fix: check competing completion orders in CEO reviews

plan-ceo-review/sections/review-sections.md

@@ -124,14 +124,19 @@ This section traces data through the system and interactions through the UI with
 For each node: what happens on each shadow path? Is it tested?
 
 **Async ordering:** For flows sharing mutable state, include a combined ASCII
-schedule with one column per operation and one for shared state. At each `await`,
+schedule with one column per operation and one for shared state. For each pair
+of overlapping awaits that can affect an invariant, show both completion orders;
+exclude an order only by naming the mechanism that prevents it. At each `await`,
 callback or job handoff: pause, let a competing operation complete, resume, then
 start a fresh consumer. Show the observed result and compare it with the exact
-caller/time boundary of the stated invariant. If safe, name the mechanism that
+caller/time boundary of the stated invariant. The invariant is a requirement,
+not proof that the implementation meets it. If safe, name the mechanism that
 prevents the violating schedule. Separate flow diagrams do not prove ordering.
-Atomic calls and a single thread do not make the whole flow atomic. An accepted
-exception needs its exact contract clause; bounded damage is insufficient. Test
-this schedule with controlled pause/release points.
+One favorable schedule is insufficient. Single-thread execution and atomic calls
+do not prevent interleaving across awaits. An accepted exception needs its exact
+contract clause; bounded damage is insufficient. Test the relevant completion
+orders with controlled pause/release points. Compare relevant pairs; exhaustive
+permutations are unnecessary.
 
 **Interaction Edge Cases:** For every new user-visible interaction, evaluate:
 ```

plan-ceo-review/sections/review-sections.md.tmpl

@@ -122,14 +122,19 @@ This section traces data through the system and interactions through the UI with
 For each node: what happens on each shadow path? Is it tested?
 
 **Async ordering:** For flows sharing mutable state, include a combined ASCII
-schedule with one column per operation and one for shared state. At each `await`,
+schedule with one column per operation and one for shared state. For each pair
+of overlapping awaits that can affect an invariant, show both completion orders;
+exclude an order only by naming the mechanism that prevents it. At each `await`,
 callback or job handoff: pause, let a competing operation complete, resume, then
 start a fresh consumer. Show the observed result and compare it with the exact
-caller/time boundary of the stated invariant. If safe, name the mechanism that
+caller/time boundary of the stated invariant. The invariant is a requirement,
+not proof that the implementation meets it. If safe, name the mechanism that
 prevents the violating schedule. Separate flow diagrams do not prove ordering.
-Atomic calls and a single thread do not make the whole flow atomic. An accepted
-exception needs its exact contract clause; bounded damage is insufficient. Test
-this schedule with controlled pause/release points.
+One favorable schedule is insufficient. Single-thread execution and atomic calls
+do not prevent interleaving across awaits. An accepted exception needs its exact
+contract clause; bounded damage is insufficient. Test the relevant completion
+orders with controlled pause/release points. Compare relevant pairs; exhaustive
+permutations are unnecessary.
 
 **Interaction Edge Cases:** For every new user-visible interaction, evaluate:
 ```

test/helpers/auq-sdk-capture.ts

