charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

feat(flows): NF-6 forge-mutation divergence parity tooling #376

Merged
claude-desktop merged 3 commits from feat/flows-divergence-forge-mutations into main 2026-04-26 11:40:40 +00:00
Conversation 0 Commits 3 Files changed 6 +876 -4
claude-desktop commented 2026-04-26 10:44:16 +00:00
Collaborator
Copy link

Summary

Phase 2 of the NF-6 cutover. Extends the divergence tooling so the operator can validate forge-side parity for the 7 NF-6 flows that follow issue.assigned — every one of which calls addLabels / createComment / requestReview / mergePullRequest / dismissReview. Without this, the soak protocol can only diff dispatch intents (currently shipped as GET /flows/divergences) and has no signal on the forge-mutation side.

What this PR does

  • New legacy_forge_calls SQLite table — (method, repo, target_number, args_json, fired_at). Migration is the same CREATE TABLE IF NOT EXISTS shape as every other flows table.
  • instrumentForLegacyParity Proxy in adapter-factory.ts — wraps every createForgeAdapterForRepo return so mutating methods append a row to legacy_forge_calls. 12 mutating methods covered (matches the forge.* mutating SPECS in forge-nodes.ts). Read-only methods are pass-through (Reflect.get returns the original fn unchanged so the JIT can inline).
  • GET /flows/divergence/summary?since=<iso> — per-method bucket of (flow_intents, legacy_calls, matched_pairs, flow_only, legacy_only) matched on (method, repo, target_number). Plus the existing agent.dispatch heuristic, plus a roll-up totals object the operator polls daily during soak.
  • 8 tests cover empty inputs, matched pairs, flow-only intents, legacy-only calls, repo / target-number mismatch, non-forge intents ignored, and the duplicate-intent fan-in case.

Operator usage during soak

GET /flows/divergence/summary?since=2026-04-23T00:00:00Z

Returns:

{
  "since": "...",
  "now": "...",
  "dispatch": { "flow_intents": N, "legacy_dispatches": N, "matched_pairs": N, "flow_only": N, "legacy_only": N },
  "forge_mutations": [
    { "method": "forge.add_labels",        "flow_intents": N, "legacy_calls": N, "matched_pairs": N, "flow_only_intents": N, "legacy_only_calls": N },
    { "method": "forge.create_comment",    ... },
    { "method": "forge.merge_pull_request", ... }
  ],
  "totals": { "flow_intents": N, "legacy_calls": N, "matched_pairs": N, "flow_only": N, "legacy_only": N }
}

For each new NF-6 flow, wait until both flow_only and legacy_only drop to zero across the soak window before adding the matching trigger to node_flows.suppress_legacy.

Failure semantics

Instrumentation row insertion is wrapped in try/catch (insertLegacyForgeCall swallows + logs). A divergence-table hiccup never breaks a real legacy call.

Tests

  • 8 new tests in flows-divergence-summary.test.ts — all pass
  • 1818 server tests pass total; 5 pre-existing failures unchanged (sweeper JSONL × 3, agents-update PATCH, foreman session CRUD)
  • Typecheck clean; biome clean on touched files

What's next

  • Phase 1 redo — fix the three blockers from the previous review and ship a default flow that's actually cuttable for issue.assigned
  • Phase 4 — author the remaining 7 NF-6 flows in their own PRs

🤖 Generated with Claude Code

