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

feat(flows): NF-6 per-trigger legacy bypass gate #375

Merged
claude-desktop merged 3 commits from feat/flows-suppress-legacy-gate into main 2026-04-26 10:36:35 +00:00
Conversation 0 Commits 3 Files changed 4 +428 -147
claude-desktop commented 2026-04-26 10:11:34 +00:00
Collaborator
Copy link

First piece of the NF-6 cutover machinery: a per-trigger gate that lets the operator skip the hard-coded webhook switch on a kind-by-kind basis once a matching flow is ready.

What this PR does

New agents.json field: node_flows.suppress_legacy: TriggerKind[]. When a kind is listed, the Forgejo webhook entry point skips the hard-coded handler switch for that kind. dispatchToFlows still fires below the gate so a mode: "live" flow takes over the surface.

Per-trigger granularity = each NF-6 surface flips with a one-line config edit, no deploy.

  • apps/server/src/shared/config/webhook-config.ts — parses + validates against TRIGGER_KINDS, dedups, frozen
  • apps/server/src/http/webhook.ts — hoists normalizeForgejoPayload + toTriggerEvent ahead of the legacy switch; the switch is skipped when triggerForGate.kind is suppressed; reuses the already-normalised event for dispatchToFlows
  • apps/server/src/shared/config/flow-mode.test.ts — 7 new test cases for parsing
  • apps/server/src/http/webhook-suppress-legacy.test.ts — 4 integration tests covering legacy default-on, gate skips legacy, non-suppressed kinds still run, unmodelled events fall through safely

Operator usage

"node_flows": {
  "mode": "live",
  "suppress_legacy": ["issue.assigned"]
}

→ legacy handler skipped for issues.assigned, only the matching flow fires. Other events keep the legacy path until their own flow lands.

What was reverted (commit 9c53eae)

The earlier Phase 1 commit (748c35b) attempted to upgrade the baked-in default flow so it could stand in for handleIssueAssigned. Code review caught three blockers that make the upgraded flow non-cuttable as authored — reverted to keep this PR scoped to the gate alone:

  1. agent-nodes.ts toTaskRequest builds a TaskRequest with forgejo_token: "pending", raw task: "implement" literal (not the rendered skill template), and no model / system_prompt / appendix. Legacy dispatchIssueForAgent (webhook-handlers.ts:567-655) loads the skill via skillForEvent, runs interpolate, layers applyAppendix + maybeApplyCavemanAppendix + applyArtifactStyleAppendix, then threads agent identity through buildAgentRequest. The flow path skips all of that — every dispatch is broken end-to-end.
  2. agent.resolve_by_login returns only { type, instance } — no token, model, system_prompt, branch_prefix, or caveman config. dispatch_resolved cannot authenticate against Forgejo.
  3. Legacy webhook.ts:184 iterates for (const assignee of payload.issue.assignees) and dispatches per-assignee. The trigger normaliser (webhook-normalize.ts:188-196) surfaces only the last assignee. Multi-assign tickets silently lose every dispatch except the trailing login.

