Skip to content

Commit 07dbdc0

Browse files
committed
fix(codex): log unsafe Stop skips
Missing turn identity skips are visible only with PLANNOTATOR_DEBUG.
1 parent 93247bd commit 07dbdc0

4 files changed

Lines changed: 129 additions & 17 deletions

File tree

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@ import { describe, expect, test, afterEach } from "bun:test";
1010
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
1111
import { tmpdir } from "node:os";
1212
import { join } from "node:path";
13-
import { findCodexRolloutByThreadId, getLastCodexMessage, getLatestCodexPlan } from "./codex-session";
13+
import {
14+
findCodexRolloutByThreadId,
15+
getCodexStopSkipReason,
16+
getLastCodexMessage,
17+
getLatestCodexPlan,
18+
logCodexStopSkip,
19+
} from "./codex-session";
1420

1521
// --- Fixture Helpers ---
1622

@@ -389,6 +395,7 @@ describe("getLatestCodexPlan", () => {
389395
text: "Authoritative plan item",
390396
source: "plan-item",
391397
});
398+
392399
});
393400

394401
test("falls back to raw proposed_plan blocks for plan-only assistant replies", () => {
@@ -408,6 +415,40 @@ describe("getLatestCodexPlan", () => {
408415
});
409416
});
410417

