fix: require complete section-loading eval reports

test/carve-section-loading.test.ts

@@ -22,7 +22,7 @@
 import { test, expect } from 'bun:test';
 import { CAPTURE_LONG_MS } from './helpers/eval-budgets';
 import { describeE2ETier } from './helpers/e2e-gate';
-import { setupSkillDir, skillFromWorktree, captureSectionReads } from './helpers/auq-sdk-capture';
+import { setupSkillDir, skillFromWorktree, captureSectionReads, LONG_SECTION_CAPTURE_MS } from './helpers/auq-sdk-capture';
 import { CARVE_GUARDS } from './helpers/carve-guards';
 
 const describeE2E = describeE2ETier('periodic');
@@ -78,7 +78,7 @@ describeE2E('carve behavioral section-loading (periodic, SDK capture)', () => {
           // their required section reads inside 60s but need 300-450s of
           // wall clock to finish the report on slower sandboxes — a timeout
           // there reads as a loading failure when the carve invariant held.
-          timeout: 480_000,
+          timeout: LONG_SECTION_CAPTURE_MS,
         });
 
         const missing = guard.requiredReads.filter((s) => !readSections.has(s));

test/ceo-section-loading-fixture.test.ts

@@ -0,0 +1,52 @@
+import { describe, expect, test } from 'bun:test';
+import {
+  CACHE_READ_WRITE_SKETCH,
+  CEO_SECTION_CACHE_PLAN,
+  hasStaleFillRaceFinding,
+} from './helpers/ceo-section-loading-fixture';
+
+describe('CEO section-loading cache fixture', () => {
+  test('the exact proposed wrapper retains a reproducible stale-fill race', async () => {
+    let releaseRead!: (value: string) => void;
+    let stored = 'old';
+    const cache = new Map<string, string>();
+    const repository = {
+      read: () => new Promise<string>((resolve) => { releaseRead = resolve; }),
+      write: async (_key: string, value: string) => { stored = value; return value; },
+    };
+    // Execute the same sketch the live reviewer receives, not a second model
+    // of its ordering. Holding the old read exposes the intended interleaving.
+    const { readProfile, writeProfile } = new Function('cache', 'repository',
+      CACHE_READ_WRITE_SKETCH + '\nreturn { readProfile, writeProfile };')(cache, repository);
+    const pending = readProfile('tenant:profile');
+    await writeProfile('tenant:profile', 'new');
+    releaseRead('old');
+    await pending;
+    expect(stored).toBe('new');
+    expect(await readProfile('tenant:profile')).toBe('old');
+    expect(CEO_SECTION_CACHE_PLAN).toContain(CACHE_READ_WRITE_SKETCH);
+    expect(hasStaleFillRaceFinding(CEO_SECTION_CACHE_PLAN)).toBe(false);
+  });
+
+  test.each([
+    // Actual finding in the unchanged fixture's successful 38 KB live report.
+    '**Missing: What happens to in-flight requests during invalidation?** If a write invalidates a key and 10 requests are simultaneously loading it (cache miss, in-flight DB fetch), all 10 will cache the same value after the invalidation. The invalidated key may get re-populated with a stale value if any of those fetches started before the write. No mention of this race.',
+    'P1: An in-flight read can repopulate stale data after a committed write invalidates the key. Guard fills with a generation token.',
+    '| Cache fill race | An older value fetched before the write is inserted after eviction, so the next read is stale. | Add a per-key epoch. |',
+    '**Invalidation race:** the pending fetch stores an outdated snapshot after cache.delete. Serialize the fill with mutation.',
+  ])('recognizes the actual ordering defect: %s', (report) => {
+    expect(hasStaleFillRaceFinding(report)).toBe(true);
+  });
+
+  test.each([
+    'The full review is complete. No issues found.',
+    'A stale value expires after 30 seconds. The LRU byte cap is adequate.',
+    'If invalidation throws after a write, the cache retains stale data. Log the failure and bypass the cache.',
+    'Read and write concurrency is covered. No stale data can be returned.',
+    '| Reads | Coalesced concurrent misses |\n| Writes | Invalidation failure leaves stale data |',
+    '```javascript\n// An in-flight read can cache stale data after invalidation.\n```',
+    '> An in-flight read can cache stale data after invalidation.',
+  ])('rejects completion, unrelated text, and quoted source: %s', (report) => {
+    expect(hasStaleFillRaceFinding(report)).toBe(false);
+  });
+});

test/helpers/auq-sdk-capture.ts

@@ -20,6 +20,14 @@ import { runSkillTest, type SkillTestResult } from './session-runner';
 
 const ROOT = path.resolve(__dirname, '..', '..');
 
