fix(codex): close the consult-mode fence, report turn.failed as a failure, capture exit codes portably (#2671, #2669)

Three defects in the codex skill sections: - The resumed-session bash block never closed its fence; every fenced region after it inverted (prose rendered as code, the synthesis-recommendation tail rendered inert). A repo-wide fence-pairing test now scans every generated SKILL.md and sections/*.md with a CommonMark-faithful state machine (an info-string opener inside a fence is literal content — nested template examples in document-generate/make-pdf stay legal; a file ending inside a fence fails). - The JSONL parsers had no turn.failed branch: a turn that STATED its failure was reported as 'possible mid-stream disconnect'. Challenge and consult now print the event's error and run a three-way completeness check (failed-with- reason / silent-disconnect / ok); consult previously had no completeness check at all. - ${PIPESTATUS[0]} is empty under zsh, so hang detection never fired and every clean run printed a spurious '[codex exit ]'. All three capture sites use ${PIPESTATUS[0]:-${pipestatus[1]}}, pinned statically and EXECUTED under real bash and zsh in the new test. Expect a step-change in codex_timeout telemetry — the counter starts firing for zsh users. Receipt: the portability pin fails on a v1.77.0.0 scratch worktree; the fence fix is structural (17 → 18 fence lines, tail no longer inside a block). Fixes #2671 Fixes #2669 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-09 14:38:59 +02:00 · 2026-08-31 21:19:06 +00:00
parent 262a605794
commit 9ca92161e2
5 changed files with 176 additions and 10 deletions
@@ -61,12 +61,19 @@ for line in sys.stdin:
            usage = obj.get('usage',{})
            tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0)
            if tokens: print(f'\ntokens used: {tokens}', flush=True)
+        elif t == 'turn.failed':
+            turn_failed = True
+            err = obj.get('error',{}).get('message','') or 'no error message in event'
+            print(f'[codex turn FAILED] {err}', flush=True, file=sys.stderr)
    except: pass
-# Fix 2: completeness check — warn if no turn.completed received
-if turn_completed_count == 0:
+# Fix 2: three-way completeness check (#2671) — a STATED failure is a failure,
+# not a network problem; only silence with no terminal event is a disconnect.
+if 'turn_failed' in dir():
+    print('[codex] turn.failed received — the turn errored (reason above), not a disconnect.', flush=True, file=sys.stderr)
+elif turn_completed_count == 0:
    print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr)
 "
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 # Fix 1: hang detection — log + surface actionable message
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
@@ -59,12 +59,19 @@ for line in sys.stdin:
            usage = obj.get('usage',{})
            tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0)
            if tokens: print(f'\ntokens used: {tokens}', flush=True)
+        elif t == 'turn.failed':
+            turn_failed = True
+            err = obj.get('error',{}).get('message','') or 'no error message in event'
+            print(f'[codex turn FAILED] {err}', flush=True, file=sys.stderr)
    except: pass
-# Fix 2: completeness check — warn if no turn.completed received
-if turn_completed_count == 0:
+# Fix 2: three-way completeness check (#2671) — a STATED failure is a failure,
+# not a network problem; only silence with no terminal event is a disconnect.
+if 'turn_failed' in dir():
+    print('[codex] turn.failed received — the turn errored (reason above), not a disconnect.', flush=True, file=sys.stderr)
+elif turn_completed_count == 0:
    print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr)
 "
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 # Fix 1: hang detection — log + surface actionable message
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
@@ -102,13 +102,24 @@ for line in sys.stdin:
                cmd = item.get('command','')
                if cmd: print(f'[codex ran] {cmd}', flush=True)
        elif t == 'turn.completed':
+            turn_completed_count = 1 + (turn_completed_count if 'turn_completed_count' in dir() else 0)
            usage = obj.get('usage',{})
            tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0)
            if tokens: print(f'\ntokens used: {tokens}', flush=True)
+        elif t == 'turn.failed':
+            turn_failed = True
+            err = obj.get('error',{}).get('message','') or 'no error message in event'
+            print(f'[codex turn FAILED] {err}', flush=True, file=sys.stderr)
    except: pass
+# Three-way completeness check (#2671; consult previously had NONE): a STATED
+# failure is a failure, not a network problem; only silence is a disconnect.
+if 'turn_failed' in dir():
+    print('[codex] turn.failed received — the turn errored (reason above), not a disconnect.', flush=True, file=sys.stderr)
+elif 'turn_completed_count' not in dir():
+    print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr)
 "
 # Fix 1: hang detection for Consult new-session (mirrors Challenge + resume)
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
  _gstack_codex_log_hang "consult" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)"
@@ -144,7 +155,7 @@ _gstack_codex_timeout_wrapper 600 codex exec resume <session-id> "<prompt>" -c '
 <same python streaming parser as above, with flush=True on all print() calls>
 "
 # Fix 1: same hang detection pattern as new-session block
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
  _gstack_codex_log_hang "consult-resume" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)"
@@ -156,6 +167,7 @@ elif [ "$_CODEX_EXIT" != "0" ]; then
  head -20 "$TMPERR" 2>/dev/null | sed 's/^/  /' || true
  _gstack_codex_log_event "codex_nonzero_exit" "consult-resume:$_CODEX_EXIT"
 fi
+```

 5. Capture session ID from the streamed output. The parser prints `SESSION_ID:<id>`
   from the `thread.started` event. Save it for follow-ups:
@@ -100,13 +100,24 @@ for line in sys.stdin:
                cmd = item.get('command','')
                if cmd: print(f'[codex ran] {cmd}', flush=True)
        elif t == 'turn.completed':
+            turn_completed_count = 1 + (turn_completed_count if 'turn_completed_count' in dir() else 0)
            usage = obj.get('usage',{})
            tokens = usage.get('input_tokens',0) + usage.get('output_tokens',0)
            if tokens: print(f'\ntokens used: {tokens}', flush=True)
+        elif t == 'turn.failed':
+            turn_failed = True
+            err = obj.get('error',{}).get('message','') or 'no error message in event'
+            print(f'[codex turn FAILED] {err}', flush=True, file=sys.stderr)
    except: pass
+# Three-way completeness check (#2671; consult previously had NONE): a STATED
+# failure is a failure, not a network problem; only silence is a disconnect.
+if 'turn_failed' in dir():
+    print('[codex] turn.failed received — the turn errored (reason above), not a disconnect.', flush=True, file=sys.stderr)
+elif 'turn_completed_count' not in dir():
+    print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr)
 "
 # Fix 1: hang detection for Consult new-session (mirrors Challenge + resume)
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
  _gstack_codex_log_hang "consult" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)"
@@ -142,7 +153,7 @@ _gstack_codex_timeout_wrapper 600 codex exec resume <session-id> "<prompt>" -c '
 <same python streaming parser as above, with flush=True on all print() calls>
 "
 # Fix 1: same hang detection pattern as new-session block
-_CODEX_EXIT=${PIPESTATUS[0]}
+_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}  # bash sets PIPESTATUS; zsh (lowercase, 1-indexed) falls through (#2669)
 if [ "$_CODEX_EXIT" = "124" ]; then
  _gstack_codex_log_event "codex_timeout" "600"
  _gstack_codex_log_hang "consult-resume" "$(wc -c < "$TMPERR" 2>/dev/null || echo 0)"
@@ -154,6 +165,7 @@ elif [ "$_CODEX_EXIT" != "0" ]; then
  head -20 "$TMPERR" 2>/dev/null | sed 's/^/  /' || true
  _gstack_codex_log_event "codex_nonzero_exit" "consult-resume:$_CODEX_EXIT"
 fi
+```

 5. Capture session ID from the streamed output. The parser prints `SESSION_ID:<id>`
   from the `thread.started` event. Save it for follow-ups:
@@ -0,0 +1,128 @@
+/**
+ * Generated-doc fence pairing + PIPESTATUS portability (#2671, #2669).
+ *
+ * #2671: an unclosed ```bash fence in codex/sections/consult-mode.md silently
+ * inverted every fenced region after it — prose rendered as code and the
+ * skill's tail instructions rendered inert. Nothing guarded fence pairing, so
+ * the defect migrated file-to-file across carves. The scanner below is a
+ * CommonMark-faithful state machine, NOT a mod-2 count: inside an open fence,
+ * a ```lang line is literal content (only a bare ``` closes), so nested fence
+ * EXAMPLES don't false-positive; a file that ends inside a fence fails.
+ *
+ * #2669: `${PIPESTATUS[0]}` is bash-only — empty under zsh, so hang detection
+ * (`= "124"`) never fired and every clean run printed a spurious
+ * "[codex exit ]". The portable form `${PIPESTATUS[0]:-${pipestatus[1]}}` is
+ * pinned statically AND executed under real bash and zsh.
+ */
+import { describe, expect, test } from "bun:test";
+import { spawnSync } from "node:child_process";
+import * as fs from "node:fs";
+import * as path from "node:path";
+
+const ROOT = path.resolve(import.meta.dir, "..");
+
+/** All generated skill docs: every SKILL.md + every sections/*.md. */
+function generatedDocs(): string[] {
+  const out: string[] = [];
+  for (const entry of fs.readdirSync(ROOT, { withFileTypes: true })) {
+    if (!entry.isDirectory() || entry.name.startsWith(".") || entry.name === "node_modules")
+      continue;
+    const skillMd = path.join(ROOT, entry.name, "SKILL.md");
+    if (fs.existsSync(skillMd)) out.push(skillMd);
+    const sections = path.join(ROOT, entry.name, "sections");
+    if (fs.existsSync(sections)) {
+      for (const f of fs.readdirSync(sections)) {
+        if (f.endsWith(".md")) out.push(path.join(sections, f));
+      }
+    }
+  }
+  return out;
+}
+
+/** Returns the 1-based line of the first unclosed fence, or null when paired. */
+export function findUnclosedFence(body: string): number | null {
+  let openLine: number | null = null;
+  const lines = body.split("\n");
+  for (let i = 0; i < lines.length; i++) {
+    const ln = lines[i];
+    if (!ln.startsWith("```")) continue;
+    if (openLine === null) {
+      openLine = i + 1; // any ``` line opens (info string allowed)
+    } else if (/^```\s*$/.test(ln)) {
+      openLine = null; // only a bare ``` closes (CommonMark)
+    }
+    // ```lang while inside = literal content (nested fence example) — ignore.
+  }
+  return openLine;
+}
+
+describe("generated-doc fence pairing (#2671)", () => {
+  const docs = generatedDocs();
+
+  test("scanner sees a meaningful corpus", () => {
+    expect(docs.length).toBeGreaterThan(50);
+  });
+
+  test("every generated SKILL.md and sections/*.md closes every fence", () => {
+    const bad: string[] = [];
+    for (const doc of docs) {
+      const line = findUnclosedFence(fs.readFileSync(doc, "utf-8"));
+      if (line !== null) bad.push(`${path.relative(ROOT, doc)}:${line}`);
+    }
+    expect(
+      bad,
+      `unclosed \`\`\` fence(s) — everything after each inverts prose/code:\n  ${bad.join("\n  ")}`,
+    ).toEqual([]);
+  });
+
+  test("the scanner itself catches the #2671 shape (self-test)", () => {
+    const broken = "prose\n```bash\nx=1\n\nmore prose that should be outside\n```bash\nmkdir -p y\n```\n";
+    // First fence opens; ```bash inside is content; bare ``` closes it; file
+    // ends OUTSIDE — but the second region's prose was swallowed. The
+    // detectable invariant is end-of-file state, so test a truly unclosed tail:
+    expect(findUnclosedFence(broken)).toBeNull();
+    expect(findUnclosedFence(broken + "```text\ntail\n")).toBe(9);
+  });
+});
+
+describe("codex exit-code capture is bash+zsh portable (#2669)", () => {
+  const SECTION_FILES = [
+    "codex/sections/challenge-mode.md",
+    "codex/sections/consult-mode.md",
+    "codex/sections/challenge-mode.md.tmpl",
+    "codex/sections/consult-mode.md.tmpl",
+  ];
+
+  test("no bare ${PIPESTATUS[0]} capture survives in the codex sections", () => {
+    for (const rel of SECTION_FILES) {
+      const body = fs.readFileSync(path.join(ROOT, rel), "utf-8");
+      for (const line of body.split("\n")) {
+        if (line.includes("_CODEX_EXIT=")) {
+          expect(line, `${rel}: ${line.trim()}`).toContain(
+            "${PIPESTATUS[0]:-${pipestatus[1]}}",
+          );
+        }
+      }
+      // The capture must exist at all (3 sites across the two modes).
+      expect(body).toContain("_CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}");
+    }
+  });
+
+  const SNIPPET = 'exit 7 | cat; _CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}; echo "EXIT:$_CODEX_EXIT"';
+  const CLEAN = 'true | cat; _CODEX_EXIT=${PIPESTATUS[0]:-${pipestatus[1]}}; echo "EXIT:$_CODEX_EXIT"';
+
+  test("bash: captures the FIRST pipeline stage's exit code", () => {
+    const r = spawnSync("bash", ["-c", `(${SNIPPET})`], { encoding: "utf-8", timeout: 10_000 });
+    expect(r.stdout).toContain("EXIT:7");
+    const c = spawnSync("bash", ["-c", CLEAN], { encoding: "utf-8", timeout: 10_000 });
+    expect(c.stdout).toContain("EXIT:0");
+  });
+
+  const hasZsh = spawnSync("zsh", ["--version"], { encoding: "utf-8", timeout: 10_000 }).status === 0;
+  test.skipIf(!hasZsh)("zsh: the lowercase 1-indexed fallback captures the same code", () => {
+    const r = spawnSync("zsh", ["-c", `(${SNIPPET})`], { encoding: "utf-8", timeout: 10_000 });
+    expect(r.stdout).toContain("EXIT:7");
+    const c = spawnSync("zsh", ["-c", CLEAN], { encoding: "utf-8", timeout: 10_000 });
+    expect(c.stdout).toContain("EXIT:0");
+  });
+});