feat(flows): NF-6 Phase 4C — review-requested baked-in flow #382

Merged

charles merged 2 commits from feat/flows-review-requested into main 2026-04-26 13:47:56 +00:00

Conversation 0 Commits 2 Files changed 8 +675 -5

claude-desktop commented 2026-04-26 13:33:29 +00:00

Collaborator

Copy link

Summary

Ports webhook-handlers.ts::handleReviewRequested into a baked-in node-flow graph. Ships review-requested as the fourth seeded flow on the pull_request_review.review_requested trigger.

src → agent.resolve_by_login (requested reviewer)
    → forge.get_pull_request (using reviewer token)
    → forge.guard_reviewer_dispatch (same-SHA verdict guard, #117)
    → forge.fetch_pr_dispatch_labels (PR + linked-issue label merge)
    → agent.render_skill (review skill, per-agent variant)
    → agent.dispatch (stateless_session: true, branch=pr.headRef)

DSL extensions

1. agent.dispatch.stateless_session arg/input → TaskRequest.stateless_session. Mirrors legacy buildAgentRequest(..., { stateless_session: true }) for review dispatch — without it, a resumed reviewer rubber-stamps "same diff, same verdict" and silently shadows updated skill instructions.

2. agent.render_skill auto-applies skillForAgent(agent.type, base). Designer pair gets design-implement.md / design-review.md instead of the generic templates. Without this the flow path silently switched designer dispatches to generic implement.md — a real divergence on every designer/design-reviewer dispatch.

3. Two new injection-shaped forge. nodes*:

   • forge.guard_reviewer_dispatch → wraps guardReviewerDispatch from domain/workflow/review-loop.ts. Two ports: dispatch (forwards on pass) / skip (carries reason on trip).
   • forge.fetch_pr_dispatch_labels → wraps fetchPrDispatchLabels from webhook-handlers.ts (newly exported). Returns merged PR + linked-issue label set.

Both follow the soft-fail-on-probe-error convention from forge.detect_outstanding_change_request so a transient 5xx degrades gracefully.

Cutover

node_flows.suppress_legacy: ["pull_request_review.review_requested"] skips the legacy switch arms at webhook.ts:263-292 (both pull_request_review_request event and the pull_request action=review_requested fallback) once the flow soaks clean.

Test plan

• 11 e2e tests pass: happy path, same-SHA guard skip, unresolved reviewer drop, design-reviewer override
• Full flows suite: 276 pass / 0 fail
• flows-routes: 63 pass (BAKED_IN_FLOWS seeding intact)
• Server typecheck clean
• AGENT_NODE_COUNT bumped from 8 to 10
• Soak in mode: "live" against real review request, confirm parity via GET /flows/divergence/summary
• Flip suppress_legacy: ["pull_request_review.review_requested"] after clean soak

🤖 Generated with Claude Code

## Summary Ports `webhook-handlers.ts::handleReviewRequested` into a baked-in node-flow graph. Ships `review-requested` as the fourth seeded flow on the `pull_request_review.review_requested` trigger. ``` src → agent.resolve_by_login (requested reviewer) → forge.get_pull_request (using reviewer token) → forge.guard_reviewer_dispatch (same-SHA verdict guard, #117) → forge.fetch_pr_dispatch_labels (PR + linked-issue label merge) → agent.render_skill (review skill, per-agent variant) → agent.dispatch (stateless_session: true, branch=pr.headRef) ``` ## DSL extensions 1. **`agent.dispatch.stateless_session`** arg/input → `TaskRequest.stateless_session`. Mirrors legacy `buildAgentRequest(..., { stateless_session: true })` for review dispatch — without it, a resumed reviewer rubber-stamps "same diff, same verdict" and silently shadows updated skill instructions. 2. **`agent.render_skill` auto-applies `skillForAgent(agent.type, base)`**. Designer pair gets `design-implement.md` / `design-review.md` instead of the generic templates. Without this the flow path silently switched designer dispatches to generic `implement.md` — a real divergence on every designer/design-reviewer dispatch. 3. **Two new injection-shaped forge.* nodes**: - `forge.guard_reviewer_dispatch` → wraps `guardReviewerDispatch` from `domain/workflow/review-loop.ts`. Two ports: `dispatch` (forwards on pass) / `skip` (carries reason on trip). - `forge.fetch_pr_dispatch_labels` → wraps `fetchPrDispatchLabels` from `webhook-handlers.ts` (newly exported). Returns merged PR + linked-issue label set. Both follow the soft-fail-on-probe-error convention from `forge.detect_outstanding_change_request` so a transient 5xx degrades gracefully. ## Cutover `node_flows.suppress_legacy: ["pull_request_review.review_requested"]` skips the legacy switch arms at `webhook.ts:263-292` (both `pull_request_review_request` event and the `pull_request` action=review_requested fallback) once the flow soaks clean. ## Test plan - [x] 11 e2e tests pass: happy path, same-SHA guard skip, unresolved reviewer drop, design-reviewer override - [x] Full flows suite: 276 pass / 0 fail - [x] flows-routes: 63 pass (BAKED_IN_FLOWS seeding intact) - [x] Server typecheck clean - [x] AGENT_NODE_COUNT bumped from 8 to 10 - [ ] Soak in `mode: "live"` against real review request, confirm parity via `GET /flows/divergence/summary` - [ ] Flip `suppress_legacy: ["pull_request_review.review_requested"]` after clean soak 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit 2026-04-26 13:33:29 +00:00

feat(flows): NF-6 Phase 4C — review-requested baked-in flow ... 9faf6b0845

Ports `webhook-handlers.ts::handleReviewRequested` into a baked-in
node-flow graph. Ships `review-requested` as the fourth seeded flow on
the `pull_request_review.review_requested` trigger:

  src → agent.resolve_by_login (requested reviewer)
      → forge.get_pull_request (using reviewer token)
      → forge.guard_reviewer_dispatch (same-SHA verdict guard, #117)
      → forge.fetch_pr_dispatch_labels (PR + linked-issue label merge)
      → agent.render_skill (review skill, per-agent variant)
      → agent.dispatch (stateless_session: true, branch=pr.headRef)

Three DSL extensions land alongside the flow:

1. `agent.dispatch` gains a `stateless_session: true` arg/input that
   propagates to `TaskRequest.stateless_session`. Mirrors the legacy
   `buildAgentRequest(..., { stateless_session: true })` wrapper used
   for review dispatch — without it, a resumed reviewer rubber-stamps
   "same diff, same verdict" and silently shadows updated skill
   instructions.

2. `agent.render_skill` now applies `skillForAgent(agent.type, base)`
   automatically. The designer pair gets `design-implement.md` /
   `design-review.md` / etc. instead of the generic templates. Without
   this the flow path silently switched designer dispatches to the
   generic `implement.md` — a real divergence on every
   designer/design-reviewer dispatch.

3. Two new injection-shaped forge.* nodes wrap legacy helpers:
   - `forge.guard_reviewer_dispatch` → `guardReviewerDispatch` from
     `domain/workflow/review-loop.ts`. Two ports: `dispatch` (forwards
     when guard passes) / `skip` (carries the reason when it trips).
   - `forge.fetch_pr_dispatch_labels` → `fetchPrDispatchLabels` from
     `webhook-handlers.ts` (newly exported). Returns the merged
     PR + linked-issue label set on the `labels` port.

Both new nodes follow the soft-fail-on-probe-error convention from
`forge.detect_outstanding_change_request` so a transient 5xx degrades
gracefully rather than dropping the dispatch.

Cutover gate: `node_flows.suppress_legacy:
["pull_request_review.review_requested"]` skips the legacy switch arms
at `webhook.ts:263-292` (both `pull_request_review_request` and the
`pull_request` action=review_requested fallback) once the flow soaks
clean on `GET /flows/divergence/summary`.

Tests: 11 new e2e tests covering happy path, same-SHA guard skip,
unresolved reviewer drop, designer per-agent skill override.
AGENT_NODE_COUNT bumped from 8 to 10.

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

charles referenced this pull request from a commit 2026-04-26 13:40:44 +00:00

fix(flows): review-requested — round-1 review parity for guard + branch fallback

charles added 1 commit 2026-04-26 13:40:44 +00:00

fix(flows): review-requested — round-1 review parity for guard + branch fallback ... 7853f4df71

Three issues caught in subagent review of #382:

1. **BLOCKER — null `headSha` hard-failed.**
   `forge.guard_reviewer_dispatch` called `asString(headSha, …)`
   unconditionally, so a draft / freshly-created PR (where Forgejo
   hasn't yet materialised the head commit) threw and suppressed
   dispatch. Legacy `webhook-handlers.ts:649` gates the entire guard
   block on `if (headSha)` — null means no-op, dispatch proceeds.
   Fix: handler-level null gate that short-circuits to `dispatch: true`
   when `headSha` is empty/missing.

2. **IMPORTANT — soft-fail on guard probe was a regression.**
   The previous try/catch fell back to `dispatch: true` on any guard
   error — which would re-dispatch against a SHA the reviewer already
   verdicted on, doubling token spend in a bounce-review storm
   (issue #117). Legacy `handleReviewRequested` doesn't try/catch
   around `guardReviewerDispatch` — exceptions propagate, dispatch
   skipped. Removed the try/catch so errors hard-fail the flow run.

3. **IMPORTANT — branch fallback to webhook payload was missing.**
   Legacy uses `prDetail?.headRef || pr.head?.ref`
   (`webhook-handlers.ts:665`); flow used `get_pr.pr.headRef` only.
   When `forge.get_pull_request` returns null (adapter call failed),
   the dispatch landed on the worker-derived `<branch_prefix>/<issue>`
   instead of the webhook payload's actual head ref. Added
   `branch_ref` (`router.race`) that picks the PR detail's headRef
   when present, else falls back to `src.out.pr.headRef`.

Tests added (3, total 14):
- null `headSha` short-circuits guard, still dispatches
- guard probe throws → `result.status === "error"`, no dispatch
- `prDetail = null` → branch falls back to trigger payload's headRef

Graph version bumped to v2 so the seeding migration rewrites the row
on restart.

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

charles merged commit 77e9358a72 into main 2026-04-26 13:47:56 +00:00

charles deleted branch feat/flows-review-requested 2026-04-26 13:47:56 +00:00

charles referenced this pull request from a commit 2026-04-26 13:47:57 +00:00

feat(flows): NF-6 Phase 4C — review-requested baked-in flow (#382)

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

No description provided.