From 93d0c41f4f865566a8f0966e33e103c7188e067d Mon Sep 17 00:00:00 2001 From: garrytan Date: Thu, 29 Oct 2026 14:19:16 +0000 Subject: [PATCH] =?UTF-8?q?fix(ship):=20Step=2011's=20review=20plan=20alon?= =?UTF-8?q?e=20decides=20which=20voices=20run;=20greptile=20fixes=20requeu?= =?UTF-8?q?e=209=20=E2=86=92=2010=20instead=20of=20an=20inline=20loop=20(s?= =?UTF-8?q?hip/SKILL.md=20workflow=20judge,=2040815828320)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- scripts/resolvers/constants.ts | 12 +++-- scripts/resolvers/outside-voice-steps.ts | 30 +++++------ scripts/resolvers/outside-voice.ts | 11 ++-- ship/sections/adversarial.md | 45 ++++++++-------- ship/sections/adversarial.md.tmpl | 8 +-- ship/sections/greptile.md | 6 +-- ship/sections/greptile.md.tmpl | 6 +-- test/outside-voice-preflight.test.ts | 8 ++- .../ship-adversarial-dispatch-clarity.test.ts | 53 +++++++++++++++++++ test/ship-control-flow.test.ts | 4 +- test/ship-skip-requeue.test.ts | 2 +- .../gsta162-ship-adversarial-dispatch.json | 4 ++ 12 files changed, 127 insertions(+), 62 deletions(-) create mode 100644 test/ship-adversarial-dispatch-clarity.test.ts create mode 100644 test/size-ledger/ship/gsta162-ship-adversarial-dispatch.json diff --git a/scripts/resolvers/constants.ts b/scripts/resolvers/constants.ts index 5f865534d..696e3ae3d 100644 --- a/scripts/resolvers/constants.ts +++ b/scripts/resolvers/constants.ts @@ -154,13 +154,19 @@ On any error: continue — ${feature} is informational, not a gate.`; * all — skip the section, no Claude fallback. * - `codex-only` (diff adversarial): disabled gates only the Codex passes; the * free Claude adversarial subagent still runs. + * `planDispatch` (/ship Step 11): the review plan's DISPATCH lines decide whether the + * native pass runs, so no mode line says anything about it. */ -export function codexPreflight(opts: { modeVar?: string; disabledBehavior: 'skip-all' | 'codex-only'; nativeReview?: boolean; role?: boolean }): string { +export function codexPreflight(opts: { modeVar?: string; disabledBehavior: 'skip-all' | 'codex-only'; nativeReview?: boolean; role?: boolean; planDispatch?: boolean }): string { const m = opts.modeVar ?? '_CODEX_MODE'; - const disabledLine = opts.disabledBehavior === 'codex-only' + const disabledLine = opts.planDispatch + ? 'Skip the Codex passes. Print: "Codex passes skipped (codex_reviews disabled)."' + : opts.disabledBehavior === 'codex-only' ? 'Skip the Codex passes only; the Claude adversarial subagent below STILL runs (it is free and fast). Print: "Codex passes skipped (codex_reviews disabled) — running Claude adversarial only."' : 'Skip this section entirely; do NOT fall back to a Claude subagent — disabled means no extra review step. Print: "Codex review skipped (codex_reviews disabled). Re-enable: `gstack-config set codex_reviews enabled`."'; - const nativeRoute = opts.disabledBehavior === 'codex-only' + const nativeRoute = opts.planDispatch + ? 'Skip the Codex passes.' + : opts.disabledBehavior === 'codex-only' ? 'Keep the required Claude adversarial pass; do not dispatch a duplicate.' : 'Fall back to the Claude subagent path.'; return `\`\`\`bash diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index f95740e02..733b686d7 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -143,7 +143,7 @@ If the subagent fails or times out, record native coverage as incomplete. Contin } function adversarialOutsideChallenge(ctx: TemplateContext, isShip: boolean): string { - return `### ${outsideVoiceFor(ctx).label} adversarial challenge (runs whenever \`CODEX_MODE\` is \`ready\` or \`unverified\`) + return `### ${outsideVoiceFor(ctx).label} adversarial challenge (${isShip ? 'on \`DISPATCH: outside-voice\`, when' : 'runs whenever'} \`CODEX_MODE\` is \`ready\` or \`unverified\`) If \`CODEX_MODE\` is \`ready\` or \`unverified\`: @@ -151,16 +151,12 @@ Outside prompt (supply repository context from the parent): "${CODEX_BOUNDARY}Review the changes on this branch against the base branch. Use the supplied branch diff. If it was not supplied and you have repository tools, run DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE". Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format \`Recommendation: because \`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativeAlreadyRequired: true, diffCommand: 'DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"' })} +${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativePass: isShip ? 'planned' : 'required', diffCommand: 'DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"' })} Present the full output verbatim. ${isShip ? 'An unavailable outside challenge does not block shipping by itself; supported findings still enter Step 11, and the structured P0/P1 and non-convergence gates still apply.' : 'This outside challenge is informational; supported findings still enter Step 5 Fix-First, whose approval and convergence gates apply.'} **Error handling:** Only this optional outside adversarial pass is non-blocking; native completion and structured-review decisions still apply. -${outsideVoiceFailurePolicy(ctx, { timeoutMinutes: 9, onTimeout: 'missing-coverage', stderrOnEmpty: true, fallback: 'none', escape: 1 })} - - - -For other modes, retain the native pass above; do not dispatch it again. +${outsideVoiceFailurePolicy(ctx, { timeoutMinutes: 9, onTimeout: 'missing-coverage', stderrOnEmpty: true, fallback: 'none', escape: 1 })}${isShip ? '' : '\n\n\n\nFor other modes, retain the native pass above; do not dispatch it again.'} ---`; } @@ -168,11 +164,11 @@ For other modes, retain the native pass above; do not dispatch it again. function adversarialStructuredReview(ctx: TemplateContext, isShip: boolean): string { return `### ${outsideVoiceFor(ctx).label} structured review (large diffs only, 200+ lines) -If \`CODEX_MODE\` is \`ready\` or \`unverified\` and either \`DIFF_TOTAL >= 200\` or the user requested the override above: +${isShip ? 'On \`DISPATCH: outside-voice\`, if' : 'If'} \`CODEX_MODE\` is \`ready\` or \`unverified\` and either \`DIFF_TOTAL >= 200\` or the user requested the override above: Prepare a structured review prompt requesting severity-tagged findings ([P0]-[P3]) or an explicit NO_FINDINGS conclusion. Preserve the base-branch scope including committed changes and working-tree changes. -${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativeAlreadyRequired: true, structuredBase: '', gate: 'structured', diffCommand: 'DIFF_BASE=$(git merge-base HEAD) && git diff "$DIFF_BASE"' })} +${outsideVoiceInvocation(ctx, { timeoutMs: 540000, nativePass: isShip ? 'planned' : 'required', structuredBase: '', gate: 'structured', diffCommand: 'DIFF_BASE=$(git merge-base HEAD) && git diff "$DIFF_BASE"' })} ${outsideVoiceFor(ctx).id === 'codex' ? 'The Codex backend uses `codex review --base` without a positional prompt: those arguments are mutually exclusive. Never drop --base to resolve an argv error; prompt-only review changes the diff scope.' : 'The Claude Code backend receives the parent-captured base diff, including committed and working-tree changes, because review mode cannot execute git.'} @@ -194,7 +190,7 @@ Read stderr for errors (same error handling as ${outsideVoiceFor(ctx).label} adv -If \`DIFF_TOTAL < 200\` without that override, skip structured review; the adversarial passes still run. +If \`DIFF_TOTAL < 200\` without that override, skip structured review; the ${isShip ? 'planned ' : ''}adversarial passes still run. ---`; } @@ -249,13 +245,15 @@ echo "DIFF_SIZE: $DIFF_TOTAL" **Detect the ${outsideVoiceFor(ctx).label} master switch + tool availability:** -${outsideVoicePreflight(ctx, { disabledBehavior: 'codex-only', nativeReview: true })} +${outsideVoicePreflight(ctx, { disabledBehavior: 'codex-only', nativeReview: true, planDispatch: isShip })} -\`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.${isShip ? '' : ` -The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs.`} +${isShip ? `\`CODEX_MODE\` gates only the planned ${outsideVoiceFor(ctx).label} passes: \`ready\` and \`unverified\` run them; every other mode skips them with the printed reason. -**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\`)). +**User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", the planned ${outsideVoiceFor(ctx).label} voice also runs its structured review regardless of diff size.` : `\`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. + +**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\`)).`} --- @@ -296,7 +294,7 @@ REVIEW_START. Keep scoped approvals. Optional outside failures retain their own incomplete records. Apply these decisions in order before leaving Step 11: -1. **Required native review incomplete:** STOP and confirm the native task stopped. +1. **Dispatched native review incomplete:** STOP and confirm the native task stopped. Outside-provider output cannot replace this pass. One recovery retry is allowed only after a concrete prerequisite correction and restored access; count it in the invocation record before launch. Capture a fresh PASS_START and persist the diff --git a/scripts/resolvers/outside-voice.ts b/scripts/resolvers/outside-voice.ts index 28675ff94..855fd2584 100644 --- a/scripts/resolvers/outside-voice.ts +++ b/scripts/resolvers/outside-voice.ts @@ -47,10 +47,10 @@ fi`; /** Caller-owned workflow role. Only enumerated plan-review generators pass it; everything else stays no-role. */ export type OutsideRole = 'plan-review'; -export function outsideVoicePreflight(ctx: TemplateContext, opts: { disabledBehavior: 'skip-all' | 'codex-only' | 'opt-in'; nativeReview?: boolean; role?: OutsideRole }): string { +export function outsideVoicePreflight(ctx: TemplateContext, opts: { disabledBehavior: 'skip-all' | 'codex-only' | 'opt-in'; nativeReview?: boolean; role?: OutsideRole; planDispatch?: boolean }): string { const v = outsideVoiceFor(ctx); if (v.id === 'codex' && (opts.disabledBehavior === 'codex-only' || (opts.disabledBehavior === 'skip-all' && !opts.role))) { - let preflight = outsideVoiceLabels(ctx, codexPreflight({ disabledBehavior: opts.disabledBehavior, nativeReview: opts.nativeReview, role: !!opts.role })) + let preflight = outsideVoiceLabels(ctx, codexPreflight({ disabledBehavior: opts.disabledBehavior, nativeReview: opts.nativeReview, role: !!opts.role, planDispatch: opts.planDispatch })) .replace('```bash\n', `\`\`\`bash\n${runtimeRootPrelude(ctx)}\n`); if (['plan-eng-review', 'plan-ceo-review'].includes(ctx.skillName)) { preflight = preflight.replace("follow the workflow's native-review instructions below", @@ -82,7 +82,7 @@ ${readiness} ${ctx.skillName === 'ship' && opts.disabledBehavior === 'opt-in' ? `Ship attempts this optional design check when frontend review applies and \`codex_reviews\` allows it. Unset means the default, enabled; the first send prints a one-time notice. -\`CODEX_MODE\` names the provider here, **${v.label}**.` : `The historical \`CODEX_MODE\` variable describes **${v.label}** availability here.`}${opts.disabledBehavior === 'opt-in' ? " `CODEX_MODE: disabled` or a send's `consent_missing` line: `codex_reviews` is off or unreadable; do not ask; record `outside_status: disabled`." : ''} ${opts.role ? 'The invocation checks auth and the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) plan-review model.' : 'Authentication and configured model validity are checked by the actual invocation, without overriding either.'} Missing/broken CLI: install or repair ${v.label}; authentication failure: run \`${v.id === 'codex' ? 'codex login' : 'claude auth login'}\`. ${opts.disabledBehavior === 'skip-all' ? "Disabled ends this entire extra review step (so does a send's `consent_missing` line), including the native fallback; record outside_status: disabled and continue after the section. Disabled is not an unavailable provider and never triggers a replacement reviewer." : opts.disabledBehavior === 'codex-only' ? "Disabled skips only the outside CLI (so does a send's `consent_missing` line); retain the native pass." : ctx.skillName === 'ship' ? '' : 'Otherwise honor this caller’s opt-in/skip choice.'} ${opts.disabledBehavior === 'skip-all' ? 'Provider failure is missing outside coverage; follow the caller’s existing fallback only when reviews are enabled.' : opts.disabledBehavior === 'codex-only' ? 'Non-ready means missing outside coverage. Keep the required native pass without duplicating it.' : 'Any non-ready outcome is missing outside coverage; follow the caller’s existing fallback.'} Never substitute another external provider.`; +\`CODEX_MODE\` names the provider here, **${v.label}**.` : `The historical \`CODEX_MODE\` variable describes **${v.label}** availability here.`}${opts.disabledBehavior === 'opt-in' ? " `CODEX_MODE: disabled` or a send's `consent_missing` line: `codex_reviews` is off or unreadable; do not ask; record `outside_status: disabled`." : ''} ${opts.role ? 'The invocation checks auth and the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) plan-review model.' : 'Authentication and configured model validity are checked by the actual invocation, without overriding either.'} Missing/broken CLI: install or repair ${v.label}; authentication failure: run \`${v.id === 'codex' ? 'codex login' : 'claude auth login'}\`. ${opts.disabledBehavior === 'skip-all' ? "Disabled ends this entire extra review step (so does a send's `consent_missing` line), including the native fallback; record outside_status: disabled and continue after the section. Disabled is not an unavailable provider and never triggers a replacement reviewer." : opts.planDispatch ? "Disabled skips the outside CLI (so does a send's `consent_missing` line)." : opts.disabledBehavior === 'codex-only' ? "Disabled skips only the outside CLI (so does a send's `consent_missing` line); retain the native pass." : ctx.skillName === 'ship' ? '' : 'Otherwise honor this caller’s opt-in/skip choice.'} ${opts.disabledBehavior === 'skip-all' ? 'Provider failure is missing outside coverage; follow the caller’s existing fallback only when reviews are enabled.' : opts.planDispatch ? 'Non-ready means missing outside coverage.' : opts.disabledBehavior === 'codex-only' ? 'Non-ready means missing outside coverage. Keep the required native pass without duplicating it.' : 'Any non-ready outcome is missing outside coverage; follow the caller’s existing fallback.'} Never substitute another external provider.`; } export interface OutsideCommandOptions { @@ -97,7 +97,8 @@ export interface OutsideCommandOptions { reasoningEffort?: 'high' | 'medium'; /** Creative proposals retain the recommendation gate with task-specific wording. */ purpose?: 'design-direction'; - nativeAlreadyRequired?: boolean; + /** The adversarial step's native pass: always required (/review) or run per the review plan's DISPATCH line (/ship). */ + nativePass?: 'required' | 'planned'; /** Resolve the plan-review policy once in the invocation shell and bind it through probe and dispatch. */ role?: OutsideRole; } @@ -188,7 +189,7 @@ export function outsideVoiceInvocation(ctx: TemplateContext, requested: OutsideC ${outsideVoiceCommand(ctx, opts)} \`\`\` -Use Bash \`timeout: ${gateMs}\`; show the full response in a \`tool-output\` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing ${planRecommendation ? 'Recommendation: because ' : opts.purpose === 'design-direction' ? 'Recommendation' : 'score/severity/completion'} markers, timeout or CLI failure means \`outside_status: unavailable\`. P0/P1 findings block like native ones; \`OUTSIDE_STATUS: unverified\` is missing coverage. ${opts.purpose === 'design-direction' ? 'Continue completed proposals; native completion does not count as outside coverage.' : opts.nativeAlreadyRequired ? 'Retain the required native pass without duplicating it; it cannot complete outside coverage.' : "Use the caller's fallback; missing coverage is never clean/PASS."} ${nativeStructured ? 'Scratch cleanup is automatic.' : 'After either outcome, delete only your private prompt; scratch cleanup is automatic.'}`; +Use Bash \`timeout: ${gateMs}\`; show the full response in a \`tool-output\` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing ${planRecommendation ? 'Recommendation: because ' : opts.purpose === 'design-direction' ? 'Recommendation' : 'score/severity/completion'} markers, timeout or CLI failure means \`outside_status: unavailable\`. P0/P1 findings block like native ones; \`OUTSIDE_STATUS: unverified\` is missing coverage. ${opts.purpose === 'design-direction' ? 'Continue completed proposals; native completion does not count as outside coverage.' : opts.nativePass === 'required' ? 'Retain the required native pass without duplicating it; it cannot complete outside coverage.' : opts.nativePass === 'planned' ? 'Never dispatch a native pass in its place; it cannot complete outside coverage.' : "Use the caller's fallback; missing coverage is never clean/PASS."} ${nativeStructured ? 'Scratch cleanup is automatic.' : 'After either outcome, delete only your private prompt; scratch cleanup is automatic.'}`; } /** diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 250d9f5a7..3de04cab0 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -1,8 +1,10 @@ -**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`. Without the native line (light depth at runtime 0; +**Step 9's last full-pass plan governs this step:** it alone decides which voices run. +The native pass runs only on `DISPATCH: adversarial-native`. The outside voice +(challenge, plus structured review at 200+ lines or on request) runs only on +`DISPATCH: outside-voice` and a preflight of `ready` or `unverified`; run the +preflight only for that line. Without the native line (light depth at runtime 0; above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`; with neither line, take decision 3 of Finish the adversarial phase. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) @@ -68,21 +70,20 @@ echo "CODEX_MODE: $_CODEX_MODE" ``` Branch on the echoed `CODEX_MODE`: -- **`disabled`** (or a send's `consent_missing` line) — `codex_reviews` is off or unreadable. Skip the Codex passes only; the Claude adversarial subagent below STILL runs (it is free and fast). Print: "Codex passes skipped (codex_reviews disabled) — running Claude adversarial only." -- **`helper_unavailable`** — the probe is missing or not executable; relay the line above (cause and fix). Keep the required Claude adversarial pass; do not dispatch a duplicate. -- **`not_installed`** — Codex CLI absent. Print: "Codex not installed; outside coverage unavailable. Install: `npm install -g @openai/codex`." Keep the required Claude adversarial pass; do not dispatch a duplicate. +- **`disabled`** (or a send's `consent_missing` line) — `codex_reviews` is off or unreadable. Skip the Codex passes. Print: "Codex passes skipped (codex_reviews disabled)." +- **`helper_unavailable`** — the probe is missing or not executable; relay the line above (cause and fix). Skip the Codex passes. +- **`not_installed`** — Codex CLI absent. Print: "Codex not installed; outside coverage unavailable. Install: `npm install -g @openai/codex`." Skip the Codex passes. - **`under_codex`** — stale artifact selected its own harness. Print: "Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage. Repair: setup --host codex." Skip the outside invocation and follow the workflow's native-review instructions below. Conflicting inherited harness markers are not grounds to guess another provider. -- **`not_authed`** — installed but no credentials. Print: "Codex not authenticated; outside coverage unavailable. Run `codex login` or set `$CODEX_API_KEY`." Keep the required Claude adversarial pass; do not dispatch a duplicate. -- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines. Keep the required Claude adversarial pass; do not dispatch a duplicate. -- **`model_unusable`** — the selected model (see `CODEX_MODEL:`) is invalid or unavailable to the account (HTTP 400 on every call). Relay the probe's HINT lines and the fix (`GSTACK_CODEX_MODEL=` or config.toml `model`); never substitute a model. Keep the required Claude adversarial pass; do not dispatch a duplicate. The ~10s round trip is cached for 1h. -- **`quota_exhausted`** — Codex usage limit: relay the probe's lines verbatim (reset time, retry); no more Codex calls this run. Keep the required Claude adversarial pass; do not dispatch a duplicate. -- **`sandbox_unavailable`** — Codex's sandbox cannot start here (containers without user namespaces); the probe printed the reason and fix. No paid call ran; outside coverage is unavailable. Keep the required Claude adversarial pass; do not dispatch a duplicate. +- **`not_authed`** — installed but no credentials. Print: "Codex not authenticated; outside coverage unavailable. Run `codex login` or set `$CODEX_API_KEY`." Skip the Codex passes. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines. Skip the Codex passes. +- **`model_unusable`** — the selected model (see `CODEX_MODEL:`) is invalid or unavailable to the account (HTTP 400 on every call). Relay the probe's HINT lines and the fix (`GSTACK_CODEX_MODEL=` or config.toml `model`); never substitute a model. Skip the Codex passes. The ~10s round trip is cached for 1h. +- **`quota_exhausted`** — Codex usage limit: relay the probe's lines verbatim (reset time, retry); no more Codex calls this run. Skip the Codex passes. +- **`sandbox_unavailable`** — Codex's sandbox cannot start here (containers without user namespaces); the probe printed the reason and fix. No paid call ran; outside coverage is unavailable. Skip the Codex passes. - **`ready`** or **`unverified`** — run the Codex pass below. `unverified` means the model check timed out or, with `(rate_limited)`, hit a 429; say so, and let the pass's own verdict decide. -`CODEX_MODE: disabled` means skip the Codex passes ONLY. -`ready` and `unverified` run them; every other mode skips them with the printed reason. +`CODEX_MODE` gates only the planned Codex passes: `ready` and `unverified` run them; every other mode skips them with the printed reason. -**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`)). +**User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", the planned Codex voice also runs its structured review regardless of diff size. --- @@ -111,7 +112,7 @@ If the subagent fails or times out, record native coverage as incomplete. Contin --- -### Codex adversarial challenge (runs whenever `CODEX_MODE` is `ready` or `unverified`) +### Codex adversarial challenge (on `DISPATCH: outside-voice`, when `CODEX_MODE` is `ready` or `unverified`) If `CODEX_MODE` is `ready` or `unverified`: @@ -160,7 +161,7 @@ esac echo 'OUTSIDE_STATUS: completed provider=codex host=claude' ``` -Use Bash `timeout: 600000`; show the full response in a `tool-output` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing score/severity/completion markers, timeout or CLI failure means `outside_status: unavailable`. P0/P1 findings block like native ones; `OUTSIDE_STATUS: unverified` is missing coverage. Retain the required native pass without duplicating it; it cannot complete outside coverage. After either outcome, delete only your private prompt; scratch cleanup is automatic. +Use Bash `timeout: 600000`; show the full response in a `tool-output` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing score/severity/completion markers, timeout or CLI failure means `outside_status: unavailable`. P0/P1 findings block like native ones; `OUTSIDE_STATUS: unverified` is missing coverage. Never dispatch a native pass in its place; it cannot complete outside coverage. After either outcome, delete only your private prompt; scratch cleanup is automatic. Present the full output verbatim. An unavailable outside challenge does not block shipping by itself; supported findings still enter Step 11, and the structured P0/P1 and non-convergence gates still apply. @@ -169,15 +170,11 @@ Present the full output verbatim. An unavailable outside challenge does not bloc - **Timeout:** "Codex timed out after 9 minutes and was terminated; this pass produced NO findings." A timed-out pass is MISSING COVERAGE, not a clean bill — say so explicitly rather than continuing as if Codex had reviewed. - **Empty response:** "Codex returned no response. Stderr: ." - - -For other modes, retain the native pass above; do not dispatch it again. - --- ### Codex structured review (large diffs only, 200+ lines) -If `CODEX_MODE` is `ready` or `unverified` and either `DIFF_TOTAL >= 200` or the user requested the override above: +On `DISPATCH: outside-voice`, if `CODEX_MODE` is `ready` or `unverified` and either `DIFF_TOTAL >= 200` or the user requested the override above: Prepare a structured review prompt requesting severity-tagged findings ([P0]-[P3]) or an explicit NO_FINDINGS conclusion. Preserve the base-branch scope including committed changes and working-tree changes. @@ -222,7 +219,7 @@ esac echo 'OUTSIDE_STATUS: completed provider=codex host=claude' ``` -Use Bash `timeout: 600000`; show the full response in a `tool-output` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing score/severity/completion markers, timeout or CLI failure means `outside_status: unavailable`. P0/P1 findings block like native ones; `OUTSIDE_STATUS: unverified` is missing coverage. Retain the required native pass without duplicating it; it cannot complete outside coverage. Scratch cleanup is automatic. +Use Bash `timeout: 600000`; show the full response in a `tool-output` fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing score/severity/completion markers, timeout or CLI failure means `outside_status: unavailable`. P0/P1 findings block like native ones; `OUTSIDE_STATUS: unverified` is missing coverage. Never dispatch a native pass in its place; it cannot complete outside coverage. Scratch cleanup is automatic. The Codex backend uses `codex review --base` without a positional prompt: those arguments are mutually exclusive. Never drop --base to resolve an argv error; prompt-only review changes the diff scope. @@ -244,7 +241,7 @@ Read stderr for errors (same error handling as Codex adversarial above). -If `DIFF_TOTAL < 200` without that override, skip structured review; the adversarial passes still run. +If `DIFF_TOTAL < 200` without that override, skip structured review; the planned adversarial passes still run. --- @@ -306,7 +303,7 @@ REVIEW_START. Keep scoped approvals. Optional outside failures retain their own incomplete records. Apply these decisions in order before leaving Step 11: -1. **Required native review incomplete:** STOP and confirm the native task stopped. +1. **Dispatched native review incomplete:** STOP and confirm the native task stopped. Outside-provider output cannot replace this pass. One recovery retry is allowed only after a concrete prerequisite correction and restored access; count it in the invocation record before launch. Capture a fresh PASS_START and persist the diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index 417be026e..4717598bc 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -1,6 +1,8 @@ -**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`. Without the native line (light depth at runtime 0; +**Step 9's last full-pass plan governs this step:** it alone decides which voices run. +The native pass runs only on `DISPATCH: adversarial-native`. The outside voice +(challenge, plus structured review at 200+ lines or on request) runs only on +`DISPATCH: outside-voice` and a preflight of `ready` or `unverified`; run the +preflight only for that line. Without the native line (light depth at runtime 0; above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`; with neither line, take decision 3 of Finish the adversarial phase. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) diff --git a/ship/sections/greptile.md b/ship/sections/greptile.md index d8da1b423..a82585dac 100644 --- a/ship/sections/greptile.md +++ b/ship/sections/greptile.md @@ -87,9 +87,9 @@ For each comment in `comments`: **SUPPRESSED:** Skip silently — these are known false positives from previous triage. -**After triage:** If fixes were approved, save their approvals and comment references. -Run Step 9's full review/fix loop, then return here. Finish the saved replies -without asking again about completed fixes, and classify new comments. +**After triage:** If fixes were approved, save their approvals and comment references, +then insert `9 → 10` before the pending work (control-flow rule 2). On that return, +finish the saved replies without asking again about completed fixes, and classify new comments. With no queued fixes, continue to Step 11. --- diff --git a/ship/sections/greptile.md.tmpl b/ship/sections/greptile.md.tmpl index d91245f14..8c5138c0e 100644 --- a/ship/sections/greptile.md.tmpl +++ b/ship/sections/greptile.md.tmpl @@ -85,9 +85,9 @@ For each comment in `comments`: **SUPPRESSED:** Skip silently — these are known false positives from previous triage. -**After triage:** If fixes were approved, save their approvals and comment references. -Run Step 9's full review/fix loop, then return here. Finish the saved replies -without asking again about completed fixes, and classify new comments. +**After triage:** If fixes were approved, save their approvals and comment references, +then insert `9 → 10` before the pending work (control-flow rule 2). On that return, +finish the saved replies without asking again about completed fixes, and classify new comments. With no queued fixes, continue to Step 11. --- diff --git a/test/outside-voice-preflight.test.ts b/test/outside-voice-preflight.test.ts index 3420603f1..4a28e8f6c 100644 --- a/test/outside-voice-preflight.test.ts +++ b/test/outside-voice-preflight.test.ts @@ -24,8 +24,12 @@ test('adversarial outside failures retain the required native pass without dupli expect(preflight).not.toMatch(/fall(?:ing)? back to (?:a|the) .*subagent/i); const output = generateAdversarialStep(ctx); 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); + if (skillName === 'review') { + 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); + } else { + expect(output.match(/Never dispatch a native pass in its place; it cannot complete outside coverage\./g)).toHaveLength(2); + } expect(output).not.toContain("Use the caller's fallback"); expectMentions(output, [['only', 'non-blocking', 'adversarial']], 'output'); expect(output).toContain('GATE: MISSING COVERAGE'); diff --git a/test/ship-adversarial-dispatch-clarity.test.ts b/test/ship-adversarial-dispatch-clarity.test.ts new file mode 100644 index 000000000..fc58ac4a7 --- /dev/null +++ b/test/ship-adversarial-dispatch-clarity.test.ts @@ -0,0 +1,53 @@ +import { expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { ALL_HOST_CONFIGS } from '../hosts/index'; +import { generateAdversarialStep } from '../scripts/resolvers/outside-voice-steps'; +import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; +import { between, compact, expectAbsent, expectMentions, expectTokens } from './helpers/prompt-structure'; + +const root = join(import.meta.dir, '..'); +const ctxFor = (skillName: string, host: string): TemplateContext => + ({ skillName, host, tmplPath: `${skillName}/SKILL.md.tmpl`, paths: HOST_PATHS[host] } as TemplateContext); + +// Paid gate run 40815828320 (ship/SKILL.md workflow judge, clarity 3, actionability 3): samples 1 and 3 found +// ship Step 11's header running the native pass only on `DISPATCH: adversarial-native` and the outside voice +// only on `DISPATCH: outside-voice`, while every CODEX_MODE branch kept a "required" native pass that "STILL +// runs" and the outside challenge ran "whenever" CODEX_MODE was ready. These retired phrases are pinned for that +// eval; /review keeps them because its native pass always runs. +const NATIVE_ALWAYS = [/STILL runs/, /required \S+(?: \(in-host\))? adversarial pass/i, /Retain the required native pass/, + /retain the native pass/i, /Keep the required native pass/, /runs whenever/, /always runs/, /Required native review incomplete/]; + +test('ship Step 11 lets the review plan alone decide which adversarial voices run, on every host', () => { + // Value: protects=ship Step 11 has one dispatch rule (the plan's DISPATCH lines); fails_when=a CODEX_MODE + // branch, heading or decision makes the native pass unconditional or runs the outside voice off-plan; + // why_new=outside-voice-preflight pins only the /review-style retain wording; seam=none + const header = compact(readFileSync(join(root, 'ship/sections/adversarial.md.tmpl'), 'utf8').split('{{ADVERSARIAL_STEP}}')[0]!); + expectTokens(header, ['`DISPATCH: adversarial-native`', '`DISPATCH: outside-voice`', '`ADVERSARIAL: skipped (review plan)`'], 'ship header'); + expectMentions(header, [['plan', 'decides', 'voices'], ['native', 'only', 'adversarial-native'], ['outside', 'only', 'outside-voice', 'preflight']], 'ship header'); + for (const { name } of ALL_HOST_CONFIGS) { + const step = generateAdversarialStep(ctxFor('ship', name)); + expectAbsent(step, NATIVE_ALWAYS, `ship ${name}`); + expect(step.match(/^### .*adversarial subagent .*$/m)?.[0], name).toContain('(on `DISPATCH: adversarial-native`)'); + expect(step.match(/^### .*adversarial challenge .*$/m)?.[0], name).toContain('on `DISPATCH: outside-voice`'); + expect(between(step, /^### .*structured review/m, 'Prepare a structured review prompt'), name).toContain('`DISPATCH: outside-voice`'); + expect(compact(between(step, '### Finish the adversarial phase')), name).toContain('1. **Dispatched native review incomplete:**'); + } +}); + +test('/review keeps its always-on native adversarial pass on every host', () => { + for (const { name } of ALL_HOST_CONFIGS) { + const step = generateAdversarialStep(ctxFor('review', name)); + expectAbsent(step, ['DISPATCH:'], `review ${name}`); + expectTokens(step, ['adversarial subagent (always runs)', 'adversarial challenge (runs whenever'], `review ${name}`); + expect(step.match(/Retain the required native pass without duplicating it/g), name).toHaveLength(2); + } +}); + +test('ship Greptile fixes return through the saved work list, not an inline loop', () => { + // Value: protects=Greptile fix approvals re-enter Step 9 through control-flow rule 2; fails_when=Step 10 + // restores its own "run Step 9, then return here" loop beside the saved list (run 40815828320 sample 2); seam=none + const after = compact(between(readFileSync(join(root, 'ship/sections/greptile.md.tmpl'), 'utf8'), '**After triage:**', '---')); + expectTokens(after, ['`9 → 10`', 'rule 2'], 'greptile after triage'); + expectAbsent(after, [/return here/i, /review\/fix loop/], 'greptile after triage'); +}); diff --git a/test/ship-control-flow.test.ts b/test/ship-control-flow.test.ts index 9c0048eec..574ccf2d9 100644 --- a/test/ship-control-flow.test.ts +++ b/test/ship-control-flow.test.ts @@ -274,7 +274,7 @@ describe('ship source controller', () => { test('native recovery has one corrected attempt and cannot borrow outside completion', () => { const finish = adversarial.slice(adversarial.indexOf('### Finish the adversarial phase')); - const recovery = finish.slice(finish.indexOf('1. **Required native'), finish.indexOf('2. **Fixes queued')); + const recovery = finish.slice(finish.indexOf('1. **Dispatched native'), finish.indexOf('2. **Fixes queued')); expectMentions(recovery, [['stop', 'confirm', 'stopped']], 'recovery'); expectMentions(recovery, [['cannot', 'outside-provider', 'replace']], 'recovery'); expectMentions(recovery, [['only', 'prerequisite', 'correction']], 'recovery'); @@ -290,7 +290,7 @@ describe('ship source controller', () => { const ctx = { host, skillName: 'ship', tmplPath: '', paths: HOST_PATHS[host] }; const ship = generateAdversarialStep(ctx); const finish = compact(ship.slice(ship.indexOf('### Finish the adversarial phase'))); - const branches = ['Required native review incomplete', 'Fixes queued after native completion', 'Native complete with no queued fixes'] + const branches = ['Dispatched native review incomplete', 'Fixes queued after native completion', 'Native complete with no queued fixes'] .map(label => finish.indexOf(label)); expect(branches.every(index => index >= 0)).toBe(true); expect(branches).toEqual([...branches].sort((a, b) => a - b)); diff --git a/test/ship-skip-requeue.test.ts b/test/ship-skip-requeue.test.ts index 42caac3f8..aee066ca0 100644 --- a/test/ship-skip-requeue.test.ts +++ b/test/ship-skip-requeue.test.ts @@ -69,7 +69,7 @@ describe.each(ALL_HOST_CONFIGS.map(({ name }) => name))('%s ship skip/requeue co }); test('fresh review after edits, native coverage and cycle limits remain required', () => { - expect(finish).toContain('**Required native review incomplete:** STOP'); + expect(finish).toContain('**Dispatched native review incomplete:** STOP'); expectMentions(finish, [['before', 'pending', 'insert']], 'finish'); expectMentions(finish, [['never', 'three-cycle', 'resets']], 'finish'); expect(review).toContain('Set CYCLES to 0 on first entry only'); diff --git a/test/size-ledger/ship/gsta162-ship-adversarial-dispatch.json b/test/size-ledger/ship/gsta162-ship-adversarial-dispatch.json new file mode 100644 index 000000000..68f2c6150 --- /dev/null +++ b/test/size-ledger/ship/gsta162-ship-adversarial-dispatch.json @@ -0,0 +1,4 @@ +{ + "bytes": -324, + "why": "GSTA-162 flaky pass: Step 11's CODEX_MODE branches and outside headings defer to the review plan's DISPATCH lines; Greptile fixes re-enter through the work list" +}