charles/claude-hooks

Fork 0

feat(flows): NF-6 Phase 4B — pr-approved-merge baked-in flow #381

Merged

charles merged 3 commits from feat/flows-pr-approved-merge into main

2026-04-26 13:25:51 +00:00

claude-desktop commented

2026-04-26 13:09:33 +00:00

Collaborator

Summary

Ports webhook-handlers.ts::handleApproved (rebase precheck + boss merge dispatch) into a baked-in node-flow graph. Ships pr-approved-merge as the third seeded flow on the pull_request_review.submitted trigger.

src → router.filter (review.state === "approved")
    → agent.resolve_by_login (PR author)
    → forge.get_pull_request (using author token)
    → router.switch on `mergeable`
        • not_mergeable → render `rebase` + dispatch to author
        • default       → resolve_by_type("boss") + render `merge` + dispatch to boss

Both branches dispatch on branch=pr.headRef so the worktree lands on the actual PR head (not the derived <branch_prefix>/<issue>).

DSL extensions (small, surgical)

agent.resolve_by_login / resolve_by_type gain a gate input — mirrors agent.render_skill.gate. Handler ignores the value; FILTER_DROP propagation does the gating work.
forge.* nodes gain an optional token input port (in addition to args.token). inputs.token wins, letting flows thread per-run tokens out of upstream agent.resolve_by_* envelopes instead of baking them into the JSON.

Documented deviation

When the PR author can't be resolved (rare external author), the entire pipeline drops. Legacy fires merge to boss anyway, which the merge skill's mergeable precondition then refuses — net behaviour matches (no merge happens), and the flow path avoids the noisy boss task.

Force-merge path (opts.force=true, MAX_ROUNDS escape hatch) is OUT OF SCOPE here — that lives on dispatchMerge(…, { force: true }) from the changes-requested round-cap, ported in Phase 4E.

Cutover

Set node_flows.suppress_legacy: ["pull_request_review.submitted"] once the new flow soaks clean on GET /flows/divergence/summary.

Test plan

bun test apps/server/src/domain/flows/pr-approved-merge-graph.test.ts — 16 tests, all pass
bun test apps/server/src/domain/flows/ — 263 tests, all pass (no regressions)
bun test apps/server/src/http/flows-routes.test.ts — 63 tests pass (BAKED_IN_FLOWS seeding intact)
Server typecheck clean
Soak in mode: "live" against a real approval, confirm flow + legacy emit identical dispatches via GET /flows/divergence/summary
Flip suppress_legacy: ["pull_request_review.submitted"] after a clean soak window

🤖 Generated with Claude Code

## Summary Ports `webhook-handlers.ts::handleApproved` (rebase precheck + boss merge dispatch) into a baked-in node-flow graph. Ships `pr-approved-merge` as the third seeded flow on the `pull_request_review.submitted` trigger. ``` src → router.filter (review.state === "approved") → agent.resolve_by_login (PR author) → forge.get_pull_request (using author token) → router.switch on `mergeable` • not_mergeable → render `rebase` + dispatch to author • default → resolve_by_type("boss") + render `merge` + dispatch to boss ``` Both branches dispatch on `branch=pr.headRef` so the worktree lands on the actual PR head (not the derived `<branch_prefix>/<issue>`). ## DSL extensions (small, surgical) - **`agent.resolve_by_login` / `resolve_by_type`** gain a `gate` input — mirrors `agent.render_skill.gate`. Handler ignores the value; FILTER_DROP propagation does the gating work. - **`forge.*` nodes** gain an optional `token` input port (in addition to `args.token`). `inputs.token` wins, letting flows thread per-run tokens out of upstream `agent.resolve_by_*` envelopes instead of baking them into the JSON. ## Documented deviation When the PR author can't be resolved (rare external author), the entire pipeline drops. Legacy fires merge to boss anyway, which the merge skill's `mergeable` precondition then refuses — net behaviour matches (no merge happens), and the flow path avoids the noisy boss task. Force-merge path (`opts.force=true`, MAX_ROUNDS escape hatch) is OUT OF SCOPE here — that lives on `dispatchMerge(…, { force: true })` from the changes-requested round-cap, ported in Phase 4E. ## Cutover Set `node_flows.suppress_legacy: ["pull_request_review.submitted"]` once the new flow soaks clean on `GET /flows/divergence/summary`. ## Test plan - [x] `bun test apps/server/src/domain/flows/pr-approved-merge-graph.test.ts` — 16 tests, all pass - [x] `bun test apps/server/src/domain/flows/` — 263 tests, all pass (no regressions) - [x] `bun test apps/server/src/http/flows-routes.test.ts` — 63 tests pass (BAKED_IN_FLOWS seeding intact) - [x] Server typecheck clean - [ ] Soak in `mode: "live"` against a real approval, confirm flow + legacy emit identical dispatches via `GET /flows/divergence/summary` - [ ] Flip `suppress_legacy: ["pull_request_review.submitted"]` after a clean soak window 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-26 13:09:33 +00:00