@@ -262,10 +262,11 @@ export async function captureSectionReads(opts: {
   // 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.
+  // Keep the model-facing config path relative to the fixture's working directory.
   const stateDir = opts.nativeReviewOnly
-    ? fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-section-state-')) : null;
+    ? fs.mkdtempSync(path.join(path.resolve(opts.planDir), '.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.`
+    ? `\n- Read ${path.relative(path.resolve(opts.planDir), 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.`
     : '';
   // Preserve full method execution while avoiding a second written walkthrough
   // of decisions already represented in the amended plan and required outputs.

test/session-runner-tools.test.ts

@@ -14,6 +14,10 @@ interface Observed {
   stateHome: string;
   stateRoot: string | null;
   outsideDisabled: boolean;
+  workingDirectory: string;
+  promptConfigPath: string | null;
+  promptConfig: string | null;
+  promptConfigMatchesState: boolean;
 }
 
 describe('outside-review disabled status in a completed report', () => {
@@ -47,12 +51,28 @@ const FAKE_CLAUDE = String.raw`
   const stateHome = process.env.GSTACK_HOME;
   let config = '';
   try { config = fs.readFileSync(path.join(stateHome, 'config.yaml'), 'utf8'); } catch {}
+  const promptConfigPath = prompt.match(/^- Read (.+), the isolated gstack configuration for this capture\./m)?.[1] ?? null;
+  let promptConfig = null;
+  let promptConfigMatchesState = false;
+  if (promptConfigPath) {
+    try {
+      const file = path.resolve(process.cwd(), promptConfigPath);
+      promptConfig = fs.readFileSync(file, 'utf8');
+      promptConfigMatchesState = fs.realpathSync(file) === fs.realpathSync(path.join(stateHome, 'config.yaml'));
+    } 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),
+    workingDirectory: process.cwd(), promptConfigPath, promptConfig, promptConfigMatchesState,
   };
   fs.writeFileSync('observed.json', JSON.stringify(observed));
+  const coordinatedState = fs.existsSync('coordinate-configs') ? path.basename(stateHome) : null;
+  if (coordinatedState) {
+    fs.writeFileSync('pending-observed-' + coordinatedState + '.json', JSON.stringify(observed));
+    fs.renameSync('pending-observed-' + coordinatedState + '.json', 'observed-' + coordinatedState + '.json');
+  }
   const outputFile = fs.existsSync('active-plan-output') ? 'PLAN.md' : 'REPORT.md';
   console.log(JSON.stringify({ type: 'system', subtype: 'init' }));
   if (fs.existsSync('diagnostic-case')) {
@@ -103,6 +123,9 @@ const FAKE_CLAUDE = String.raw`
     console.log(JSON.stringify({ type: 'assistant', message: { content: [
       { type: 'tool_use', name: 'Read', input: { file_path } },
     ] } }));
+    if (coordinatedState) {
+      while (!fs.existsSync('release-' + coordinatedState)) await Bun.sleep(5);
+    }
     if (fs.existsSync('wait-for-report')) {
       fs.writeFileSync('work-ready', '');
       while (!fs.existsSync('release-report')) await Bun.sleep(5);
@@ -425,7 +448,11 @@ Rules for this run:
       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(path.isAbsolute(child.stateHome)).toBe(true);
+      expect(path.dirname(child.stateHome)).toBe(path.resolve(dir));
+      expect(child.promptConfigPath).toBe(path.join(path.basename(child.stateHome), 'config.yaml'));
+      expect(child.promptConfig).toBe('codex_reviews: disabled\n');
+      expect(child.promptConfigMatchesState).toBe(true);
       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);
@@ -436,6 +463,66 @@ Rules for this run:
     });
   });
 
+  test('native-only config resolves from the prompt inside a nested fixture working directory', async () => {
+    await withFakeClaude(async (dir) => {
+      const planDir = path.join(dir, 'gstack-paid-shard-fixture', 'tmp', 'review fixture');
+      fs.mkdirSync(planDir, { recursive: true });
+      const result = await captureSectionReads({
+        planDir, skillName: 'plan-ceo-review',
+        scenario: 'Review the full plan', testName: 'section-config-nested',
+        nativeReviewOnly: true, timeout: 5_000,
+      });
+      const child: Observed = JSON.parse(fs.readFileSync(path.join(planDir, 'observed.json'), 'utf8'));
+      expect(fs.realpathSync(child.workingDirectory)).toBe(fs.realpathSync(planDir));
+      expect(path.isAbsolute(child.stateHome)).toBe(true);
+      expect(path.dirname(child.stateHome)).toBe(path.resolve(planDir));
+      expect(child.stateRoot).toBe(child.stateHome);
+      expect(child.promptConfigPath).toMatch(/^\.gstack-section-state-[^/\\]+[/\\]config\.yaml$/);
+      expect(path.isAbsolute(child.promptConfigPath!)).toBe(false);
+      expect(child.promptConfig).toBe('codex_reviews: disabled\n');
+      expect(child.promptConfigMatchesState).toBe(true);
+      expect(child.outsideDisabled).toBe(true);
+      expect(result.reportProduced).toBe(true);
+      expect(fs.existsSync(child.stateHome)).toBe(false);
+      expect(fs.readdirSync(planDir).filter(name => name.startsWith('.gstack-section-state-'))).toEqual([]);
+    });
+  });
+
+  test('overlapping native-only captures in one fixture retain independent config until each exits', async () => {
+    await withFakeClaude(async (dir) => {
+      fs.writeFileSync(path.join(dir, 'coordinate-configs'), '');
+      const captures = [0, 1].map(index => captureSectionReads({
+        planDir: dir, skillName: 'plan-ceo-review', scenario: 'Review the full plan',
+        testName: 'section-config-overlap-' + index, nativeReviewOnly: true, timeout: 5_000,
+      }));
+      const readChildren = (): Observed[] => fs.readdirSync(dir)
+        .filter(name => /^observed-.*\.json$/.test(name))
+        .map(name => JSON.parse(fs.readFileSync(path.join(dir, name), 'utf8')));
+      let children: Observed[] = [];
+      try {
+        const deadline = Date.now() + 3_000;
+        while ((children = readChildren()).length < 2 && Date.now() < deadline) await Bun.sleep(5);
+        expect(children).toHaveLength(2);
+        expect(new Set(children.map(child => child.stateHome)).size).toBe(2);
+        for (const child of children) {
+          expect(path.dirname(child.stateHome)).toBe(path.resolve(dir));
+          expect(child.stateRoot).toBe(child.stateHome);
+          expect(child.promptConfigMatchesState).toBe(true);
+          expect(child.promptConfig).toBe('codex_reviews: disabled\n');
+        }
+        fs.writeFileSync(path.join(dir, 'release-' + path.basename(children[0].stateHome)), '');
+        expect((await Promise.race(captures)).reportProduced).toBe(true);
+        expect(fs.existsSync(children[0].stateHome)).toBe(false);
+        expect(fs.readFileSync(path.join(children[1].stateHome, 'config.yaml'), 'utf8')).toBe('codex_reviews: disabled\n');
+      } finally {
+        for (const child of readChildren()) fs.writeFileSync(path.join(dir, 'release-' + path.basename(child.stateHome)), '');
+        const results = await Promise.all(captures);
+        expect(results.every(result => result.reportProduced)).toBe(true);
+      }
+      for (const child of children) expect(fs.existsSync(child.stateHome)).toBe(false);
+    });
+  }, 10_000);
+
   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);
@@ -445,10 +532,27 @@ Rules for this run:
         reportMarker: /^## GSTACK REVIEW REPORT\s*$/m,
       });
       expect(observed().outsideDisabled).toBe(true);
