Skip to content

Commit 474ca86

Browse files
voltcrashmaria-rcks
authored and
Pipyakas
committed
fix(server): show why Codex hooks stopped a turn (pingdotgg#27399)
Co-authored-by: maria-rcks <maria@kuuro.net> (cherry picked from commit 9b72088)
1 parent f06c759 commit 474ca86

7 files changed

Lines changed: 185 additions & 21 deletions

File tree

‎apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2366,6 +2366,116 @@ describe("CodexAdapterV2 post-settle continuation", () => {
23662366
),
23672367
);
23682368

2369+
it.effect.each([
2370+
{ hook: "stopped", status: "interrupted", expected: true },
2371+
{ hook: "completed", status: "interrupted", expected: false },
2372+
{ hook: "warning", status: "interrupted", expected: false },
2373+
{ hook: "empty", status: "interrupted", expected: false },
2374+
{ hook: "wrong-turn", status: "interrupted", expected: false },
2375+
{ hook: "wrong-thread", status: "interrupted", expected: false },
2376+
{ hook: "unscoped", status: "interrupted", expected: false },
2377+
{ hook: "stopped", status: "completed", expected: false },
2378+
{ hook: "stopped", status: "failed", expected: false },
2379+
] as const)(
2380+
"correlates $hook hook output with a $status turn only",
2381+
({ hook, status, expected }) =>
2382+
Effect.gen(function* () {
2383+
const nativeThreadId = "hook-thread";
2384+
const stopReason = "Save a current task checkpoint before compaction.";
2385+
const hookCompleted = (turnId: string | null) => ({
2386+
type: "emit_inbound" as const,
2387+
label: "hook/completed",
2388+
frame: {
2389+
method: "hook/completed",
2390+
params: {
2391+
threadId: hook === "wrong-thread" ? "other-thread" : nativeThreadId,
2392+
turnId: hook === "unscoped" ? null : hook === "wrong-turn" ? "other-turn" : turnId,
2393+
run: {
2394+
id: "checkpoint-hook",
2395+
eventName: "preCompact",
2396+
source: "project",
2397+
sourcePath: "/workspace/.codex/hooks.json",
2398+
scope: "turn",
2399+
executionMode: "sync",
2400+
handlerType: "command",
2401+
displayOrder: 1,
2402+
startedAt: 1782622440,
2403+
status: hook === "completed" ? "completed" : "stopped",
2404+
entries: [
2405+
{ kind: "warning", text: "Unrelated hook diagnostic" },
2406+
{
2407+
kind: hook === "warning" ? "warning" : "stop",
2408+
text: hook === "empty" ? " " : stopReason,
2409+
},
2410+
],
2411+
},
2412+
},
2413+
},
2414+
});
2415+
const transcript = makeCodexReplayTranscript({
2416+
scenario: `hook-stop-${hook}-${status}`,
2417+
entries: [
2418+
...codexReplayPreamble({ nativeThreadId, nativeTurnId: "hook-turn", prompt: "work" }),
2419+
hookCompleted("hook-turn"),
2420+
{
2421+
type: "emit_inbound",
2422+
label: "turn/completed",
2423+
frame: {
2424+
method: "turn/completed",
2425+
params: {
2426+
threadId: nativeThreadId,
2427+
turn: makeCodexReplayTurn({ id: "hook-turn", status }),
2428+
},
2429+
},
2430+
},
2431+
...codexReplayPreamble({ nativeThreadId, nativeTurnId: "next-turn", prompt: "work" })
2432+
.slice(-3)
2433+
.map((entry, index) => (index < 2 ? withReplayRequestId(entry, 4) : entry)),
2434+
hookCompleted("hook-turn"),
2435+
{
2436+
type: "emit_inbound",
2437+
label: "turn/completed/next",
2438+
frame: {
2439+
method: "turn/completed",
2440+
params: {
2441+
threadId: nativeThreadId,
2442+
turn: makeCodexReplayTurn({ id: "next-turn", status: "interrupted" }),
2443+
},
2444+
},
2445+
},
2446+
],
2447+
});
2448+
const harness = yield* makeCodexReplayHarness(transcript);
2449+
for (const turn of ["hook-turn", "next-turn"]) {
2450+
yield* harness.runtime.startTurn(
2451+
makeCodexTestTurnInput({
2452+
threadId: harness.threadId,
2453+
providerThread: harness.providerThread,
2454+
now: yield* DateTime.now,
2455+
attemptId: RunAttemptId.make(turn),
2456+
text: "work",
2457+
}),
2458+
);
2459+
yield* harness.awaitTerminal(turn === "hook-turn" ? 1 : 2);
2460+
}
2461+
assert.deepEqual(
2462+
harness.terminalEvents().map((event) => ({
2463+
status: event.status,
2464+
reason: event.status === "failed" ? undefined : event.reason,
2465+
})),
2466+
[
2467+
{ status, reason: expected ? `preCompact hook (project): ${stopReason}` : undefined },
2468+
{ status: "interrupted", reason: undefined },
2469+
],
2470+
);
2471+
}).pipe(
2472+
Effect.scoped,
2473+
Effect.provide(
2474+
Layer.mergeAll(IdAllocator.layer, McpProviderSessions.layer, NodeServices.layer),
2475+
),
2476+
),
2477+
);
2478+
23692479
it.effect("resumes a provider thread without requesting or decoding its history", () =>
23702480
Effect.scoped(
23712481
Effect.gen(function* () {

‎apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,7 @@ function withCodexManagedFailure(
11201120
}
11211121

11221122
interface ActiveCodexTurnContext {
1123+
hookStopReason?: string;
11231124
capacityRetryAttempt?: number;
11241125
capacityRetryPending?: Deferred.Deferred<ActiveCodexTurnContext | undefined>;
11251126
closedCommandItemIds?: ReadonlySet<string>;
@@ -4479,6 +4480,22 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* (
44794480
updateSubagentSelection(payload.threadId, payload.toModel),
44804481
);
44814482

4483+
yield* client.handleServerNotification("hook/completed", (payload) =>
4484+
Effect.gen(function* () {
4485+
if (payload.turnId == null || payload.run.status !== "stopped") return;
4486+
const context = (yield* Ref.get(activeTurns)).get(payload.turnId);
4487+
if (context?.providerThread.nativeThreadRef?.nativeId !== payload.threadId) return;
4488+
const reason = payload.run.entries
4489+
.filter((entry) => entry.kind === "stop")
4490+
.map((entry) => entry.text.trim())
4491+
.filter(Boolean)
4492+
.join("\n");
4493+
if (!reason) return;
4494+
const source = payload.run.source === undefined ? "" : ` (${payload.run.source})`;
4495+
context.hookStopReason = `${payload.run.eventName} hook${source}: ${reason}`;
4496+
}),
4497+
);
4498+
44824499
yield* client.handleServerNotification("turn/started", (payload) =>
44834500
Effect.gen(function* () {
44844501
const context = (yield* Ref.get(activeTurns)).get(payload.turn.id);
@@ -5746,6 +5763,9 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* (
57465763
runOrdinal: input.context.input.runOrdinal,
57475764
status: terminalStatus,
57485765
failure: null,
5766+
...(terminalStatus === "interrupted" && input.context.hookStopReason !== undefined
5767+
? { reason: input.context.hookStopReason }
5768+
: {}),
57495769
threadDisposition: "reusable",
57505770
};
57515771
},

‎apps/server/src/orchestration-v2/RunExecutionService.test.ts‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2561,22 +2561,43 @@ it.effect("does not overwrite Stop when ownership changes after the finalization
25612561
}),
25622562
);
25632563

2564-
it.effect("emits run_interrupt_result when hard-stop finalizes the active attempt", () =>
2565-
Effect.gen(function* () {
2566-
const { written, observed, committedEffects } = yield* captureRootRunTermination({
2567-
key: "hard-stop",
2568-
shouldFinalizeRun: () => Effect.succeed(true),
2569-
});
2570-
assert.deepEqual(
2571-
written.map((item) => item.type),
2572-
["run_interrupt_result"],
2573-
);
2574-
assert.deepEqual(observed, ["run:interrupted", "pull-requests-refreshed"]);
2575-
assert.deepEqual(
2576-
committedEffects.map((effect) => effect.request.type),
2577-
["checkpoint.capture"],
2578-
);
2579-
}),
2564+
it.effect.each([
2565+
{ providerReason: undefined, requestReason: undefined, expected: "Stopped by the provider" },
2566+
{
2567+
providerReason: "preCompact hook (project): Save a checkpoint.",
2568+
requestReason: undefined,
2569+
expected: "preCompact hook (project): Save a checkpoint.",
2570+
},
2571+
{
2572+
providerReason: "preCompact hook (project): Save a checkpoint.",
2573+
requestReason: "Stopped by the user",
2574+
expected: "Stopped by the user",
2575+
},
2576+
])(
2577+
"emits run_interrupt_result with $expected when finalizing the active attempt",
2578+
({ providerReason, requestReason, expected }) =>
2579+
Effect.gen(function* () {
2580+
const { written, observed, committedEffects } = yield* captureRootRunTermination({
2581+
key: "hard-stop",
2582+
shouldFinalizeRun: () => Effect.succeed(true),
2583+
loadRunInterruptReason: () => Effect.succeed(requestReason),
2584+
events: (ids) =>
2585+
Stream.make({ ...rootTerminalEvent(ids, "interrupted"), reason: providerReason }),
2586+
});
2587+
assert.deepEqual(
2588+
written.map((item) => item.type),
2589+
["run_interrupt_result"],
2590+
);
2591+
assert.deepEqual(observed, ["run:interrupted", "pull-requests-refreshed"]);
2592+
assert.equal(
2593+
written[0]?.type === "run_interrupt_result" ? written[0].message : undefined,
2594+
expected,
2595+
);
2596+
assert.deepEqual(
2597+
committedEffects.map((effect) => effect.request.type),
2598+
["checkpoint.capture"],
2599+
);
2600+
}),
25802601
);
25812602

25822603
it.effect("records a finished run as failed when its ownership check cannot be read", () =>
@@ -3054,6 +3075,7 @@ function captureRootRunTermination(input: {
30543075
readonly shouldFinalizeRun: () => Effect.Effect<boolean, ProjectionStore.ProjectionStoreV2Error>;
30553076
readonly rejectTerminalWrite?: boolean;
30563077
readonly hasUnpairedRunInterruptRequest?: () => Effect.Effect<boolean, never>;
3078+
readonly loadRunInterruptReason?: () => Effect.Effect<string | undefined, never>;
30573079
readonly hasPendingBackgroundWorkForThread?: ProviderAdapter.ProviderAdapterV2SessionRuntime["hasPendingBackgroundWorkForThread"];
30583080
readonly seedOpenSubagent?: boolean;
30593081
readonly events?: (
@@ -3238,6 +3260,9 @@ function captureRootRunTermination(input: {
32383260
attemptId: ids.attemptId,
32393261
providerTurnOrdinal: 1,
32403262
shouldFinalizeRun: input.shouldFinalizeRun,
3263+
...(input.loadRunInterruptReason === undefined
3264+
? {}
3265+
: { loadRunInterruptReason: input.loadRunInterruptReason }),
32413266
...(input.shouldStartProviderTurn === undefined
32423267
? {}
32433268
: { shouldStartProviderTurn: input.shouldStartProviderTurn }),

‎apps/server/src/orchestration-v2/RunExecutionService.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -686,8 +686,10 @@ export const layer: Layer.Layer<
686686
}
687687
const allocateEventId = () => idAllocator.allocate.event({ threadId: input.run.threadId });
688688
const reason =
689-
input.terminal.status === "interrupted" && input.loadRunInterruptReason !== undefined
690-
? yield* input.loadRunInterruptReason()
689+
input.terminal.status === "interrupted"
690+
? ((input.loadRunInterruptReason === undefined
691+
? undefined
692+
: yield* input.loadRunInterruptReason()) ?? input.terminal.reason)
691693
: undefined;
692694
const open = input.openRunOwnedSubagents ?? emptyOpenRunOwnedSubagentProjection();
693695
const hasOpenSubagentProjection =
@@ -1775,7 +1777,6 @@ export function makeInterruptResultTurnItem(input: {
17751777
readonly run: OrchestrationV2Run;
17761778
readonly rootNode: OrchestrationV2ExecutionNode;
17771779
readonly providerThread: OrchestrationV2ProviderThread;
1778-
/** The Stop's own reason, from its run_interrupt_request item. */
17791780
readonly reason: string | undefined;
17801781
readonly completedAt: DateTime.Utc;
17811782
}): OrchestrationV2TurnItem {

‎apps/web/src/components/chat/TimelineSystemDivider.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export function TimelineSystemDivider(props: {
1010
readonly tone?: "neutral" | "danger";
1111
readonly icon?: LucideIcon;
1212
readonly showDetailSeparator?: boolean;
13+
readonly wrapDetail?: boolean;
1314
readonly actionLabel?: string;
1415
readonly onAction?: () => void;
1516
}) {
@@ -22,7 +23,8 @@ export function TimelineSystemDivider(props: {
2223
<span
2324
className={cn(
2425
"inline-flex min-w-0 items-center opacity-70",
25-
typeof props.detail === "string" && "max-w-80 truncate",
26+
typeof props.detail === "string" &&
27+
(props.wrapDetail ? "max-w-80 whitespace-pre-wrap" : "max-w-80 truncate"),
2628
)}
2729
>
2830
{props.showDetailSeparator === false ? null : "·\u00a0"}

‎apps/web/src/components/chat/V2LifecycleRow.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,12 @@ export function V2LifecycleRow(props: {
7272
const { item } = props;
7373
if (item.type === "run_interrupt_result") {
7474
return (
75-
<TimelineSystemDivider label="Run interrupted" detail={item.message} icon={CircleStopIcon} />
75+
<TimelineSystemDivider
76+
label="Run interrupted"
77+
detail={item.message}
78+
icon={CircleStopIcon}
79+
wrapDetail
80+
/>
7681
);
7782
}
7883
if (item.type === "handoff") {

‎packages/provider-core/src/server/ProviderAdapter.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ export const ProviderAdapterV2Event = Schema.Union([
154154
runOrdinal: PositiveInt,
155155
status: Schema.Literals(["completed", "interrupted", "cancelled"]),
156156
failure: Schema.Null,
157+
reason: Schema.optional(Schema.String),
157158
threadDisposition: Schema.Literals(["reusable", "broken"]),
158159
}),
159160
Schema.Struct({

0 commit comments

Comments
 (0)