+/**
+ * Existing long section-loader work budget (v1.71): complete workflows can
+ * load their sections quickly, then need 300–450s to generate the full report.
+ * Keep 120s of the CAPTURE_LONG_MS outer budget for setup and reporting.
+ * Ordinary captureSectionReads callers retain the 300s default below.
+ */
+export const LONG_SECTION_CAPTURE_MS = 480_000;
+
 /** The 7 decision-brief format elements graded on the captured AUQ text. */
 export const AUQ_FORMAT_ELEMENTS: Array<{ field: string; re: RegExp }> = [
   { field: 'ELI10:', re: /ELI10\s*:/i },
@@ -217,6 +225,22 @@ This is a capture test, not an interactive session. Skip any system-audit / envi
  * resolves to, so Read/Grep/Glob/Write is all the agent needs (no Bash → it cannot
  * `find /` its way out, nor run git/gh mutations).
  */
+export function hasDisabledOutsideReview(output: string): boolean {
+  const headings = [...output.matchAll(/^## GSTACK REVIEW REPORT[ \t]*\r?$/gm)];
+  const heading = headings.at(-1);
+  if (!heading) return false;
+  const section = output.slice(heading.index! + heading[0].length).split(/^##[ \t]+/m, 1)[0];
+  const plain = (cell: string) => cell.replace(/[*_`]/g, '').trim().replace(/\s+/g, ' ').toLowerCase();
+  for (const line of section.split('\n')) {
+    if (!line.trimStart().startsWith('|')) continue;
+    const cells = line.split('|').map(plain);
+    if (cells[1] === 'outside review') {
+      return /^disabled(?:$|\s|[(:—–-])/.test(cells[5] ?? '');
+    }
+  }
+  return false;
+}
+
 export async function captureSectionReads(opts: {
   planDir: string;
   skillName: string;
@@ -230,9 +254,19 @@ export async function captureSectionReads(opts: {
   model?: string;
   maxTurns?: number;
   timeout?: number;
+  /** Measure native section loading with the documented extra-review opt-out. */
+  nativeReviewOnly?: boolean;
 }): Promise<{ readSections: Set<string>; reportProduced: boolean; toolCalls: SkillTestResult['toolCalls']; output: string }> {
   const outFile = path.join(opts.planDir, opts.reportFile ?? 'REPORT.md');
   const skillPath = path.join(opts.planDir, opts.skillName, 'SKILL.md');
+  // Outside-review dispatch has separate behavioral coverage. Native-only
+  // captures use the real supported control in state owned by this call;
+  // never mutate the operator's or another capture's gstack configuration.
+  const stateDir = opts.nativeReviewOnly
+    ? fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-section-state-')) : null;
+  const nativeReviewRule = stateDir
+    ? `\n- Read ${path.join(stateDir, 'config.yaml')}, the isolated gstack configuration for this capture. It sets codex_reviews: disabled. Follow that documented control: skip the entire extra outside-review step, including its native fallback, and report outside coverage as disabled. Complete all native review sections and the full required report.`
+    : '';
   const prompt = `You are running an automated skill-execution test. No human is present, so AskUserQuestion is unavailable. The ONLY skill file you may read is this absolute path: ${skillPath}. Do NOT Glob/find/search for any other SKILL.md anywhere — especially nothing under ~/.claude or /Users.
 
 Read ${skillPath} and EXECUTE its workflow for this scenario:
@@ -244,18 +278,26 @@ Rules for this run:
 - At any decision point that would call AskUserQuestion, silently pick the skill's recommended option and continue. Do NOT stop to ask.
 - This skill's body has been carved into on-demand sections/. When the skill gives a STOP-Read directive (for example "Read \`.../sections/<file>\` and execute it in full"), you MUST actually Read that sections/ file with the Read tool BEFORE doing the work it covers. Do not work from memory.
 - Do NOT run git, gh, commit, push, or any mutating command.
-- When the workflow is complete, write the skill's final output (the full review report / ship plan, including any required report table) to ${outFile}.`;
+- When the workflow is complete, write the skill's final output (the full review report / ship plan, including any required report table) to ${outFile}.${nativeReviewRule}`;
 
-  const result = await runSkillTest({
-    prompt,
-    workingDirectory: opts.planDir,
-    allowedTools: ['Read', 'Grep', 'Glob', 'Write'],
-    maxTurns: opts.maxTurns ?? 25,
-    timeout: opts.timeout ?? 300_000,
-    testName: opts.testName,
-    runId: opts.runId,
-    model: resolveEvalModel('capture', opts.model),
-  });
+  let result: SkillTestResult;
+  try {
+    if (stateDir) fs.writeFileSync(path.join(stateDir, 'config.yaml'), 'codex_reviews: disabled\n');
+    result = await runSkillTest({
+      prompt,
+      workingDirectory: opts.planDir,
+      allowedTools: ['Read', 'Grep', 'Glob', 'Write'],
+      tools: ['Read', 'Grep', 'Glob', 'Write'],
+      maxTurns: opts.maxTurns ?? 25,
+      timeout: opts.timeout ?? 300_000,
+      testName: opts.testName,
+      runId: opts.runId,
+      model: resolveEvalModel('capture', opts.model),
+      ...(stateDir ? { env: { GSTACK_HOME: stateDir, GSTACK_STATE_ROOT: stateDir } } : {}),
+    });
+  } finally {
+    if (stateDir) fs.rmSync(stateDir, { recursive: true, force: true });
+  }
 
   const readSections = new Set<string>();
   for (const c of result.toolCalls) {
@@ -267,7 +309,8 @@ Rules for this run:
 
   let output = '';
   try { output = fs.readFileSync(outFile, 'utf-8'); } catch { output = result.output ?? ''; }
-  const reportProduced = opts.reportMarker ? opts.reportMarker.test(output) : output.trim().length > 0;
+  const reportProduced = result.exitReason === 'success'
+    && (opts.reportMarker ? opts.reportMarker.test(output) : output.trim().length > 0);
 
   return { readSections, reportProduced, toolCalls: result.toolCalls, output };
 }

test/helpers/ceo-section-loading-fixture.ts

@@ -0,0 +1,93 @@
+/**
+ * Section loading needs a complete, bounded plan, not a finding-count fixture.
+ * The former two-bullet plan made a successful review invent key isolation,
+ * error, concurrency, observability, rollout, and test contracts in a 38 KB
+ * report. Those surrounding contracts are explicit here; the read/write sketch
+ * still permits an old in-flight read to refill a key after write invalidation.
+ */
+export const CACHE_READ_WRITE_SKETCH = `async function readProfile(key) {
+  const cached = cache.get(key);
+  if (cached !== undefined) return cached;
+  const value = await repository.read(key);
+  cache.set(key, value);
+  return value;
+}
+
+async function writeProfile(key, update) {
+  const saved = await repository.write(key, update);
+  cache.delete(key);
+  return saved;
+}`;
+
+export const CEO_SECTION_CACHE_PLAN = `# Plan: cache profile summaries in one process
+
+## Measured problem and accepted scope
+The existing profile-summary service has one active process. A one-week trace
+shows repeated reads of about 900 hot keys: DB CPU is 70%, with read p95 120 ms.
+Add a process-local LRU wrapper to the existing repository. Acceptance targets
+are at least 60% cache hits, DB CPU below 50%, and read p95 below 60 ms, with the
+existing error-rate and correctness SLOs unchanged. This is an internal backend
+change with no UI, API, schema, pricing, or developer onboarding change.
+
+## Existing contracts retained
+- All reads and writes use this repository in the same process; there are no
+  external DB writers. Multi-process operation remains unsupported and startup
+  rejects that configuration while caching is enabled.
+- Authentication and authorization run before repository access. Keys encode
+  the authenticated tenant ID and validated profile ID without ambiguity.
+  Values are immutable profile-summary DTOs; secrets and cache keys are never
+  logged. Cached results cannot bypass authorization.
+- The existing LRU adapter supports 1000 entries, a 16 MiB byte cap, and a
+  30-second TTL. Recorded hot data fits those limits. Absent records use a
+  distinct sentinel with a 10-second TTL; undefined means a cache miss.
+- Cache operations are synchronous and atomic in the single JS event loop.
+  On any cache failure the existing adapter bypasses the cache until an empty
+  cache is reinitialized; repository errors keep the current typed API error
+  mapping. The existing per-key
+  single-flight wrapper coalesces simultaneous misses and releases on failure.
+- A read already in progress when a write commits may return its earlier DB
+  snapshot to that caller. Every read begun after that write completes must
+  observe the committed version. TTL expiry is not a substitute for this rule.
+
+## Proposed wrapper integration
+Keep the current read-through repository interface and shared adapters. These
+are the complete new read/write ordering rules; no additional version checks or
+coordination between a cache fill and a write are proposed:
+
+\`\`\`javascript
+${CACHE_READ_WRITE_SKETCH}
+\`\`\`
+
+## Verification and rollout
+Existing repository contract tests cover tenant isolation, key validation,
+absence, DB failures, and authorization. New wrapper tests cover hit/miss,
+eviction and byte limits, TTL, adapter-failure fallback, successful-write
+invalidation, failed-write preservation, and concurrent-miss coalescing.
+The rollout uses the existing runtime feature flag: enable for 10% of keys,
+then 50%, then all keys after one healthy hour at each stage. Monitor hit/miss,
+eviction, cache bytes, fallback errors, DB CPU, and read p95 without raw IDs.
+On error-rate or latency regression, disable the flag immediately; both reads
+and writes bypass the cache while disabled, and enabling creates an empty cache.
+Cold starts remain within the existing DB capacity. The service owner monitors
+the rollout and records the results against the acceptance targets.
+
+## Out of scope
+Distributed caching, cross-process coherence, prewarming, changing consistency
+semantics, or adding new product surfaces. The repository interface preserves a
+future replacement path without introducing a general cache framework now.
+`;
+
+/** Recognize the seeded concurrency finding across ordinary report wording. */
+export function hasStaleFillRaceFinding(report: string): boolean {
+  // Ignore quoted source: a copied plan or code sketch is not a finding.
+  const prose = report.replace(/```[\s\S]*?```/g, '').replace(/^\s*>.*$/gm, '');
+  const blocks = prose.split(/\n\s*\n|\n(?=\s*\|)/);
+  return blocks.some((block) => {
+    const text = block.replace(/[*_`]/g, '').replace(/\s+/g, ' ')
+      .replace(/\bno\s+(?:stale|outdated)\b[^.!?]*(?:[.!?]|$)/gi, '');
+    return /\b(?:stale|outdated)\b|\bold(?:er)?\s+(?:value|data|result|version)\b/i.test(text)
+      && /\b(?:race|racing|concurrent|concurrency|in[- ]flight|pending)\b/i.test(text)
+      && /\b(?:read|fetch|fill|refill|repopulat|insert|store|set)\w*\b/i.test(text)
+      && /\b(?:invalidat|evict|write|commit)\w*\b/i.test(text);
+  });
+}

test/helpers/session-runner.ts

@@ -132,7 +132,10 @@ export async function runSkillTest(options: {
   prompt: string;
   workingDirectory: string;
   maxTurns?: number;
+  /** Approval allowlist; does not restrict which tools the model can see. */
   allowedTools?: string[];
+  /** Optional built-in tool availability. Omit to preserve the CLI defaults. */
+  tools?: string[];
   timeout?: number;
   testName?: string;
   runId?: string;
@@ -197,6 +200,10 @@ export async function runSkillTest(options: {
     '--max-turns', String(maxTurns),
     '--allowed-tools', ...allowedTools,
   ];
+  // --allowed-tools controls approval, including when permissions are skipped;
+  // only --tools removes unrelated built-ins such as Agent, Bash, and Skill.
+  // Keep this opt-in: existing workflow evals intentionally use CLI defaults.
+  if (options.tools !== undefined) args.push('--tools', options.tools.join(','));
   // Hermetic children get zero MCP servers (no --mcp-config is passed).
   // Gated on the same call-time check as the env scrub so EVALS_HERMETIC=0
   // restores operator MCP along with the operator env.
@@ -426,9 +433,9 @@ export async function runSkillTest(options: {
     if (resultLine.subtype === 'success' && resultLine.is_error) {
       // claude -p can return subtype=success with is_error=true (e.g. API connection failure)
       exitReason = 'error_api';
-    } else if (resultLine.subtype === 'success') {
+    } else if (resultLine.subtype === 'success' && exitCode === 0 && !timedOut) {
       exitReason = 'success';
-    } else if (resultLine.subtype) {
+    } else if (resultLine.subtype && resultLine.subtype !== 'success') {
       // Preserve known subtypes like error_max_turns even if is_error is set
       exitReason = resultLine.subtype;
     }

test/helpers/touchfiles-data.ts

@@ -147,13 +147,13 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
   'tpa-broken':                  ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'],
   'tpa-absent-darwin':           ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'],
   'tpa-apple-ban':               ['scripts/resolvers/third-party-actions.ts', 'ship/SKILL.md.tmpl', 'ship/sections/apple-release.md.tmpl', 'scripts/gen-skill-docs.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-third-party-actions.test.ts'],
-  'ship-section-loading':        ['ship/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-ship-section-loading.test.ts'],
-  'plan-ceo-section-loading':    ['plan-ceo-review/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts'],
+  'ship-section-loading':        ['ship/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/skill-e2e-ship-section-loading.test.ts'],
+  'plan-ceo-section-loading':    ['plan-ceo-review/**', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/helpers/ceo-section-loading-fixture.ts', 'test/ceo-section-loading-fixture.test.ts', 'test/skill-e2e-plan-ceo-review-section-loading.test.ts'],
   // Data-driven behavioral guard for the 'plan'/'prompt' carves (eng, design,
   // devex, office-hours + future PR2 carves). One file iterating CARVE_GUARDS;
   // the selector sets GSTACK_CARVE_SKILL=<name> to scope cost to the changed
   // skill (D-CODEX A). Touching the registry/helper or sections.ts runs all.
-  'carve-section-loading':       ['design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/carve-section-loading.test.ts'],
+  'carve-section-loading':       ['design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/carve-section-loading.test.ts'],
   'autoplan-chain-pty':          ['autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/ui-heavy-feature.md', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'],
 
   // Per-finding AskUserQuestion count + review-report-at-bottom assertion.

test/session-runner-tools.test.ts

@@ -0,0 +1,289 @@
+import { 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 { captureSectionReads, hasDisabledOutsideReview, LONG_SECTION_CAPTURE_MS } from './helpers/auq-sdk-capture';
+import { CAPTURE_MS, CAPTURE_LONG_MS } from './helpers/eval-budgets';
+import { getHermeticDirs } from './helpers/hermetic-env';
+import { runSkillTest } from './helpers/session-runner';
+
+interface Observed {
+  args: string[];
+  prompt: string;
+  stateHome: string;
+  stateRoot: string | null;
+  outsideDisabled: boolean;
+}
+
+describe('outside-review disabled status in a completed report', () => {
+  const report = (status: string) => [
+    '## GSTACK REVIEW REPORT', '',
+    '| Review | Trigger | Why | Runs | Status | Findings |',
+    '| Outside Review | codex_reviews | Independent review | 0 | ' + status + ' | None |',
+  ].join('\n');
+
+  test.each(['disabled', 'Disabled', '**disabled**', '`disabled`',
+    'disabled (codex_reviews)', '__Disabled__ — configured opt-out'])('accepts semantic status %s', (status) => {
+    expect(hasDisabledOutsideReview(report(status))).toBe(true);
+  });
+
+  test.each(['', 'unavailable', 'completed', 'not disabled', 'disabledness'])('rejects status %s', (status) => {
+    expect(hasDisabledOutsideReview(report(status))).toBe(false);
+  });
+
+  test('requires the Outside Review row in the final report section', () => {
+    expect(hasDisabledOutsideReview('Outside coverage: disabled')).toBe(false);
+    expect(hasDisabledOutsideReview(report('disabled') + '\n\n' + report('completed'))).toBe(false);
+    expect(hasDisabledOutsideReview('## GSTACK REVIEW REPORT\n\n## Other context\n'
+      + '| Outside Review | codex_reviews | Independent review | 0 | disabled | None |')).toBe(false);
+  });
+});
+
+const FAKE_CLAUDE = String.raw`
+  const fs = require('node:fs');
+  const path = require('node:path');
+  const prompt = await Bun.stdin.text();
+  const stateHome = process.env.GSTACK_HOME;
+  let config = '';
+  try { config = fs.readFileSync(path.join(stateHome, 'config.yaml'), 'utf8'); } catch {}
+  const observed = {
+    args: process.argv.slice(2), prompt, stateHome,
+    stateRoot: process.env.GSTACK_STATE_ROOT ?? null,
+    outsideDisabled: /^codex_reviews:\s*disabled\s*$/m.test(config),
+  };
+  fs.writeFileSync('observed.json', JSON.stringify(observed));
+  console.log(JSON.stringify({ type: 'system', subtype: 'init' }));
+  if (fs.existsSync('fail-cli')) {
+    const failure = fs.readFileSync('fail-cli', 'utf8');
+    const report = '## GSTACK REVIEW REPORT\nA report written before the run failed.\n';
+    fs.writeFileSync('REPORT.md', report);
+    console.log(JSON.stringify({ type: 'result',
+      subtype: failure === 'error' ? 'error_during_execution' : 'success',
+      is_error: failure !== 'nonzero', result: report,
+    }));
+    process.exitCode = failure === 'is_error' ? 0 : 1;
+  } else {
+    const file_path = path.join(process.cwd(), 'plan-ceo-review', 'sections', 'review-sections.md');
+    console.log(JSON.stringify({ type: 'assistant', message: { content: [
+      { type: 'tool_use', name: 'Read', input: { file_path } },
+    ] } }));
+    if (fs.existsSync('wait-for-report')) {
+      fs.writeFileSync('work-ready', '');
+      while (!fs.existsSync('release-report')) await Bun.sleep(5);
+    }
+    fs.writeFileSync('REPORT.md', '## GSTACK REVIEW REPORT\nFull native review fixture.\nOutside review: '
+      + (observed.outsideDisabled ? 'disabled' : 'default') + '\n');
+    console.log(JSON.stringify({ type: 'result', subtype: 'success', result: JSON.stringify(observed) }));
+  }
+`;
+
+function flagValue(args: string[], flag: string): string | undefined {
+  const at = args.indexOf(flag);
+  return at < 0 ? undefined : args[at + 1];
+}
+
+async function withFakeClaude(run: (dir: string, readObserved: () => Observed) => Promise<void>): Promise<void> {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'session-tools-'));
+  const shimDir = path.join(dir, 'shim');
+  const script = path.join(dir, 'fake-claude.ts');
+  fs.mkdirSync(shimDir);
+  fs.writeFileSync(script, FAKE_CLAUDE);
+  const quote = (value: string) => "'" + value.replace(/'/g, "'\\''") + "'";
+  // Explicit POSIX shim: Windows CreateProcess does not interpret /bin/sh.
+  fs.writeFileSync(path.join(shimDir, 'claude'),
+    `#!/bin/sh\nexec ${quote(process.execPath)} ${quote(script)} "$@"\n`, { mode: 0o755 });
+  const saved = Object.fromEntries(['PATH', 'EVALS_HERMETIC', 'GSTACK_HOME', 'GSTACK_STATE_ROOT']
+    .map((key) => [key, process.env[key]]));
+  process.env.PATH = `${shimDir}${path.delimiter}${process.env.PATH ?? ''}`;
+  process.env.EVALS_HERMETIC = '1';
+  try {
+    await run(dir, () => JSON.parse(fs.readFileSync(path.join(dir, 'observed.json'), 'utf8')));
+  } finally {
+    for (const [key, value] of Object.entries(saved)) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+}
+
+describe.skipIf(process.platform === 'win32')('session-runner explicit tool availability', () => {
+  test('long section work fits the existing long capture tier with reporting headroom', () => {
+    expect(LONG_SECTION_CAPTURE_MS).toBeGreaterThan(CAPTURE_MS);
+    expect(CAPTURE_LONG_MS - LONG_SECTION_CAPTURE_MS).toBe(120_000);
+  });
+
+  test('a long capture can finish after the ordinary deadline; the default still times out', async () => {
+    // Run the controlled clock in a subprocess: it cannot affect sibling tests
+    // or hooks in Bun's shared shard. The fake CLI blocks after a real Read
+    // event until the worker advances time and allows report completion.
+    for (const long of [false, true]) {
+      await withFakeClaude(async (dir) => {
+        fs.writeFileSync(path.join(dir, 'wait-for-report'), '');
+        const worker = path.join(dir, 'budget-worker.ts');
+        const captureModule = path.resolve(import.meta.dir, 'helpers/auq-sdk-capture.ts');
+        fs.writeFileSync(worker, `
+          import * as fs from 'node:fs';
+          import { captureSectionReads, LONG_SECTION_CAPTURE_MS } from ${JSON.stringify(captureModule)};
+          const realNow = Date.now;
+          const realSetTimeout = globalThis.setTimeout;
+          const realClearTimeout = globalThis.clearTimeout;
+          let advanced = 0;
+          const timers = new Map();
+          Date.now = () => realNow() + advanced;
+          globalThis.setTimeout = ((callback, delay = 0, ...args) => {
+            const id = realSetTimeout(() => {
+              timers.delete(id);
+              callback(...args);
+            }, delay);
+            timers.set(id, { callback, args, delay, due: Date.now() + delay });
+            return id;
+          });
+          globalThis.clearTimeout = ((id) => { timers.delete(id); realClearTimeout(id); });
+          const pause = () => new Promise(resolve => realSetTimeout(resolve, 5));
+          let pending;
+          let finished = false;
+          try {
+            pending = captureSectionReads({
+              planDir: process.cwd(), skillName: 'plan-ceo-review', scenario: 'Complete the full review',
+              testName: 'section-budget', ${long ? 'timeout: LONG_SECTION_CAPTURE_MS,' : ''}
+            });
+            const waitStarted = realNow();
+            while (!fs.existsSync('work-ready') || ![...timers.values()].some(timer => timer.delay > 200_000)) {
+              if (realNow() - waitStarted > 5_000) throw new Error('Fake CLI did not enter the work phase');
+              await pause();
+            }
+            // 360s is beyond the ordinary 300s work limit, inside the existing
+            // 480s long-workflow limit. No real-time deadline is relaxed.
+            advanced = 360_000;
+            for (const [id, timer] of [...timers]) {
+              if (timer.due <= Date.now()) {
+                globalThis.clearTimeout(id);
+                timer.callback(...timer.args);
+              }
+            }
+            fs.writeFileSync('release-report', '');
+            const result = await pending;
+            finished = true;
+            console.log(JSON.stringify({
+              reportProduced: result.reportProduced,
+              readSection: result.readSections.has('review-sections.md'),
+            }));
+          } finally {
+            // A failed handshake must still terminate the fake CLI through
+            // the runner's own scoped timeout, not orphan it behind the worker.
+            if (!finished) {
+              for (const [id, timer] of [...timers]) {
+                if (timer.delay >= 10_000) {
+                  globalThis.clearTimeout(id);
+                  timer.callback(...timer.args);
+                }
+              }
+              await pending?.catch(() => {});
+            }
+            for (const id of timers.keys()) realClearTimeout(id);
+            Date.now = realNow;
+            globalThis.setTimeout = realSetTimeout;
+            globalThis.clearTimeout = realClearTimeout;
+          }
+        `);
+        const result = Bun.spawnSync([process.execPath, worker], {
+          cwd: dir, env: process.env, stdout: 'pipe', stderr: 'pipe', timeout: 15_000,
+        });
+        expect(result.exitCode, result.stderr.toString()).toBe(0);
+        expect(JSON.parse(result.stdout.toString())).toEqual({ reportProduced: long, readSection: true });
+      });
+    }
+  }, 35_000);
+
+  test('passes available tools separately from the approval allowlist and preserves stdin', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      const prompt = 'Read the literal text: `$(do not execute)` and "quotes".\nSecond line.';
+      const result = await runSkillTest({
+        prompt, workingDirectory: dir, allowedTools: ['Read'], tools: ['Read', 'Write'],
+        model: 'capture-model', maxTurns: 7, timeout: 5_000,
+      });
+      expect(result.exitReason).toBe('success');
+      expect(observed().prompt).toBe(prompt);
+      expect(flagValue(observed().args, '--allowed-tools')).toBe('Read');
+      expect(flagValue(observed().args, '--tools')).toBe('Read,Write');
+      expect(flagValue(observed().args, '--model')).toBe('capture-model');
+      expect(flagValue(observed().args, '--max-turns')).toBe('7');
+    });
+  });
+
+  test('omitting tools preserves default tool availability for existing callers', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      await runSkillTest({ prompt: 'Default tools', workingDirectory: dir, allowedTools: ['Read'], timeout: 5_000 });
+      expect(observed().args).not.toContain('--tools');
+      expect(flagValue(observed().args, '--allowed-tools')).toBe('Read');
+    });
+  });
+
+  test('an explicit empty list uses the CLI no-tools argument', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      await runSkillTest({ prompt: 'No tools', workingDirectory: dir, tools: [], timeout: 5_000 });
+      expect(flagValue(observed().args, '--tools')).toBe('');
+    });
+  });
+
+  test('section capture exposes only its declared tools without opting out of outside reviews by default', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      const result = await captureSectionReads({
+        planDir: dir, skillName: 'plan-ceo-review', scenario: 'Review the full plan',
+        testName: 'section-tools-default', timeout: 5_000,
+      });
+      expect(flagValue(observed().args, '--tools')).toBe('Read,Grep,Glob,Write');
+      expect(observed().stateHome).toBe(getHermeticDirs().gstackHome);
+      expect(observed().prompt).not.toContain('codex_reviews: disabled');
+      expect(result.readSections.has('review-sections.md')).toBe(true);
+      expect(result.reportProduced).toBe(true);
+    });
+  });
+
+  test('native-only capture gives its child real isolated disabled config and cleans only that state', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      const hostState = path.join(dir, 'host-state');
+      fs.mkdirSync(hostState);
+      const hostConfig = 'codex_reviews: enabled\nmarker: preserve\n';
+      fs.writeFileSync(path.join(hostState, 'config.yaml'), hostConfig);
+      process.env.GSTACK_HOME = hostState;
+      process.env.GSTACK_STATE_ROOT = hostState;
+      const sharedConfigPath = path.join(getHermeticDirs().gstackHome, 'config.yaml');
+      const sharedBefore = fs.existsSync(sharedConfigPath) ? fs.readFileSync(sharedConfigPath, 'utf8') : null;
+      const result = await captureSectionReads({
+        planDir: dir, skillName: 'plan-ceo-review', scenario: 'Review the full plan',
+        testName: 'section-tools-native', nativeReviewOnly: true, timeout: 5_000,
+      });
+      const child = observed();
+      expect(child.outsideDisabled).toBe(true);
+      expect(child.stateRoot).toBe(child.stateHome);
+      expect(child.stateHome).not.toBe(hostState);
+      expect(child.stateHome).not.toBe(getHermeticDirs().gstackHome);
+      expect(child.prompt).toContain(path.join(child.stateHome, 'config.yaml'));
+      expect(child.prompt).toContain('Complete all native review sections and the full required report.');
+      expect(child.prompt).toContain('report outside coverage as disabled');
+      expect(fs.existsSync(child.stateHome)).toBe(false);
+      expect(fs.readFileSync(path.join(hostState, 'config.yaml'), 'utf8')).toBe(hostConfig);
+      expect(fs.existsSync(sharedConfigPath) ? fs.readFileSync(sharedConfigPath, 'utf8') : null).toBe(sharedBefore);
+      expect(result.reportProduced).toBe(true);
+      expect(result.output).toContain('Outside review: disabled');
+    });
+  });
+
+  test.each(['nonzero', 'is_error', 'error'])('a %s capture cannot pass using a report left behind before failure', async (failure) => {
+    await withFakeClaude(async (dir, observed) => {
+      fs.writeFileSync(path.join(dir, 'fail-cli'), failure);
+      const result = await captureSectionReads({
+        planDir: dir, skillName: 'plan-ceo-review', scenario: 'Review the full plan',
+        testName: 'section-tools-failed', nativeReviewOnly: true, timeout: 5_000,
+        reportMarker: /^## GSTACK REVIEW REPORT\s*$/m,
+      });
+      expect(observed().outsideDisabled).toBe(true);
+      expect(fs.existsSync(observed().stateHome)).toBe(false);
+      expect(result.reportProduced).toBe(false);
+      // Preserve useful diagnostics; failed completion does not erase output.
+      expect(result.output).toContain('## GSTACK REVIEW REPORT');
+    });
+  });
+});