418+
describe("Codex Stop skip diagnostics", () => {
419+
test("classifies a missing Stop turn id without reading stale plan content", () => {
420+
expect(getCodexStopSkipReason("not-read.jsonl")).toBe("missing-turn-id");
421+
});
422+
423+
test("requires an id-carrying rollout turn marker", () => {
424+
const turnId = "turn-without-marker";
425+
const path = writeTempRollout(
426+
buildRollout(
427+
sessionMeta(),
428+
turnStarted("other-turn"),
429+
completedPlanItem("Plan item without matching start marker", turnId),
430+
),
431+
);
432+
433+
expect(getCodexStopSkipReason(path, turnId)).toBe("missing-turn-marker");
434+
});
435+
436+
test("writes the exact skip breadcrumb only when debug is enabled", () => {
437+
const messages: string[] = [];
438+
const write = (message: string) => messages.push(message);
439+
440+
logCodexStopSkip("missing-turn-id", { debug: "", write });
441+
expect(messages).toEqual([]);
442+
443+
logCodexStopSkip("missing-turn-id", { debug: "1", write });
444+
logCodexStopSkip("missing-turn-marker", { debug: "1", write });
445+
expect(messages).toEqual([
446+
"[DEBUG] Codex Stop plan review skipped: missing Stop payload turn_id.",
447+
"[DEBUG] Codex Stop plan review skipped: missing id-carrying rollout turn marker.",
448+
]);
449+
});
450+
});
451+
411452
test("extracts plan blocks surrounded by assistant prose", () => {
412453
const turnId = "turn-prose";
413454
const path = writeTempRollout(

apps/hook/server/codex-session.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ export interface GetLatestCodexPlanOptions {
6060
stopHookActive?: boolean;
6161
}
6262

63+
export type CodexStopSkipReason = "missing-turn-id" | "missing-turn-marker";
64+
6365
const TURN_START_TYPES = new Set(["task_started", "turn_started"]);
6466
const TURN_COMPLETE_TYPES = new Set(["task_complete", "turn_completed"]);
6567
const PROPOSED_PLAN_RE = /<proposed_plan>([\s\S]*?)<\/proposed_plan>/gi;
@@ -182,23 +184,26 @@ function findLastIndex(
182184
return -1;
183185
}
184186

185-
function findTurnStartIndex(entries: RolloutEntry[], turnId?: string): number {
186-
const matchingTurnStart = findLastIndex(
187+
function findMatchingTurnMarkerIndex(
188+
entries: RolloutEntry[],
189+
turnId: string,
190+
): number {
191+
return findLastIndex(
187192
entries,
188193
(entry) =>
189-
entry.type === "event_msg" &&
190-
TURN_START_TYPES.has(entry.payload?.type || "") &&
191-
(!turnId || entry.payload?.turn_id === turnId)
194+
(
195+
(entry.type === "event_msg" && TURN_START_TYPES.has(entry.payload?.type || "")) ||
196+
entry.type === "turn_context"
197+
) &&
198+
getTurnId(entry) === turnId,
192199
);
193-
if (matchingTurnStart !== -1) return matchingTurnStart;
200+
}
194201

195-
const matchingTurnContext = findLastIndex(
196-
entries,
197-
(entry) =>
198-
entry.type === "turn_context" &&
199-
(!turnId || entry.payload?.turn_id === turnId)
200-
);
201-
if (matchingTurnContext !== -1) return matchingTurnContext;
202+
function findTurnStartIndex(entries: RolloutEntry[], turnId?: string): number {
203+
const matchingTurnStart = turnId
204+
? findMatchingTurnMarkerIndex(entries, turnId)
205+
: -1;
206+
if (matchingTurnStart !== -1) return matchingTurnStart;
202207

203208
const lastTurnStart = findLastIndex(
204209
entries,
@@ -283,6 +288,32 @@ function getTurnId(entry: RolloutEntry): string | null {
283288
return typeof turnId === "string" && turnId ? turnId : null;
284289
}
285290

291+
export function getCodexStopSkipReason(
292+
rolloutPath: string,
293+
turnId?: string,
294+
): CodexStopSkipReason | null {
295+
if (!turnId) return "missing-turn-id";
296+
const entries = parseRolloutEntries(rolloutPath);
297+
return findMatchingTurnMarkerIndex(entries, turnId) === -1
298+
? "missing-turn-marker"
299+
: null;
300+
}
301+
302+
export function logCodexStopSkip(
303+
reason: CodexStopSkipReason,
304+
opts: {
305+
debug?: string;
306+
write?: (message: string) => void;
307+
} = {},
308+
): void {
309+
if (!opts.debug) return;
310+
const detail =
311+
reason === "missing-turn-id"
312+
? "missing Stop payload turn_id."
313+
: "missing id-carrying rollout turn marker.";
314+
(opts.write ?? console.error)(`[DEBUG] Codex Stop plan review skipped: ${detail}`);
315+
}
316+
286317
function collectPlanCandidates(
287318
entries: RolloutEntry[],
288319
startIndex: number,

apps/hook/server/index.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,13 @@ import {
133133
findActiveExitPlanModeOccurrenceInTranscript,
134134
type RenderedMessage,
135135
} from "./session-log";
136-
import { findCodexRolloutByThreadId, getLatestCodexPlan, getRecentCodexMessages } from "./codex-session";
136+
import {
137+
findCodexRolloutByThreadId,
138+
getCodexStopSkipReason,
139+
getLatestCodexPlan,
140+
getRecentCodexMessages,
141+
logCodexStopSkip,
142+
} from "./codex-session";
137143
import {
138144
recordPlanApprovalForSubmission,
139145
shouldReusePlanApproval,
@@ -1939,8 +1945,18 @@ if (args[0] === "sessions") {
19391945
process.exit(0);
19401946
}
19411947

1948+
const turnId =
1949+
typeof event.turn_id === "string" && event.turn_id
1950+
? event.turn_id
1951+
: undefined;
1952+
const skipReason = getCodexStopSkipReason(rolloutPath, turnId);
1953+
if (skipReason) {
1954+
logCodexStopSkip(skipReason, { debug: process.env.PLANNOTATOR_DEBUG });
1955+
process.exit(0);
1956+
}
1957+
19421958
const latestPlan = getLatestCodexPlan(rolloutPath, {
1943-
turnId: typeof event.turn_id === "string" ? event.turn_id : undefined,
1959+
turnId,
19441960
stopHookActive: !!event.stop_hook_active,
19451961
});
19461962

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
getRecentRenderedMessages,
1818
resolveActiveBranchIndices,
1919
findActiveExitPlanModeOccurrence,
20+
findActiveExitPlanModeOccurrenceInTranscript,
2021
findDroidSessionLogsForCwd,
2122
resolveDroidSessionLogForCwd,
2223
projectSlugFromCwd,
@@ -30,7 +31,7 @@ import {
3031
resolveSessionLogByCwdScan,
3132
type SessionLogEntry,
3233
} from "./session-log";
33-
import { mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs";
34+
import { mkdirSync, mkdtempSync, writeFileSync, rmSync, utimesSync } from "node:fs";
3435
import { join } from "node:path";
3536
import { tmpdir } from "node:os";
3637

@@ -951,6 +952,29 @@ describe("findActiveExitPlanModeOccurrence", () => {
951952

952953
expect(findActiveExitPlanModeOccurrence(entries, { plan: "# Plan" })).toBeNull();
953954
});
955+
956+
test("resolves ExitPlanMode from the transcript state before PermissionRequest", () => {
957+
const plan = "# Plan\n- step";
958+
const dir = mkdtempSync(join(tmpdir(), "plannotator-pre-permission-"));
959+
const transcriptPath = join(dir, "session.jsonl");
960+
try {
961+
writeFileSync(
962+
transcriptPath,
963+
buildLog(
964+
userPrompt("make a plan"),
965+
exitPlanModeToolUse("toolu_before_permission", plan),
966+
),
967+
);
968+
969+
expect(
970+
findActiveExitPlanModeOccurrenceInTranscript(transcriptPath, { plan }),
971+
).toMatchObject({
972+
toolUseId: "toolu_before_permission",
973+
});
974+
} finally {
975+
rmSync(dir, { recursive: true, force: true });
976+
}
977+
});
954978
});
955979

956980
describe("extractRecentRenderedMessages — after a rewind", () => {

0 commit comments

Comments
 (0)