fix(codex): resolve the current turn from the rollout when the Stop payload carries no turn_id (#1534)

Codex added turn_id to StopCommandInput in rust-v0.117.0 (0.116.0-alpha.12),
after the hooks engine shipped in rust-v0.114.0, so stable rust-v0.114.0,
v0.115.0 and v0.116.0 fire the Stop hook with no turn_id at all. #1169 made
resolveCodexStopPlan fail closed on that payload, which silently disabled plan
review on those versions.

When the payload omits the field, derive the turn from the rollout instead: the
id of the last id-carrying turn_context/task_started marker, then the same
first-marker-anchored scan, so mid-turn compaction safety and per-turn plan
filtering are unchanged. A rollout with no turn markers still fails closed, and
so does a payload that carries turn_id as a blank string (truncated or foreign,
refused before the file is read).

Using the fallback writes one line to stderr, unconditionally — stdout is the
hook's JSON decision channel, and Codex only reads a Stop hook's stderr on exit
code 2, which this hook never uses.

Docs: minimum-Codex-version table in the Codex troubleshooting guide and a
turn-resolution section in AGENTS.md.

Author: backnotprop

AGENTS.md

@@ -204,6 +204,38 @@ Approve → stdout: {"hookSpecificOutput":{"decision":{"behavior":"allow"}}}
 Deny    → stdout: {"hookSpecificOutput":{"decision":{"behavior":"deny","message":"..."}}}
 ```
 
+### Codex Stop hook: which turn the plan belongs to
+
+Codex has no `ExitPlanMode`, so plan review rides its experimental `Stop` hook:
+the hook reads the session rollout and returns the plan **the turn that just
+stopped** produced. Anchoring on a turn is what keeps an older, already-decided
+`<proposed_plan>` from being reopened (#1169), and the turn is resolved in
+`resolveCodexStopPlan` (`apps/hook/server/codex-session.ts`) in two tiers:
+
+1. **The payload's own `turn_id`** when the Stop JSON carries the field.
+2. **The rollout's own turn markers** when it does not: the id of the LAST
+   id-carrying `turn_context` / `task_started` marker in the newest rollout
+   segment. A turn still in flight owns the last markers by construction, and
+   when Stop fires at turn end the last marker is that turn's own, so one rule
+   covers both shapes. The scan then still anchors on that turn's **first**
+   marker, so a mid-turn compaction (which re-emits `turn_context` with the same
+   id) cannot hide a plan the turn produced before it.
+
+Tier 2 exists because `StopCommandInput.turn_id` only landed in Codex
+**rust-v0.117.0** (`rust-v0.116.0-alpha.12`), while the hooks engine itself
+shipped in **rust-v0.114.0**: stable `rust-v0.114.0`, `v0.115.0` and `v0.116.0`
+fire the Stop hook with no `turn_id` at all. Their rollouts do record the id on
+every turn marker (`TurnContext::to_turn_context_item` writes
+`turn_id: Some(sub_id)`), which is what the fallback reads. Using it writes one
+unconditional line to **stderr** (`logCodexStopTurnIdFallback`; stdout is the
+hook's JSON decision channel, and Codex only reads a Stop hook's stderr on
+exit code 2, which this hook never uses).
+
+Fail-closed cases are unchanged: a payload that carries `turn_id` as a blank
+string is truncated or foreign, not an old Codex, and is refused **before the
+rollout is read**; a rollout with no id-carrying turn marker at all skips too.
+Both skips stay silent unless `PLANNOTATOR_DEBUG` is set.
+
 ## Code Review Flow
 
 ```

apps/hook/server/codex-session.test.ts

@@ -16,6 +16,7 @@ import {
   getLastCodexMessage,
   getLatestCodexPlan,
   logCodexStopSkip,
+  logCodexStopTurnIdFallback,
   getRecentCodexMessages,
   resolveCodexStopPlan,
 } from "./codex-session";
@@ -700,14 +701,16 @@ describe("getLatestCodexPlan", () => {
   });
 
   describe("Codex Stop skip diagnostics", () => {
-    test("classifies a missing Stop turn id without reading stale plan content", () => {
-      // The path does not exist: reaching the file at all would throw, which
-      // is the point — a turn-id-less Stop must never load plan content.
-      expect(resolveCodexStopPlan("not-read.jsonl")).toEqual({
+    test("classifies a blank Stop turn id without reading stale plan content", () => {
+      // A PRESENT but blank turn id is a truncated or foreign payload, never an
+      // old Codex, so it still fails closed. The path does not exist: reaching
+      // the file at all would throw, which is the point — that payload must
+      // never load plan content.
+      expect(resolveCodexStopPlan("not-read.jsonl", { turnId: "   " })).toEqual({
         plan: null,
         skipReason: "missing-turn-id",
       });
-      expect(resolveCodexStopPlan("not-read.jsonl", { turnId: "   " })).toEqual({
+      expect(resolveCodexStopPlan("not-read.jsonl", { turnId: "" })).toEqual({
         plan: null,
         skipReason: "missing-turn-id",
       });
@@ -745,6 +748,123 @@ describe("getLatestCodexPlan", () => {
     });
   });
 
+  // Codex rust-v0.114.0/0.115.0/0.116.0 ship the hooks engine but send a Stop
+  // payload with no `turn_id` field at all (it arrived in rust-v0.117.0). On
+  // those versions the turn is derived from the rollout instead of failing
+  // closed, which would leave plan review silently disabled.
+  describe("Codex Stop turn id fallback", () => {
+    test("resolves the plan of the rollout's current turn", () => {
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnStarted("turn-only"),
+          completedPlanItem("Plan from the turn that just stopped", "turn-only"),
+        ),
+      );
+
+      expect(resolveCodexStopPlan(path)).toEqual({
+        plan: {
+          text: "Plan from the turn that just stopped",
+          source: "plan-item",
+        },
+        skipReason: null,
+        fallbackTurnId: "turn-only",
+      });
+    });
+
+    test("anchors on the turn's FIRST marker, so a mid-turn compaction keeps the plan visible", () => {
+      // Compaction re-emits a `turn_context` carrying the in-flight turn's id.
+      // Naming the turn from the LAST marker is fine; anchoring the scan there
+      // would hide every plan the turn produced before the compaction point.
+      const turnId = "turn-compacted";
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnStarted(turnId),
+          completedPlanItem("Plan written before compaction", turnId),
+          compacted("Summary of the conversation so far"),
+          turnContext(turnId),
+          assistantMessage("Continuing after compaction."),
+        ),
+      );
+
+      const result = resolveCodexStopPlan(path);
+      expect(result.plan).toEqual({
+        text: "Plan written before compaction",
+        source: "plan-item",
+      });
+      expect(result.fallbackTurnId).toBe(turnId);
+    });
+
+    test("never re-reviews a plan from an older, already-completed turn", () => {
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnStarted("turn-old"),
+          completedPlanItem("Already-decided plan", "turn-old"),
+          turnCompleted("turn-old"),
+          turnStarted("turn-new"),
+          assistantMessage("Unrelated follow-up answer."),
+          turnCompleted("turn-new"),
+        ),
+      );
+
+      expect(resolveCodexStopPlan(path)).toEqual({
+        plan: null,
+        skipReason: null,
+        fallbackTurnId: "turn-new",
+      });
+    });
+
+    test("fails closed when the rollout carries no id-carrying turn marker", () => {
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnContext(),
+          assistantMessage("<proposed_plan>\nStale plan\n</proposed_plan>"),
+        ),
+      );
+
+      expect(resolveCodexStopPlan(path)).toEqual({
+        plan: null,
+        skipReason: "missing-turn-marker",
+      });
+    });
+
+    test("a payload-named turn is unaffected by the fallback", () => {
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnStarted("turn-old"),
+          completedPlanItem("Older turn's plan", "turn-old"),
+          turnCompleted("turn-old"),
+          turnStarted("turn-new"),
+          assistantMessage("Unrelated follow-up answer."),
+        ),
+      );
+
+      // The payload names the older turn: its plan is returned, and no fallback
+      // is reported — the rollout's newest turn never enters the decision.
+      expect(resolveCodexStopPlan(path, { turnId: "turn-old" })).toEqual({
+        plan: { text: "Older turn's plan", source: "plan-item" },
+        skipReason: null,
+      });
+    });
+
+    test("announces the fallback on stderr unconditionally", () => {
+      const messages: string[] = [];
+      logCodexStopTurnIdFallback("turn-42", {
+        write: (message) => messages.push(message),
+      });
+
+      expect(messages).toHaveLength(1);
+      // The line must name the missing field and the turn it fell back to —
+      // that is the whole diagnostic value. No debug flag gates it.
+      expect(messages[0]).toContain("turn_id");
+      expect(messages[0]).toContain("turn-42");
+    });
+  });
+
   test("extracts plan blocks surrounded by assistant prose", () => {
     const turnId = "turn-prose";
     const path = writeTempRollout(
@@ -808,14 +928,20 @@ describe("getLatestCodexPlan", () => {
     expect(getLatestCodexPlan(path, { turnId: requestedTurnId })).toBeNull();
   });
 
-  test("does not scrape an assistant proposed plan for a Stop event without a turn id", () => {
+  test("does not scrape an older turn's assistant proposed plan for a Stop event without a turn id", () => {
+    // A turn-id-less payload now resolves the turn from the rollout instead of
+    // failing closed (Codex < 0.117 sends no turn_id), but the resolved turn is
+    // the CURRENT one — an earlier, already-decided turn's plan stays invisible.
     const completedTurnId = "turn-completed";
+    const currentTurnId = "turn-current";
     const path = writeTempRollout(
       buildRollout(
         sessionMeta(),
         turnStarted(completedTurnId),
         assistantMessage("<proposed_plan>\nPrevious turn plan\n</proposed_plan>"),
         turnCompleted(completedTurnId),
+        turnStarted(currentTurnId),
+        assistantMessage("Just answering a regular question."),
       ),
     );
 

apps/hook/server/codex-session.ts

@@ -67,6 +67,12 @@ export interface CodexStopPlanLookup {
    * produced none. Diagnostic only: both outcomes are a silent no-op.
    */
   skipReason: CodexStopSkipReason | null;
+  /**
+   * The turn id the scan used, set ONLY when it was derived from the rollout
+   * because the Stop payload carried no `turn_id` field. The caller surfaces
+   * it on stderr; a payload-named turn leaves this undefined.
+   */
+  fallbackTurnId?: string;
 }
 
 const TURN_START_TYPES = new Set(["task_started", "turn_started"]);
@@ -254,6 +260,26 @@ function findTurnStartIndex(entries: RolloutEntry[], turnId: string): number {
   );
 }
 
+/**
+ * The turn the rollout says is current: the id of the LAST id-carrying turn
+ * marker in the file, or null when the file has none.
+ *
+ * That single rule covers both shapes the Stop hook can catch. A turn still in
+ * flight owns the last markers by construction — a completed turn's markers can
+ * only be followed by a newer turn's — and when Stop fires at turn end the last
+ * marker is that turn's own. Compaction is safe because this only names the
+ * turn; where it BEGINS is still `findTurnStartIndex`'s first-marker anchor.
+ */
+function findLatestRolloutTurnId(entries: RolloutEntry[]): string | null {
+  for (let i = entries.length - 1; i >= 0; i--) {
+    const entry = entries[i];
+    if (!isTurnMarker(entry)) continue;
+    const turnId = getTurnId(entry);
+    if (turnId) return turnId;
+  }
+  return null;
+}
+
 function findActiveTurnStartIndex(entries: RolloutEntry[]): number {
   const latestTurnStart = findLastIndex(
     entries,
@@ -332,6 +358,24 @@ export function logCodexStopSkip(
   (opts.write ?? console.error)(`[DEBUG] Codex Stop plan review skipped: ${detail}`);
 }
 
+/**
+ * One unconditional stderr line whenever the rollout fallback named the turn,
+ * so a user on a Codex that sends no `turn_id` can see WHY plan review behaves
+ * the way it does without setting PLANNOTATOR_DEBUG.
+ *
+ * stderr, never stdout: stdout is the Stop hook's JSON decision channel. Codex
+ * only reads a Stop hook's stderr when it exits with code 2, and this hook
+ * always exits 0, so the line is a captured breadcrumb, never agent input.
+ */
+export function logCodexStopTurnIdFallback(
+  turnId: string,
+  opts: { write?: (message: string) => void } = {},
+): void {
+  (opts.write ?? console.error)(
+    `[plannotator] Codex Stop payload carried no turn_id (Codex < 0.117); resolved the current turn from the rollout instead (turn ${turnId}).`,
+  );
+}
+
 function collectPlanCandidates(
   entries: RolloutEntry[],
   startIndex: number,
@@ -472,8 +516,9 @@ export function getRecentCodexMessages(
  * - no plan after the last hook prompt => null
  * - identical plan after the last hook prompt => null
  *
- * Requires options.turnId, and requires that turn to be anchored in the file:
- * see resolveCodexStopPlan, which this thin wrapper drops the reason from.
+ * The turn comes from options.turnId, or from the rollout's own turn markers
+ * when the Stop payload carried no `turn_id` field, and must be anchored in the
+ * file: see resolveCodexStopPlan, which this thin wrapper drops the reason from.
  */
 export function getLatestCodexPlan(
   rolloutPath: string,
@@ -491,15 +536,26 @@ export function resolveCodexStopPlan(
   rolloutPath: string,
   options: GetLatestCodexPlanOptions = {}
 ): CodexStopPlanLookup {
-  // A Stop payload without a turn id cannot tell a plan produced in this turn
-  // from an already-decided <proposed_plan> earlier in the thread, so fail
-  // closed — before the file is read, so stale content is never even loaded.
-  // Codex itself always sends one (`StopCommandInput.turn_id` is a required
-  // String), so this is a guard against a foreign or truncated payload.
-  const turnId = options.turnId?.trim();
-  if (!turnId) return { plan: null, skipReason: "missing-turn-id" };
+  // A payload that NAMES a turn but names it blank is truncated or foreign, not
+  // an old Codex: fail closed before the file is read, so stale plan content is
+  // never even loaded. Keyed on the field being present, because an absent
+  // field is the old-Codex shape handled below.
+  const payloadTurnId = options.turnId?.trim();
+  if (options.turnId !== undefined && !payloadTurnId) {
+    return { plan: null, skipReason: "missing-turn-id" };
+  }
 
   const entries = parseRolloutEntries(rolloutPath);
+
+  // Codex only added `turn_id` to the Stop payload in rust-v0.117.0
+  // (rust-v0.116.0-alpha.12); rust-v0.114.0/0.115.0/0.116.0 ship the hooks
+  // engine and send a Stop payload without the field. Their rollouts still
+  // record the turn id on every turn marker (`TurnContext::to_turn_context_item`
+  // writes `turn_id: Some(sub_id)`), so derive the current turn from the file
+  // rather than failing closed and silently disabling plan review there.
+  const turnId = payloadTurnId ?? findLatestRolloutTurnId(entries);
+  if (!turnId) return { plan: null, skipReason: "missing-turn-marker" };
+
   // The named turn must be anchored in THIS rollout file. Without an anchor,
   // every plan in the file belongs to some other turn — the normal shape of an
   // older segment of a split thread, which routinely ends with an
@@ -512,6 +568,7 @@ export function resolveCodexStopPlan(
   return {
     plan: findPlanInTurn(entries, turnStartIndex, turnId, !!options.stopHookActive),
     skipReason: null,
+    ...(payloadTurnId ? {} : { fallbackTurnId: turnId }),
   };
 }
 

apps/hook/server/index.ts

@@ -167,6 +167,7 @@ import {
   findCodexRolloutsByThreadId,
   getRecentCodexMessages,
   logCodexStopSkip,
+  logCodexStopTurnIdFallback,
   resolveCodexStopPlan,
 } from "./codex-session";
 import { findCopilotPlanContent, findCopilotSessionByAncestorPids, findCopilotSessionForCwd, getRecentCopilotMessages } from "./copilot-session";
@@ -2360,13 +2361,21 @@ if (args[0] === "sessions") {
       process.exit(0);
     }
 
-    const { plan: latestPlan, skipReason } = resolveCodexStopPlan(rolloutPath, {
-      turnId: typeof event.turn_id === "string" ? event.turn_id : undefined,
+    // Absent `turn_id` means an older Codex (the field arrived in rust-v0.117.0)
+    // and hands the lookup its rollout fallback; a PRESENT but unusable value is
+    // a truncated or foreign payload and must still fail closed, so it is passed
+    // through as a blank string rather than collapsed to "absent".
+    const rawTurnId = event.turn_id;
+    const { plan: latestPlan, skipReason, fallbackTurnId } = resolveCodexStopPlan(rolloutPath, {
+      turnId: rawTurnId === undefined ? undefined : typeof rawTurnId === "string" ? rawTurnId : "",
       stopHookActive: !!event.stop_hook_active,
     });
     if (skipReason) {
       logCodexStopSkip(skipReason, { debug: process.env.PLANNOTATOR_DEBUG });
     }
+    if (fallbackTurnId) {
+      logCodexStopTurnIdFallback(fallbackTurnId);
+    }
 
     if (!latestPlan?.text) {
       process.exit(0);

apps/marketing/src/content/docs/guides/troubleshooting.md

@@ -92,6 +92,32 @@ If a Codex plan turn completes without opening Plannotator:
 3. Check `~/.codex/config.toml` contains `hooks = true` under `[features]`
 4. Check `~/.codex/hooks.json` has a `Stop` hook whose command points to `plannotator`
 5. Run `plannotator sessions` in case the browser failed to open but the session is running
+6. Check your Codex version: `codex --version`
+
+### Minimum Codex version
+
+The `Stop` hook Plannotator uses arrived in Codex **rust-v0.114.0**, so anything
+older has no Codex plan review at all. Two later changes matter:
+
+| Codex version | What changed |
+|---------------|--------------|
+| `rust-v0.114.0` | Hooks engine ships; the feature key is `codex_hooks` and it is off by default |
+| `rust-v0.117.0` (`0.116.0-alpha.12`) | The Stop payload starts carrying `turn_id` |
+| `rust-v0.125.0` | Hooks become enabled by default |
+| `rust-v0.131.0` | The feature key is renamed to `hooks`, with `codex_hooks` kept as a legacy alias |
+
+Step 3 above assumes `rust-v0.131.0` or newer, which is the version this
+installer targets. On `rust-v0.114.0`–`rust-v0.130.x` the key Codex reads is
+`codex_hooks = true`, so a config that says `hooks = true` alone has no effect
+there — write `codex_hooks = true` (both keys together are fine) and restart
+Codex.
+
+You do **not** need a Codex new enough to send `turn_id`. On `rust-v0.114.0`,
+`v0.115.0` and `v0.116.0` the Stop payload omits that field, and Plannotator
+resolves the turn from the session rollout instead, which is why plan review
+opens there as well. When that fallback runs it prints one line to stderr
+naming the turn it used, so a hook run captured with stderr visible shows
+exactly what happened.
 
 Codex hooks are currently disabled on Windows in the official Codex docs, so the Windows installer prints manual guidance instead of changing Codex config automatically.