diff --git a/work/v2-phase-2/spec.md b/work/v2-phase-2/spec.md index 2efc5c5..e18128a 100644 --- a/work/v2-phase-2/spec.md +++ b/work/v2-phase-2/spec.md @@ -6,13 +6,34 @@ drafted: 2026-08-26 # Spec: v2 autonomous lane (Phase 2) From the accepted intent: the chain must advance without a live session — merged -intent → spec PR, merged spec → implementation PR, every PR reviewed with a bounded -fix loop; the operator's only actions are the gate merges. +intent → spec PR, merged spec → implementation PR, every PR reviewed. The operator's +actions are the gate merges, plus answering review findings in a session until the +deferred fix stage lands (see the amendment note). + +**Amended 2026-08-27 (operator ruling at the review cap).** This phase ships three +workflows, not four: the fix loop (R4) is deferred to its own intent. Two reasons, +stated here so this spec stands on its own: R5 allows two `claude[bot]` trigger +edges — review-of-agent-PRs, and the fix-on-review comment. The fix stage needed +both of those **plus** a third, the dispatch that re-runs review after a push-back +that changes no code; and it was the one job holding write +credentials, running PR-authored code, and reading untrusted PR text at once — it +earns a design from scratch rather than patches. + +**Where the next intent starts — split the credential**, so no one job holds all three +powers: the agent that edits the PR's code holds no token and runs nothing from the PR; +CI verifies the result on the pushed branch as it verifies any branch; a deterministic +step holds the app credential and does the writing. Answering review findings stays the +operator's job in a session until that intent lands. This is a scope reduction the +operator ruled on; no rail is weakened by it. ## Requirements -Each requirement is verifiable; R1–R4 are jointly proven by the phase exit test -(one real change flows through the lane with the operator only merging). +Each requirement is verifiable; R1–R3 are jointly proven by the phase exit test: +one real change flows through the lane, the operator merging at each gate and +answering whatever the review finds. The run counts only if the review actually +reported something and the operator resolved it in a session — a smoke change so +clean that nothing was found proves the happy path, not the handoff this phase +depends on. R4 is deferred with the fix stage — see the amendment note above. - **R1 — spec stage.** An operator merge to main that adds or changes `work//intent.md` triggers a job that opens (or updates) a PR titled @@ -29,19 +50,60 @@ Each requirement is verifiable; R1–R4 are jointly proven by the phase exit tes - **R3 — review stage.** Every same-repo PR gets a review per `REVIEW.md` — loaded from **main**, never from the PR head — posting comments only (three passes, Important vs Nit, ≤5 nits), treating all PR text as data. -- **R4 — bounded fix loop.** Review findings trigger a fix pass that bumps the - `round-N` label **before** acting; at `round-3` it applies `needs-human`, posts - the productive-cap comment (land the converged core, split the remainder), and - stops. Fix pushes re-trigger review; the labels bound the loop. -- **R5 — safety invariants (all four workflows).** One global `claude-quota` - concurrency group serializes every agent job; explicit actor gates - (`github.actor == operator`, or `allowed_bots: claude[bot]` only on the two - deliberately-opened bot edges: review-of-agent-PRs and fix-on-review-comment); +- **R4 — bounded fix loop. DEFERRED** to its own intent (see the amendment note + above). It described: review findings trigger a fix pass that bumps the `round-N` + label before acting; at `round-3` it applies `needs-human`, posts the productive-cap + comment, and stops. That design is superseded — the next intent starts from the + credential split stated in the amendment note above, not from this text and not + from any plan written for the four-workflow shape. +- **R5 — safety invariants (all three workflows in this phase).** One global `claude-quota` + concurrency group serializes every agent job. + + **Every gate checks BOTH actors — one rule, no exceptions.** On a re-run + `github.actor` stays whoever (or whatever) started the original run, and the + person who pressed re-run appears only in `github.triggering_actor`. So any gate + written on `github.actor` alone lets a write-capable collaborator re-run someone + else's job — an operator's dispatch, or a bot-triggered review — and spend the + operator's subscription under a passing check. So every job in this lane checks + the triggering actor as well, and accepts exactly two: **the operator** — who may + re-run anything by hand, including a bot-originated review that failed + transiently — **or `claude[bot]` itself**, which is what the triggering actor is + on a run the lane started. Anyone else is refused, whatever `github.actor` or + `allowed_bots` says. The probe workflow already carries both checks; this rule + exists so no new job forgets them. + + Which gates: the two stage jobs and any dispatch are operator-gated; the review + job opens exactly one bot edge, review-of-agent-PRs (`allowed_bots: claude[bot]`). + R5's second allowed edge, the fix-on-review comment, went with the deferred stage, + and the third edge that stage would also have needed is precisely why it was + deferred. Then: `timeout-minutes` and `--max-turns` on every job; PR-creation steps assert the - PR exists and fail loudly; the fix stage never pushes to a PR that already has - an approval; stage write-limits stated in the stage skills (mechanical - enforcement arrives with Phase 3 hooks). -- **R6 — plumbing proven first.** Before the four workflows are finalized, a + PR exists and fail loudly; stage write-limits stated in the stage skills + (mechanical enforcement arrives with Phase 3 hooks). **No stage pushes to a PR + the operator has already approved** — R1 and R2 let a re-triggered producer + stage update an existing PR, so this is not only the deferred fix stage's + concern: an approval means the operator read that diff, and a later push would + silently move what they approved. When a producer stage would have to update an + already-approved PR — the upstream artifact moved while the PR sat approved and + unmerged — it does not push. It labels the PR `stale`, says so in one comment, + and stops. **Merging it is not one of the ways out:** its recorded upstream hash + no longer matches main, so merging would land an artifact the chain reads as + stale and no stage would rebuild it — the merge changes the artifact, not the + upstream that moved. The operator closes the stale PR, then **re-runs the stage + by dispatch** — closing a PR or dismissing an approval fires no event, so the + push-triggered stages would otherwise never wake and the slug would sit stranded. + Both stage workflows therefore carry `workflow_dispatch` alongside their push + trigger, gated to the operator like every other dispatch here. The one-PR-per-slug + rule means **one OPEN PR per slug**: a closed PR is history, so the rebuild opens + a fresh one rather than reopening the stale PR and pushing into an approval. **The runaway + brake counts push-triggered runs only** (`--event push`): a dispatch by anyone + who fails the actor gate still records a run whose job was skipped, and counting + those would let a collaborator spam dispatches until the brake trips and starves + the lane. Push-to-main runs keep the property the brake needs — the ruleset means + only an operator merge causes them, so such a run always means the agent ran. An approved PR + belongs to the operator, so the lane never edits one behind them; and a stale one + is rebuilt, never merged. +- **R6 — plumbing proven first.** Before the three workflows are finalized, a disposable `workflow_dispatch` test proves: the action (app token, no `github_token` input) can push a branch and create a PR via allowlisted `gh pr create`, and that app-created events trigger downstream workflows. @@ -61,22 +123,24 @@ Each requirement is verifiable; R1–R4 are jointly proven by the phase exit tes At the file/component altitude; order of work belongs to `plan.md`. - **Workflows** (`.github/workflows/`): `spec-on-intent.yml`, - `implement-on-spec.yml`, `review-on-pr.yml`, `fix-on-review.yml` — per the v2 - design doc's skeletons: push-to-main path triggers for the two stage jobs; - `pull_request` (same-repo guard, per-PR `cancel-in-progress`) for review; - `issue_comment` with the reviewer's `YSTACK-REVIEW` marker for the fix pass. - All use `claude_code_oauth_token`, no `github_token` input (app-token events - must cascade), and invoke stage skills as their prompt. + `implement-on-spec.yml`, `review-on-pr.yml` — push-to-main path triggers for + the two stage jobs **plus `workflow_dispatch` on each, so the operator can + restart a stage after clearing a stale PR** (see the approved-PR rule in R5), + and `pull_request` (same-repo guard, per-PR `cancel-in-progress`) for review. All use `claude_code_oauth_token`, no + `github_token` input (app-token events must cascade), and invoke stage skills + as their prompt. (`fix-on-review.yml` and its `issue_comment` trigger belonged + to the deferred R4 — see the amendment note.) - **Stage skills** (`.claude/skills/`): reuse `spec-draft` and `plan-draft` unchanged; add `implement` (runs plan-draft first, then codes to the plan, - runs verify, opens the impl PR), `review-pr` (applies REVIEW.md), and - `address-review` (adopt-or-push-back per finding; never touches test files - during a fix — hook-enforced in Phase 3). + runs verify, opens the impl PR) and `review-pr` (applies REVIEW.md). + (`address-review` belonged to the deferred R4.) - **Helpers** (`scripts/v2/`, each with a hermetic test in `scripts/test/`): `pending-spec.sh` / `pending-impl.sh` (which slug needs work — the - hash-comparison idempotency guards), `round-cap.sh` (read/bump round labels, - emit proceed/stop), `quota-preflight.sh` (count recent agent runs, skip over - budget). Deterministic bash, no model calls. + hash-comparison idempotency guards) and `quota-preflight.sh` (count recent + agent runs, stop over budget). Deterministic bash, no model calls. + (`round-cap.sh` is already merged and stays in the repo, but nothing in this + phase calls it: round labels bound the fix loop, which is deferred. It wakes + again with that intent.) - **Model policy:** stage skills carry it — producer stages (`implement`) at the fixed coder ceiling from `config/models.conf`; gate stages (`review-pr`) at high effort, never downgraded (v1 tiering, unchanged). @@ -101,8 +165,8 @@ first; adding Codex is additive and needs no workflow changes). one-concern budget. Recommendation for the plan stage: land as a stacked sequence under one `plan.md` — (a) helpers + tests, (b) workflows + skills + docs — each PR independently green. -3. **Quota:** all four jobs share the operator's subscription window with their - interactive use. Serialization + preflight (R5) mitigate; the sizing question +3. **Quota:** the three jobs share the operator's subscription window with the + operator's own interactive use. Serialization + preflight (R5) mitigate; the sizing question stays open until measured in this phase. 4. **North-star fit:** user-directed intent (operator-approved at G1), so no consensus gate required. The lane serves north star B indirectly — an