## Summary Phase 2 of the NF-6 cutover. Extends the divergence tooling so the operator can validate forge-side parity for the 7 NF-6 flows that follow `issue.assigned` — every one of which calls `addLabels` / `createComment` / `requestReview` / `mergePullRequest` / `dismissReview`. Without this, the soak protocol can only diff dispatch intents (currently shipped as `GET /flows/divergences`) and has no signal on the forge-mutation side. ## What this PR does - New `legacy_forge_calls` SQLite table — `(method, repo, target_number, args_json, fired_at)`. Migration is the same `CREATE TABLE IF NOT EXISTS` shape as every other flows table. - `instrumentForLegacyParity` Proxy in `adapter-factory.ts` — wraps every `createForgeAdapterForRepo` return so mutating methods append a row to `legacy_forge_calls`. 12 mutating methods covered (matches the `forge.*` mutating SPECS in `forge-nodes.ts`). Read-only methods are pass-through (`Reflect.get` returns the original fn unchanged so the JIT can inline). - `GET /flows/divergence/summary?since=<iso>` — per-method bucket of `(flow_intents, legacy_calls, matched_pairs, flow_only, legacy_only)` matched on `(method, repo, target_number)`. Plus the existing `agent.dispatch` heuristic, plus a roll-up totals object the operator polls daily during soak. - 8 tests cover empty inputs, matched pairs, flow-only intents, legacy-only calls, repo / target-number mismatch, non-forge intents ignored, and the duplicate-intent fan-in case. ## Operator usage during soak ``` GET /flows/divergence/summary?since=2026-04-23T00:00:00Z ``` Returns: ```json { "since": "...", "now": "...", "dispatch": { "flow_intents": N, "legacy_dispatches": N, "matched_pairs": N, "flow_only": N, "legacy_only": N }, "forge_mutations": [ { "method": "forge.add_labels", "flow_intents": N, "legacy_calls": N, "matched_pairs": N, "flow_only_intents": N, "legacy_only_calls": N }, { "method": "forge.create_comment", ... }, { "method": "forge.merge_pull_request", ... } ], "totals": { "flow_intents": N, "legacy_calls": N, "matched_pairs": N, "flow_only": N, "legacy_only": N } } ``` For each new NF-6 flow, wait until both `flow_only` and `legacy_only` drop to zero across the soak window before adding the matching trigger to `node_flows.suppress_legacy`. ## Failure semantics Instrumentation row insertion is wrapped in try/catch (`insertLegacyForgeCall` swallows + logs). A divergence-table hiccup never breaks a real legacy call. ## Tests - 8 new tests in `flows-divergence-summary.test.ts` — all pass - 1818 server tests pass total; 5 pre-existing failures unchanged (sweeper JSONL × 3, agents-update PATCH, foreman session CRUD) - Typecheck clean; biome clean on touched files ## What's next - Phase 1 redo — fix the three blockers from the previous review and ship a default flow that's actually cuttable for `issue.assigned` - Phase 4 — author the remaining 7 NF-6 flows in their own PRs 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(flows): NF-6 forge-mutation divergence parity tooling
All checks were successful
qa / qa (pull_request) Successful in 5m50s
Details
qa / dockerfile (pull_request) Successful in 10s
Details
45e4d1727f
 
The existing /flows/divergences endpoint correlates `agent.dispatch`
intents against `task_history` rows but doesn't see any other forge
mutation. Every NF-6 flow except `issue.assigned` calls `addLabels` /
`createComment` / `requestReview` / `mergePullRequest` / `dismissReview`
etc. — without an instrumentation hook on the legacy adapter, the
operator has no way to validate parity before flipping `suppress_legacy`
on those triggers.

This PR adds:

- New `legacy_forge_calls` SQLite table — `(method, repo, target_number,
  args_json, fired_at)`. Migration is the same `CREATE TABLE IF NOT
  EXISTS` shape as every other flows table; idempotent.
- `instrumentForLegacyParity` Proxy in `adapter-factory.ts` — wraps
  every `createForgeAdapterForRepo` return so mutating methods append a
  row to `legacy_forge_calls`. 12 mutating methods covered (matches the
  `forge.*` mutating SPECS in `forge-nodes.ts`). Read-only methods are
  pass-through (Reflect.get returns the original fn).
- `GET /flows/divergence/summary?since=<iso>` — per-method bucket of
  `(flow_intents, legacy_calls, matched_pairs, flow_only, legacy_only)`
  matched on `(method, repo, target_number)`. Plus the existing
  `agent.dispatch` heuristic, plus a roll-up totals object the operator
  polls daily during soak.
- 8 tests cover empty inputs, matched pairs, flow-only intents,
  legacy-only calls, repo / target-number mismatch, non-forge intents
  ignored, and the duplicate-intent fan-in case.

Operator usage during soak:
  - Watch `GET /flows/divergence/summary?since=<24h>` daily.
  - For each new flow, wait until both `flow_only` and `legacy_only`
    drop to zero across the soak window before adding the matching
    trigger to `node_flows.suppress_legacy`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(flows): NF-6 divergence — record post-await + drop ghost dismissReview
All checks were successful
qa / qa (pull_request) Successful in 6m5s
Details
qa / dockerfile (pull_request) Successful in 11s
Details
a5beda8def
 
Code review of #376 flagged four majors. All addressed:

