feat(flows): NF-6 Phase 5B + 5C — cutover to flows, simplify webhook dispatcher #388

Merged

charles merged 2 commits from feat/flows-cutover-delete-legacy into main

2026-04-26 15:47:29 +00:00

claude-desktop commented

2026-04-26 15:39:04 +00:00

Collaborator

Summary

Switches the production dispatch path from the hard-coded webhook switch to the node-flow runner. After 7 baked-in flows shipped in Phase 4 + 1 in Phase 5A, every trigger kind now has flow coverage — the legacy switch arms are redundant.

Stacks on PR #387 (Phase 5A — slash-commands-other). Merge order matters; this PR depends on the slash-commands-other flow existing for the issue_comment.slash_command suppression to be safe.

Phase 5B — rewire `onAssignedReady` callback

propagateDependencyClosure.onAssignedReady previously re-fired the legacy handleIssueAssigned helper on each unblocked dependent. After cutover that helper is no longer the canonical entry point; swapped to a synthetic ForgeEvent + dispatchToFlows re-entry so dep-triggered re-dispatches go through the same flow-runner + flow_runs observability. Lazy import breaks the flow-dispatch.ts → webhook-handlers.ts load cycle.

Phase 5C — flip cutover + simplify dispatcher

config/agents.json: node_flows.mode = "live" + suppress_legacy covers all 7 trigger kinds shipped (issue.assigned, issue.labeled, issue.closed, pull_request_review.review_requested, pull_request_review.submitted, issue_comment.slash_command, check_suite.completed).
webhook.ts: ripped out the legacy switch (~150 lines) + per-trigger bypass gate plumbing. Forgejo webhook now does only:
- signature verify
- parse + multi-repo gate
- normalise payload to ForgeEvent
- run handlePullRequestOpened / handlePullRequestClosed inline (no flow yet covers these — Phase 6)
- dispatchToFlows(forgeEvent) (canonical path)
Deleted webhook-suppress-legacy.test.ts (gate is gone).
Removed two test cases asserting "legacy switch fires cleanupIssue on issues.closed" — equivalent coverage in issue-closed-deps-graph.test.ts.

What's NOT deleted

Legacy handler functions in webhook-handlers.ts (handleIssueAssigned, handleIssueLabeled, handleApproved, handleChangesRequested, handleReviewRequested, handleIssueComment, handleStatusEvent, handleActionRunEvent) are dead code in production but kept in the file. Their direct unit tests still verify them. Phase 6 rips them out + their tests in one cleanup pass.

handleIssueClosed, handleSlashCommandOther, handleCheckSuiteCompleted, handlePullRequestOpened, handlePullRequestClosed, handlePostMergeRebase STAY — flow injections (or surviving inline calls in webhook.ts) still need them.

Test plan

Server typecheck clean
Test suite: 1968 pass / 5 fail (all pre-existing — session-pruning + foreman CRUD, unrelated)
Production soak: watch flow_runs for flow-only failures; drop to mode = "off" in config/agents.json if needed (trivial rollback)
Phase 6 follow-up: delete dead handlers + their tests

🤖 Generated with Claude Code

## Summary Switches the production dispatch path from the hard-coded webhook switch to the node-flow runner. After 7 baked-in flows shipped in Phase 4 + 1 in Phase 5A, every trigger kind now has flow coverage — the legacy switch arms are redundant. **Stacks on PR #387** (Phase 5A — `slash-commands-other`). Merge order matters; this PR depends on the slash-commands-other flow existing for the `issue_comment.slash_command` suppression to be safe. ## Phase 5B — rewire `onAssignedReady` callback `propagateDependencyClosure.onAssignedReady` previously re-fired the legacy `handleIssueAssigned` helper on each unblocked dependent. After cutover that helper is no longer the canonical entry point; swapped to a synthetic `ForgeEvent` + `dispatchToFlows` re-entry so dep-triggered re-dispatches go through the same flow-runner + `flow_runs` observability. Lazy import breaks the `flow-dispatch.ts → webhook-handlers.ts` load cycle. ## Phase 5C — flip cutover + simplify dispatcher 1. **`config/agents.json`**: `node_flows.mode = "live"` + `suppress_legacy` covers all 7 trigger kinds shipped (issue.assigned, issue.labeled, issue.closed, pull_request_review.review_requested, pull_request_review.submitted, issue_comment.slash_command, check_suite.completed). 2. **`webhook.ts`**: ripped out the legacy switch (~150 lines) + per-trigger bypass gate plumbing. Forgejo webhook now does only: - signature verify - parse + multi-repo gate - normalise payload to ForgeEvent - run `handlePullRequestOpened` / `handlePullRequestClosed` inline (no flow yet covers these — Phase 6) - `dispatchToFlows(forgeEvent)` (canonical path) 3. Deleted `webhook-suppress-legacy.test.ts` (gate is gone). 4. Removed two test cases asserting "legacy switch fires `cleanupIssue` on `issues.closed`" — equivalent coverage in `issue-closed-deps-graph.test.ts`. ## What's NOT deleted Legacy handler functions in `webhook-handlers.ts` (`handleIssueAssigned`, `handleIssueLabeled`, `handleApproved`, `handleChangesRequested`, `handleReviewRequested`, `handleIssueComment`, `handleStatusEvent`, `handleActionRunEvent`) are dead code in production but kept in the file. Their direct unit tests still verify them. **Phase 6** rips them out + their tests in one cleanup pass. `handleIssueClosed`, `handleSlashCommandOther`, `handleCheckSuiteCompleted`, `handlePullRequestOpened`, `handlePullRequestClosed`, `handlePostMergeRebase` STAY — flow injections (or surviving inline calls in webhook.ts) still need them. ## Test plan - [x] Server typecheck clean - [x] Test suite: 1968 pass / 5 fail (all pre-existing — session-pruning + foreman CRUD, unrelated) - [ ] **Production soak**: watch `flow_runs` for flow-only failures; drop to `mode = "off"` in `config/agents.json` if needed (trivial rollback) - [ ] Phase 6 follow-up: delete dead handlers + their tests 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 2 commits

