fix(hook): skip re-review of a plan already decided this session

AGENTS.md

@@ -141,6 +141,7 @@ claude --plugin-dir ./apps/hook
 | `PLANNOTATOR_JINA` | Set to `0` / `false` to disable Jina Reader for URL annotation, or `1` / `true` to enable. Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "jina": false }`) or per-invocation via `--no-jina`. |
 | `PLANNOTATOR_ANNOTATE_HISTORY` | Set to `0` / `false` to disable per-file version history in annotate mode (no copies of annotated files are written to the data dir; the annotate version diff is unavailable). Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "annotateHistory": false }`); the env var takes precedence. |
 | `PLANNOTATOR_GUIDE_HISTORY` | Set to `0` / `false` to disable persisting successful Guided Reviews (no guide copies are written to the data dir; the "Previous guides" list is then never populated, though already-saved guides remain readable and listed). Default: enabled. Can also be set via `~/.plannotator/config.json` (`{ "guideHistory": false }`); the env var takes precedence. |
+| `PLANNOTATOR_PLAN_DECISION_REUSE` | **Hook-runtime only.** Set to `0` / `false` to disable reuse, so every plan opens a review. Default: enabled — only a fresh Claude approval for the *same active* `ExitPlanMode` occurrence may be reused (for a short retry window), with a visible hook message. A new identical tool occurrence, a rewound/compacted occurrence, or missing transcript identity opens a fresh review; denials never replay. Codex uses current-turn filtering for proposed plans and never replays decisions. Approval records are scoped to project/session/normalized plan and occurrence under the data dir. Can also be set via `~/.plannotator/config.json` (`{ "planDecisionReuse": false }`); the env var takes precedence. |
 | `PLANNOTATOR_CURSOR_SANDBOX` | Set to `0` / `false` / `disabled` to stop passing `--sandbox enabled` when launching Cursor's `agent` CLI for review jobs — the flag pair is omitted entirely, deferring to the user's own Cursor Agent sandbox configuration. For systems where Cursor's sandbox cannot start (NixOS, AppArmor-restricted Linux). Default: enabled (`--sandbox enabled` is passed). Can also be set via `~/.plannotator/config.json` (`{ "cursorSandbox": false }`); the env var takes precedence. Note: opting out means the review job's write protection relies on `--mode ask` plus the user's own Cursor configuration. |
 | `PLANNOTATOR_TODO_PROVIDER` | Set to `off` / `0` / `false` / `disabled` to stop mirroring the approved plan checklist into an editable todo provider during execution. Default: enabled, which syncs only when a provider is detected (currently pi-todos: detected when its todo directory exists — `<cwd>/.pi/todos` by default, or wherever `PI_TODO_PATH` redirects it when set). The repo-implied `<cwd>/.pi/todos` must realpath to a location inside the project or the provider reads as absent and never writes, so a symlink committed into a hostile repo cannot redirect todo writes out of it; an explicitly set `PI_TODO_PATH` is the user's own choice and is honored verbatim, including outside the project. The mirror is additive — the progress widget is unaffected either way — and sync is one-way, so provider-side edits never feed back into plan execution. Can also be set via `~/.plannotator/config.json` (`{ "todoProvider": "off" }`); the env var takes precedence. |
 | `JINA_API_KEY` | Optional Jina Reader API key for higher rate limits (500 RPM vs 20 RPM unauthenticated). Free keys include 10M tokens. |

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

@@ -10,7 +10,13 @@ import { describe, expect, test, afterEach } from "bun:test";
 import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { findCodexRolloutByThreadId, getLastCodexMessage, getLatestCodexPlan } from "./codex-session";
+import {
+  findCodexRolloutByThreadId,
+  getCodexStopSkipReason,
+  getLastCodexMessage,
+  getLatestCodexPlan,
+  logCodexStopSkip,
+} from "./codex-session";
 
 // --- Fixture Helpers ---
 
@@ -71,10 +77,11 @@ function sessionMeta(): string {
   });
 }
 
-function turnContext(): string {
+function turnContext(turnId?: string): string {
   return rolloutLine("turn_context", {
     cwd: "/tmp/test",
     model: "o3",
+    ...(turnId && { turn_id: turnId }),
   });
 }
 
@@ -388,6 +395,7 @@ describe("getLatestCodexPlan", () => {
       text: "Authoritative plan item",
       source: "plan-item",
     });
+
   });
 
   test("falls back to raw proposed_plan blocks for plan-only assistant replies", () => {
@@ -407,6 +415,40 @@ describe("getLatestCodexPlan", () => {
     });
   });
 
