feat(flows): NF-6 Phase 4E — pr-changes-requested baked-in flow #384

Merged

charles merged 2 commits from feat/flows-pr-changes-requested into main 2026-04-26 14:59:45 +00:00

Conversation 0 Commits 2 Files changed 7 +865 -8

claude-desktop commented 2026-04-26 14:21:20 +00:00

Collaborator

Copy link

Summary

Ports webhook-handlers.ts::handleChangesRequested (lines 751-817) + domain/workflow/review-loop.ts::guardAuthorDispatch into a baked-in node-flow graph. Ships pr-changes-requested as the sixth seeded flow on the pull_request_review.submitted trigger.

src → router.filter (review.state === "changes_requested")
    → 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 →
            agent.reviewer_for_pr (label-aware reviewer pick)
            forge.guard_author_dispatch (round-cap, #117)
                • proceed → fetch labels + render `address-review`
                            + dispatch to author
                • force_merge → forge.dispatch_force_merge to boss
                                with FORCE MERGE preamble + SSE

DSL extensions (3 new nodes)

1. forge.guard_author_dispatch → wraps guardAuthorDispatch. Two ports: proceed (pass) / force_merge (carries {reviewer, rounds} on cap-trip).

2. agent.reviewer_for_pr → wraps reviewerForPr from webhook-routing.ts. Picks reviewer agent type considering label match (M17-4 / #155) + author mapping. Without this the round-cap guard would count the wrong reviewer's submissions on dev-authored area:dashboard PRs.

3. forge.dispatch_force_merge → wraps dispatchMerge(…, { force: true, reviewer, rounds }). Encapsulating the whole thing as one node preserves the force_merge_dispatched SSE broadcast (issue #141) and the ⚠️ FORCE MERGE preamble that instructs boss to skip the "latest review APPROVED" check. Splitting into render + dispatch primitives would require teaching agent.dispatch about SSE.

Also added gate input to forge.guard_author_dispatch and agent.reviewer_for_pr so the rebase branch (mergeable=false) drops the entire guard + dispatch chain via FILTER_DROP propagation.

Cutover

Same gate as pr-approved-merge (Phase 4B): node_flows.suppress_legacy: ["pull_request_review.submitted"] skips BOTH pull_request_approved AND pull_request_rejected legacy switch arms at webhook.ts:300-316. Only flip once BOTH pr-approved-merge AND this flow have soaked clean on GET /flows/divergence/summary. Documented in the loader header.

Test plan

• 12 new e2e tests: non-changes-requested no-op, happy-path address-review, rebase precheck, round-cap force-merge, author unresolved drop
• Full flows suite: 368 pass / 0 fail
• Server typecheck clean
• AGENT_NODE_COUNT bumped from 11 to 14
• Soak in mode: "live" against real REQUEST_CHANGES review, confirm parity via GET /flows/divergence/summary
• Soak round-cap path by intentionally hitting MAX_ROUNDS on a test PR
• Flip suppress_legacy: ["pull_request_review.submitted"] after BOTH 4B + 4E soak

🤖 Generated with Claude Code

## Summary Ports `webhook-handlers.ts::handleChangesRequested` (lines 751-817) + `domain/workflow/review-loop.ts::guardAuthorDispatch` into a baked-in node-flow graph. Ships `pr-changes-requested` as the sixth seeded flow on the `pull_request_review.submitted` trigger. ``` src → router.filter (review.state === "changes_requested") → 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 → agent.reviewer_for_pr (label-aware reviewer pick) forge.guard_author_dispatch (round-cap, #117) • proceed → fetch labels + render `address-review` + dispatch to author • force_merge → forge.dispatch_force_merge to boss with FORCE MERGE preamble + SSE ``` ## DSL extensions (3 new nodes) 1. **`forge.guard_author_dispatch`** → wraps `guardAuthorDispatch`. Two ports: `proceed` (pass) / `force_merge` (carries `{reviewer, rounds}` on cap-trip). 2. **`agent.reviewer_for_pr`** → wraps `reviewerForPr` from `webhook-routing.ts`. Picks reviewer agent type considering label match (M17-4 / #155) + author mapping. Without this the round-cap guard would count the wrong reviewer's submissions on dev-authored `area:dashboard` PRs. 3. **`forge.dispatch_force_merge`** → wraps `dispatchMerge(…, { force: true, reviewer, rounds })`. Encapsulating the whole thing as one node preserves the `force_merge_dispatched` SSE broadcast (issue #141) and the `⚠️ FORCE MERGE` preamble that instructs boss to skip the "latest review APPROVED" check. Splitting into render + dispatch primitives would require teaching `agent.dispatch` about SSE. Also added `gate` input to `forge.guard_author_dispatch` and `agent.reviewer_for_pr` so the rebase branch (mergeable=false) drops the entire guard + dispatch chain via FILTER_DROP propagation. ## Cutover Same gate as `pr-approved-merge` (Phase 4B): `node_flows.suppress_legacy: ["pull_request_review.submitted"]` skips BOTH `pull_request_approved` AND `pull_request_rejected` legacy switch arms at `webhook.ts:300-316`. **Only flip once BOTH `pr-approved-merge` AND this flow have soaked clean** on `GET /flows/divergence/summary`. Documented in the loader header. ## Test plan - [x] 12 new e2e tests: non-changes-requested no-op, happy-path address-review, rebase precheck, round-cap force-merge, author unresolved drop - [x] Full flows suite: 368 pass / 0 fail - [x] Server typecheck clean - [x] AGENT_NODE_COUNT bumped from 11 to 14 - [ ] Soak in `mode: "live"` against real REQUEST_CHANGES review, confirm parity via `GET /flows/divergence/summary` - [ ] Soak round-cap path by intentionally hitting `MAX_ROUNDS` on a test PR - [ ] Flip `suppress_legacy: ["pull_request_review.submitted"]` after BOTH 4B + 4E soak 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit 2026-04-26 14:21:20 +00:00

feat(flows): NF-6 Phase 4E — pr-changes-requested baked-in flow ... f60bd31878

Ports `webhook-handlers.ts::handleChangesRequested` (lines 751-817) +
`domain/workflow/review-loop.ts::guardAuthorDispatch` into a baked-in
node-flow graph. Ships `pr-changes-requested` as the sixth seeded flow
on the `pull_request_review.submitted` trigger:

  src → router.filter (review.state === "changes_requested")
      → 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 →
              agent.reviewer_for_pr (label-aware reviewer pick)
              forge.guard_author_dispatch (round-cap, #117)
                  • proceed → fetch labels + render `address-review`
                              + dispatch to author
                  • force_merge → forge.dispatch_force_merge to boss
                                  with FORCE MERGE preamble + SSE

Three new injection-shaped nodes:

1. `forge.guard_author_dispatch` → wraps `guardAuthorDispatch` from
   `domain/workflow/review-loop.ts`. Two ports: `proceed` (fires on
   pass) / `force_merge` (carries `{ reviewer, rounds }` on cap-trip).

2. `agent.reviewer_for_pr` → wraps `reviewerForPr` from
   `webhook-routing.ts`. Picks reviewer agent type for the PR
   considering label match (M17-4 / #155) + author mapping. Without
   this the round-cap guard would count the wrong reviewer's
   submissions on dev-authored `area:dashboard` PRs.

3. `forge.dispatch_force_merge` → wraps `dispatchMerge(…, { force: true,
   reviewer, rounds })` from `webhook-ci.ts`. Encapsulating the whole
   thing as one node preserves the `force_merge_dispatched` SSE
   broadcast (issue #141) and the `⚠️ FORCE MERGE` preamble that
   instructs the boss skill to skip the "latest review APPROVED"
   precondition. Splitting into render + dispatch primitives would
   require teaching `agent.dispatch` about SSE.

Also added `gate` input to `forge.guard_author_dispatch` and
`agent.reviewer_for_pr` so the rebase branch (mergeable=false) drops
the entire guard + dispatch chain via FILTER_DROP propagation.

Cutover: `node_flows.suppress_legacy:
["pull_request_review.submitted"]` skips BOTH the `pull_request_approved`
AND `pull_request_rejected` legacy switch arms at `webhook.ts:300-316`.
Only flip once BOTH `pr-approved-merge` (Phase 4B) AND this flow have
soaked clean on `GET /flows/divergence/summary`. Documented prominently
in the loader header.

Tests: 12 new e2e tests covering non-changes-requested no-op, happy
path, rebase precheck, round-cap force-merge, author unresolved drop.
AGENT_NODE_COUNT bumped from 11 to 14.

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

charles referenced this pull request from a commit 2026-04-26 14:27:05 +00:00

fix(flows): pr-changes-requested — round-1 review parity for caveman appendix

charles added 1 commit 2026-04-26 14:27:05 +00:00

fix(flows): pr-changes-requested — round-1 review parity for caveman appendix ... 53a9c47387

One IMPORTANT divergence + 2 test gaps caught in subagent review of #384:

1. **Caveman appendix divergence on address-review.** Legacy
   `handleChangesRequested` (`webhook-handlers.ts:797-806`)
   deliberately omits `maybeApplyCavemanAppendix` from the address-
   review prompt chain — only `applyAppendix` +
   `applyArtifactStyleAppendix`. The flow's `render_address` node
   used the default `appendix_mode: "full"` which fires caveman, so a
   chore PR receiving REQUEST_CHANGES would silently get
   caveman-compressed address-review prompts under flow mode where
   legacy did not.

   Fix: added `appendix_mode: "no-caveman"` to `agent.render_skill`
   (skips caveman, keeps `applyAppendix` + artifact-style). Wired the
   flag on the `render_address` node. Bumped graph version to v2 so
   the seeding migration rewrites the row on restart.

   `handleReviewRequested` (Phase 4C) DOES apply caveman intentionally
   (`webhook-handlers.ts:693`), so the flow's full-mode default is
   correct there — only the address-review path needs the opt-out.

2. **Test gap: `mergeable === null`.** Added a parity test pinning
   the fall-through arm (matches legacy `mergeable !== false`).

3. **Test gap: address-review caveman opt-out.** Added an assertion
   that the rendered prompt carries the per-instance prompt_appendix
   (legacy applies it) but NOT the caveman appendix marker (legacy
   doesn't), even when `token_economy.caveman = true` is forced ON.

Tests: 12 → 14, all pass. Typecheck clean.

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

claude-desktop referenced this pull request 2026-04-26 14:33:18 +00:00

feat(flows): NF-6 Phase 4F — issue-closed-deps baked-in flow #385

charles referenced this pull request from a commit 2026-04-26 14:45:37 +00:00

feat(flows): NF-6 Phase 4F — issue-closed-deps baked-in flow (#385)

charles referenced this pull request from a commit 2026-04-26 14:48:22 +00:00

fix(flows): pr-changes-requested — round-1 review parity for caveman appendix

charles force-pushed feat/flows-pr-changes-requested from 53a9c47387 to 32c392eb6e 2026-04-26 14:48:22 +00:00 Compare

charles merged commit a6b8ec9107 into main 2026-04-26 14:59:45 +00:00

charles referenced this pull request from a commit 2026-04-26 14:59:46 +00:00

feat(flows): NF-6 Phase 4E — pr-changes-requested baked-in flow (#384)

charles deleted branch feat/flows-pr-changes-requested 2026-04-26 14:59:46 +00:00

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

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

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

Subscribe

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!384

No description provided.