diff --git a/AGENTS.md b/AGENTS.md index 0859318ef..3da031a13 100644 --- a/AGENTS.md +++ b/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 — `/.pi/todos` by default, or wherever `PI_TODO_PATH` redirects it when set). The repo-implied `/.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. | diff --git a/apps/hook/server/codex-session.test.ts b/apps/hook/server/codex-session.test.ts index 450ad27ab..f74380b96 100644 --- a/apps/hook/server/codex-session.test.ts +++ b/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("\nPlan from the later task\n"), + ) + ); + + 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("\nPrevious turn plan\n"), + 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("\nCurrent turn plan\n"), + ), + ); + + 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( diff --git a/apps/hook/server/codex-session.ts b/apps/hook/server/codex-session.ts index b6a4e8d17..d89ed73b2 100644 --- a/apps/hook/server/codex-session.ts +++ b/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 = /([\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 ; 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; } diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index d2e1939c5..7f7f3e157 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -89,7 +89,7 @@ import { handleGoalSetupServerReady, } from "@plannotator/server/goal-setup"; import { type DiffType, detectManagedVcs, prepareLocalReviewDiff, gitRuntime } from "@plannotator/server/vcs"; -import { loadConfig, resolveDefaultDiffType, resolveSharingEnabled } from "@plannotator/shared/config"; +import { loadConfig, resolveDefaultDiffType, resolveSharingEnabled, resolvePlanDecisionReuse } from "@plannotator/shared/config"; import { parseReviewArgs } from "@plannotator/shared/review-args"; import { normalizeGoalSetupBundle, @@ -142,9 +142,20 @@ import { resolveDroidSessionLogForCwd, resolveSessionLogByAncestorPids, resolveSessionLogByCwdScan, + findActiveExitPlanModeOccurrenceInTranscript, type RenderedMessage, } from "./session-log"; -import { findCodexRolloutByThreadId, getLatestCodexPlan, getRecentCodexMessages } from "./codex-session"; +import { + findCodexRolloutByThreadId, + getCodexStopSkipReason, + getLatestCodexPlan, + getRecentCodexMessages, + logCodexStopSkip, +} from "./codex-session"; +import { + recordPlanApprovalForSubmission, + shouldReusePlanApproval, +} from "./plan-decision-policy"; import { findCopilotPlanContent, findCopilotSessionByAncestorPids, findCopilotSessionForCwd, getRecentCopilotMessages } from "./copilot-session"; import { formatInteractiveNoArgClarification, @@ -2008,8 +2019,18 @@ if (args[0] === "sessions") { process.exit(0); } + const turnId = + typeof event.turn_id === "string" && event.turn_id + ? event.turn_id + : undefined; + const skipReason = getCodexStopSkipReason(rolloutPath, turnId); + if (skipReason) { + logCodexStopSkip(skipReason, { debug: process.env.PLANNOTATOR_DEBUG }); + process.exit(0); + } + const latestPlan = getLatestCodexPlan(rolloutPath, { - turnId: typeof event.turn_id === "string" ? event.turn_id : undefined, + turnId, stopHookActive: !!event.stop_hook_active, }); @@ -2094,43 +2115,86 @@ if (args[0] === "sessions") { } const planProject = (await detectProjectName()) ?? "_unknown"; - - // Start the plan review server - const server = await startPlannotatorServer({ + const planSessionId = typeof event.session_id === "string" ? event.session_id : ""; + + const planDecisionReuseEnabled = resolvePlanDecisionReuse(loadConfig()); + const planOccurrence = + !isGemini && + typeof event.transcript_path === "string" && + event.transcript_path + ? findActiveExitPlanModeOccurrenceInTranscript(event.transcript_path, { + plan: planContent, + toolUseId: + typeof event.tool_use_id === "string" ? event.tool_use_id : undefined, + }) + : null; + const planApprovalContext = { + enabled: planDecisionReuseEnabled, + isGemini, + project: planProject, + sessionId: planSessionId, plan: planContent, - origin: isGemini ? "gemini-cli" : detectedOrigin, - permissionMode, - sharingEnabled, - shareBaseUrl, - pasteApiUrl, - htmlContent: planHtmlContent, - onReady: async (url, isRemote, port) => { - handleServerReady(url, isRemote, port); + occurrence: planOccurrence, + }; + const priorApproved = shouldReusePlanApproval(planApprovalContext); + + let result: { + approved: boolean; + feedback?: string; + savedPath?: string; + agentSwitch?: string; + permissionMode?: string; + }; + let reusedApproval = false; - if (isRemote && sharingEnabled) { - await writeRemoteShareLink(planContent, shareBaseUrl, "review the plan", "plan only").catch(() => {}); - } - }, - }); + if (priorApproved) { + // The permission mode set on the first approval is session-scoped, so it is + // deliberately not re-asserted here. + result = { approved: true }; + reusedApproval = true; + } else { + // Start the plan review server + const server = await startPlannotatorServer({ + plan: planContent, + origin: isGemini ? "gemini-cli" : detectedOrigin, + permissionMode, + sharingEnabled, + shareBaseUrl, + pasteApiUrl, + htmlContent: planHtmlContent, + onReady: async (url, isRemote, port) => { + handleServerReady(url, isRemote, port); - registerSession({ - pid: process.pid, - port: server.port, - url: server.url, - mode: "plan", - project: planProject, - startedAt: new Date().toISOString(), - label: `plan-${planProject}`, - }); + if (isRemote && sharingEnabled) { + await writeRemoteShareLink(planContent, shareBaseUrl, "review the plan", "plan only").catch(() => {}); + } + }, + }); - // Wait for user decision (blocks until approve/deny) - const result = await server.waitForDecision(); + registerSession({ + pid: process.pid, + port: server.port, + url: server.url, + mode: "plan", + project: planProject, + startedAt: new Date().toISOString(), + label: `plan-${planProject}`, + }); - // Give browser time to receive response and update UI - await Bun.sleep(1500); + // Wait for user decision (blocks until approve/deny) + result = await server.waitForDecision(); - // Cleanup - server.stop(); + // Give browser time to receive response and update UI + await Bun.sleep(1500); + + // Cleanup + server.stop(); + + recordPlanApprovalForSubmission({ + ...planApprovalContext, + approved: result.approved, + }); + } // Output decision in the appropriate format for the harness if (isGemini) { @@ -2162,6 +2226,10 @@ if (args[0] === "sessions") { console.log( JSON.stringify({ + ...(reusedApproval && { + systemMessage: + "Reused the approval for this same ExitPlanMode submission.", + }), hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { diff --git a/apps/hook/server/plan-decision-policy.test.ts b/apps/hook/server/plan-decision-policy.test.ts new file mode 100644 index 000000000..f6ec4a0cb --- /dev/null +++ b/apps/hook/server/plan-decision-policy.test.ts @@ -0,0 +1,179 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + findActiveExitPlanModeOccurrence, + type SessionLogEntry, +} from "./session-log"; +import { + recordPlanApprovalForSubmission, + shouldReusePlanApproval, +} from "./plan-decision-policy"; + +const dirs: string[] = []; +const dataDir = (): string => { + const dir = mkdtempSync(join(tmpdir(), "plan-decision-policy-test-")); + dirs.push(dir); + return dir; +}; + +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +const plan = "# Plan\n- step"; + +function exitPlanEntry( + entryUuid: string, + parentUuid: string | null, + toolUseId: string, +): SessionLogEntry { + return { + type: "assistant", + uuid: entryUuid, + parentUuid, + message: { + id: `msg-${toolUseId}`, + role: "assistant", + content: [ + { + type: "tool_use", + id: toolUseId, + name: "ExitPlanMode", + input: { plan }, + }, + ], + }, + }; +} + +function context( + dir: string, + occurrence: ReturnType, + overrides: Partial[0]> = {}, +) { + return { + enabled: true, + isGemini: false, + project: "project", + sessionId: "session", + plan, + occurrence, + baseDir: dir, + now: 1_000, + ...overrides, + }; +} + +describe("plan approval reuse policy", () => { + test("reuses an approval only for the same active ExitPlanMode retry", () => { + const dir = dataDir(); + const occurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-a", null, "tool-a")], + { plan, toolUseId: "tool-a" }, + ); + const input = context(dir, occurrence); + + recordPlanApprovalForSubmission({ ...input, approved: true }); + + expect(shouldReusePlanApproval(input)).toBe(true); + }); + + test("opens a fresh review for a genuine identical resubmission", () => { + const dir = dataDir(); + const first = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-a", null, "tool-a")], + { plan, toolUseId: "tool-a" }, + ); + const second = findActiveExitPlanModeOccurrence( + [ + exitPlanEntry("entry-a", null, "tool-a"), + exitPlanEntry("entry-b", "entry-a", "tool-b"), + ], + { plan, toolUseId: "tool-b" }, + ); + recordPlanApprovalForSubmission({ ...context(dir, first), approved: true }); + + expect(shouldReusePlanApproval(context(dir, second))).toBe(false); + }); + + test("does not replay an approval from a rewound occurrence", () => { + const dir = dataDir(); + const oldOccurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-old", null, "tool-old")], + { plan }, + ); + const liveOccurrence = findActiveExitPlanModeOccurrence( + [ + { type: "user", uuid: "root", parentUuid: null, message: { role: "user", content: "plan" } }, + exitPlanEntry("entry-old", "root", "tool-old"), + exitPlanEntry("entry-live", "root", "tool-live"), + ], + { plan }, + ); + recordPlanApprovalForSubmission({ ...context(dir, oldOccurrence), approved: true }); + + expect(shouldReusePlanApproval(context(dir, liveOccurrence))).toBe(false); + }); + + test("does not replay an approval from before a compact boundary", () => { + const dir = dataDir(); + const oldOccurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-old", null, "tool-old")], + { plan }, + ); + const compactedOccurrence = findActiveExitPlanModeOccurrence( + [ + { type: "user", uuid: "root", parentUuid: null, message: { role: "user", content: "plan" } }, + exitPlanEntry("entry-old", "root", "tool-old"), + exitPlanEntry("entry-after-compact", null, "tool-after-compact"), + ], + { plan }, + ); + recordPlanApprovalForSubmission({ ...context(dir, oldOccurrence), approved: true }); + + expect(shouldReusePlanApproval(context(dir, compactedOccurrence))).toBe(false); + }); + + test("fails open for a corrupt approval store", () => { + const dir = dataDir(); + const occurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-a", null, "tool-a")], + { plan }, + ); + const input = context(dir, occurrence); + recordPlanApprovalForSubmission({ ...input, approved: true }); + const decisionsDir = join(dir, "plan-decisions"); + writeFileSync(join(decisionsDir, readdirSync(decisionsDir)[0]), "{not-json"); + + expect(shouldReusePlanApproval(input)).toBe(false); + }); + + test("does not reuse when disabled or when occurrence identity is unavailable", () => { + const dir = dataDir(); + const occurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-a", null, "tool-a")], + { plan }, + ); + const input = context(dir, occurrence); + recordPlanApprovalForSubmission({ ...input, approved: true }); + + expect(shouldReusePlanApproval({ ...input, enabled: false })).toBe(false); + expect(shouldReusePlanApproval({ ...input, occurrence: null })).toBe(false); + }); + + test("never persists a denial for a later retry", () => { + const dir = dataDir(); + const occurrence = findActiveExitPlanModeOccurrence( + [exitPlanEntry("entry-a", null, "tool-a")], + { plan }, + ); + const input = context(dir, occurrence); + + recordPlanApprovalForSubmission({ ...input, approved: false }); + + expect(shouldReusePlanApproval(input)).toBe(false); + }); +}); diff --git a/apps/hook/server/plan-decision-policy.ts b/apps/hook/server/plan-decision-policy.ts new file mode 100644 index 000000000..d04e7608b --- /dev/null +++ b/apps/hook/server/plan-decision-policy.ts @@ -0,0 +1,59 @@ +import { + getPlanApproval, + recordPlanApproval, +} from "./plan-decision-store"; +import type { ActiveExitPlanModeOccurrence } from "./session-log"; + +interface PlanApprovalReuseContext { + enabled: boolean; + isGemini: boolean; + project: string; + sessionId: string; + plan: string; + occurrence: ActiveExitPlanModeOccurrence | null; + baseDir?: string; + now?: number; +} + +export function shouldReusePlanApproval(context: PlanApprovalReuseContext): boolean { + if ( + !context.enabled || + context.isGemini || + !context.occurrence || + !context.sessionId + ) { + return false; + } + + return !!getPlanApproval( + context.project, + context.sessionId, + context.plan, + context.occurrence.key, + context.baseDir, + context.now, + ); +} + +export function recordPlanApprovalForSubmission( + context: PlanApprovalReuseContext & { approved: boolean }, +): void { + if ( + !context.enabled || + context.isGemini || + !context.approved || + !context.occurrence || + !context.sessionId + ) { + return; + } + + recordPlanApproval( + context.project, + context.sessionId, + context.plan, + context.occurrence.key, + context.baseDir, + context.now, + ); +} diff --git a/apps/hook/server/plan-decision-store.test.ts b/apps/hook/server/plan-decision-store.test.ts new file mode 100644 index 000000000..8e804dfee --- /dev/null +++ b/apps/hook/server/plan-decision-store.test.ts @@ -0,0 +1,151 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { + existsSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + utimesSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + APPROVAL_REUSE_MAX_AGE_MS, + getPlanApproval, + recordPlanApproval, +} from "./plan-decision-store"; + +const dirs: string[] = []; +const tmp = (): string => { + const dir = mkdtempSync(join(tmpdir(), "plan-decisions-")); + dirs.push(dir); + return dir; +}; + +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +const P = "my-project"; + +describe("plan-decision-store (#1075)", () => { + test("keeps the source file free of raw NUL bytes", () => { + const source = readFileSync(join(import.meta.dir, "plan-decision-store.ts")); + expect(source.includes(0)).toBe(false); + }); + + test("returns null before any approval is recorded", () => { + expect(getPlanApproval(P, "sess-a", "# Plan\n", "occurrence-a", tmp(), 1_000)).toBeNull(); + }); + + test("records an approval only for the same project, session, plan, and occurrence", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "# Plan\n\n- step", "occurrence-a", dir, 1_000); + expect( + getPlanApproval(P, "sess-a", "# Plan\n\n- step", "occurrence-a", dir, 1_001), + ).toEqual({ occurrence: "occurrence-a", approvedAt: 1_000 }); + expect( + getPlanApproval(P, "sess-a", "# Plan\n\n- step", "occurrence-b", dir, 1_001), + ).toBeNull(); + }); + + test("matches plans after normalizing CRLF and surrounding whitespace", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "# Plan\n\n- a\n- b", "occurrence-a", dir, 1_000); + // A Windows resubmission (CRLF) with a trailing blank line still matches. + expect( + getPlanApproval( + P, + "sess-a", + " # Plan\r\n\r\n- a\r\n- b\r\n", + "occurrence-a", + dir, + 1_001, + ), + ).toEqual({ occurrence: "occurrence-a", approvedAt: 1_000 }); + }); + + test("does not match a plan whose body actually changed", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "# Plan\n- a", "occurrence-a", dir, 1_000); + expect( + getPlanApproval(P, "sess-a", "# Plan\n- a\n- b", "occurrence-a", dir, 1_001), + ).toBeNull(); + }); + + test("keeps approvals separate per project, session, and plan", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "plan one", "occurrence-a", dir, 1_000); + expect(getPlanApproval("other-project", "sess-a", "plan one", "occurrence-a", dir, 1_001)).toBeNull(); + expect(getPlanApproval(P, "sess-b", "plan one", "occurrence-a", dir, 1_001)).toBeNull(); + expect(getPlanApproval(P, "sess-a", "plan two", "occurrence-a", dir, 1_001)).toBeNull(); + }); + + test("treats legacy denial and malformed records as absent", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "plan", "occurrence-a", dir, 1_000); + const file = join(dir, "plan-decisions", readdirSync(join(dir, "plan-decisions"))[0]); + const key = Object.keys(JSON.parse(readFileSync(file, "utf-8")))[0]; + + writeFileSync( + file, + JSON.stringify({ + [key]: { decision: "denied", feedback: "add tests" }, + malformed: { occurrence: 42, approvedAt: "now" }, + }), + ); + + expect(getPlanApproval(P, "sess-a", "plan", "occurrence-a", dir, 1_001)).toBeNull(); + }); + + test("does not reuse approvals older than the freshness window", () => { + const dir = tmp(); + recordPlanApproval(P, "sess-a", "plan", "occurrence-a", dir, 1_000); + expect( + getPlanApproval( + P, + "sess-a", + "plan", + "occurrence-a", + dir, + 1_000 + APPROVAL_REUSE_MAX_AGE_MS + 1, + ), + ).toBeNull(); + }); + + test("an empty session id never records or matches (safe fallback)", () => { + const dir = tmp(); + recordPlanApproval(P, "", "plan", "occurrence-a", dir, 1_000); + expect(getPlanApproval(P, "", "plan", "occurrence-a", dir, 1_001)).toBeNull(); + }); + + test("tolerates session ids that are file paths or contain separators", () => { + const dir = tmp(); + const pathLikeId = "/Users/x/.codex/sessions/2026/07/rollout-abc.jsonl"; + recordPlanApproval(P, pathLikeId, "plan", "occurrence-a", dir, 1_000); + expect(getPlanApproval(P, pathLikeId, "plan", "occurrence-a", dir, 1_001)).toEqual({ + occurrence: "occurrence-a", + approvedAt: 1_000, + }); + }); + + test("prunes session files older than the retention window on write", () => { + const dir = tmp(); + recordPlanApproval(P, "old-session", "plan", "occurrence-a", dir, 1_000); + const decisionsDir = join(dir, "plan-decisions"); + const [oldFile] = readdirSync(decisionsDir); + const oldPath = join(decisionsDir, oldFile); + const ancientSeconds = Date.now() / 1000 - 8 * 24 * 60 * 60; + utimesSync(oldPath, ancientSeconds, ancientSeconds); + + recordPlanApproval(P, "new-session", "plan", "occurrence-a", dir, 1_000); + expect(existsSync(oldPath)).toBe(false); + expect(getPlanApproval(P, "new-session", "plan", "occurrence-a", dir, 1_001)).toEqual({ + occurrence: "occurrence-a", + approvedAt: 1_000, + }); + expect(getPlanApproval(P, "old-session", "plan", "occurrence-a", dir, 1_001)).toBeNull(); + }); +}); diff --git a/apps/hook/server/plan-decision-store.ts b/apps/hook/server/plan-decision-store.ts new file mode 100644 index 000000000..66263fddf --- /dev/null +++ b/apps/hook/server/plan-decision-store.ts @@ -0,0 +1,164 @@ +/** + * Per-session record of recent Claude ExitPlanMode approvals. A hook retry for + * the same active tool occurrence can reuse the approval without opening a + * duplicate review. + * + * Records are keyed by (session, normalized-plan hash) and stored one JSON file + * per session under the data dir. Every operation is best-effort: a failure here + * must never break plan review, it just means the plan may be reviewed again. + */ +import { createHash, randomUUID } from "node:crypto"; +import { + existsSync, + mkdirSync, + readFileSync, + readdirSync, + renameSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; + +import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; + +import { normalizePlanText } from "./plan-normalization"; + +/** A recent approval for one plan submission occurrence. */ +export interface RecordedPlanApproval { + occurrence: string; + approvedAt: number; +} + +const SUBDIR = "plan-decisions"; +/** Session files older than this are pruned opportunistically on write. */ +const MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; +/** A retry normally follows immediately; never replay an old hook approval. */ +export const APPROVAL_REUSE_MAX_AGE_MS = 5 * 60 * 1000; + +const decisionsDir = (baseDir?: string): string => + join(baseDir ?? getPlannotatorDataDir(), SUBDIR); + +/** A filesystem-safe file name scoped to (project, session), so two projects + * that ever share a session id can never read each other's decisions. Any id + * shape (UUID, thread id, path) is safe because it is hashed. */ +const sessionFile = (dir: string, project: string, sessionId: string): string => + join( + dir, + `${createHash("sha256").update(`${project}\0${sessionId}`).digest("hex").slice(0, 32)}.json`, + ); + +/** Plans are matched after normalizing CRLF to LF and trimming surrounding + * whitespace, so a Windows resubmission (or a trailing-newline difference) of the + * same plan matches. */ +const planKey = (plan: string): string => + createHash("sha256").update(normalizePlanText(plan)).digest("hex"); + +function isRecordedPlanApproval(value: unknown): value is RecordedPlanApproval { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + const record = value as Record; + return ( + Object.keys(record).length === 2 && + typeof record.occurrence === "string" && + record.occurrence.length > 0 && + typeof record.approvedAt === "number" && + Number.isFinite(record.approvedAt) && + record.approvedAt >= 0 + ); +} + +const readSession = (file: string): Record => { + try { + const parsed = JSON.parse(readFileSync(file, "utf-8")); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + const approvals: Record = {}; + for (const [key, value] of Object.entries(parsed)) { + if (/^[a-f0-9]{64}$/.test(key) && isRecordedPlanApproval(value)) { + approvals[key] = value; + } + } + return approvals; + } catch { + return {}; + } +}; + +/** A fresh approval for this exact active submission, or null. */ +export function getPlanApproval( + project: string, + sessionId: string, + plan: string, + occurrence: string, + baseDir?: string, + now = Date.now(), +): RecordedPlanApproval | null { + if (!sessionId || !occurrence || !Number.isFinite(now)) return null; + const file = sessionFile(decisionsDir(baseDir), project, sessionId); + if (!existsSync(file)) return null; + const approval = readSession(file)[planKey(plan)]; + if ( + !approval || + approval.occurrence !== occurrence || + approval.approvedAt > now || + now - approval.approvedAt > APPROVAL_REUSE_MAX_AGE_MS + ) { + return null; + } + return approval; +} + +/** Record an approval for one active submission. Best-effort: never throws. */ +export function recordPlanApproval( + project: string, + sessionId: string, + plan: string, + occurrence: string, + baseDir?: string, + approvedAt = Date.now(), +): void { + if (!sessionId || !occurrence || !Number.isFinite(approvedAt) || approvedAt < 0) return; + let tmp: string | null = null; + try { + const dir = decisionsDir(baseDir); + mkdirSync(dir, { recursive: true }); + pruneOldSessions(dir); + const file = sessionFile(dir, project, sessionId); + const approvals = readSession(file); + approvals[planKey(plan)] = { occurrence, approvedAt }; + // Write to a temp file and rename so a concurrent reader never sees a + // half-written file (rename is atomic on the same filesystem). + tmp = `${file}.${process.pid}.${randomUUID()}.tmp`; + writeFileSync(tmp, JSON.stringify(approvals), "utf-8"); + renameSync(tmp, file); + tmp = null; + } catch { + // A failed record just means the plan may be reviewed again, which is safe. + } finally { + if (tmp) { + try { + rmSync(tmp, { force: true }); + } catch { + // Ignore a temporary file that cannot be cleaned up. + } + } + } +} + +function pruneOldSessions(dir: string): void { + const cutoff = Date.now() - MAX_AGE_MS; + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return; + } + for (const name of entries) { + if (!name.endsWith(".json")) continue; + const path = join(dir, name); + try { + if (statSync(path).mtimeMs < cutoff) rmSync(path, { force: true }); + } catch { + // Ignore a file that vanished or cannot be stat'd. + } + } +} diff --git a/apps/hook/server/plan-normalization.ts b/apps/hook/server/plan-normalization.ts new file mode 100644 index 000000000..26ddccbb2 --- /dev/null +++ b/apps/hook/server/plan-normalization.ts @@ -0,0 +1,3 @@ +export function normalizePlanText(plan: string): string { + return plan.replace(/\r\n/g, "\n").trim(); +} diff --git a/apps/hook/server/session-log.test.ts b/apps/hook/server/session-log.test.ts index 458a1562f..09b0f4501 100644 --- a/apps/hook/server/session-log.test.ts +++ b/apps/hook/server/session-log.test.ts @@ -16,6 +16,8 @@ import { extractRecentRenderedMessages, getRecentRenderedMessages, resolveActiveBranchIndices, + findActiveExitPlanModeOccurrence, + findActiveExitPlanModeOccurrenceInTranscript, findDroidSessionLogsForCwd, resolveDroidSessionLogForCwd, projectSlugFromCwd, @@ -29,7 +31,7 @@ import { resolveSessionLogByCwdScan, type SessionLogEntry, } from "./session-log"; -import { mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeFileSync, rmSync, utimesSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -80,6 +82,27 @@ function assistantToolUse( }); } +function exitPlanModeToolUse(toolUseId: string, plan: string): string { + return JSON.stringify({ + type: "assistant", + message: { + id: `msg_${toolUseId}`, + role: "assistant", + content: [ + { + type: "tool_use", + id: toolUseId, + name: "ExitPlanMode", + input: { plan }, + }, + ], + stop_reason: "tool_use", + }, + uuid: crypto.randomUUID(), + parentUuid: crypto.randomUUID(), + }); +} + /** Assistant entry with both text and tool_use */ function assistantTextAndToolUse( msgId: string, @@ -863,6 +886,97 @@ describe("resolveActiveBranchIndices", () => { }); }); +describe("findActiveExitPlanModeOccurrence", () => { + test("identifies the exact active ExitPlanMode tool occurrence", () => { + const plan = "# Plan\n- step"; + const entries = parseSessionLog( + buildLog(userPrompt("make a plan"), exitPlanModeToolUse("toolu_plan", plan)), + ); + + expect( + findActiveExitPlanModeOccurrence(entries, { plan, toolUseId: "toolu_plan" }), + ).toMatchObject({ + toolUseId: "toolu_plan", + entryUuid: entries[1].uuid, + }); + }); + + test("uses the live occurrence after a rewind instead of the orphaned occurrence", () => { + const plan = "# Same plan"; + const entries = parseSessionLog( + buildRewoundLog({ + kept: [userPrompt("make a plan")], + abandoned: [exitPlanModeToolUse("toolu_orphaned", plan)], + resumed: [exitPlanModeToolUse("toolu_live", plan)], + }), + ); + + expect(findActiveExitPlanModeOccurrence(entries, { plan })).toMatchObject({ + toolUseId: "toolu_live", + }); + }); + + test("does not resolve an occurrence before a compact boundary", () => { + const plan = "# Same plan"; + const preCompact = linkChain([ + userPrompt("make a plan"), + exitPlanModeToolUse("toolu_before_compact", plan), + ]); + const postCompact = linkChain( + [exitPlanModeToolUse("toolu_after_compact", plan)], + null, + ); + const entries = parseSessionLog([...preCompact, ...postCompact].join("\n")); + + expect(findActiveExitPlanModeOccurrence(entries, { plan })).toMatchObject({ + toolUseId: "toolu_after_compact", + }); + }); + + test("fails open when the active transcript cannot identify a tool occurrence", () => { + const entries = parseSessionLog( + JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + { + type: "tool_use", + name: "ExitPlanMode", + input: { plan: "# Plan" }, + }, + ], + }, + }), + ); + + expect(findActiveExitPlanModeOccurrence(entries, { plan: "# Plan" })).toBeNull(); + }); + + test("resolves ExitPlanMode from the transcript state before PermissionRequest", () => { + const plan = "# Plan\n- step"; + const dir = mkdtempSync(join(tmpdir(), "plannotator-pre-permission-")); + const transcriptPath = join(dir, "session.jsonl"); + try { + writeFileSync( + transcriptPath, + buildLog( + userPrompt("make a plan"), + exitPlanModeToolUse("toolu_before_permission", plan), + ), + ); + + expect( + findActiveExitPlanModeOccurrenceInTranscript(transcriptPath, { plan }), + ).toMatchObject({ + toolUseId: "toolu_before_permission", + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + describe("extractRecentRenderedMessages — after a rewind", () => { const rewoundLog = () => buildRewoundLog({ diff --git a/apps/hook/server/session-log.ts b/apps/hook/server/session-log.ts index e209e462f..39001b082 100644 --- a/apps/hook/server/session-log.ts +++ b/apps/hook/server/session-log.ts @@ -20,6 +20,8 @@ import { spawnSync } from "node:child_process"; import { join, dirname, basename } from "node:path"; import { homedir } from "node:os"; +import { normalizePlanText } from "./plan-normalization"; + const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR || join(homedir(), ".claude"); const DEFAULT_SESSIONS_DIR = join(claudeConfigDir, "sessions"); @@ -76,6 +78,12 @@ export interface RenderedMessage { timestamp?: string; } +export interface ActiveExitPlanModeOccurrence { + entryUuid: string; + toolUseId: string; + key: string; +} + // --- Session File Discovery --- /** @@ -777,6 +785,74 @@ export function resolveActiveBranchIndices( } } +/** + * Find the live Claude ExitPlanMode tool occurrence for a hook payload. The + * transcript branch and stable entry/tool ids bind reuse to a submission, not + * merely to equal plan text. + */ +export function findActiveExitPlanModeOccurrence( + entries: SessionLogEntry[], + opts: { plan: string; toolUseId?: string }, +): ActiveExitPlanModeOccurrence | null { + const branchIndices = resolveActiveBranchIndices(entries); + if (!branchIndices) return null; + + const expectedPlan = normalizePlanText(opts.plan); + for (let i = entries.length - 1; i >= 0; i--) { + if (!branchIndices.has(i)) continue; + const entry = entries[i]; + const entryUuid = entry?.uuid; + if (typeof entryUuid !== "string" || !entryUuid) continue; + if (getEntryRole(entry) !== "assistant") continue; + const content = entry.message?.content; + if (!Array.isArray(content)) continue; + + for (let blockIndex = content.length - 1; blockIndex >= 0; blockIndex--) { + const block = content[blockIndex]; + if (!block || block.type !== "tool_use") continue; + const toolUseId = typeof block.id === "string" ? block.id : ""; + const toolName = typeof block.name === "string" ? block.name : ""; + const input = block.input; + const submittedPlan = + input && typeof input === "object" && typeof (input as { plan?: unknown }).plan === "string" + ? (input as { plan: string }).plan + : null; + if ( + !toolUseId || + toolName !== "ExitPlanMode" || + submittedPlan === null || + normalizePlanText(submittedPlan) !== expectedPlan || + (opts.toolUseId && toolUseId !== opts.toolUseId) + ) { + continue; + } + + return { + entryUuid, + toolUseId, + key: JSON.stringify([entryUuid, toolUseId]), + }; + } + } + + return null; +} + +/** Read and resolve a live ExitPlanMode occurrence, failing open on any issue. */ +export function findActiveExitPlanModeOccurrenceInTranscript( + transcriptPath: string, + opts: { plan: string; toolUseId?: string }, +): ActiveExitPlanModeOccurrence | null { + try { + return findActiveExitPlanModeOccurrence( + parseSessionLog(readFileSync(transcriptPath, "utf-8")), + opts, + ); + } catch { + return null; + } +} + /** * Extract up to `limit` of the most recent rendered assistant messages. * diff --git a/apps/marketing/src/content/docs/reference/environment-variables.md b/apps/marketing/src/content/docs/reference/environment-variables.md index 42a6340c6..53b2234b1 100644 --- a/apps/marketing/src/content/docs/reference/environment-variables.md +++ b/apps/marketing/src/content/docs/reference/environment-variables.md @@ -23,6 +23,7 @@ All Plannotator environment variables and their defaults. | `PLANNOTATOR_SHARE` | enabled | Set to `disabled` to turn off sharing. Hides share UI and import options. Can also be set via `~/.plannotator/config.json` (`{ "share": "disabled" }`); the env var takes precedence. | | `PLANNOTATOR_SHARE_URL` | `https://share.plannotator.ai` | Base URL for share links. Set this when self-hosting the share portal. | | `PLANNOTATOR_DATA_DIR` | `~/.plannotator` | Override the base directory for Plannotator-managed files (plans, history, drafts, config, hooks, sessions).* Some UI preferences remain in functional browser cookies. When unset, an existing `~/.plannotator` is always used; if it doesn't exist and `$XDG_DATA_HOME` is set to an absolute path, `$XDG_DATA_HOME/plannotator` is used; otherwise `~/.plannotator`. (The XDG spec's implicit `~/.local/share` default is deliberately not applied — only an explicitly-set `$XDG_DATA_HOME` moves the directory.) | +| `PLANNOTATOR_PLAN_DECISION_REUSE` | enabled | **Hook-runtime only.** Set to `0` or `false` to make every plan open a review. Otherwise, only an approval for the same active Claude `ExitPlanMode` occurrence is reused during a short retry window, with a visible hook message. New identical occurrences, rewound/compacted history, missing transcript identity, and all denials open a fresh review. Codex filters proposed plans to the current turn and never replays a decision. Also configurable with `{ "planDecisionReuse": false }` in `~/.plannotator/config.json`; the environment variable takes precedence. | | `PLANNOTATOR_PLAN_TIMEOUT_SECONDS` | `345600` | OpenCode only. `submit_plan` wait timeout in seconds. Set `0` to disable timeout. | | `PLANNOTATOR_TODO_PROVIDER` | auto | Pi/oh-my-pi only. Set to `off` (or `0` / `false` / `disabled`) to stop mirroring the approved plan checklist into an editable todo provider during execution. When enabled, Plannotator syncs the checklist only if a provider is detected — currently [pi-todos](https://github.com/mitsuhiko/agent-stuff), detected by its todo directory existing (`.pi/todos` by default, or wherever `PI_TODO_PATH` redirects it when set). The mirror is additive: the progress widget behaves the same either way, and sync is one-way, so edits made in `/todos` never feed back into plan execution. Can also be set via `~/.plannotator/config.json` (`{ "todoProvider": "off" }`); the env var takes precedence. | diff --git a/packages/shared/config.test.ts b/packages/shared/config.test.ts index 4e78f8798..cbab2f2c5 100644 --- a/packages/shared/config.test.ts +++ b/packages/shared/config.test.ts @@ -4,6 +4,7 @@ import { resolveCursorSandbox, resolveUseGlimpse, resolveAnnotateHistory, + resolvePlanDecisionReuse, resolveGuideHistory, resolveUseJina, resolveTodoProviderEnabled, @@ -135,6 +136,12 @@ describe("config.json boolean coercion", () => { key: "guideHistory", resolve: resolveGuideHistory, }, + { + name: "resolvePlanDecisionReuse", + envVar: "PLANNOTATOR_PLAN_DECISION_REUSE", + key: "planDecisionReuse", + resolve: resolvePlanDecisionReuse, + }, { name: "resolveUseJina", envVar: "PLANNOTATOR_JINA", diff --git a/packages/shared/config.ts b/packages/shared/config.ts index c1d5c5561..05c58fded 100644 --- a/packages/shared/config.ts +++ b/packages/shared/config.ts @@ -133,6 +133,13 @@ export interface PlannotatorConfig { * annotate sessions fully stateless. Default: true. */ annotateHistory?: boolean; + /** + * Skip re-opening a plan review for a plan already decided in the same agent + * session (#1075): an identical ExitPlanMode resubmission, or a Codex Stop turn + * that re-scrapes the previous turn's decided plan. Set to false to always + * re-review. Default: true. + */ + planDecisionReuse?: boolean; /** * Persist successful Guided Reviews (guide content + per-section reviewed * state) under ~/.plannotator/guides/ (or PLANNOTATOR_DATA_DIR) so they @@ -324,6 +331,21 @@ export function resolveAnnotateHistory(config: PlannotatorConfig): boolean { return coerceConfigBoolean(config.annotateHistory, true); } +/** + * Resolve whether an already-decided plan is re-used instead of re-reviewed + * within the same session (#1075). + * + * Priority (highest wins): + * PLANNOTATOR_PLAN_DECISION_REUSE env var → config.planDecisionReuse → default true + */ +export function resolvePlanDecisionReuse(config: PlannotatorConfig): boolean { + const envVal = process.env.PLANNOTATOR_PLAN_DECISION_REUSE; + if (envVal !== undefined) { + return envVal === "1" || envVal.toLowerCase() === "true"; + } + return coerceConfigBoolean(config.planDecisionReuse, true); +} + /** * Resolve whether successful Guided Reviews are persisted to disk. *