- **Phantom legacy_only on forge failure** — `instrumentForLegacyParity`
  inserted the `legacy_forge_calls` row BEFORE `value.apply()`. A 4xx /
  5xx / network blip on the underlying call left a row that never
  matched a flow intent, surfacing as a false-positive `legacy_only`
  during soak and blocking cutover decisions. Recording now happens
  AFTER the call resolves: sync return → record inline; Promise →
  record on the resolution arm; rejection → no row. New end-to-end
  Proxy test (`adapter-factory-proxy.test.ts`) covers async failure +
  sync failure + happy path.

- **`dismissReview` ghost entry** — listed in `MUTATING_METHODS` but
  `ForgePort` doesn't have such a method (it's `removeReviewRequest`).
  Dead code that never fires; meanwhile `removeReviewRequest` IS
  mutating and is now also unrecorded — but no SPEC entry exists for
  either yet, so dropping the ghost is the correct fix until the
  dismiss-review flow + matching SPEC land together.

- **Module-load contract** — added an assertion at boot that every
  `MUTATING_METHODS` key snake-cases to a node type that exists in
  `FORGE_NODE_TYPES`. Catches a future `getURL` → `get_u_r_l`
  regression at startup, not during silent soak misreporting. Pulls
  `FORGE_NODE_TYPES` from `domain/flows/forge-nodes.ts`.

- **`dispatchFlowIntents` underflow** — the previous formula
  `dispatch.flow_intents - intents.length` could go negative when the
  dispatch correlator's `fired_at_ms <= nowMs` filter excluded rows
  that the summary's raw `intents` list still saw (clock skew, future
  test seeds). Compute dispatch flow_intents directly from intent
  rows now, no subtraction. New test seeds 3 forge intents + 0
  agent.dispatch + 0 legacy and asserts `summary.dispatch.flow_intents
  === 0` (was -3 before).

Plus 6 new Proxy contract tests + 1 new dispatch-underflow test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(flows): NF-6 divergence — kill circular import, sync nowMs filter
All checks were successful
qa / qa (pull_request) Successful in 6m6s
Details
qa / dockerfile (pull_request) Successful in 12s
Details
cc9adf95d3
 
Round 3 review feedback:

- **Circular import** between `adapter-factory` (value-imported
  `FORGE_NODE_TYPES` from `forge-nodes`) and `forge-nodes` (already
  value-imports `createForgeAdapterForRepo` from `adapter-factory`).
  Module evaluation order matters under HMR / test ordering — the
  IIFE-style assertion would have thrown a misleading "no matching
  forge.* node type" or `TypeError` if forge-nodes was the first entry
  point. Replaced the import with a hard-coded `KNOWN_FORGE_MUTATING_TYPES`
  Set inline; the assertion still catches `getURL → get_u_r_l` and
  copy-paste typos at boot, and the comment explicitly documents that
  drift between the two lists is the whole risk this assertion exists
  to catch (so independent declarations are correct, not duplication).
- **`dispatchFlowIntents` lost the `nowMs` filter** that
  `dispatch.matched_pairs` and `dispatch.flow_only_intents` still
  apply. With clock-skewed / future-dated dispatch intents,
  `summary.dispatch.flow_intents` could exceed `matched + flow_only`.
  Now reuses `parseIntentRows` with the same `fired_at_ms <= nowMs`
  filter so the three reported numbers reconcile, and avoids drift
  if the intent envelope shape changes.
- Dropped the redundant `(err) => { throw err; }` rejection arm on the
  Proxy's `result.then(...)`. An absent rejection handler propagates
  rejection identically — same effect, less noise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-desktop deleted branch feat/flows-divergence-forge-mutations 2026-04-26 11:40:40 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
area:agents
Agent types, pool scheduling, per-instance config

  
area:dashboard
Dashboard UI and observability surfaces

  
area:database
DB layer — schema, migrations, ORM, raw SQL

  
area:design
UI/UX mockup work — routes to designer agent

  
area:design-review
Design review dispatch — routes to design-reviewer agent

  
area:flows
Flow runner — YAML loader, executor, op registry, expression eval

  
area:infra
Deployment, isolation, containers, systemd units

  
area:meta
Tracking, scaffolding, project setup

  
area:security
Security — routes to reviewer-security (opus)

  
area:sessions
Session-id store, Claude SDK resume logic

  
area:webhook
Forgejo webhook routing and handlers

  
area:workdir
Clone cache, worktrees, git identity

  
security
Security-sensitive issue

  
type:bug
Bug

  
type:chore
Chore

  
type:meta
Tracking or decisions, not implementation work

  
type:user-story
User story

No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!376
No description provided.