test/skill-e2e-plan-ceo-review-section-loading.test.ts

@@ -31,7 +31,10 @@ import {
   setupSkillDir,
   skillFromWorktree,
   captureSectionReads,
+  hasDisabledOutsideReview,
+  LONG_SECTION_CAPTURE_MS,
 } from './helpers/auq-sdk-capture';
+import { CEO_SECTION_CACHE_PLAN, hasStaleFillRaceFinding } from './helpers/ceo-section-loading-fixture';
 
 const describeE2E = describeE2ETier('periodic');
 const runId = `plan-ceo-section-loading-${process.env.EVALS_RUN_ID ?? 'local'}`;
@@ -39,22 +42,6 @@ const runId = `plan-ceo-section-loading-${process.env.EVALS_RUN_ID ?? 'local'}`;
 // Sections every plan-ceo-review run must consult after Step 0.
 const REQUIRED_SECTIONS = ['review-sections.md'];
 
-const PLAN_MD = [
-  '# Plan: add an in-memory cache layer',
-  '',
-  '## Context',
-  'Reads hit the DB on every request. Add a process-local LRU cache in front of',
-  'the read path to cut DB load.',
-  '',
-  '## Approach',
-  '- Wrap the read repository in a cache that stores the last 1000 keys.',
-  '- Invalidate on write.',
-  '',
-  '## Out of scope',
-  'Distributed cache, cross-process coherence.',
-  '',
-].join('\n');
-
 describeE2E('/plan-ceo-review section-loading E2E (periodic, SDK capture)', () => {
   test(
     'a real review Reads the carved section before producing the report',
@@ -64,7 +51,7 @@ describeE2E('/plan-ceo-review section-loading E2E (periodic, SDK capture)', () =
         skillName: 'plan-ceo-review',
         skillMd,
         sectionsFrom,
-        fixtures: { 'PLAN.md': PLAN_MD },
+        fixtures: { 'PLAN.md': CEO_SECTION_CACHE_PLAN },
         tmpPrefix: 'gstack-ceo-secload-',
       });
 
@@ -74,9 +61,16 @@ describeE2E('/plan-ceo-review section-loading E2E (periodic, SDK capture)', () =
         scenario:
           'Review the plan in PLAN.md. Hold the current scope (HOLD SCOPE mode) — do not challenge or expand scope. Run the full CEO review and produce the review report.',
         requiredSections: REQUIRED_SECTIONS,
-        reportMarker: /GSTACK REVIEW REPORT|COMPLETION SUMMARY|review/i,
+        reportMarker: /^## GSTACK REVIEW REPORT\s*$/m,
         testName: 'plan-ceo-section-loading',
         runId,
+        // This external carve missed the generic loader's v1.71 budget fix:
+        // a full 11-section review needs its long work budget, not the helper's
+        // ordinary 300s default. The outer CAPTURE_LONG_MS remains unchanged.
+        timeout: LONG_SECTION_CAPTURE_MS,
+        // This case measures native section loading and report completion;
+        // outside-provider dispatch is covered by the cross-harness evals.
+        nativeReviewOnly: true,
       });
 
       const missing = REQUIRED_SECTIONS.filter(s => !readSections.has(s));
@@ -87,6 +81,13 @@ describeE2E('/plan-ceo-review section-loading E2E (periodic, SDK capture)', () =
       });
       // Guard against an empty pass: the report must have real content.
       expect(output.trim().length).toBeGreaterThan(200);
