From 1de52b06b5c5c715e87424bbb81a063e0491bb5c Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 7 Nov 2026 20:30:19 +0000 Subject: [PATCH] Repin the outside reviewer briefs and fix the tests the verdict line broke (GST-1821) The outside brief, both adversarial sections and the review-army prompt-file line now carry the last-line VERDICT request; their pins move under the preregistered no-loss replay GST-1821. Test fakes that wrote a clean reply now end it with VERDICT: CLEAN, the review carve marker follows the codex exec sentence, size pins drop where skills shrank, and the plan-review prompt-file line is shortened so /plan-eng-review stays at its pin. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- plan-ceo-review/sections/review-sections.md | 4 +- plan-eng-review/sections/review-sections.md | 4 +- scripts/resolvers/outside-voice-steps.ts | 2 +- scripts/resolvers/outside-voice.ts | 4 +- .../review-pins/brief-bodies/outside.json | 2 +- .../files/review/sections/adversarial.md.json | 2 +- .../files/ship/sections/adversarial.md.json | 2 +- .../files/ship/sections/review-army.md.json | 2 +- .../review-pins/replays/GST-1821.json | 1 + test/fixtures/skill-md-size-allowlist.json | 76 +++++++++---------- test/helpers/carve-guards.ts | 2 +- test/review-depth.test.ts | 5 +- test/run-in-background-guidance.test.ts | 4 +- 13 files changed, 57 insertions(+), 53 deletions(-) create mode 100644 test/fixtures/review-pins/replays/GST-1821.json diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index b26a3dda6..936afbaf6 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -443,7 +443,7 @@ took for granted, missing dependencies or sequencing issues, and strategic miscalibration (is this the right thing to build at all?). Be direct. Be terse. No compliments. Just the problems. -Write Recommendation: because . If there are no findings, say so and explain why the plan is ready. End with exactly `VERDICT: CLEAN` (no findings) or `VERDICT: FINDINGS` as the last line. +Write Recommendation: because ; if no findings, explain why the plan is ready. End with exactly `VERDICT: CLEAN` (no findings) or `VERDICT: FINDINGS` as the last line. THE PLAN: @@ -456,7 +456,7 @@ Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native fallback below, never a replacement provider. Finish termination before fallback and consume only completed output. Use private temporary paths, with no background jobs. -Create a private prompt file: run `umask 077; mktemp "${TMPDIR:-/tmp}/gstack-plan-prompt.XXXXXXXX"` in Bash and keep the returned path. Use Write to put the **complete prompt and context**, including actual plan/spec/source, in that file. Substitute its shell-quoted path for ``; never interpolate user text into shell source. Request severity (Critical, High, Medium or Low) per finding, a Recommendation: because line, and `VERDICT: CLEAN` or `VERDICT: FINDINGS` as the last line. +Create a private prompt file: run `umask 077; mktemp "${TMPDIR:-/tmp}/gstack-plan-prompt.XXXXXXXX"` in Bash and keep the returned path. Use Write to put the **complete prompt and context**, including actual plan/spec/source, in that file. Substitute its shell-quoted path for ``; never interpolate user text into shell source. Request severity (Critical, High, Medium or Low) per finding and the prompt's Recommendation and VERDICT lines. ```bash # GSTACK_ACTIVE_HOST names the harness, never the model. diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index 07993e82f..8457c408e 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -958,7 +958,7 @@ took for granted, missing dependencies or sequencing issues, and strategic miscalibration (is this the right thing to build at all?). Be direct. Be terse. No compliments. Just the problems. -Write Recommendation: because . If there are no findings, say so and explain why the plan is ready. End with exactly `VERDICT: CLEAN` (no findings) or `VERDICT: FINDINGS` as the last line. +Write Recommendation: because ; if no findings, explain why the plan is ready. End with exactly `VERDICT: CLEAN` (no findings) or `VERDICT: FINDINGS` as the last line. THE PLAN: @@ -971,7 +971,7 @@ Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native fallback below, never a replacement provider. Finish termination before fallback and consume only completed output. Use private temporary paths, with no background jobs. -Create a private prompt file: run `umask 077; mktemp "${TMPDIR:-/tmp}/gstack-plan-prompt.XXXXXXXX"` in Bash and keep the returned path. Use Write to put the **complete prompt and context**, including actual plan/spec/source, in that file. Substitute its shell-quoted path for ``; never interpolate user text into shell source. Request severity (Critical, High, Medium or Low) per finding, a Recommendation: because line, and `VERDICT: CLEAN` or `VERDICT: FINDINGS` as the last line. +Create a private prompt file: run `umask 077; mktemp "${TMPDIR:-/tmp}/gstack-plan-prompt.XXXXXXXX"` in Bash and keep the returned path. Use Write to put the **complete prompt and context**, including actual plan/spec/source, in that file. Substitute its shell-quoted path for ``; never interpolate user text into shell source. Request severity (Critical, High, Medium or Low) per finding and the prompt's Recommendation and VERDICT lines. ```bash # GSTACK_ACTIVE_HOST names the harness, never the model. diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index a5068a99b..207874d96 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -389,7 +389,7 @@ survived the review scrutiny, overcomplexity (is there a fundamentally simpler approach the review was too deep in the weeds to see?), feasibility risks the review took for granted, missing dependencies or sequencing issues, and strategic miscalibration (is this the right thing to build at all?). Be direct. Be terse. No -compliments. Just the problems.${needsApprovalReadiness ? '\n\nWrite Recommendation: because . If there are no findings, say so and explain why the plan is ready. ' + OUTSIDE_VERDICT_REQUEST + '\n' : ''} +compliments. Just the problems.${needsApprovalReadiness ? '\n\nWrite Recommendation: because ; if no findings, explain why the plan is ready. ' + OUTSIDE_VERDICT_REQUEST + '\n' : ''} ${ctx.skillName === 'plan-devex-review' ? ` REVIEW CONTEXT (from the full working list, outside the truncated plan body): diff --git a/scripts/resolvers/outside-voice.ts b/scripts/resolvers/outside-voice.ts index 52c2819fd..07b37c661 100644 --- a/scripts/resolvers/outside-voice.ts +++ b/scripts/resolvers/outside-voice.ts @@ -176,7 +176,9 @@ export function outsideVoiceInvocation(ctx: TemplateContext, requested: OutsideC ? `Request [P0]-[P3] severity tags and ${VERDICT_LAST_LINE}.` : opts.purpose === 'design-direction' ? 'Request a complete design proposal ending with Recommendation: because .' - : `Request severity (Critical, High, Medium or Low) per finding, a Recommendation: because line, and ${VERDICT_LAST_LINE}.`; + : planRecommendation + ? "Request severity (Critical, High, Medium or Low) per finding and the prompt's Recommendation and VERDICT lines." + : `Request severity (Critical, High, Medium or Low) per finding, a Recommendation: because line, and ${VERDICT_LAST_LINE}.`; const preparation = `${['plan-ceo-review', 'plan-eng-review'].includes(ctx.skillName) ? 'Create a private prompt file: run `umask 077; mktemp "${TMPDIR:-/tmp}/gstack-plan-prompt.XXXXXXXX"` in Bash and keep the returned path. Use Write to put the **complete prompt and context**, including actual plan/spec/source, in that file' : 'Write the **complete prompt and context**, including actual plan/spec/source, to a private file'}${outsideVoiceFor(ctx).id === 'claude-code' ? ' (Claude Code has no tools, git or path access)' : ''}. Substitute its shell-quoted path for \`\`; never interpolate user text into shell source. ${completion}`; diff --git a/test/fixtures/review-pins/brief-bodies/outside.json b/test/fixtures/review-pins/brief-bodies/outside.json index 9cdf30705..c2be7dfa2 100644 --- a/test/fixtures/review-pins/brief-bodies/outside.json +++ b/test/fixtures/review-pins/brief-bodies/outside.json @@ -1 +1 @@ -{"sha":"bd96406da28b03742384560425a333b6d6d7ed2be95bd2026c90e0039632cee2"} +{"sha":"7c90f42c528b1f8093e0a64068dc889c94af7f84efb6f890185da1472220e67b"} diff --git a/test/fixtures/review-pins/files/review/sections/adversarial.md.json b/test/fixtures/review-pins/files/review/sections/adversarial.md.json index 39bc50c43..d68a7f652 100644 --- a/test/fixtures/review-pins/files/review/sections/adversarial.md.json +++ b/test/fixtures/review-pins/files/review/sections/adversarial.md.json @@ -1 +1 @@ -{"sha":"e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255","replay":"G1"} +{"sha":"7031c39570a4fcc5243c84285e729e252b87c277f61520f0b376851275bb5e7e","replay":"GST-1821"} diff --git a/test/fixtures/review-pins/files/ship/sections/adversarial.md.json b/test/fixtures/review-pins/files/ship/sections/adversarial.md.json index b8f1e9c67..75a6ff958 100644 --- a/test/fixtures/review-pins/files/ship/sections/adversarial.md.json +++ b/test/fixtures/review-pins/files/ship/sections/adversarial.md.json @@ -1 +1 @@ -{"sha":"09f01971dfc2d174ba739a6c5e745b2663e08bf1c22a19671acbfb6995f70640","replay":"GST-1346"} +{"sha":"c4012593ec9fddb17f60b205b12fbaf12b35f4a2c3832e7b8834c6eed85ddcb9","replay":"GST-1821"} diff --git a/test/fixtures/review-pins/files/ship/sections/review-army.md.json b/test/fixtures/review-pins/files/ship/sections/review-army.md.json index 3f329f9cf..ba93430d2 100644 --- a/test/fixtures/review-pins/files/ship/sections/review-army.md.json +++ b/test/fixtures/review-pins/files/ship/sections/review-army.md.json @@ -1 +1 @@ -{"sha":"5edd49d21b5acc4a92001c2d8ed0768917bdc2d278b209b322359607ac2058d8","replay":"GSTA-258-C+GSTA-258-L4"} +{"sha":"873430e7ea838133b7a30069959edac8139e4c23ba0b1317fbdf1fb7460b7df7","replay":"GST-1821"} diff --git a/test/fixtures/review-pins/replays/GST-1821.json b/test/fixtures/review-pins/replays/GST-1821.json new file mode 100644 index 000000000..f7fcdc427 --- /dev/null +++ b/test/fixtures/review-pins/replays/GST-1821.json @@ -0,0 +1 @@ +{"replay":"GST-1821","justification":"GST-1821 (an outside review reads clean only from a last-line VERDICT token): preregistered no-loss probe, gpt-6-astra at high effort, codex-cli 0.167.0, six seeded-bug repos and three clean repos, two trials per case and arm, 54 scored runs. Codex's structured step moved from `codex review --base` (arm R) to `codex exec` with gstack's prompt, [P0]-[P3] tags and the last-line VERDICT request (arm X). X caught the seeded bug in 12 of 12 bug runs, R in 12 of 12 and the no-gstack arm in 12 of 12, so no catch was lost; all three arms are saturated, so a loss on subtler bugs is not ruled out. X ended with the VERDICT line in 18 of 18 runs and read clean on all 6 clean-case runs. The adversarial outside brief and the other outside prompt-file lines change only their closing request (the Recommendation line is kept and the same VERDICT request is appended); their finding, severity and Blocks instructions are byte-identical and were not probed separately. Free replay of 681 captured outside replies through the new parser: 0 moved toward clean."} diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index 70b40ba6c..3fa426f56 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -2,56 +2,56 @@ "_comment": "Generated SKILL.md files over the limit, pinned at their exact character count (#3199). test/skill-md-size-guard.test.ts fails when a pinned skill grows past its pin or shrinks below it: lower the pin when a skill shrinks and delete it once the skill is within the limit. Raising a pin is an owner decision. The shared-preamble hoist (GSTA-96) moved every tier 2+ skill's shared preamble into sections/preamble.md; the pins below are what remains.", "limit": 100000, "pins": { - "agy/gstack-autoplan": 109601, - "agy/gstack-plan-design-review": 109100, - "agy/gstack-plan-devex-review": 114928, + "agy/gstack-autoplan": 109589, + "agy/gstack-plan-design-review": 109097, + "agy/gstack-plan-devex-review": 114925, "agy/gstack-plan-eng-review": 143581, - "agy/gstack-review": 104744, - "codex/gstack-autoplan": 108344, - "codex/gstack-plan-design-review": 110457, - "codex/gstack-plan-devex-review": 116436, + "agy/gstack-review": 104741, + "codex/gstack-autoplan": 108332, + "codex/gstack-plan-design-review": 110454, + "codex/gstack-plan-devex-review": 116433, "codex/gstack-plan-eng-review": 144746, "codex/gstack-review": 104267, - "copilot/gstack-autoplan": 109044, - "copilot/gstack-plan-design-review": 109366, - "copilot/gstack-plan-devex-review": 114390, + "copilot/gstack-autoplan": 109032, + "copilot/gstack-plan-design-review": 109363, + "copilot/gstack-plan-devex-review": 114387, "copilot/gstack-plan-eng-review": 143746, - "copilot/gstack-review": 104325, - "cursor/gstack-autoplan": 108276, - "cursor/gstack-plan-design-review": 107908, - "cursor/gstack-plan-devex-review": 113660, + "copilot/gstack-review": 104322, + "cursor/gstack-autoplan": 108264, + "cursor/gstack-plan-design-review": 107905, + "cursor/gstack-plan-devex-review": 113657, "cursor/gstack-plan-eng-review": 142336, - "cursor/gstack-review": 122797, - "factory/gstack-autoplan": 110554, - "factory/gstack-plan-design-review": 111006, - "factory/gstack-plan-devex-review": 117023, + "cursor/gstack-review": 122794, + "factory/gstack-autoplan": 110542, + "factory/gstack-plan-design-review": 111003, + "factory/gstack-plan-devex-review": 117020, "factory/gstack-plan-eng-review": 145444, - "factory/gstack-review": 126570, - "gbrain/gstack-autoplan": 108683, + "factory/gstack-review": 126567, + "gbrain/gstack-autoplan": 108671, "gbrain/gstack-plan-eng-review": 124963, - "gemini/gstack-autoplan": 108834, - "gemini/gstack-plan-design-review": 109194, - "gemini/gstack-plan-devex-review": 114218, + "gemini/gstack-autoplan": 108822, + "gemini/gstack-plan-design-review": 109191, + "gemini/gstack-plan-devex-review": 114215, "gemini/gstack-plan-eng-review": 143579, - "gemini/gstack-review": 104147, - "hermes/gstack-autoplan": 108637, + "gemini/gstack-review": 104144, + "hermes/gstack-autoplan": 108625, "hermes/gstack-plan-eng-review": 124999, - "kiro/gstack-autoplan": 108050, - "kiro/gstack-plan-design-review": 107754, - "kiro/gstack-plan-devex-review": 113512, + "kiro/gstack-autoplan": 108038, + "kiro/gstack-plan-design-review": 107751, + "kiro/gstack-plan-devex-review": 113509, "kiro/gstack-plan-eng-review": 142198, - "kiro/gstack-review": 122621, - "openclaw/gstack-autoplan": 108506, + "kiro/gstack-review": 122618, + "openclaw/gstack-autoplan": 108494, "openclaw/gstack-plan-eng-review": 123478, - "opencode/gstack-autoplan": 109025, - "opencode/gstack-plan-design-review": 109222, - "opencode/gstack-plan-devex-review": 114268, + "opencode/gstack-autoplan": 109013, + "opencode/gstack-plan-design-review": 109219, + "opencode/gstack-plan-devex-review": 114265, "opencode/gstack-plan-eng-review": 143605, - "opencode/gstack-review": 123419, - "slate/gstack-autoplan": 108163, - "slate/gstack-plan-design-review": 107831, - "slate/gstack-plan-devex-review": 113586, + "opencode/gstack-review": 123416, + "slate/gstack-autoplan": 108151, + "slate/gstack-plan-design-review": 107828, + "slate/gstack-plan-devex-review": 113583, "slate/gstack-plan-eng-review": 142267, - "slate/gstack-review": 122709 + "slate/gstack-review": 122706 } } diff --git a/test/helpers/carve-guards.ts b/test/helpers/carve-guards.ts index 6cd1fc44f..9840b10ef 100644 --- a/test/helpers/carve-guards.ts +++ b/test/helpers/carve-guards.ts @@ -485,7 +485,7 @@ do not launch the downstream skill or open a browser.`, 'Plan File Discovery', 'MULTI-SPECIALIST CONFIRMED', 'Cross-model synthesis', - 'codex review --base', + 'Codex runs `codex exec` read-only and reads that diff itself.', ], gateAfterStop: undefined, // operational multi-STOP skill, like ship }, diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index 517b72bbd..8dfa06ac5 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -194,10 +194,11 @@ describe('GST-967 invariant: no gate-class owner exit before Step 11\'s voices; * with each voices step read back as main's stop (its reason is the parenthesised tail, no dispatch), hashes to the same value, * and every voices step is a gate-class stop before Step 11 that adds exactly Step 11's three voices. No grant is passed (main * takes none). The brief bodies lib/review-brief.ts cuts from the ship sections hash to MAIN_SECTIONS: main's bodies plus - * GST-508's exclude-pathspec placeholders (replay 'GST-508+GST-464+GST-967': the merged bodies minus main's are the wave's minus fc292e22f). + * GST-508's exclude-pathspec placeholders (replay 'GST-508+GST-464+GST-967': the merged bodies minus main's are the wave's minus fc292e22f), + * then GST-1821's last-line VERDICT request in the outside brief (replay 'GST-1821'). */ const MAIN_PLANS = '9d13d6bae86bfc9207ce7e0cec8da5b33db2518c1653e8e009160a98b0f04c89'; -const MAIN_SECTIONS = 'cec8872ff5a4aacb610133294706e452952b172fbe9c4891039679e98d895f62'; +const MAIN_SECTIONS = '7eb6b4c2dd31edbd7c69f02bd31f8884407187fac8ace4e796632d60957b6bbb'; describe('GST-967 dispatch-superset no-loss check', () => { // Value: protects=the no-loss rule for review-depth changes: GST-967 drops, renames and re-efforts no reviewer main dispatches on any grid case, and changes no brief body; fails_when=a non-stop plan changes, a voices step appears off gate class or after Step 11, it dispatches anything but the three voices, or a brief body moves; // why_new=GST-967; seam=planReview, sectionTexts; no_claim=what the added voices find (they only add coverage) diff --git a/test/run-in-background-guidance.test.ts b/test/run-in-background-guidance.test.ts index 8475a2661..6dda5d456 100644 --- a/test/run-in-background-guidance.test.ts +++ b/test/run-in-background-guidance.test.ts @@ -74,7 +74,7 @@ out=; prev=; for a in "$@"; do [ "$prev" = -o ] && out=$a; prev=$a; done [ -n "$out" ] || { printf '%s\\n' OK; exit 0; } printf '%s\\n' "$FAKE_REVIEW_ID" >> "$FAKE_CALLS" cat > /dev/null -printf '%s\\n' "$FAKE_REVIEW_ID: current findings" "No issues found." "Recommendation: fix $FAKE_REVIEW_ID because this is the current finding." > "$out" +printf '%s\\n' "$FAKE_REVIEW_ID: current findings" "No issues found." "Recommendation: fix $FAKE_REVIEW_ID because this is the current finding." 'VERDICT: CLEAN' > "$out" printf '%s\\n' '{"type":"turn.completed"}' printf '%s\\n' "$FAKE_REVIEW_ID: current stderr" >&2 exit "$FAKE_CODEX_STATUS" @@ -105,7 +105,7 @@ exec ${quote(Bun.which('cat')!)} "$@" cleanup: () => fs.rmSync(dir, { recursive: true, force: true }) }; } - const findings = (id: string) => `${id}: current findings\nNo issues found.\nRecommendation: fix ${id} because this is the current finding.\n`; + const findings = (id: string) => `${id}: current findings\nNo issues found.\nRecommendation: fix ${id} because this is the current finding.\nVERDICT: CLEAN\n`; // INV-1: the verdict-form validator prints its verdict before the completed status. const unavailable = (id: string) => `${findings(id)}VERDICT: unavailable\nFINDINGS: none\nOUTSIDE_USAGE: unknown\nREASON: execution_failed\n`; const completed = (id: string) => `${findings(id)}VERDICT: clean\nFINDINGS: none\nOUTSIDE_USAGE: unknown\nOUTSIDE_STATUS: completed provider=codex host=claude\n`;