mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
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>
This commit is contained in:
garrytanandcapy-ai[bot] committed
1 parent
c5e13f9167
commit
243ca8ea21
17 files changed
+110
-118
No files matched your search
+2
-1
@@ -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
|
||||
|
||||
|
||||
@@ -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.'}
|
||||
|
||||
---`;
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
+10
-11
@@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
+9
-9
@@ -136,10 +136,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
@@ -2,11 +2,12 @@
|
||||
<!-- Regenerate: bun run gen:skill-docs -->
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
@@ -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}}
|
||||
|
||||
|
||||
@@ -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 <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 <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 <fp>`);
|
||||
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 <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 <fp>`).
|
||||
`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 <f> --hunks <f> --tests <f>
|
||||
--callers <f>`, plus the outside voice only if the plan lists it. Log item 6's
|
||||
row as `"skill":"review-fix-check"` with `--finish <token>`; 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.
|
||||
|
||||
@@ -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 <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 <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 <fp>`);
|
||||
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 <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 <fp>`).
|
||||
`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 <f> --hunks <f> --tests <f>
|
||||
--callers <f>`, plus the outside voice only if the plan lists it. Log item 6's
|
||||
row as `"skill":"review-fix-check"` with `--finish <token>`; 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.
|
||||
|
||||
+10
-11
@@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
+10
-11
@@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
+10
-11
@@ -327,10 +327,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
+10
-11
@@ -329,10 +329,10 @@ repository-landing asks, including on Apple repos.
|
||||
|
||||
4. **Carry check.** If `gh pr list --head <branch-name> --state open --json number`
|
||||
names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr <n> --base <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
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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 <fp>']) 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 <token>`');
|
||||
expect(compact(read('ship/sections/adversarial.md.tmpl'))).toContain("Step 9's last full-pass plan governs this step");
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in new issue
Block a user