2026-04-26 15:39:04 +00:00

feat(flows): NF-6 Phase 5A — slash-commands-other baked-in flow

qa / qa (pull_request) Successful in 6m21s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

56e53f29e6

Closes the slash-breakdown cutover-gate scope problem documented in
PR #383. Phase 4D shipped `slash-breakdown` for `/breakdown` only, but
the cutover gate `node_flows.suppress_legacy: ["issue_comment.slash_command"]`
collapses ALL slash commands to one trigger kind — flipping it would
have skipped `/hold`, `/no-ready`, `/ready`, `/raise-cap`, `/unassign`
since the legacy switch arm at `webhook.ts:320-330` handles them all
in one block.

Ships:
1. New helper `handleSlashCommandOther` in webhook-handlers.ts that
   wraps the four arms above the `/breakdown` branch in legacy
   `handleIssueComment` (lines 213-265). Trust-gated, side-effect
   only (no task dispatch). Returns `true` when a command matched.
2. New flow node `slash.handle_command_other` (monolithic — same
   shape as `slash.handle_breakdown` from Phase 4D).
3. New baked-in flow `slash-commands-other` filtered on
   `command.command in {hold, no-ready, ready, raise-cap, unassign}`.

Combined with `slash-breakdown`, the two flows now cover every
command the legacy `case "issue_comment"` arm handles. Cutover gate
flip is safe.

Tests: 16 e2e tests covering each command + breakdown filter-out +
unknown-command filter-out + rawBody trim + not-wired guard.
AGENT_NODE_COUNT bumped from 16 to 17.

This is Phase 5A of the cutover plan. Phase 5B + 5C ship next.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(flows): NF-6 Phase 5B + 5C — cutover to flows, simplify webhook dispatcher

qa / qa (pull_request) Successful in 6m44s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

8ab69349c1

Switches the production dispatch path from the hard-coded webhook
switch to the node-flow runner. After 7 baked-in flows shipped in
Phase 4 + 1 in Phase 5A, every trigger kind now has flow coverage —
the legacy switch arms are redundant.

## Phase 5B — rewire onAssignedReady callback

`propagateDependencyClosure.onAssignedReady` previously re-fired the
legacy `handleIssueAssigned` helper on each unblocked dependent.
After the cutover that helper is no longer the canonical entry point;
swapped to a synthetic `ForgeEvent` + `dispatchToFlows` re-entry so
dep-triggered re-dispatches go through the same flow-runner +
`flow_runs` observability as everything else. Lazy import breaks the
`flow-dispatch.ts → webhook-handlers.ts` load cycle.

## Phase 5C — flip cutover + simplify webhook dispatcher

1. `config/agents.json`: `node_flows.mode = "live"` + `suppress_legacy`
   covers all 7 trigger kinds shipped in Phase 4 + 5A.

2. `webhook.ts`: ripped out the legacy switch (~150 lines) +
   per-trigger bypass gate plumbing. The Forgejo webhook now does
   only:
     - signature verify
     - parse + multi-repo gate
     - normalise payload to ForgeEvent
     - run `handlePullRequestOpened` / `handlePullRequestClosed`
       inline (no flow yet covers these — Phase 6 will add them)
     - `dispatchToFlows(forgeEvent)` (canonical path)

3. Deleted `webhook-suppress-legacy.test.ts` (gate is gone).

4. Removed the two test cases in `webhook.test.ts` +
   `webhook-handlers.test.ts` that asserted "legacy switch fires
   `cleanupIssue` on `issues.closed`" — the equivalent coverage
   lives in `issue-closed-deps-graph.test.ts`.

## What's NOT deleted

The legacy handler functions in `webhook-handlers.ts`
(`handleIssueAssigned`, `handleIssueLabeled`, `handleApproved`,
`handleChangesRequested`, `handleReviewRequested`, `handleIssueComment`,
`handleStatusEvent`, `handleActionRunEvent`) are dead code in
production now (webhook.ts no longer calls them) but kept in the
file. Their direct unit tests in `webhook-handlers.test.ts`,
`webhook-assign-dedup.test.ts`, and
`force-merge.integration.test.ts` still verify them — and the
flow-side tests duplicate the coverage by injection. Phase 6 will
rip them out + their tests in one cleanup pass.

`handleIssueClosed`, `handleSlashCommandOther`,
`handleCheckSuiteCompleted`, `handlePullRequestOpened`,
`handlePullRequestClosed`, `handlePostMergeRebase` STAY — flow
injections (or the surviving inline calls in webhook.ts) still need
them.

## Test plan

- [x] Server typecheck clean
- [x] Test suite: 1968 pass / 5 fail (all pre-existing —
      session-pruning + foreman CRUD, unrelated to this PR)
- [ ] Soak in production: watch `flow_runs` for any flow-only
      failures; drop into `mode = "off"` if needed (flips trivially)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

charles merged commit 7be09d7cfb into main

2026-04-26 15:47:29 +00:00

charles deleted branch feat/flows-cutover-delete-legacy

2026-04-26 15:47:29 +00:00

charles referenced this pull request from a commit

2026-04-26 15:47:30 +00:00

feat(flows): NF-6 Phase 5B + 5C — cutover to flows, simplify webhook dispatcher (#388)