From 243ca8ea21e1cf43a9b073fd6d6592c0bc2f3c91 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 28 Oct 2026 02:50:35 +0000 Subject: [PATCH] fix(ship): one decision each for the review-loop exits, the planned adversarial skip and Step 16's TODOS rerun (screen judge repair) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 3 ++- scripts/resolvers/outside-voice-steps.ts | 14 +++++----- scripts/resolvers/review-dashboard.ts | 4 +-- ship/SKILL.md | 21 +++++++-------- ship/SKILL.md.tmpl | 18 ++++++------- ship/sections/adversarial.md | 14 +++++----- ship/sections/adversarial.md.tmpl | 3 ++- ship/sections/review-army.md | 26 +++++++++---------- ship/sections/review-army.md.tmpl | 26 +++++++++---------- test/fixtures/golden/agy-ship-SKILL.md | 21 +++++++-------- test/fixtures/golden/claude-ship-SKILL.md | 21 +++++++-------- test/fixtures/golden/codex-ship-SKILL.md | 21 +++++++-------- test/fixtures/golden/factory-ship-SKILL.md | 21 +++++++-------- test/outside-voice-preflight.test.ts | 2 +- test/ship-control-flow.test.ts | 2 +- .../garrytan-gsta131-token-efficiency.json | 4 +-- test/skill-validation.test.ts | 7 ++--- 17 files changed, 110 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6b17c899..9eec3ba85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,9 +18,10 @@ - Specialist prompts cap findings at 400 words. `docs/unattended.md` adds report-length limits (READY at most 150 words plus required lines, reports at most 300). ### Gate integrity -- Ship's union growth is a +1,877-byte entry in GSTA-132's size ledger (test/size-ledger/ship/garrytan-gsta131-token-efficiency.json; measured 330,901 bytes, SIZE_LEDGER: ok), and the ship skeleton cap in test/helpers/carve-guards.ts moves 60,913 → 60,960 bytes, pinned at the measured size. The five /review size pins in test/fixtures/skill-md-size-allowlist.json go down by 2 bytes each. +- Ship's union growth is a +1,621-byte entry in GSTA-132's size ledger (test/size-ledger/ship/garrytan-gsta131-token-efficiency.json; measured 330,645 bytes, SIZE_LEDGER: ok), and the ship skeleton cap in test/helpers/carve-guards.ts moves 60,913 → 60,960 bytes, pinned at the measured size. The five /review size pins in test/fixtures/skill-md-size-allowlist.json go down by 2 bytes each. - `test/autoplan-token-efficiency.test.ts` gives its subprocess-driven cases a 30-second timeout; its resumed-run case persists `max_rounds: 1` instead of 3, a value inside the new cap of 2. - `test/ship-receipt-carry-pregate.test.ts` joins `KNOWN_WINDOWS_INCOMPATIBLE` in `scripts/lib/windows-curation.ts`, like the four other tests that pass `--pregate` to the receipt CLI. On a Windows host the fixture's Linux lane is a requires-remote obligation, so `write` refuses the pregate (windows-free-shard run 40611810409). It still runs on Linux and macOS. +- Ship's Step 11 now runs the adversarial passes the review plan dispatches, so three prose pins move with it: `test/skill-validation.test.ts` and `test/outside-voice-preflight.test.ts` expect the ship heading without "always-on" and the native pass "on `DISPATCH: adversarial-native`" (/review keeps "always runs"), and `test/ship-control-flow.test.ts` pins the one plan call that picks decisions 2–4. ## [1.91.286.0] - 2026-10-28 diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index 93d403b6f..f95740e02 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -116,7 +116,7 @@ If A: revise the premise and note the revision. If B: proceed (and note that the // ─── Adversarial Review (always-on) ────────────────────────────────── function adversarialNativePass(ctx: TemplateContext, isShip: boolean): string { - return `### ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent (always runs) + return `### ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent (${isShip ? 'on \`DISPATCH: adversarial-native\`' : 'always runs'}) Before dispatch, run \`~/.claude/skills/gstack/bin/gstack-review-log --start adversarial-review\` and save the returned token for this native attempt. Do the same before each outside @@ -235,9 +235,9 @@ export function generateAdversarialStep(ctx: TemplateContext): string { const isShip = ctx.skillName === 'ship'; const stepNum = isShip ? '11' : '4.8'; - return `## Step ${stepNum}: Adversarial review (always-on) + return `## Step ${stepNum}: Adversarial review${isShip ? '' : ' (always-on)'} -Every diff gets the ${outsideVoiceFor(ctx).nativeLabel} adversarial pass. Add ${outsideVoiceFor(ctx).label} when its preflight is ready; unavailable or disabled outside coverage stays explicit. +${isShip ? 'Unavailable' : `Every diff gets the ${outsideVoiceFor(ctx).nativeLabel} adversarial pass. Add ${outsideVoiceFor(ctx).label} when its preflight is ready; unavailable`} or disabled outside coverage stays explicit. **Detect diff size:** @@ -252,8 +252,8 @@ echo "DIFF_SIZE: $DIFF_TOTAL" ${outsideVoicePreflight(ctx, { disabledBehavior: 'codex-only', nativeReview: true })} \`CODEX_MODE: disabled\` means skip the ${outsideVoiceFor(ctx).label} passes ONLY. -\`ready\` and \`unverified\` run them; every other mode skips them with the printed reason. -The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs. +\`ready\` and \`unverified\` run them; every other mode skips them with the printed reason.${isShip ? '' : ` +The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs.`} **User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the ${outsideVoiceFor(ctx).label} structured review regardless of diff size (still requires \`CODEX_MODE: ready\` (or \`unverified\`)). @@ -274,7 +274,7 @@ ${outsideVoiceProvenance(ctx, 'adversarial')} After all passes complete, synthesize findings across all sources: \`\`\` -ADVERSARIAL REVIEW SYNTHESIS (always-on, N lines): +ADVERSARIAL REVIEW SYNTHESIS (${isShip ? '' : 'always-on, '}N lines): ════════════════════════════════════════════════════════════ High confidence (found by multiple sources): [findings agreed on by >1 pass] Unique to the parent checklist/specialists: [from earlier steps] @@ -309,7 +309,7 @@ in order before leaving Step 11: Step 9 completes full review before fixes; any further repair inserts its checks ahead of the remaining items. These fresh reviews after code edits are not recovery retries. Returning here never resets Step 9's three-cycle fix limit. -3. **Native complete with no queued fixes:** Finish the memory updates below, +3. **Native complete with no queued fixes, or not dispatched:** Finish the memory updates below, then continue to Step 11.5. Never jump directly to release preparation.` : 'The native pass is required for Step 5.8 completion. Optional outside failures remain separately recorded, not completed by native coverage. Return all findings and structured-review decisions to Step 5; the parent owns fixes and the full rerun.'} ---`; diff --git a/scripts/resolvers/review-dashboard.ts b/scripts/resolvers/review-dashboard.ts index 89a6c1a73..4b2e58237 100644 --- a/scripts/resolvers/review-dashboard.ts +++ b/scripts/resolvers/review-dashboard.ts @@ -65,9 +65,7 @@ ${ctx.skillName === 'ship' ? 'This verdict never skips Step 9 or its finding, ap Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. -- Outside Voice is the default-on plan review after CEO/Eng review. \`codex_reviews\` +${ctx.skillName === 'ship' ? '- Step 9\'s review plan decides the adversarial passes (Step 11).\n' : '- Adversarial review always includes a native pass. Available, enabled outside\n challenges supplement it; diffs of 200+ lines also get the structured P1 gate.\n'}- Outside Voice is the default-on plan review after CEO/Eng review. \`codex_reviews\` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. diff --git a/ship/SKILL.md b/ship/SKILL.md index 3d1978825..881cd3934 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -396,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -509,10 +508,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -752,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index ec7995e29..16f65656e 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -136,10 +136,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -232,10 +232,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -473,7 +473,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index bfff86465..a0024fba6 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -2,11 +2,12 @@ **Step 9's last full-pass plan governs this step:** run the native pass only on `DISPATCH: adversarial-native` and the outside voice only on -`DISPATCH: outside-voice`; otherwise print `ADVERSARIAL: skipped (review plan)`. +`DISPATCH: outside-voice`. With neither, print `ADVERSARIAL: skipped (review plan)` +and take decision 3 of Finish the adversarial phase. -## Step 11: Adversarial review (always-on) +## Step 11: Adversarial review -Every diff gets the Claude adversarial pass. Add Codex when its preflight is ready; unavailable or disabled outside coverage stays explicit. +Unavailable or disabled outside coverage stays explicit. **Detect diff size:** @@ -77,13 +78,12 @@ Branch on the echoed `CODEX_MODE`: `CODEX_MODE: disabled` means skip the Codex passes ONLY. `ready` and `unverified` run them; every other mode skips them with the printed reason. -The Claude adversarial subagent always runs. **User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the Codex structured review regardless of diff size (still requires `CODEX_MODE: ready` (or `unverified`)). --- -### Claude adversarial subagent (always runs) +### Claude adversarial subagent (on `DISPATCH: adversarial-native`) Before dispatch, run `~/.claude/skills/gstack/bin/gstack-review-log --start adversarial-review` and save the returned token for this native attempt. Do the same before each outside @@ -281,7 +281,7 @@ Retain the historical review-log skill ID; add `"host":"claude","outside_provide After all passes complete, synthesize findings across all sources: ``` -ADVERSARIAL REVIEW SYNTHESIS (always-on, N lines): +ADVERSARIAL REVIEW SYNTHESIS (N lines): ════════════════════════════════════════════════════════════ High confidence (found by multiple sources): [findings agreed on by >1 pass] Unique to the parent checklist/specialists: [from earlier steps] @@ -316,7 +316,7 @@ in order before leaving Step 11: Step 9 completes full review before fixes; any further repair inserts its checks ahead of the remaining items. These fresh reviews after code edits are not recovery retries. Returning here never resets Step 9's three-cycle fix limit. -3. **Native complete with no queued fixes:** Finish the memory updates below, +3. **Native complete with no queued fixes, or not dispatched:** Finish the memory updates below, then continue to Step 11.5. Never jump directly to release preparation. --- diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index ddffca679..a8215d657 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -1,6 +1,7 @@ **Step 9's last full-pass plan governs this step:** run the native pass only on `DISPATCH: adversarial-native` and the outside voice only on -`DISPATCH: outside-voice`; otherwise print `ADVERSARIAL: skipped (review plan)`. +`DISPATCH: outside-voice`. With neither, print `ADVERSARIAL: skipped (review plan)` +and take decision 3 of Finish the adversarial phase. {{ADVERSARIAL_STEP}} diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index daff334ee..c8f9d4064 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -16,7 +16,8 @@ Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan --base `; a repeat uses decision 2's run. Save its `REVIEW_PLAN:` line and fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step -11's voices), each at its `effort=` on the same model family; a `SKIP:` reviewer +11's voices), each at its `effort=` on the same model family, in place of 9.1's and +9.2's own selection rules; a `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds `"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. @@ -798,25 +799,22 @@ finding makes that PR a draft. ### Decide whether to repeat Step 9 After persistence, record missing output, CYCLES and fixes in the invocation record. -Apply these decisions in order: +Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` +with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, +`--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass +logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `); +its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +Other scopes apply decisions 2–4 in order. 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, resume at the next decision; otherwise run a fresh complete Step 9. A successful peer or a QA exception cannot replace missing dispatched coverage. 2. **Third fixing cycle reached (`CYCLES >= 3`):** STOP and report recurring findings with - `converged:false`; do not run a fourth fixing cycle. Under `frozen` the plan - decides sooner: `gstack-review-plan --base --pass $PASS --last full` with - each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, - `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass - logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `). - `NEXT: stop` stops here, unfixed; `full` is decision 3, `fix-check` 3.5, `done` 4. - Name each blocking finding (path, severity, problem) for the owner. Deferred and noted items are never recurring findings, so a capped pass - whose remaining findings are all below the floor applies no edits and continues - at decision 4: the loop ships below the floor. The cap is a stop, not an - approval: a blocker it leaves standing is reported, never waved through. + `converged:false`; do not run a fourth fixing cycle. Name each blocking finding (path, + severity, problem) for the owner, unfixed. The cap is a stop, not an approval. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of - Step 9 before the pending Step 10 in the work list (`NEXT: full`). If the fixes grew the diff + Step 9 before the pending Step 10 in the work list. If the fixes grew the diff past what the request needs (a helper, a guard, a case nothing takes), cut that growth before anything else; the repeat reviews the cut as part of the range. Tests must pass or retain approval for the same verified pre-existing failures and scope. Keep CYCLES and scoped approvals across this repeat; PASS increments on entry. @@ -826,7 +824,7 @@ Apply these decisions in order: on `gstack-review-plan --fix-check-prompt --fix-list --hunks --tests --callers `, plus the outside voice only if the plan lists it. Log item 6's row as `"skill":"review-fix-check"` with `--finish `; not a pass. A - CRITICAL stops as decision 2; else decision 4. + CRITICAL goes to decision 2; otherwise decision 4. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it clears may you continue to Step 10. A pass whose only findings were deferred or noted applied no edits and continues here: the loop converges below the floor. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index e7e617506..e5d0b4c83 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -14,7 +14,8 @@ Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan --base `; a repeat uses decision 2's run. Save its `REVIEW_PLAN:` line and fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step -11's voices), each at its `effort=` on the same model family; a `SKIP:` reviewer +11's voices), each at its `effort=` on the same model family, in place of 9.1's and +9.2's own selection rules; a `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds `"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. @@ -212,25 +213,22 @@ finding makes that PR a draft. ### Decide whether to repeat Step 9 After persistence, record missing output, CYCLES and fixes in the invocation record. -Apply these decisions in order: +Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` +with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, +`--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass +logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `); +its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +Other scopes apply decisions 2–4 in order. 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, resume at the next decision; otherwise run a fresh complete Step 9. A successful peer or a QA exception cannot replace missing dispatched coverage. 2. **Third fixing cycle reached (`CYCLES >= 3`):** STOP and report recurring findings with - `converged:false`; do not run a fourth fixing cycle. Under `frozen` the plan - decides sooner: `gstack-review-plan --base --pass $PASS --last full` with - each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, - `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass - logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `). - `NEXT: stop` stops here, unfixed; `full` is decision 3, `fix-check` 3.5, `done` 4. - Name each blocking finding (path, severity, problem) for the owner. Deferred and noted items are never recurring findings, so a capped pass - whose remaining findings are all below the floor applies no edits and continues - at decision 4: the loop ships below the floor. The cap is a stop, not an - approval: a blocker it leaves standing is reported, never waved through. + `converged:false`; do not run a fourth fixing cycle. Name each blocking finding (path, + severity, problem) for the owner, unfixed. The cap is a stop, not an approval. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of - Step 9 before the pending Step 10 in the work list (`NEXT: full`). If the fixes grew the diff + Step 9 before the pending Step 10 in the work list. If the fixes grew the diff past what the request needs (a helper, a guard, a case nothing takes), cut that growth before anything else; the repeat reviews the cut as part of the range. Tests must pass or retain approval for the same verified pre-existing failures and scope. Keep CYCLES and scoped approvals across this repeat; PASS increments on entry. @@ -240,7 +238,7 @@ Apply these decisions in order: on `gstack-review-plan --fix-check-prompt --fix-list --hunks --tests --callers `, plus the outside voice only if the plan lists it. Log item 6's row as `"skill":"review-fix-check"` with `--finish `; not a pass. A - CRITICAL stops as decision 2; else decision 4. + CRITICAL goes to decision 2; otherwise decision 4. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it clears may you continue to Step 10. A pass whose only findings were deferred or noted applied no edits and continues here: the loop converges below the floor. diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 0a8e260df..7494fe0c4 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -396,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -509,10 +508,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -752,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 3d1978825..881cd3934 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -396,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -509,10 +508,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -752,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index d3689b753..c4d937f91 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -327,10 +327,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -395,8 +395,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -508,10 +507,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -755,7 +754,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index c88d04459..374d879a4 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos. 4. **Carry check.** If `gh pr list --head --state open --json number` names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base --dry-run`. - `CARRY: would carry` (same product patch-id): skip review, fix what the merge broke - (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without - `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD: stop. - A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -397,8 +397,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -510,10 +509,10 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree` (none: `review_freshness` - `CURRENT`), the Step 9 record's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -757,7 +756,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/outside-voice-preflight.test.ts b/test/outside-voice-preflight.test.ts index fc81af532..3420603f1 100644 --- a/test/outside-voice-preflight.test.ts +++ b/test/outside-voice-preflight.test.ts @@ -23,7 +23,7 @@ test('adversarial outside failures retain the required native pass without dupli expect(preflight).toMatch(/(?:do not dispatch a duplicate|without duplicating it)/); expect(preflight).not.toMatch(/fall(?:ing)? back to (?:a|the) .*subagent/i); const output = generateAdversarialStep(ctx); - expect(output).toContain('adversarial subagent (always runs)'); + expect(output).toContain(skillName === 'ship' ? 'adversarial subagent (on `DISPATCH: adversarial-native`)' : 'adversarial subagent (always runs)'); expectMentions(output, [['do not', 'dispatch', 'retain']], 'output'); expect(output.match(/Retain the required native pass without duplicating it; it cannot complete outside coverage\./g)).toHaveLength(2); expect(output).not.toContain("Use the caller's fallback"); diff --git a/test/ship-control-flow.test.ts b/test/ship-control-flow.test.ts index 3373cb28f..9c0048eec 100644 --- a/test/ship-control-flow.test.ts +++ b/test/ship-control-flow.test.ts @@ -43,7 +43,7 @@ describe('ship source controller', () => { test('the review plan drives the repeat, the fix-check binds under its own skill, and Step 11.5 accepts a fix-check or a planned adversarial skip', () => { const decide = review.slice(review.indexOf('### Decide whether to repeat Step 9'), review.indexOf('**Required-probe parent gate:**')); for (const flag of ['--fixed-blocking', '--fixed-below', '--blocking-open', '--touched-gate', '--critical-found', '--adversarial-ran', '--reviewed-numbers ']) expect(decide).toContain(flag); - expect(decide).toContain('`NEXT: stop` stops here, unfixed; `full` is decision 3, `fix-check` 3.5, `done` 4.'); + expect(decide).toContain('its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4.'); expect(decide).toContain('`gstack-review-log --start review-fix-check` token'); expect(decide).toContain('`"skill":"review-fix-check"` with `--finish `'); expect(compact(read('ship/sections/adversarial.md.tmpl'))).toContain("Step 9's last full-pass plan governs this step"); diff --git a/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json b/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json index 7257ec6b0..7610f4c75 100644 --- a/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json +++ b/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json @@ -1,4 +1,4 @@ { - "bytes": 1877, - "why": "GSTA-131 token efficiency: review plan dispatch, fix-check 3.5, plan-governed adversarial step, Step 1 delta carry, Step 11.5 fix-check and planned-skip binding, 400-word findings cap, light Step 7/8 audits" + "bytes": 1621, + "why": "GSTA-131 token efficiency: review plan dispatch, fix-check 3.5, plan-governed adversarial step, Step 1 delta carry, Step 11.5 fix-check and planned-skip binding, 400-word findings cap, light Step 7/8 audits; screen repair: one decision each for the loop exits, adversarial dispatch and Step 16 receipts" } diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 08b897411..88753fe81 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1619,13 +1619,14 @@ describe('Codex skill', () => { expect(content).toContain('200'); }); - test('adversarial review in /ship always runs both passes', () => { + test('adversarial review in /ship runs the passes the review plan dispatches', () => { const content = readShipUnion(); - expect(content).toContain('Adversarial review (always-on)'); + expect(content).toContain("Step 9's last full-pass plan governs this step"); + expect(content).toContain('## Step 11: Adversarial review\n'); expect(content).toContain('adversarial-review'); expect(content).toContain('run-with-timeout --effort high '); expect(content).toContain('Investigate and fix'); - expect(content).toContain('Claude adversarial subagent (always runs)'); + expect(content).toContain('Claude adversarial subagent (on `DISPATCH: adversarial-native`)'); }); test('scope drift detection in /review and /ship', () => {