+  describe("Codex Stop skip diagnostics", () => {
+    test("classifies a missing Stop turn id without reading stale plan content", () => {
+      expect(getCodexStopSkipReason("not-read.jsonl")).toBe("missing-turn-id");
+    });
+
+    test("requires an id-carrying rollout turn marker", () => {
+      const turnId = "turn-without-marker";
+      const path = writeTempRollout(
+        buildRollout(
+          sessionMeta(),
+          turnStarted("other-turn"),
+          completedPlanItem("Plan item without matching start marker", turnId),
+        ),
+      );
+
+      expect(getCodexStopSkipReason(path, turnId)).toBe("missing-turn-marker");
+    });
+
+    test("writes the exact skip breadcrumb only when debug is enabled", () => {
+      const messages: string[] = [];
+      const write = (message: string) => messages.push(message);
+
+      logCodexStopSkip("missing-turn-id", { debug: "", write });
+      expect(messages).toEqual([]);
+
+      logCodexStopSkip("missing-turn-id", { debug: "1", write });
+      logCodexStopSkip("missing-turn-marker", { debug: "1", write });
+      expect(messages).toEqual([
+        "[DEBUG] Codex Stop plan review skipped: missing Stop payload turn_id.",
+        "[DEBUG] Codex Stop plan review skipped: missing id-carrying rollout turn marker.",
+      ]);
+    });
+  });
+
   test("extracts plan blocks surrounded by assistant prose", () => {
     const turnId = "turn-prose";
     const path = writeTempRollout(
@@ -453,6 +495,55 @@ describe("getLatestCodexPlan", () => {
     expect(result).toBeNull();
   });
 
+  test("does not scrape a proposed plan from a later task when the requested turn has none", () => {
+    const requestedTurnId = "turn-requested";
+    const laterTurnId = "turn-later";
+    const path = writeTempRollout(
+      buildRollout(
+        sessionMeta(),
+        turnStarted(requestedTurnId),
+        assistantMessage("I have no plan to submit for this task."),
+        turnCompleted(requestedTurnId),
+        turnStarted(laterTurnId),
+        assistantMessage("<proposed_plan>\nPlan from the later task\n</proposed_plan>"),
+      )
+    );
+
+    expect(getLatestCodexPlan(path, { turnId: requestedTurnId })).toBeNull();
+  });
+
+  test("does not scrape an assistant proposed plan for a Stop event without a turn id", () => {
+    const completedTurnId = "turn-completed";
+    const path = writeTempRollout(
+      buildRollout(
+        sessionMeta(),
+        turnStarted(completedTurnId),
+        assistantMessage("<proposed_plan>\nPrevious turn plan\n</proposed_plan>"),
+        turnCompleted(completedTurnId),
+      ),
+    );
+
+    expect(getLatestCodexPlan(path, { stopHookActive: true })).toBeNull();
+  });
+
+  test("keeps the active task id when a turn context has no id", () => {
+    const turnId = "turn-with-context";
+    const path = writeTempRollout(
+      buildRollout(
+        sessionMeta(),
+        turnStarted(turnId),
+        turnContext(),
+        eventMsg("task_started"),
+        assistantMessage("<proposed_plan>\nCurrent turn plan\n</proposed_plan>"),
+      ),
+    );
+
+    expect(getLatestCodexPlan(path, { turnId })).toEqual({
+      text: "Current turn plan",
+      source: "assistant-message",
+    });
+  });
+
   test("returns null when Stop re-entry has no revised plan after the hook prompt", () => {
     const turnId = "turn-stop-no-revision";
     const path = writeTempRollout(

apps/hook/server/codex-session.ts

@@ -17,6 +17,8 @@ import { readFileSync, readdirSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { homedir } from "node:os";
 
+import { normalizePlanText } from "./plan-normalization";
+
 // --- Types ---
 
 type CodexPlanSource = "plan-item" | "assistant-message";
@@ -58,6 +60,8 @@ export interface GetLatestCodexPlanOptions {
   stopHookActive?: boolean;
 }
 
+export type CodexStopSkipReason = "missing-turn-id" | "missing-turn-marker";
+
 const TURN_START_TYPES = new Set(["task_started", "turn_started"]);
 const TURN_COMPLETE_TYPES = new Set(["task_complete", "turn_completed"]);
 const PROPOSED_PLAN_RE = /<proposed_plan>([\s\S]*?)<\/proposed_plan>/gi;
@@ -170,10 +174,6 @@ function extractLastProposedPlan(text: string): string | null {
   return latest || null;
 }
 
-function normalizePlan(text: string): string {
-  return text.replace(/\r\n/g, "\n").trim();
-}
-
 function findLastIndex(
   entries: RolloutEntry[],
   predicate: (entry: RolloutEntry) => boolean
@@ -184,23 +184,26 @@ function findLastIndex(
   return -1;
 }
 
-function findTurnStartIndex(entries: RolloutEntry[], turnId?: string): number {
-  const matchingTurnStart = findLastIndex(
+function findMatchingTurnMarkerIndex(
+  entries: RolloutEntry[],
+  turnId: string,
+): number {
+  return findLastIndex(
     entries,
     (entry) =>
-      entry.type === "event_msg" &&
-      TURN_START_TYPES.has(entry.payload?.type || "") &&
-      (!turnId || entry.payload?.turn_id === turnId)
+      (
+        (entry.type === "event_msg" && TURN_START_TYPES.has(entry.payload?.type || "")) ||
+        entry.type === "turn_context"
+      ) &&
+      getTurnId(entry) === turnId,
   );
-  if (matchingTurnStart !== -1) return matchingTurnStart;
+}
 
-  const matchingTurnContext = findLastIndex(
-    entries,
-    (entry) =>
-      entry.type === "turn_context" &&
-      (!turnId || entry.payload?.turn_id === turnId)
-  );
-  if (matchingTurnContext !== -1) return matchingTurnContext;
+function findTurnStartIndex(entries: RolloutEntry[], turnId?: string): number {
+  const matchingTurnStart = turnId
+    ? findMatchingTurnMarkerIndex(entries, turnId)
+    : -1;
+  if (matchingTurnStart !== -1) return matchingTurnStart;
 
   const lastTurnStart = findLastIndex(
     entries,
@@ -280,22 +283,64 @@ function getAssistantProposedPlanText(entry: RolloutEntry): string | null {
   return extractLastProposedPlan(messageText);
 }
 
+function getTurnId(entry: RolloutEntry): string | null {
+  const turnId = entry.payload?.turn_id;
+  return typeof turnId === "string" && turnId ? turnId : null;
+}
+
+export function getCodexStopSkipReason(
+  rolloutPath: string,
+  turnId?: string,
+): CodexStopSkipReason | null {
+  if (!turnId) return "missing-turn-id";
+  const entries = parseRolloutEntries(rolloutPath);
+  return findMatchingTurnMarkerIndex(entries, turnId) === -1
+    ? "missing-turn-marker"
+    : null;
+}
+
+export function logCodexStopSkip(
+  reason: CodexStopSkipReason,
+  opts: {
+    debug?: string;
+    write?: (message: string) => void;
+  } = {},
+): void {
+  if (!opts.debug) return;
+  const detail =
+    reason === "missing-turn-id"
+      ? "missing Stop payload turn_id."
+      : "missing id-carrying rollout turn marker.";
+  (opts.write ?? console.error)(`[DEBUG] Codex Stop plan review skipped: ${detail}`);
+}
+
 function collectPlanCandidates(
   entries: RolloutEntry[],
   startIndex: number,
   turnId?: string
 ): CodexPlanCandidate[] {
   const candidates: CodexPlanCandidate[] = [];
+  let activeTurnId: string | null = null;
 
   for (let i = Math.max(startIndex, 0); i < entries.length; i++) {
     const entry = entries[i];
+    if (
+      (entry.type === "event_msg" && TURN_START_TYPES.has(entry.payload?.type || "")) ||
+      entry.type === "turn_context"
+    ) {
+      const boundaryTurnId = getTurnId(entry);
+      if (boundaryTurnId) activeTurnId = boundaryTurnId;
+    }
 
     const planItemText = getPlanItemText(entry, turnId);
     if (planItemText) {
       candidates.push({ index: i, text: planItemText, source: "plan-item" });
     }
 
-    const assistantPlanText = getAssistantProposedPlanText(entry);
+    const assistantPlanText =
+      !turnId || activeTurnId === turnId
+        ? getAssistantProposedPlanText(entry)
+        : null;
     if (assistantPlanText) {
       candidates.push({
         index: i,
@@ -417,6 +462,9 @@ export function getLatestCodexPlan(
 ): CodexPlanResult | null {
   const entries = parseRolloutEntries(rolloutPath);
   if (entries.length === 0) return null;
+  // Stop payloads without a turn id cannot safely distinguish a new plan from
+  // a previous assistant <proposed_plan>; fail closed instead of resurfacing it.
+  if (!options.turnId) return null;
 
   const turnStartIndex = findTurnStartIndex(entries, options.turnId);
   const candidates = collectPlanCandidates(
@@ -457,8 +505,8 @@ export function getLatestCodexPlan(
 
   if (
     latestBeforeHookPrompt &&
-    normalizePlan(latestBeforeHookPrompt.text) ===
-      normalizePlan(latestAfterHookPrompt.text)
+    normalizePlanText(latestBeforeHookPrompt.text) ===
+      normalizePlanText(latestAfterHookPrompt.text)
   ) {
     return null;
   }