Two majors also flagged:

  • The outstanding-changes_requested pivot (issue #270, webhook-handlers.ts:583-602) is load-bearing for the dependency-closure cascade. Without it, dev runs implement against dev/N instead of address-review against pr.headRef and the review loop wedges.
  • mode === "off" + non-empty suppress_legacy silently drops events. Either reject this combo at config load OR only suppress when at least one enabled flow on the row matches the trigger kind.

A follow-up PR will add the missing pieces (agent.resolve_by_login returning the full ResolvedAgent, an agent.dispatch.render_skill hop or args extension, a router.fan_out over assignees, the address-review pivot, and the empty-flow guard) before any operator should set suppress_legacy: ["issue.assigned"].

Tests

  • 1817 server tests pass; 5 pre-existing failures unchanged (sweeper JSONL × 3, agents-update PATCH, foreman session CRUD).
  • Typecheck clean; biome clean on all touched files.

Next steps after merge

  1. The gate is dormant until an operator sets it. Default config behaviour is unchanged.
  2. Phase 2 — extend the divergence diff tooling (flows-divergence.ts) to cover forge.* mutations (currently dispatch-only). Needed to validate every flow except the issue.assigned one before any cutover.
  3. Phase 1 (re-do) — fix the three blockers above, then the gate is actually cuttable for issue.assigned.
  4. Phase 4 — author the remaining 7 NF-6 flows in their own PRs.

🤖 Generated with Claude Code

First piece of the NF-6 cutover machinery: a per-trigger gate that lets the operator skip the hard-coded webhook switch on a kind-by-kind basis once a matching flow is ready. ## What this PR does New `agents.json` field: `node_flows.suppress_legacy: TriggerKind[]`. When a kind is listed, the Forgejo webhook entry point skips the hard-coded handler switch for that kind. `dispatchToFlows` still fires below the gate so a `mode: "live"` flow takes over the surface. Per-trigger granularity = each NF-6 surface flips with a one-line config edit, no deploy. - `apps/server/src/shared/config/webhook-config.ts` — parses + validates against `TRIGGER_KINDS`, dedups, frozen - `apps/server/src/http/webhook.ts` — hoists `normalizeForgejoPayload` + `toTriggerEvent` ahead of the legacy switch; the switch is skipped when `triggerForGate.kind` is suppressed; reuses the already-normalised event for `dispatchToFlows` - `apps/server/src/shared/config/flow-mode.test.ts` — 7 new test cases for parsing - `apps/server/src/http/webhook-suppress-legacy.test.ts` — 4 integration tests covering legacy default-on, gate skips legacy, non-suppressed kinds still run, unmodelled events fall through safely ## Operator usage ```json "node_flows": { "mode": "live", "suppress_legacy": ["issue.assigned"] } ``` → legacy handler skipped for `issues.assigned`, only the matching flow fires. Other events keep the legacy path until their own flow lands. ## What was reverted (commit 9c53eae) The earlier Phase 1 commit (`748c35b`) attempted to upgrade the baked-in `default` flow so it could stand in for `handleIssueAssigned`. Code review caught **three blockers** that make the upgraded flow non-cuttable as authored — reverted to keep this PR scoped to the gate alone: 1. `agent-nodes.ts toTaskRequest` builds a `TaskRequest` with `forgejo_token: "pending"`, raw `task: "implement"` literal (not the rendered skill template), and no model / system_prompt / appendix. Legacy `dispatchIssueForAgent` (`webhook-handlers.ts:567-655`) loads the skill via `skillForEvent`, runs `interpolate`, layers `applyAppendix` + `maybeApplyCavemanAppendix` + `applyArtifactStyleAppendix`, then threads agent identity through `buildAgentRequest`. The flow path skips all of that — every dispatch is broken end-to-end. 2. `agent.resolve_by_login` returns only `{ type, instance }` — no token, model, system_prompt, branch_prefix, or caveman config. `dispatch_resolved` cannot authenticate against Forgejo. 3. Legacy `webhook.ts:184` iterates `for (const assignee of payload.issue.assignees)` and dispatches per-assignee. The trigger normaliser (`webhook-normalize.ts:188-196`) surfaces only the **last** assignee. Multi-assign tickets silently lose every dispatch except the trailing login. Two majors also flagged: - The outstanding-`changes_requested` pivot (issue #270, `webhook-handlers.ts:583-602`) is load-bearing for the dependency-closure cascade. Without it, dev runs `implement` against `dev/N` instead of `address-review` against `pr.headRef` and the review loop wedges. - `mode === "off"` + non-empty `suppress_legacy` silently drops events. Either reject this combo at config load OR only suppress when at least one enabled flow on the row matches the trigger kind. A follow-up PR will add the missing pieces (`agent.resolve_by_login` returning the full `ResolvedAgent`, an `agent.dispatch.render_skill` hop or args extension, a `router.fan_out` over assignees, the address-review pivot, and the empty-flow guard) before any operator should set `suppress_legacy: ["issue.assigned"]`. ## Tests - 1817 server tests pass; 5 pre-existing failures unchanged (sweeper JSONL × 3, agents-update PATCH, foreman session CRUD). - Typecheck clean; biome clean on all touched files. ## Next steps after merge 1. The gate is dormant until an operator sets it. Default config behaviour is unchanged. 2. Phase 2 — extend the divergence diff tooling (`flows-divergence.ts`) to cover `forge.*` mutations (currently dispatch-only). Needed to validate every flow except the `issue.assigned` one before any cutover. 3. Phase 1 (re-do) — fix the three blockers above, then the gate is actually cuttable for `issue.assigned`. 4. Phase 4 — author the remaining 7 NF-6 flows in their own PRs. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Adds `node_flows.suppress_legacy: TriggerKind[]` to agents.json. When a
trigger kind is listed, the Forgejo webhook entry point skips the legacy
hard-coded handler switch for that kind. `dispatchToFlows` still fires
so a `mode: "live"` flow takes over the surface.

Lets each NF-6 cutover (issue.assigned, review_requested, CI, etc.)
flip independently with a one-line config edit, with no double-dispatch
during the transition window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(flows): NF-6 default flow parity for handleIssueAssigned cutover
All checks were successful
qa / qa (pull_request) Successful in 5m30s
Details
qa / dockerfile (pull_request) Successful in 13s
Details
748c35b07e
 
Closes the gap inventory blocking the legacy `issues.assigned` cutover:

- New `agent.resolve_by_login` node — wraps `resolveAgentByUser` so
  Forgejo logins map to agent types via the canonical config lookup
  (host-mode + no-instance + unknown-user soft-skip via FILTER_DROP),
  matching legacy `if (!agent?.token) return null` semantics.
- `agent.dispatch` gains `enable_dedup` + `cancel_stale` args. When set,
  the node honours the same `(repo, issue, type)` dedup window and
  cross-type queued-task cancel that legacy `handleIssueAssigned` used,
  via a `DedupSurface` injection.
- Dedup helpers lifted from `webhook-handlers.ts` to
  `domain/dispatch/assign-dedup.ts` so the flow node can share state
  without depending on the HTTP layer (back-compat re-export kept).
- `executor.ts` accepts `argDefaults` — `flow-dispatch.ts` injects
  production defaults (`dispatch`, `dedup`, `resolveAgentByLogin`) so
  the JSON DSL doesn't have to carry function references.
- `default-graph.json` rewritten (v3) to use the new nodes; trigger
  fan-out: src.assignee → type_switch (designer / design-reviewer /
  default→resolve_agent) → dispatch_*. Every dispatch opts into dedup +
  cancel-stale.

Five end-to-end execute tests cover the four routing branches plus the
dedup-hit drop path. With this, setting `node_flows.suppress_legacy:
["issue.assigned"]` is the operator's only remaining step to retire
`handleIssueAssigned`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revert "feat(flows): NF-6 default flow parity for handleIssueAssigned cutover"
All checks were successful
qa / qa (pull_request) Successful in 5m32s
Details
qa / dockerfile (pull_request) Successful in 11s
Details
9c53eae2f7
 
This reverts commit 748c35b07e.
claude-desktop changed title from feat(flows): NF-6 legacy-bypass gate + default flow parity to feat(flows): NF-6 per-trigger legacy bypass gate 2026-04-26 10:23:18 +00:00
claude-desktop deleted branch feat/flows-suppress-legacy-gate 2026-04-26 10:36:35 +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!375
No description provided.