From 4df031177bdf563202853b461232e47ac5adcd16 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:34:48 +0000 Subject: [PATCH] fix: honor and persist disabled outside plan reviews --- document-release/sections/release-body.md | 29 ++- plan-ceo-review/sections/review-sections.md | 30 ++- plan-devex-review/sections/review-sections.md | 30 ++- plan-eng-review/sections/review-sections.md | 30 ++- scripts/resolvers/outside-voice.ts | 2 +- scripts/resolvers/review.ts | 56 ++++- test/disabled-plan-review-evidence.test.ts | 222 ++++++++++++++++++ test/helpers/disabled-plan-review-fixture.ts | 114 +++++++++ test/helpers/touchfiles-data.ts | 12 + test/skill-e2e-outside-plan-disabled.test.ts | 72 ++++++ test/touchfiles.test.ts | 5 +- 11 files changed, 591 insertions(+), 11 deletions(-) create mode 100644 test/disabled-plan-review-evidence.test.ts create mode 100644 test/helpers/disabled-plan-review-fixture.ts create mode 100644 test/skill-e2e-outside-plan-disabled.test.ts diff --git a/document-release/sections/release-body.md b/document-release/sections/release-body.md index 484788da4..9e19cf3fa 100644 --- a/document-release/sections/release-body.md +++ b/document-release/sections/release-body.md @@ -484,6 +484,27 @@ Branch on the echoed `CODEX_MODE`: A stale artifact selecting its own harness must report missing coverage and run no outside CLI. Repair: `setup --host codex`. Never infer a replacement provider from inherited environment markers. The invocation below repeats this guard. +**Disabled is a terminal branch for this section.** If the preflight prints +`CODEX_MODE: disabled`, persist `outside_status: disabled` with the guarded +command below, then finish the documentation workflow. Do not construct a review prompt, invoke an outside CLI, +dispatch an Agent/Task fallback, or ask the apply question below. A disabled review +is an intentional opt-out, not a provider failure that needs a replacement reviewer. + +Run this guarded command before leaving the disabled branch. It starts a fresh +shell and re-reads the control; enabled workflows never append a disabled record. +If logging fails, report the persistence failure and retain the disabled opt-out. + +```bash + +_DISABLED_REVIEW_MODE=$("$HOME/.claude/skills/gstack/bin/gstack-config" get codex_reviews 2>/dev/null) || { + echo 'Cannot read codex_reviews; disabled outside coverage was not recorded.' >&2 + exit 1 +} +if [ "$_DISABLED_REVIEW_MODE" = disabled ]; then + "$HOME/.claude/skills/gstack/bin/gstack-review-log" '{"skill":"codex-doc-review","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","status":"skipped","source":"none","host":"claude","outside_provider":"codex","outside_status":"disabled","phase":"documentation","commit":"'"$(git rev-parse --short HEAD 2>/dev/null || true)"'"}' +fi +``` + When the mode is anything except `disabled`, print one line so the off-switch stays discoverable: "Running the Codex doc review automatically (standard step). Disable: `gstack-config set codex_reviews disabled`." @@ -558,7 +579,13 @@ Present the full output verbatim under `CODEX SAYS (documentation review):`. Provider failures are informational; report the named provider, diagnosis, and missing coverage, then use the native fallback below. -**If preflight is unavailable for any reason (or Codex errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +`CODEX_MODE: disabled`, finish this section with `outside_status: disabled`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with the same prompt, passing `run_in_background: false` (subagents default to background since Claude Code v2.1.198). Bound it at a 5-minute timeout; if it never completes, treat the review as unavailable and continue. Present findings under `DOCUMENTATION REVIEW (Claude subagent):`. If it fails: "Doc review unavailable. Continuing." diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index e10373111..677e10047 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -309,6 +309,28 @@ Branch on the echoed `CODEX_MODE`: A stale artifact selecting its own harness must report missing coverage and run no outside CLI. Repair: `setup --host codex`. Never infer a replacement provider from inherited environment markers. The invocation below repeats this guard. +**Disabled is a terminal branch for this section.** If the preflight prints +`CODEX_MODE: disabled`, persist `outside_status: disabled` with the guarded +command below, then continue directly to the workflow's required outputs after this section. Do not construct a challenge, +invoke an outside CLI, dispatch an Agent/Task fallback, or ask about outside findings. +The native plan review is already complete. A disabled review is an intentional +opt-out, not a provider failure that needs a replacement reviewer. + +Run this guarded command before leaving the disabled branch. It starts a fresh +shell and re-reads the control; enabled workflows never append a disabled record. +If logging fails, report the persistence failure and retain the disabled opt-out. + +```bash + +_DISABLED_REVIEW_MODE=$("$HOME/.claude/skills/gstack/bin/gstack-config" get codex_reviews 2>/dev/null) || { + echo 'Cannot read codex_reviews; disabled outside coverage was not recorded.' >&2 + exit 1 +} +if [ "$_DISABLED_REVIEW_MODE" = disabled ]; then + "$HOME/.claude/skills/gstack/bin/gstack-review-log" '{"skill":"codex-plan-review","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","status":"skipped","source":"none","host":"claude","outside_provider":"codex","outside_status":"disabled","phase":"plan-review","commit":"'"$(git rev-parse --short HEAD 2>/dev/null || true)"'"}' +fi +``` + When the mode is anything except `disabled`, print one line so the off-switch stays discoverable: "Running the outside voice automatically (standard step). Disable: `gstack-config set codex_reviews disabled`." @@ -387,7 +409,13 @@ CODEX SAYS (plan review — outside voice): - Timeout: "Codex timed out after 5 minutes." Fall back to the Claude subagent below. - Empty response: "Codex returned no response." Fall back to the Claude subagent below. -**If preflight is unavailable for any reason (or Codex errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +`CODEX_MODE: disabled`, finish this section with `outside_status: disabled`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with `run_in_background: false` (subagents default to background since Claude Code v2.1.198; the findings must land before the workflow continues). The subagent has fresh context and no conversation bias — but it is the same harness; model identity stays unknown unless the runtime reports it; weigh its agreement accordingly. Bound it the same way as Codex: cap the dispatch at a 5-minute timeout so "never blocking" diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 37d3018e3..60d11a10f 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -295,6 +295,28 @@ Branch on the echoed `CODEX_MODE`: A stale artifact selecting its own harness must report missing coverage and run no outside CLI. Repair: `setup --host codex`. Never infer a replacement provider from inherited environment markers. The invocation below repeats this guard. +**Disabled is a terminal branch for this section.** If the preflight prints +`CODEX_MODE: disabled`, persist `outside_status: disabled` with the guarded +command below, then continue directly to the workflow's required outputs after this section. Do not construct a challenge, +invoke an outside CLI, dispatch an Agent/Task fallback, or ask about outside findings. +The native plan review is already complete. A disabled review is an intentional +opt-out, not a provider failure that needs a replacement reviewer. + +Run this guarded command before leaving the disabled branch. It starts a fresh +shell and re-reads the control; enabled workflows never append a disabled record. +If logging fails, report the persistence failure and retain the disabled opt-out. + +```bash + +_DISABLED_REVIEW_MODE=$("$HOME/.claude/skills/gstack/bin/gstack-config" get codex_reviews 2>/dev/null) || { + echo 'Cannot read codex_reviews; disabled outside coverage was not recorded.' >&2 + exit 1 +} +if [ "$_DISABLED_REVIEW_MODE" = disabled ]; then + "$HOME/.claude/skills/gstack/bin/gstack-review-log" '{"skill":"codex-plan-review","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","status":"skipped","source":"none","host":"claude","outside_provider":"codex","outside_status":"disabled","phase":"plan-review","commit":"'"$(git rev-parse --short HEAD 2>/dev/null || true)"'"}' +fi +``` + When the mode is anything except `disabled`, print one line so the off-switch stays discoverable: "Running the outside voice automatically (standard step). Disable: `gstack-config set codex_reviews disabled`." @@ -373,7 +395,13 @@ CODEX SAYS (plan review — outside voice): - Timeout: "Codex timed out after 5 minutes." Fall back to the Claude subagent below. - Empty response: "Codex returned no response." Fall back to the Claude subagent below. -**If preflight is unavailable for any reason (or Codex errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +`CODEX_MODE: disabled`, finish this section with `outside_status: disabled`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with `run_in_background: false` (subagents default to background since Claude Code v2.1.198; the findings must land before the workflow continues). The subagent has fresh context and no conversation bias — but it is the same harness; model identity stays unknown unless the runtime reports it; weigh its agreement accordingly. Bound it the same way as Codex: cap the dispatch at a 5-minute timeout so "never blocking" diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index 15ead6fb0..7d93d9ca7 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -390,6 +390,28 @@ Branch on the echoed `CODEX_MODE`: A stale artifact selecting its own harness must report missing coverage and run no outside CLI. Repair: `setup --host codex`. Never infer a replacement provider from inherited environment markers. The invocation below repeats this guard. +**Disabled is a terminal branch for this section.** If the preflight prints +`CODEX_MODE: disabled`, persist `outside_status: disabled` with the guarded +command below, then continue directly to the workflow's required outputs after this section. Do not construct a challenge, +invoke an outside CLI, dispatch an Agent/Task fallback, or ask about outside findings. +The native plan review is already complete. A disabled review is an intentional +opt-out, not a provider failure that needs a replacement reviewer. + +Run this guarded command before leaving the disabled branch. It starts a fresh +shell and re-reads the control; enabled workflows never append a disabled record. +If logging fails, report the persistence failure and retain the disabled opt-out. + +```bash + +_DISABLED_REVIEW_MODE=$("$HOME/.claude/skills/gstack/bin/gstack-config" get codex_reviews 2>/dev/null) || { + echo 'Cannot read codex_reviews; disabled outside coverage was not recorded.' >&2 + exit 1 +} +if [ "$_DISABLED_REVIEW_MODE" = disabled ]; then + "$HOME/.claude/skills/gstack/bin/gstack-review-log" '{"skill":"codex-plan-review","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","status":"skipped","source":"none","host":"claude","outside_provider":"codex","outside_status":"disabled","phase":"plan-review","commit":"'"$(git rev-parse --short HEAD 2>/dev/null || true)"'"}' +fi +``` + When the mode is anything except `disabled`, print one line so the off-switch stays discoverable: "Running the outside voice automatically (standard step). Disable: `gstack-config set codex_reviews disabled`." @@ -468,7 +490,13 @@ CODEX SAYS (plan review — outside voice): - Timeout: "Codex timed out after 5 minutes." Fall back to the Claude subagent below. - Empty response: "Codex returned no response." Fall back to the Claude subagent below. -**If preflight is unavailable for any reason (or Codex errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +`CODEX_MODE: disabled`, finish this section with `outside_status: disabled`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with `run_in_background: false` (subagents default to background since Claude Code v2.1.198; the findings must land before the workflow continues). The subagent has fresh context and no conversation bias — but it is the same harness; model identity stays unknown unless the runtime reports it; weigh its agreement accordingly. Bound it the same way as Codex: cap the dispatch at a 5-minute timeout so "never blocking" diff --git a/scripts/resolvers/outside-voice.ts b/scripts/resolvers/outside-voice.ts index 139ee8d25..974c695d4 100644 --- a/scripts/resolvers/outside-voice.ts +++ b/scripts/resolvers/outside-voice.ts @@ -90,7 +90,7 @@ else fi \`\`\` -The historical \`CODEX_MODE\` variable describes **${v.label}** availability here. 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 skips this entire extra review step, including the native fallback.' : opts.disabledBehavior === 'codex-only' ? 'Disabled skips only the outside CLI; retain the native pass.' : 'Honor this caller’s existing opt-in/skip choice.'} Any non-ready outcome is missing outside coverage; follow the caller’s existing fallback. Never substitute another external provider.`; +The historical \`CODEX_MODE\` variable describes **${v.label}** availability here. 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, 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; retain the native pass.' : 'Honor this caller’s existing 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.' : 'Any non-ready outcome is missing outside coverage; follow the caller’s existing fallback.'} Never substitute another external provider.`; } export interface OutsideCommandOptions { diff --git a/scripts/resolvers/review.ts b/scripts/resolvers/review.ts index 824633822..7c181396a 100644 --- a/scripts/resolvers/review.ts +++ b/scripts/resolvers/review.ts @@ -12,10 +12,10 @@ * Review logs are stored locally at ~/.gstack/reviews/review-log.jsonl. * Outside CLI prompts are written to temp files to prevent shell injection. */ -import type { TemplateContext } from './types'; +import { toShellPath, type TemplateContext } from './types'; import { generateInvokeSkill } from './composition'; import { CC_BACKGROUND_DEFAULT_SINCE } from './constants'; -import { outsideVoiceFor, outsideVoiceInvocation, outsideVoicePreflight, outsideVoiceProvenance } from './outside-voice'; +import { outsideVoiceFor, outsideVoiceInvocation, outsideVoicePreflight, outsideVoiceProvenance, outsideVoiceRuntime } from './outside-voice'; import { DESIGN_DOC_DISCOVERY_BLOCK } from './design-doc-discovery'; import { getHostConfig } from '../../hosts/index'; @@ -602,6 +602,25 @@ High-confidence findings (agreed on by multiple sources) should be prioritized f ---`; } +/** A disabled pass must supersede earlier completed coverage before the section exits. */ +function generateDisabledOutsideRecord(ctx: TemplateContext, skill: string, phase: string): string { + const bin = toShellPath(ctx.paths.binDir); + return `Run this guarded command before leaving the disabled branch. It starts a fresh +shell and re-reads the control; enabled workflows never append a disabled record. +If logging fails, report the persistence failure and retain the disabled opt-out. + +\`\`\`bash +${outsideVoiceRuntime(ctx)} +_DISABLED_REVIEW_MODE=$("${bin}/gstack-config" get codex_reviews 2>/dev/null) || { + echo 'Cannot read codex_reviews; disabled outside coverage was not recorded.' >&2 + exit 1 +} +if [ "$_DISABLED_REVIEW_MODE" = disabled ]; then + "${bin}/gstack-review-log" '{"skill":"${skill}","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","status":"skipped","source":"none","host":"${ctx.host}","outside_provider":"${outsideVoiceFor(ctx).id}","outside_status":"disabled","phase":"${phase}","commit":"'"$(git rev-parse --short HEAD 2>/dev/null || true)"'"}' +fi +\`\`\``; +} + export function generateCodexPlanReview(ctx: TemplateContext): string { return `## Outside Voice — Independent Plan Challenge (default-on) @@ -616,6 +635,15 @@ review. The user turns this off only by asking explicitly ${outsideVoicePreflight(ctx, { disabledBehavior: 'skip-all' })} +**Disabled is a terminal branch for this section.** If the preflight prints +\`CODEX_MODE: disabled\`, persist \`outside_status: disabled\` with the guarded +command below, then continue directly to the workflow's required outputs after this section. Do not construct a challenge, +invoke an outside CLI, dispatch an Agent/Task fallback, or ask about outside findings. +The native plan review is already complete. A disabled review is an intentional +opt-out, not a provider failure that needs a replacement reviewer. + +${generateDisabledOutsideRecord(ctx, 'codex-plan-review', 'plan-review')} + When the mode is anything except \`disabled\`, print one line so the off-switch stays discoverable: "Running the outside voice automatically (standard step). Disable: \`gstack-config set codex_reviews disabled\`." @@ -658,7 +686,13 @@ ${outsideVoiceFor(ctx).label.toUpperCase()} SAYS (plan review — outside voice) - Timeout: "${outsideVoiceFor(ctx).label} timed out after 5 minutes." Fall back to the ${outsideVoiceFor(ctx).nativeLabel} subagent below. - Empty response: "${outsideVoiceFor(ctx).label} returned no response." Fall back to the ${outsideVoiceFor(ctx).nativeLabel} subagent below. -**If preflight is unavailable for any reason (or ${outsideVoiceFor(ctx).label} errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +\`CODEX_MODE: disabled\`, finish this section with \`outside_status: disabled\`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with \`run_in_background: false\` (subagents default to background since ${CC_BACKGROUND_DEFAULT_SINCE}; the findings must land before the workflow continues). The subagent has fresh context and no conversation bias — but it is the same harness; model identity stays unknown unless the runtime reports it; weigh its agreement accordingly. Bound it the same way as ${outsideVoiceFor(ctx).label}: cap the dispatch at a 5-minute timeout so "never blocking" @@ -739,6 +773,14 @@ Step 9 doc health summary you already produced) and finish the workflow. ${outsideVoicePreflight(ctx, { disabledBehavior: 'skip-all' })} +**Disabled is a terminal branch for this section.** If the preflight prints +\`CODEX_MODE: disabled\`, persist \`outside_status: disabled\` with the guarded +command below, then finish the documentation workflow. Do not construct a review prompt, invoke an outside CLI, +dispatch an Agent/Task fallback, or ask the apply question below. A disabled review +is an intentional opt-out, not a provider failure that needs a replacement reviewer. + +${generateDisabledOutsideRecord(ctx, 'codex-doc-review', 'documentation')} + When the mode is anything except \`disabled\`, print one line so the off-switch stays discoverable: "Running the ${outsideVoiceFor(ctx).label} doc review automatically (standard step). Disable: \`gstack-config set codex_reviews disabled\`." @@ -777,7 +819,13 @@ Present the full output verbatim under \`${outsideVoiceFor(ctx).label.toUpperCas Provider failures are informational; report the named provider, diagnosis, and missing coverage, then use the native fallback below. -**If preflight is unavailable for any reason (or ${outsideVoiceFor(ctx).label} errored at runtime):** +**Native fallback — provider unavailable or execution failed, with reviews enabled:** + +Immediately before dispatching, check the preflight result again. On +\`CODEX_MODE: disabled\`, finish this section with \`outside_status: disabled\`; +do not dispatch. Otherwise, use this fallback for missing/broken CLI, failed +authentication/model selection, a failed preflight, or a failed outside invocation. +The disabled branch never reaches this fallback. Dispatch via the Agent tool with the same prompt, passing \`run_in_background: false\` (subagents default to background since ${CC_BACKGROUND_DEFAULT_SINCE}). Bound it at a 5-minute timeout; if it never completes, treat the review as unavailable and continue. Present findings under \`DOCUMENTATION REVIEW (${outsideVoiceFor(ctx).nativeLabel} subagent):\`. If it fails: "Doc review unavailable. Continuing." diff --git a/test/disabled-plan-review-evidence.test.ts b/test/disabled-plan-review-evidence.test.ts new file mode 100644 index 000000000..7a32f6d9c --- /dev/null +++ b/test/disabled-plan-review-evidence.test.ts @@ -0,0 +1,222 @@ +import { afterAll, describe, expect, test } from 'bun:test'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { disabledPlanReviewEvidence, installDisabledPlanReviewFixture } from './helpers/disabled-plan-review-fixture'; + +const ROOT = resolve(import.meta.dir, '..'); +const TEMP = mkdtempSync(join(tmpdir(), 'gstack-disabled-plan-oracle-')); +afterAll(() => rmSync(TEMP, { recursive: true, force: true })); + +function completed() { + return { + exitReason: 'success', output: 'Outside review: disabled (codex_reviews disabled). Native review complete.', + transcript: [ + { type: 'system', subtype: 'init', tools: ['Bash', 'Read', 'Write', 'Agent'] }, + { type: 'assistant', message: { content: [{ type: 'tool_use', id: 'preflight', name: 'Bash', input: { command: './runtime/bin/gstack-config get codex_reviews; echo "CODEX_MODE: $_CODEX_MODE"' } }] } }, + { type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 'preflight', content: 'CODEX_MODE: disabled\n' }] } }, + { type: 'result', subtype: 'success', is_error: false, result: 'Outside review: disabled.' }, + ] as any[], + }; +} + +function dispatch(name: string, input: Record) { + return { type: 'assistant', message: { content: [{ type: 'tool_use', id: 'forbidden', name, input }] } }; +} + +const PRIOR_RECORD = { + skill: 'codex-plan-review', timestamp: '2026-09-08T00:00:00Z', status: 'clean', + source: 'codex', host: 'claude', outside_provider: 'codex', outside_status: 'completed', phase: 'plan-review', +}; +const disabledRecord = () => ({ ...PRIOR_RECORD, timestamp: '2026-09-08T00:00:01Z', status: 'skipped', source: 'none', outside_status: 'disabled' }); +function oracle(result: ReturnType, cliDispatchLog = '') { + return disabledPlanReviewEvidence(result, cliDispatchLog, [PRIOR_RECORD, disabledRecord()].map(record => JSON.stringify(record)).join('\n'), PRIOR_RECORD); +} + +describe('disabled outside-plan live oracle', () => { + test('requires a real disabled preflight with Agent available and valid completion', () => { + expect(oracle(completed(), '').passed).toBe(true); + }); + test('claimed disabled status without matching successful tool evidence cannot pass', () => { + for (const mutate of [ + (r: ReturnType) => { r.transcript.splice(1, 2); }, + (r: ReturnType) => { r.transcript[2].message.content[0].tool_use_id = 'unrelated'; }, + (r: ReturnType) => { r.transcript[2].message.content[0].is_error = true; }, + (r: ReturnType) => { r.transcript[1].message.content[0].input.command = 'cat OUTSIDE-PLAN.md'; }, + ]) { + const result = completed(); mutate(result); + expect(oracle(result, '').passed).toBe(false); + } + }); + test('missing Agent availability cannot make a no-fallback result pass', () => { + const result = completed(); result.transcript[0].tools = ['Bash', 'Read']; + expect(oracle(result, '').passed).toBe(false); + }); + test('empty, malformed, unsuccessful and timed-out completions cannot pass', () => { + for (const mutate of [ + (r: ReturnType) => { r.transcript.pop(); }, + (r: ReturnType) => { r.transcript.at(-1).is_error = true; }, + (r: ReturnType) => { r.transcript.at(-1).result = ''; }, + (r: ReturnType) => { r.transcript.at(-1).result = {}; }, + (r: ReturnType) => { r.transcript.at(-1).subtype = 'error_max_turns'; }, + (r: ReturnType) => { r.exitReason = 'timeout'; }, + ]) { + const result = completed(); mutate(result); + expect(oracle(result, '').passed).toBe(false); + } + }); + test('Agent/Task fallback dispatch fails even when the parent reports disabled', () => { + for (const tool of ['Agent', 'Task']) { + const result = completed(); result.transcript.splice(-1, 0, dispatch(tool, { prompt: 'Review the plan independently' })); + const evidence = oracle(result, ''); + expect(evidence.passed).toBe(false); + expect(evidence.fallbackCalls).toHaveLength(1); + } + }); + test('an observed outside CLI invocation fails even when the parent reports disabled', () => { + expect(oracle(completed(), 'codex invoked\n').passed).toBe(false); + }); + test('an unexecuted outside branch in a combined Bash block is not dispatch evidence', () => { + const result = completed(); + result.transcript[1].message.content[0].input.command = './runtime/bin/gstack-config get codex_reviews; echo "CODEX_MODE: disabled"; if [ "$mode" != disabled ]; then codex exec --json -; fi'; + const evidence = oracle(result, ''); + expect(evidence.passed).toBe(true); + expect(evidence.outsideCommandMentions).toHaveLength(1); + }); + test('requires a new persisted disabled record after the preserved completed record', () => { + for (const records of [ + [], [PRIOR_RECORD], [disabledRecord()], + [PRIOR_RECORD, { ...disabledRecord(), status: 'clean' }], + [PRIOR_RECORD, { ...disabledRecord(), source: 'codex' }], + [PRIOR_RECORD, { ...disabledRecord(), host: 'codex' }], + [PRIOR_RECORD, { ...disabledRecord(), outside_provider: 'claude-code' }], + [PRIOR_RECORD, { ...disabledRecord(), outside_status: 'completed' }], + [PRIOR_RECORD, { ...disabledRecord(), phase: 'documentation' }], + [PRIOR_RECORD, { ...disabledRecord(), timestamp: PRIOR_RECORD.timestamp }], + [PRIOR_RECORD, disabledRecord(), { ...PRIOR_RECORD, timestamp: '2026-09-08T00:00:02Z' }], + ]) { + expect(disabledPlanReviewEvidence(completed(), '', records.map(record => JSON.stringify(record)).join('\n'), PRIOR_RECORD).passed).toBe(false); + } + expect(disabledPlanReviewEvidence(completed(), '', '{bad-json}', PRIOR_RECORD).passed).toBe(false); + }); + + test('missing or falsely completed attribution fails', () => { + for (const output of ['Done.', 'Outside review is not disabled.', 'outside_status: completed', 'Outside review disabled. Both reviewers agree.']) { + const result = completed(); result.output = output; + expect(oracle(result, '').passed).toBe(false); + } + }); + + test('generated preflight reads isolated disabled config without executing the available CLI', () => { + const rendered = join(TEMP, 'render'); + const generation = spawnSync(process.execPath, ['run', 'scripts/gen-skill-docs.ts', '--host', 'claude', '--out-dir', rendered], { + cwd: ROOT, encoding: 'utf8', timeout: 120_000, + }); + expect(generation.status, generation.stderr).toBe(0); + const repo = mkdtempSync(join(TEMP, 'repo-')); + const fixture = installDisabledPlanReviewFixture(rendered, repo, ROOT); + const preflight = fixture.instructions.match(/```bash\n([\s\S]*?)\n```/)?.[1]; + expect(preflight).toBeDefined(); + const result = spawnSync('bash', ['-c', preflight!], { + cwd: repo, env: { ...process.env, ...fixture.env }, encoding: 'utf8', timeout: 5_000, + }); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toMatch(/^CODEX_MODE: disabled\s*$/m); + expect(existsSync(fixture.cliDispatchLog)).toBe(false); + const persist = [...fixture.instructions.matchAll(/```bash\n([\s\S]*?)\n```/g)] + .map(match => match[1]).find(command => command.includes('"outside_status":"disabled"')); + expect(persist).toBeDefined(); + const logged = spawnSync('bash', ['-c', persist!], { + cwd: repo, env: { ...process.env, ...fixture.env }, encoding: 'utf8', timeout: 5_000, + }); + expect(logged.status, logged.stderr).toBe(0); + const reviewLog = readFileSync(fixture.reviewLogPath, 'utf8'); + const records = reviewLog.trim().split('\n').map(line => JSON.parse(line)); + expect(records).toHaveLength(2); + expect(records[0]).toEqual(fixture.priorRecord); + expect(records[1]).toMatchObject({ skill: 'codex-plan-review', host: 'claude', outside_provider: 'codex', + outside_status: 'disabled', phase: 'plan-review', status: 'skipped', source: 'none' }); + expect(disabledPlanReviewEvidence(completed(), '', reviewLog, fixture.priorRecord).persistedDisabled).toBe(true); + // A fresh enabled shell must not append a disabled record, even if the + // model unnecessarily executes this guarded fence after the preflight. + const enable = spawnSync(join(ROOT, 'bin/gstack-config'), ['set', 'codex_reviews', 'enabled'], { + cwd: repo, env: { ...process.env, ...fixture.env }, encoding: 'utf8', timeout: 5_000, + }); + expect(enable.status, enable.stderr).toBe(0); + const enabledLog = spawnSync('bash', ['-c', persist!], { + cwd: repo, env: { ...process.env, ...fixture.env }, encoding: 'utf8', timeout: 5_000, + }); + expect(enabledLog.status, enabledLog.stderr).toBe(0); + expect(readFileSync(fixture.reviewLogPath, 'utf8')).toBe(reviewLog); + // Prove the sentinel observes a real invocation; an empty broken spy is + // not acceptable evidence that the generated off branch skipped the CLI. + const forbidden = spawnSync('codex', ['--version'], { cwd: repo, env: { ...process.env, ...fixture.env }, encoding: 'utf8', timeout: 5_000 }); + expect(forbidden.status).toBe(73); + expect(readFileSync(fixture.cliDispatchLog, 'utf8')).toBe('codex invoked\n'); + }, 150_000); + test('generated Codex plan and documentation log fences resolve runtime in fresh shells', () => { + const rendered = join(TEMP, 'codex-render'); + const generated = spawnSync(process.execPath, ['run', 'scripts/gen-skill-docs.ts', '--host', 'codex', '--out-dir', rendered], { + cwd: ROOT, encoding: 'utf8', timeout: 120_000, + }); + expect(generated.status, generated.stderr).toBe(0); + const repo = mkdtempSync(join(TEMP, 'codex-repo-')); + const home = join(repo, 'owned-home'); + const codexHome = join(home, 'custom codex'); + const state = join(repo, 'gstack-state'); + mkdirSync(join(codexHome, 'skills'), { recursive: true }); + symlinkSync(ROOT, join(codexHome, 'skills', 'gstack'), 'dir'); + const env = { ...process.env, HOME: home, CODEX_HOME: codexHome, GSTACK_HOME: state, GSTACK_STATE_ROOT: state, + GSTACK_PROJECT_SLUG: 'codex-disabled-fixture', GSTACK_ROOT: '', GSTACK_BIN: '', GSTACK_ACTIVE_HOST: 'codex' }; + const run = (command: string, overrides: NodeJS.ProcessEnv = {}) => spawnSync('bash', ['-c', command], { cwd: repo, env: { ...env, ...overrides }, encoding: 'utf8', timeout: 5_000 }); + const config = (mode: string) => spawnSync(join(ROOT, 'bin/gstack-config'), ['set', 'codex_reviews', mode], { + cwd: repo, env, encoding: 'utf8', timeout: 5_000, + }); + const slug = spawnSync(join(ROOT, 'bin/gstack-slug'), [], { cwd: repo, env, encoding: 'utf8', timeout: 5_000 }); + expect(slug.status, slug.stderr).toBe(0); + const branch = /^BRANCH=([a-zA-Z0-9._-]+)$/m.exec(slug.stdout)?.[1]; + expect(branch).toBeDefined(); + const logPath = join(state, 'projects', env.GSTACK_PROJECT_SLUG, `${branch}-reviews.jsonl`); + const brokenRuntime = join(repo, 'broken-runtime'); + mkdirSync(join(brokenRuntime, 'bin'), { recursive: true }); + mkdirSync(join(brokenRuntime, 'lib')); + writeFileSync(join(brokenRuntime, 'lib/claude-bin.ts'), '// valid explicit runtime marker\n'); + writeFileSync(join(brokenRuntime, 'bin/gstack-config'), '#!/bin/sh\nexit 19\n', { mode: 0o755 }); + const unexpectedWrite = join(repo, 'unexpected-review-write'); + writeFileSync(join(brokenRuntime, 'bin/gstack-review-log'), '#!/bin/sh\nprintf invoked > "$UNEXPECTED_REVIEW_WRITE"\n', { mode: 0o755 }); + for (const [skill, id, phase] of [ + ['gstack-plan-eng-review', 'codex-plan-review', 'plan-review'], + ['gstack-document-release', 'codex-doc-review', 'documentation'], + ]) { + const instructions = readFileSync(join(rendered, '.agents', 'skills', skill, 'SKILL.md'), 'utf8'); + const fence = [...instructions.matchAll(/```bash\n([\s\S]*?)\n```/g)] + .map(match => match[1]).find(command => command.includes(`"skill":"${id}"`) && command.includes('"outside_status":"disabled"')); + expect(fence, skill).toBeDefined(); + expect(config('disabled').status).toBe(0); + const prior = { skill: id, timestamp: new Date(Date.now() - 60_000).toISOString(), status: 'clean', source: 'claude-code', + host: 'codex', outside_provider: 'claude-code', outside_status: 'completed', phase }; + const seed = spawnSync(join(ROOT, 'bin/gstack-review-log'), [JSON.stringify(prior)], { cwd: repo, env, encoding: 'utf8', timeout: 5_000 }); + expect(seed.status, seed.stderr).toBe(0); + const before = readFileSync(logPath, 'utf8'); + const logged = run(fence!); // No variables survive from a preflight shell. + expect(logged.status, logged.stderr).toBe(0); + const after = readFileSync(logPath, 'utf8'); + expect(after.startsWith(before)).toBe(true); + const records = after.trim().split('\n').map(line => JSON.parse(line)).filter(record => record.skill === id); + expect(records).toHaveLength(2); + expect(records[1]).toMatchObject({ skill: id, host: 'codex', outside_provider: 'claude-code', + outside_status: 'disabled', phase, status: 'skipped', source: 'none' }); + expect(config('enabled').status).toBe(0); + const skipped = run(fence!); + expect(skipped.status, skipped.stderr).toBe(0); + expect(readFileSync(logPath, 'utf8')).toBe(after); + const failedRead = run(fence!, { GSTACK_ROOT: brokenRuntime, UNEXPECTED_REVIEW_WRITE: unexpectedWrite }); + expect(failedRead.status).not.toBe(0); + expect(failedRead.stderr).toContain('Cannot read codex_reviews'); + expect(readFileSync(logPath, 'utf8')).toBe(after); + expect(existsSync(unexpectedWrite)).toBe(false); + } + }, 150_000); + +}); diff --git a/test/helpers/disabled-plan-review-fixture.ts b/test/helpers/disabled-plan-review-fixture.ts new file mode 100644 index 000000000..431fc5244 --- /dev/null +++ b/test/helpers/disabled-plan-review-fixture.ts @@ -0,0 +1,114 @@ +/** Isolated real-parent fixture and execution oracle for the plan-review off switch. */ +import { mkdirSync, readFileSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; +import { delimiter, join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { extractSkillSections } from './skill-fixture'; +import { claudeOutsideExecutions } from './outside-voice-evidence'; + +export const OUTSIDE_PLAN_SECTION = 'Outside Voice — Independent Plan Challenge (default-on)'; + +/** Extract generated instructions; runtime paths are the only content substitution. */ +export function installDisabledPlanReviewFixture(rendered: string, repo: string, runtimeRoot: string) { + const source = join(rendered, 'plan-eng-review'); + const main = readFileSync(join(source, 'SKILL.md'), 'utf8'); + const frontmatter = main.match(/^---\r?\n[\s\S]*?\r?\n---\r?\n/)?.[0]; + if (!frontmatter) throw new Error('Generated plan-eng-review has no frontmatter'); + // The shared plan challenge is lazily loaded. Give the established, + // fence-aware extractor its frontmatter without copying any workflow prose. + const input = join(repo, 'outside-plan-source.md'); + writeFileSync(input, frontmatter + readFileSync(join(source, 'sections/review-sections.md'), 'utf8')); + let generated: string; + try { generated = extractSkillSections(input, [OUTSIDE_PLAN_SECTION]); } + finally { unlinkSync(input); } + symlinkSync(runtimeRoot, join(repo, 'runtime'), 'dir'); + const instructions = generated + .replaceAll('$HOME/.claude/skills/gstack', '$PWD/runtime') + .replaceAll('~/.claude/skills/gstack', './runtime'); + const workflowPath = join(repo, 'OUTSIDE-PLAN.md'); + writeFileSync(workflowPath, instructions); + + const stateDir = join(repo, 'gstack-state'); + const configDir = join(repo, 'claude-config'); + const spyDir = join(repo, 'cli-bin'); + const cliDispatchLog = join(repo, 'outside-cli-dispatch.log'); + for (const dir of [stateDir, configDir, spyDir]) mkdirSync(dir, { recursive: true }); + // A forbidden invocation is observable without buying another model call. + // No prompt/credentials are recorded; even --version/auth probes count. + writeFileSync(join(spyDir, 'codex'), '#!/bin/sh\nprintf "codex invoked\\n" >> "$GSTACK_DISABLED_CLI_LOG"\nexit 73\n', { mode: 0o755 }); + const env = { + PATH: `${spyDir}${delimiter}${process.env.PATH ?? ''}`, + CLAUDE_CONFIG_DIR: configDir, + GSTACK_HOME: stateDir, + GSTACK_STATE_ROOT: stateDir, + GSTACK_DISABLED_CLI_LOG: cliDispatchLog, + GSTACK_ACTIVE_HOST: 'claude', + GSTACK_PROJECT_SLUG: 'disabled-plan-fixture', + }; + const config = spawnSync(join(runtimeRoot, 'bin/gstack-config'), ['set', 'codex_reviews', 'disabled'], { + cwd: repo, env: { ...process.env, ...env }, encoding: 'utf8', timeout: 5_000, + }); + if (config.status !== 0) throw new Error(`Cannot seed isolated review control: ${config.stderr}`); + // Seed real historical coverage so the new disabled record must replace it + // in the dashboard's latest-record view, not merely appear in final prose. + const prior = { + skill: 'codex-plan-review', timestamp: new Date(Date.now() - 60_000).toISOString(), + status: 'clean', source: 'codex', host: 'claude', outside_provider: 'codex', + outside_status: 'completed', phase: 'plan-review', + }; + const logged = spawnSync(join(runtimeRoot, 'bin/gstack-review-log'), [JSON.stringify(prior)], { + cwd: repo, env: { ...process.env, ...env }, encoding: 'utf8', timeout: 5_000, + }); + if (logged.status !== 0) throw new Error(`Cannot seed historical review: ${logged.stderr}`); + const slug = spawnSync(join(runtimeRoot, 'bin/gstack-slug'), [], { + cwd: repo, env: { ...process.env, ...env }, encoding: 'utf8', timeout: 5_000, + }); + const branch = /^BRANCH=([a-zA-Z0-9._-]+)$/m.exec(slug.stdout)?.[1]; + if (slug.status !== 0 || !branch) throw new Error('Cannot resolve isolated review-log branch'); + const reviewLogPath = join(stateDir, 'projects', env.GSTACK_PROJECT_SLUG, `${branch}-reviews.jsonl`); + const priorRecord = JSON.parse(readFileSync(reviewLogPath, 'utf8').trim()); + return { workflowPath, instructions, generated, stateDir, cliDispatchLog, reviewLogPath, priorRecord, env }; +} + +export function disabledPlanReviewEvidence(result: { + exitReason: string; output: string; transcript: any[]; +}, cliDispatchLog: string, reviewLog = '', priorRecord?: Record) { + const init = result.transcript.find(event => event?.type === 'system' && event.subtype === 'init'); + const terminal = result.transcript.filter(event => event?.type === 'result').at(-1); + const toolCalls = result.transcript.flatMap(event => event?.type === 'assistant' && Array.isArray(event.message?.content) + ? event.message.content.filter((block: any) => block.type === 'tool_use') : []); + const fallbackCalls = toolCalls.filter((call: any) => call.name === 'Agent' || call.name === 'Task'); + // Command text may contain an unexecuted guarded branch. Keep it for + // diagnostics; only the executable sentinel establishes CLI dispatch. + const outsideCommandMentions = toolCalls.filter((call: any) => call.name === 'Bash' + && /\bcodex\s+(?:exec|review)\b|\bgstack-claude-code(?:['"])?\s+--/.test(String(call.input?.command ?? ''))); + const disabledPreflight = claudeOutsideExecutions(result.transcript).filter(call => call.succeeded + && /gstack-config\b/.test(call.command) && /codex_reviews\b/.test(call.command) + && /^CODEX_MODE: disabled\s*$/m.test(call.output)); + const completed = result.exitReason === 'success' && terminal?.subtype === 'success' + && terminal.is_error !== true && typeof terminal.result === 'string' && terminal.result.trim().length > 0; + const disabledAttribution = /outside_status["'`*]*\s*[:=]\s*["'`*]*disabled\b/i.test(result.output) + || result.output.split('\n').some(line => /\b(?:outside(?:\s+(?:voice|review))?|codex(?:\s+review)?)\b/i.test(line) + && /\bdisabled\b/i.test(line) && !/\bnot\s+disabled\b/i.test(line)); + const falseCompletion = /\b(?:both reviewers agree|outside_status["'`*]*\s*[:=]\s*["'`*]*completed)\b/i.test(result.output); + const agentAvailable = Array.isArray(init?.tools) && init.tools.includes('Agent'); + let records: any[] = []; + let malformedLog = false; + try { records = reviewLog.trim() ? reviewLog.trim().split('\n').map(line => JSON.parse(line)) : []; } + catch { malformedLog = true; } + const planRecords = records.filter(record => record && typeof record === 'object' && record.skill === 'codex-plan-review'); + const priorIndex = priorRecord ? planRecords.findIndex(record => JSON.stringify(record) === JSON.stringify(priorRecord)) : -1; + const latestRecord = planRecords.at(-1); + const persistedDisabled = !malformedLog && priorIndex >= 0 && priorIndex < planRecords.length - 1 + && priorRecord?.outside_status === 'completed' && latestRecord.status === 'skipped' + && latestRecord.source === 'none' && latestRecord.host === 'claude' + && latestRecord.outside_provider === 'codex' && latestRecord.outside_status === 'disabled' + && latestRecord.phase === 'plan-review' && typeof latestRecord.timestamp === 'string' + && typeof priorRecord.timestamp === 'string' && Number.isFinite(Date.parse(latestRecord.timestamp)) + && Date.parse(latestRecord.timestamp) > Date.parse(priorRecord.timestamp); + return { + passed: completed && agentAvailable && disabledPreflight.length > 0 && fallbackCalls.length === 0 + && cliDispatchLog.trim() === '' && disabledAttribution && !falseCompletion && persistedDisabled, + completed, agentAvailable, disabledAttribution, falseCompletion, persistedDisabled, latestRecord, malformedLog, + disabledPreflight, fallbackCalls, outsideCommandMentions, cliDispatchLog, + }; +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 009216534..d19825cc8 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -291,6 +291,17 @@ export const E2E_TOUCHFILES: Record = { 'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-fixture.ts', 'test/helpers/outside-voice-evidence.ts', 'test/skill-e2e-outside-voice.test.ts', ], + // Disabled means no extra plan review, including a native Agent fallback. + 'outside-plan-disabled-no-fallback': [ + 'plan-eng-review/**', 'plan-ceo-review/**', 'hosts/claude.ts', 'hosts/define-host.ts', + 'scripts/gen-skill-docs.ts', 'scripts/resolvers/index.ts', 'scripts/resolvers/sections.ts', + 'scripts/resolvers/review.ts', 'scripts/resolvers/outside-voice.ts', 'scripts/resolvers/constants.ts', + 'bin/gstack-config', 'bin/gstack-codex-probe', 'bin/gstack-review-log', 'bin/gstack-slug', 'bin/gstack-wtree', 'bin/gstack-brain-enqueue', 'test/helpers/session-runner.ts', + 'test/helpers/hermetic-env.ts', 'test/helpers/skill-fixture.ts', 'test/helpers/outside-voice-evidence.ts', + 'test/helpers/disabled-plan-review-fixture.ts', 'test/disabled-plan-review-evidence.test.ts', + 'test/skill-e2e-outside-plan-disabled.test.ts', + ], + // GPT-5.6 Sol scope-termination E2E (Codex CLI, full generated investigate skill) 'codex-sol-scope-termination': ['model-overlays/gpt-5.6-sol.md', 'scripts/models.ts', 'scripts/resolvers/model-overlay.ts', 'scripts/resolvers/preamble/**', 'investigate/**', 'test/helpers/codex-session-runner.ts', 'test/codex-e2e-sol-scope.test.ts'], @@ -746,6 +757,7 @@ export const E2E_TIERS: Record = { 'codex-review-findings': 'periodic', 'outside-voice-codex-to-claude-code': 'periodic', 'outside-voice-claude-code-to-codex': 'periodic', + 'outside-plan-disabled-no-fallback': 'periodic', 'codex-sol-scope-termination': 'periodic', 'gemini-smoke': 'periodic', diff --git a/test/skill-e2e-outside-plan-disabled.test.ts b/test/skill-e2e-outside-plan-disabled.test.ts new file mode 100644 index 000000000..85c861f7b --- /dev/null +++ b/test/skill-e2e-outside-plan-disabled.test.ts @@ -0,0 +1,72 @@ +/** Periodic paid regression: the plan-review off switch never dispatches a replacement reviewer. */ +import { afterAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { e2eTierEnabled } from './helpers/e2e-gate'; +import { runSkillTest } from './helpers/session-runner'; +import { EvalCollector, getProjectEvalDir } from './helpers/eval-store'; +import { CAPTURE_MS, CAPTURE_LONG_MS } from './helpers/eval-budgets'; +import { disabledPlanReviewEvidence, installDisabledPlanReviewFixture } from './helpers/disabled-plan-review-fixture'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const periodic = e2eTierEnabled('periodic'); +const enabled = periodic && !!Bun.which('claude'); +if (periodic && !enabled) process.stderr.write('Disabled plan-review E2E: SKIPPED — Claude CLI is unavailable. No behavior was exercised.\n'); +const selected = enabled ? (await import('./helpers/e2e-helpers')).selectedTests : []; +const collector = enabled ? new EvalCollector('e2e-outside-plan-disabled') : null; +const describeLive = enabled ? describe : describe.skip; +const testName = 'outside-plan-disabled-no-fallback'; +const testSelected = selected === null || selected.includes(testName) ? test : test.skip; +afterAll(async () => { if (collector) await collector.finalize(); }); + +describeLive('Disabled outside plan review', () => { + testSelected('outside-plan-disabled-no-fallback', async () => { + const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-outside-disabled-')); + const repo = path.join(fixtureRoot, 'repo'); + const rendered = path.join(fixtureRoot, 'render'); + fs.mkdirSync(repo); + const runId = `${testName}-${process.env.GSTACK_EVAL_RUN_ID ?? Date.now()}-${path.basename(fixtureRoot)}`; + const evidenceDir = path.join(process.env.GSTACK_EVAL_DIR || getProjectEvalDir(), 'disabled-plan-review', runId); + fs.mkdirSync(evidenceDir, { recursive: true }); + try { + const render = Bun.spawnSync([process.execPath, 'run', 'scripts/gen-skill-docs.ts', '--host', 'claude', '--out-dir', rendered], { + cwd: ROOT, stdout: 'pipe', stderr: 'pipe', timeout: 120_000, + }); + if (render.exitCode !== 0) throw new Error(`Isolated generation failed: ${render.stderr}`); + const fixture = installDisabledPlanReviewFixture(rendered, repo, ROOT); + fs.writeFileSync(path.join(repo, 'PLAN.md'), '# Accepted plan\n\nAdd a documented --json flag to the existing status command. Preserve the text output by default, and test both formats. Native review is complete.\n'); + // The parent must discover the control through the actual preflight. + // Do not tell it the expected branch or forbid Agent in the prompt/tools. + const prompt = 'The native engineering review of PLAN.md is complete. Read OUTSIDE-PLAN.md and execute that final workflow section for this plan, following its actual configuration and branch rules. Then return the workflow completion summary, including outside-review attribution. This is an isolated review fixture; all required reviewer tools are authorized.'; + fs.writeFileSync(path.join(evidenceDir, 'instructions.md'), fixture.instructions); + const result = await runSkillTest({ + workingDirectory: repo, prompt, testName, runId, + timeout: CAPTURE_MS, maxTurns: 15, + tools: ['Bash', 'Read', 'Write', 'Grep', 'Glob', 'Agent'], + allowedTools: ['Bash', 'Read', 'Write', 'Grep', 'Glob', 'Agent'], + env: fixture.env, + }); + const cliDispatchLog = fs.existsSync(fixture.cliDispatchLog) ? fs.readFileSync(fixture.cliDispatchLog, 'utf8') : ''; + const reviewLog = fs.existsSync(fixture.reviewLogPath) ? fs.readFileSync(fixture.reviewLogPath, 'utf8') : ''; + const evidence = disabledPlanReviewEvidence(result, cliDispatchLog, reviewLog, fixture.priorRecord); + fs.writeFileSync(path.join(evidenceDir, 'review-log.jsonl'), reviewLog); + // Persist positive and negative native tool evidence before assertions or + // fixture cleanup. A timeout, missing init/result, or empty run never passes. + fs.writeFileSync(path.join(evidenceDir, 'transcript.ndjson'), result.transcript.map(event => JSON.stringify(event)).join('\n') + '\n'); + fs.writeFileSync(path.join(evidenceDir, 'evidence.json'), JSON.stringify({ ...evidence, exitReason: result.exitReason, model: result.model, output: result.output, cost: result.costEstimate }, null, 2) + '\n'); + collector?.addTest({ name: testName, suite: 'outside-plan-disabled', tier: 'e2e', passed: evidence.passed, + duration_ms: result.duration, cost_usd: result.costEstimate.estimatedCost, model: result.model, + turns_used: result.costEstimate.turnsUsed, exit_reason: result.exitReason, + transcript: result.transcript, output: result.output, prompt, + }); + expect(evidence, `Native evidence: ${evidenceDir}`).toMatchObject({ + passed: true, completed: true, agentAvailable: true, disabledAttribution: true, + falseCompletion: false, persistedDisabled: true, fallbackCalls: [], cliDispatchLog: '', + }); + expect(evidence.disabledPreflight.length).toBeGreaterThan(0); + } finally { + fs.rmSync(fixtureRoot, { recursive: true, force: true }); + } + }, CAPTURE_LONG_MS); // One normal capture plus bounded isolated generation. +}); diff --git a/test/touchfiles.test.ts b/test/touchfiles.test.ts index 1bec835a6..f0499ecee 100644 --- a/test/touchfiles.test.ts +++ b/test/touchfiles.test.ts @@ -121,8 +121,9 @@ describe('selectTests', () => { expect(result.selected).toContain('plan-ceo-split-overflow'); // v2 plan Phase B carve: the section-loading E2E depends on plan-ceo-review/**. expect(result.selected).toContain('plan-ceo-section-loading'); - expect(result.selected.length).toBe(21); - expect(result.skipped.length).toBe(Object.keys(E2E_TOUCHFILES).length - 21); + expect(result.selected).toContain('outside-plan-disabled-no-fallback'); + expect(result.selected.length).toBe(22); + expect(result.skipped.length).toBe(Object.keys(E2E_TOUCHFILES).length - 22); }); test('global touchfile triggers ALL tests', () => {