+      expect(observed().promptConfigMatchesState).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('a timed-out native-only capture removes its config after the CLI has read it', async () => {
+    await withFakeClaude(async (dir, observed) => {
+      fs.writeFileSync(path.join(dir, 'wait-for-report'), '');
+      const result = await captureSectionReads({
+        planDir: dir, skillName: 'plan-ceo-review', scenario: 'Review the full plan',
+        testName: 'section-config-timeout', nativeReviewOnly: true, timeout: 1_500,
+      });
+      expect(fs.existsSync(path.join(dir, 'work-ready'))).toBe(true);
+      expect(observed().promptConfigMatchesState).toBe(true);
+      expect(observed().promptConfig).toBe('codex_reviews: disabled\n');
+      expect(fs.existsSync(observed().stateHome)).toBe(false);
+      expect(result.reportProduced).toBe(false);
+      expect(fs.existsSync(path.join(dir, 'REPORT.md'))).toBe(false);
+    });
+  }, 5_000);
 });

test/skill-ceo-section-ordering.test.ts

@@ -123,7 +123,13 @@ describe('plan-ceo-review carve — static ordering', () => {
       expect(instructions).toContain('If safe, name the mechanism that prevents the violating schedule.');
       expect(instructions).toContain('Separate flow diagrams do not prove ordering.');
       expect(instructions).toContain('An accepted exception needs its exact contract clause; bounded damage is insufficient.');
-      expect(instructions).toContain('Test this schedule with controlled pause/release points.');
+      expect(instructions).toContain('For each pair of overlapping awaits that can affect an invariant, show both completion orders;');
+      expect(instructions).toContain('exclude an order only by naming the mechanism that prevents it.');
+      expect(instructions).toContain('The invariant is a requirement, not proof that the implementation meets it.');
+      expect(instructions).toContain('One favorable schedule is insufficient.');
+      expect(instructions).toContain('Single-thread execution and atomic calls do not prevent interleaving across awaits.');
+      expect(instructions).toContain('Test the relevant completion orders with controlled pause/release points.');
+      expect(instructions).toContain('Compare relevant pairs; exhaustive permutations are unnecessary.');
     }
   });