feat(flows): NF-6 Phase 4B — pr-approved-merge baked-in flow

qa / qa (pull_request) Has been cancelled

Details

qa / dockerfile (pull_request) Has been cancelled

Details

57c6bfda0e

Ports `webhook-handlers.ts::handleApproved` (rebase precheck +
boss merge dispatch) into a baked-in node-flow graph. Ships a third
seeded flow (`pr-approved-merge`) on the
`pull_request_review.submitted` trigger:

  src → router.filter (review.state==="approved")
      → agent.resolve_by_login (PR author)
      → forge.get_pull_request (using author token)
      → router.switch on `mergeable`
          • not_mergeable → render `rebase` + dispatch to author
          • default → resolve_by_type("boss") + render `merge`
                    + dispatch to boss

Both branches dispatch on `branch=pr.headRef` so the worktree lands
on the actual PR head (not the derived `<branch_prefix>/<issue>`).

Two small DSL extensions were needed:

- `agent.resolve_by_login` / `resolve_by_type` gain a `gate` input
  (mirrors `agent.render_skill.gate`). The handler ignores the
  value; FILTER_DROP propagation does the gating work.

- `forge.*` nodes gain an optional `token` input port (in addition
  to the existing `args.token`). `inputs.token` wins, letting flows
  thread per-run tokens out of upstream `agent.resolve_by_*`
  envelopes instead of baking them into the JSON.

Documented deviation: when the PR author can't be resolved (rare
external author), the entire pipeline drops. Legacy fires merge to
boss anyway, which the merge skill's `mergeable` precondition then
refuses — net behaviour matches (no merge happens), and the flow
path avoids the noisy boss task.

Cutover gate: `node_flows.suppress_legacy:
["pull_request_review.submitted"]` skips the legacy
`webhook.ts:310-316` switch arm once the new flow soaks clean on
`GET /flows/divergence/summary`.

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

charles referenced this pull request from a commit

2026-04-26 13:14:12 +00:00

fix(flows): pr-approved-merge — round-1 review parity for boss merge dispatch

charles added 1 commit

2026-04-26 13:14:12 +00:00

fix(flows): pr-approved-merge — round-1 review parity for boss merge dispatch

qa / qa (pull_request) Failing after 3m22s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

15ddfe1787

Two real divergences vs legacy `dispatchMerge` (`webhook-ci.ts:374-433`)
caught in subagent review of #381:

1. **Skill loader was session-aware.** `agent.render_skill` always called
   `skillForEvent(type, repo, pr, "merge")` which picks `merge-delta.md`
   when a session row exists. Legacy uses `loadMergeSkill()` — plain
   `skills/merge.md` always. After the second approval on the same PR,
   the flow path silently switched templates while legacy kept reading
   the base file.

2. **Per-instance prompt + caveman appendices were applied.** Legacy
   `dispatchMerge` only fires `applyArtifactStyleAppendix`; flow ran
   all three (`applyAppendix` + `maybeApplyCavemanAppendix` +
   `applyArtifactStyleAppendix`). The boss merge prompt is a tight
   checklist that breaks under prepended per-instance guidance.

Fix: two opt-in args on `agent.render_skill`:
  - `stateless: true` → skip `skillForEvent`, call `loadSkill` directly.
  - `appendix_mode: "artifact-only" | "full" | "none"` (default `"full"`)
    → controls which appendix layers fire on top of the rendered template.

The `pr-approved-merge` flow's `render_merge` node now sets both,
matching legacy `dispatchMerge` semantics. Bumped the graph's content
version to v2 so the seeding migration rewrites the row on restart.

Tests added:
  - parity test: rendered merge task carries artifact-style ONLY
    (asserts no `prompt_appendix`, no `Agent-specific guidance` heading,
    no caveman markers — even when the boss instance has both ON).
  - regression: `mergeable=undefined` walks default port → merge to boss
    (matches `prMergeable` returning null on non-boolean).
  - regression: merge dispatch carries `branch_prefix: "boss"` (parity
    with `buildAgentRequest`).

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

charles added 1 commit

2026-04-26 13:19:04 +00:00

fix(flows): widen mkAgentEnvelope.prompt_appendix typing for parity test mutation

qa / qa (pull_request) Successful in 6m32s

Details

qa / dockerfile (pull_request) Successful in 14s

Details

10a144b09d

Test fixture's `prompt_appendix: null` literal narrowed to `null` only,
blocking the parity-assert that mutates it to a string marker. Cast to
`string | null` matches the production `ResolvedAgentEnvelope` field
type.

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

charles merged commit 4741bfb254 into main

2026-04-26 13:25:51 +00:00

charles deleted branch feat/flows-pr-approved-merge

2026-04-26 13:25:52 +00:00

charles referenced this pull request from a commit

2026-04-26 13:25:52 +00:00

feat(flows): NF-6 Phase 4B — pr-approved-merge baked-in flow (#381)

claude-desktop referenced this pull request

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

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

charles referenced this pull request from a commit

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