feat(flows): NF-6 Phase 4G — check-suite-completed baked-in flow #386

Merged

charles merged 2 commits from feat/flows-pr-fixci-probe into main

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

claude-desktop commented

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

Collaborator

Summary

Final flow in the Phase 4 set. Ports the post-CI routing path that legacy webhook-handlers.ts::handleStatusEvent + handleActionRunEvent cover into one baked-in node-flow graph.

src → forge.handle_check_suite_completed

One monolithic node wraps the new handleCheckSuiteCompleted helper (added in this PR) which unifies handleStatusEvent + handleActionRunEvent into one canonical entry point so the flow node can call into one symbol instead of branching on the wire event name.

Routing tree

3 arms, plus decidePostCiAction's 6 sub-arms on the success path:

failure / error → dispatchFixCi to PR author with the fix-ci skill template.
success → confirm aggregate is green (other workflows may still be running), then route via decidePostCiAction (self-review skip, approved+merge, verdict-at-head skip, stale-bounce, pending skip, fresh-request POST).

Monolithic because the legacy routing tree is monolithic — splitting into granular nodes would either duplicate the entire 6-arm decidePostCiAction table in the flow JSON (which would routinely drift from legacy) or expose 6+ new node types operators can't customise meaningfully.

Cutover

node_flows.suppress_legacy: ["check_suite.completed"] skips the legacy status, action_run_failure, action_run_success, AND action_run_recover switch arms at webhook.ts:341-361. The trigger normaliser collapses every wire variant (Forgejo status/action_run_*, GitHub workflow_run/check_suite, GitLab Pipeline Hook) into one check_suite.completed kind, so one suppression entry covers all of them.

Test plan

13 e2e tests: failure / error / success arms, empty-runs defensive path, invalid-conclusion error surface, not-wired guard
Full flows suite: 330 pass / 0 fail
Server typecheck clean
AGENT_NODE_COUNT bumped from 15 to 16
Soak in mode: "live" against real CI completion, confirm parity via GET /flows/divergence/summary
Flip suppress_legacy: ["check_suite.completed"] after clean soak

Phase 4 set complete

This is the seventh and final baked-in flow in the Phase 4 set. Phase 4 ships:

4A issue-labeled-route (#380)
4B pr-approved-merge (#381)
4C review-requested (#382)
4D slash-breakdown (#383)
4E pr-changes-requested (#384)
4F issue-closed-deps (#385)
4G check-suite-completed (this PR)

After every flow soaks clean on GET /flows/divergence/summary, the operator can flip every suppress_legacy entry in one go and the legacy webhook switch in webhook.ts becomes deletable.

🤖 Generated with Claude Code

## Summary **Final flow in the Phase 4 set.** Ports the post-CI routing path that legacy `webhook-handlers.ts::handleStatusEvent` + `handleActionRunEvent` cover into one baked-in node-flow graph. ``` src → forge.handle_check_suite_completed ``` One monolithic node wraps the new `handleCheckSuiteCompleted` helper (added in this PR) which unifies `handleStatusEvent` + `handleActionRunEvent` into one canonical entry point so the flow node can call into one symbol instead of branching on the wire event name. ## Routing tree 3 arms, plus `decidePostCiAction`'s 6 sub-arms on the success path: - `failure` / `error` → `dispatchFixCi` to PR author with the fix-ci skill template. - `success` → confirm aggregate is green (other workflows may still be running), then route via `decidePostCiAction` (self-review skip, approved+merge, verdict-at-head skip, stale-bounce, pending skip, fresh-request POST). Monolithic because the legacy routing tree is monolithic — splitting into granular nodes would either duplicate the entire 6-arm `decidePostCiAction` table in the flow JSON (which would routinely drift from legacy) or expose 6+ new node types operators can't customise meaningfully. ## Cutover `node_flows.suppress_legacy: ["check_suite.completed"]` skips the legacy `status`, `action_run_failure`, `action_run_success`, AND `action_run_recover` switch arms at `webhook.ts:341-361`. The trigger normaliser collapses every wire variant (Forgejo `status`/`action_run_*`, GitHub `workflow_run`/`check_suite`, GitLab `Pipeline Hook`) into one `check_suite.completed` kind, so one suppression entry covers all of them. ## Test plan - [x] 13 e2e tests: failure / error / success arms, empty-runs defensive path, invalid-conclusion error surface, not-wired guard - [x] Full flows suite: 330 pass / 0 fail - [x] Server typecheck clean - [x] AGENT_NODE_COUNT bumped from 15 to 16 - [ ] Soak in `mode: "live"` against real CI completion, confirm parity via `GET /flows/divergence/summary` - [ ] Flip `suppress_legacy: ["check_suite.completed"]` after clean soak ## Phase 4 set complete This is the **seventh and final** baked-in flow in the Phase 4 set. Phase 4 ships: - 4A `issue-labeled-route` (#380) - 4B `pr-approved-merge` (#381) - 4C `review-requested` (#382) - 4D `slash-breakdown` (#383) - 4E `pr-changes-requested` (#384) - 4F `issue-closed-deps` (#385) - 4G `check-suite-completed` (this PR) After every flow soaks clean on `GET /flows/divergence/summary`, the operator can flip every `suppress_legacy` entry in one go and the legacy webhook switch in `webhook.ts` becomes deletable. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

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

feat(flows): NF-6 Phase 4G — check-suite-completed baked-in flow

qa / qa (pull_request) Successful in 6m36s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

dd5c4e5fe1

Final flow in the Phase 4 set. Ports the post-CI routing path that
legacy `webhook-handlers.ts::handleStatusEvent` +
`handleActionRunEvent` cover into one baked-in node-flow graph
`check-suite-completed`:

  src → forge.handle_check_suite_completed

One monolithic node wraps `handleCheckSuiteCompleted` (a new helper
added in this PR that unifies `handleStatusEvent` +
`handleActionRunEvent` into one canonical entry point so the flow
node can call into one symbol instead of branching on the wire event
name).

Routing tree (3 arms, plus `decidePostCiAction`'s 6 sub-arms on the
success path):
  - `failure` / `error` → `dispatchFixCi` to PR author with the
    fix-ci skill template.
  - `success` → confirm aggregate is green (other workflows may still
    be running), then route via `decidePostCiAction` (self-review
    skip, approved+merge, verdict-at-head skip, stale-bounce, pending
    skip, fresh-request POST).

Monolithic because the legacy routing tree is monolithic — splitting
into granular nodes would either duplicate the entire
`decidePostCiAction` 6-arm table in the flow JSON (which would
routinely drift from legacy) or expose 6+ new node types operators
can't customise meaningfully.

Returns `taskId` when fix-ci dispatches; `taskId: FILTER_DROP` on the
success path (no single taskId summarises the post-CI action — could
be a forge `requestReview` POST, a forge `removeReviewRequest` +
`requestReview` bounce, a boss merge dispatch, or just a log line).

Cutover gate: `node_flows.suppress_legacy: ["check_suite.completed"]`
skips the legacy `status`, `action_run_failure`, `action_run_success`,
AND `action_run_recover` switch arms at `webhook.ts:341-361`. The
trigger normaliser collapses every wire variant (Forgejo `status`/
`action_run_*`, GitHub `workflow_run`/`check_suite`, GitLab
`Pipeline Hook`) into one `check_suite.completed` kind, so one
suppression entry covers all of them.

Tests: 13 e2e tests covering failure / error / success arms,
empty-runs defensive path, invalid-conclusion error surface, not-wired
guard. AGENT_NODE_COUNT bumped from 15 to 16.

This is the seventh and final baked-in flow in the Phase 4 set.
After it soaks clean alongside the six earlier flows, the operator
can flip every `suppress_legacy` entry in one go and the legacy
webhook switch becomes deletable.

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

charles referenced this pull request from a commit

2026-04-26 15:14:55 +00:00

fix(flows): check-suite-completed — round-1 review parity (runId + branches)

charles added 1 commit

2026-04-26 15:14:55 +00:00

fix(flows): check-suite-completed — round-1 review parity (runId + branches)

qa / qa (pull_request) Successful in 6m37s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

bc2d9623e7

One BLOCKER + one IMPORTANT caught in subagent review of #386:

1. **BLOCKER — `runId` semantic mismatch silenced fix-ci log fetches.**
   Legacy `handleActionRunEvent` (`webhook-handlers.ts:991`) passes
   `run.index_in_repo` (the per-repo run number you see in the URL)
   to `dispatchFixCi`, which feeds `getWorkflowJobLogs` — and the
   log-fetch endpoint at `forgejo-api.ts` builds
   `…/actions/runs/{runId}/jobs/{jobIdx}/attempt/1/logs` which only
   resolves with the per-repo number. The trigger normaliser was
   passing `run.id` (the global DB id) as `runId`, which silently
   404s the log prefetch — every fix-ci dispatch from a Forgejo
   `action_run_failure` would land on the agent with the
   "(could not fetch logs — inspect manually)" placeholder instead
   of the actual failing job log lines.

   Fix: `webhook-normalize.ts:339` now uses
   `asNumber(run.index_in_repo) ?? asNumber(run.id)` — falling back
   to `run.id` only when the per-repo number is missing (defensive
   for non-Forgejo forges).

2. **IMPORTANT — dropping `status.branches` regressed PR resolution
   on force-pushed PRs.** Legacy `handleStatusEvent` passes
   `status.branches` to `findOpenPRForHead` (`webhook-handlers.ts:933`),
   which uses it as a fallback match when the head SHA doesn't line
   up against any open PR (`webhook-ci.ts:159-160` checks
   `branchNames.has(p.headRef)`). Without it, force-pushed PRs slip
   through SHA-only lookup and dispatch silently no-ops where legacy
   would have recovered.

   Threaded `branches: string[]` through the trigger envelope:
   - `ForgeEvent.ci_run` gains `branches: string[]` (Forgejo `status`
     populates it; `action_run_*` doesn't; GitHub `workflow_run` uses
     `head_branch`; GitHub `status` mirrors Forgejo; GitLab Pipeline
     Hook uses `oa.ref`).
   - `TriggerEventCheckSuiteCompleted` gains `branches: string[]`.
   - `handleCheckSuiteCompleted` accepts `branches?: string[]` and
     maps to `findOpenPRForHead`'s `{name}[]` shape.
   - `forge.handle_check_suite_completed` flow node wires `branches`
     from `src.out.branches`.

Bumped `CHECK_SUITE_COMPLETED_GRAPH_VERSION` to v2 so the seeding
migration rewrites the row on restart.

NOT fixed (deferred): direct unit test for `handleCheckSuiteCompleted`.
Legacy `handleStatusEvent` + `handleActionRunEvent` also ship with no
direct unit tests in `webhook-handlers.test.ts` — the flow-side tests
in `check-suite-completed-graph.test.ts` already exercise both branches
of the helper's return value (null → taskId FILTER_DROP, string →
taskId passthrough). Adding direct unit tests would require heavy
mocking of probeToken + findOpenPRForHead + fetchAggregateState +
dispatchFixCi + decidePostCiAction; same coverage achievable through
the flow tests.

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

charles merged commit d0094b7873 into main

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

charles referenced this pull request from a commit

2026-04-26 15:20:21 +00:00

feat(flows): NF-6 Phase 4G — check-suite-completed baked-in flow (#386)

charles deleted branch feat/flows-pr-fixci-probe

2026-04-26 15:20:22 +00:00