+      expect(output).toMatch(/^\|\s*Review\s*\|\s*Trigger\s*\|\s*Why\s*\|\s*Runs\s*\|\s*Status\s*\|\s*Findings\s*\|/m);
+      expect(output).toMatch(/^\|\s*CEO Review\s*\|/m);
+      // A native capture must not invent an outside dispatch or claim coverage.
+      expect(hasDisabledOutsideReview(output)).toBe(true);
+      // Loading a section and producing a table alone must not hide an empty
+      // review: the complete fixture still contains a real ordering defect.
+      expect(hasStaleFillRaceFinding(output)).toBe(true);
     },
     CAPTURE_LONG_MS,
   );

test/touchfiles.test.ts

@@ -132,6 +132,14 @@ describe('selectTests', () => {
     expect(result.reason).toContain('global');
   });
 
+  test('section-capture tool isolation regression selects only its three capture workflows', () => {
+    const result = selectTests(['test/session-runner-tools.test.ts'], E2E_TOUCHFILES);
+    expect(result.selected.sort()).toEqual([
+      'carve-section-loading', 'plan-ceo-section-loading', 'ship-section-loading',
+    ]);
+    expect(result.reason).toBe('diff');
+  });
+
   test('gen-skill-docs.ts is a scoped touchfile, not global', () => {
     const result = selectTests(['scripts/gen-skill-docs.ts'], E2E_TOUCHFILES);
     // Should select tests that list gen-skill-docs.ts in their touchfiles, not ALL tests