Skip to content

Commit 695b18f

Browse files
authored
Plugin: stop treating approval cancel as auth failure (#6)
1 parent 2fb635e commit 695b18f

File tree

6 files changed

+438
-36
lines changed

6 files changed

+438
-36
lines changed

src/client.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,3 +458,90 @@ describe("createPendingInputCoordinator", () => {
458458
await expect(second.response).resolves.toEqual({ index: 0, option: "approve" });
459459
});
460460
});
461+
462+
describe("turn stop detection", () => {
463+
it("treats silent approval cancels as an approval stop reason", () => {
464+
expect(
465+
__testing.resolveTurnStoppedReason({
466+
interrupted: false,
467+
terminalStatus: "completed",
468+
approvalCancelled: true,
469+
assistantText: "",
470+
hasPlanArtifact: false,
471+
}),
472+
).toBe("approval");
473+
});
474+
475+
it("does not hide assistant output after an approval cancel", () => {
476+
expect(
477+
__testing.resolveTurnStoppedReason({
478+
interrupted: false,
479+
terminalStatus: "completed",
480+
approvalCancelled: true,
481+
assistantText: "Cancelled, but here is context.",
482+
hasPlanArtifact: false,
483+
}),
484+
).toBeUndefined();
485+
});
486+
487+
it("keeps explicit turn cancellations distinct from approval cancels", () => {
488+
expect(
489+
__testing.resolveTurnStoppedReason({
490+
interrupted: false,
491+
terminalStatus: "interrupted",
492+
approvalCancelled: false,
493+
assistantText: "",
494+
hasPlanArtifact: false,
495+
}),
496+
).toBe("cancelled");
497+
});
498+
});
499+
500+
describe("extractTurnTerminalState", () => {
501+
it("parses failed turn/completed notifications with unauthorized details", () => {
502+
expect(
503+
__testing.extractTurnTerminalState("turn/completed", {
504+
turn: {
505+
id: "turn-1",
506+
status: "failed",
507+
error: {
508+
message: "unauthorized",
509+
codexErrorInfo: "unauthorized",
510+
},
511+
},
512+
}),
513+
).toEqual({
514+
status: "failed",
515+
error: {
516+
message: "unauthorized",
517+
codexErrorInfo: "unauthorized",
518+
httpStatusCode: undefined,
519+
},
520+
});
521+
});
522+
523+
it("extracts upstream http status codes from nested codex error info", () => {
524+
expect(
525+
__testing.extractTurnTerminalState("turn/completed", {
526+
turn: {
527+
status: "failed",
528+
error: {
529+
message: "request failed",
530+
codexErrorInfo: {
531+
httpConnectionFailed: {
532+
httpStatusCode: 401,
533+
},
534+
},
535+
},
536+
},
537+
}),
538+
).toEqual({
539+
status: "failed",
540+
error: {
541+
message: "request failed",
542+
codexErrorInfo: "httpConnectionFailed:401",
543+
httpStatusCode: 401,
544+
},
545+
});
546+
});
547+
});

src/client.ts

Lines changed: 190 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {
2424
ThreadReplay,
2525
ThreadState,
2626
ThreadSummary,
27+
TurnTerminalError,
2728
TurnResult,
2829
} from "./types.js";
2930

@@ -163,6 +164,19 @@ function pickFiniteNumber(record: Record<string, unknown>, keys: string[]): numb
163164
return undefined;
164165
}
165166

167+
function parseFiniteNumber(value: unknown): number | undefined {
168+
if (typeof value === "number" && Number.isFinite(value)) {
169+
return value;
170+
}
171+
if (typeof value === "string") {
172+
const parsed = Number(value.trim());
173+
if (Number.isFinite(parsed)) {
174+
return parsed;
175+
}
176+
}
177+
return undefined;
178+
}
179+
166180
function pickBoolean(record: Record<string, unknown>, keys: string[]): boolean | undefined {
167181
for (const key of keys) {
168182
const value = record[key];
@@ -336,6 +350,38 @@ function findFirstNestedValue(value: unknown, keys: readonly string[], depth = 0
336350
return undefined;
337351
}
338352

353+
function findFirstNestedNumber(value: unknown, keys: readonly string[], depth = 0): number | undefined {
354+
if (depth > 6) {
355+
return undefined;
356+
}
357+
if (Array.isArray(value)) {
358+
for (const entry of value) {
359+
const match = findFirstNestedNumber(entry, keys, depth + 1);
360+
if (match !== undefined) {
361+
return match;
362+
}
363+
}
364+
return undefined;
365+
}
366+
const record = asRecord(value);
367+
if (!record) {
368+
return undefined;
369+
}
370+
for (const key of keys) {
371+
const parsed = parseFiniteNumber(record[key]);
372+
if (parsed !== undefined) {
373+
return parsed;
374+
}
375+
}
376+
for (const nested of Object.values(record)) {
377+
const match = findFirstNestedNumber(nested, keys, depth + 1);
378+
if (match !== undefined) {
379+
return match;
380+
}
381+
}
382+
return undefined;
383+
}
384+
339385
function collectStreamingText(value: unknown): string {
340386
if (typeof value === "string") {
341387
return value;
@@ -2022,6 +2068,98 @@ function extractContextCompactionProgress(
20222068
};
20232069
}
20242070

2071+
function normalizeTurnTerminalStatus(
2072+
value: string | undefined,
2073+
): TurnResult["terminalStatus"] | undefined {
2074+
const normalized = value?.trim().toLowerCase();
2075+
switch (normalized) {
2076+
case "completed":
2077+
return "completed";
2078+
case "interrupted":
2079+
case "cancelled":
2080+
case "canceled":
2081+
return "interrupted";
2082+
case "failed":
2083+
case "error":
2084+
return "failed";
2085+
default:
2086+
return undefined;
2087+
}
2088+
}
2089+
2090+
function summarizeCodexErrorInfo(value: unknown): string | undefined {
2091+
if (typeof value === "string") {
2092+
return value.trim() || undefined;
2093+
}
2094+
const record = asRecord(value);
2095+
if (!record) {
2096+
return undefined;
2097+
}
2098+
for (const [key, nested] of Object.entries(record)) {
2099+
const nestedRecord = asRecord(nested);
2100+
const httpStatusCode = nestedRecord
2101+
? pickFiniteNumber(nestedRecord, ["httpStatusCode", "http_status_code"])
2102+
: undefined;
2103+
if (httpStatusCode !== undefined) {
2104+
return `${key}:${httpStatusCode}`;
2105+
}
2106+
const nestedSummary = summarizeCodexErrorInfo(nested);
2107+
if (nestedSummary) {
2108+
return `${key}:${nestedSummary}`;
2109+
}
2110+
return key;
2111+
}
2112+
return undefined;
2113+
}
2114+
2115+
function extractTurnTerminalState(
2116+
method: string,
2117+
params: unknown,
2118+
): { status?: TurnResult["terminalStatus"]; error?: TurnTerminalError } | undefined {
2119+
const methodLower = method.trim().toLowerCase();
2120+
if (
2121+
methodLower !== "turn/completed" &&
2122+
methodLower !== "turn/failed" &&
2123+
methodLower !== "turn/cancelled"
2124+
) {
2125+
return undefined;
2126+
}
2127+
const record = asRecord(params) ?? {};
2128+
const turn = asRecord(record.turn) ?? record;
2129+
const errorRecord = asRecord(turn.error) ?? asRecord(record.error) ?? null;
2130+
const status =
2131+
normalizeTurnTerminalStatus(
2132+
pickString(turn, ["status"]) ??
2133+
(methodLower === "turn/failed"
2134+
? "failed"
2135+
: methodLower === "turn/cancelled"
2136+
? "interrupted"
2137+
: "completed"),
2138+
) ??
2139+
(methodLower === "turn/failed"
2140+
? "failed"
2141+
: methodLower === "turn/cancelled"
2142+
? "interrupted"
2143+
: undefined);
2144+
if (!errorRecord) {
2145+
return { status };
2146+
}
2147+
const codexErrorInfoValue =
2148+
errorRecord.codexErrorInfo ?? errorRecord.codex_error_info ?? errorRecord.type;
2149+
const error: TurnTerminalError = {
2150+
message:
2151+
pickString(errorRecord, ["message", "text", "summary", "reason"], { trim: true }) ??
2152+
undefined,
2153+
codexErrorInfo: summarizeCodexErrorInfo(codexErrorInfoValue),
2154+
httpStatusCode: findFirstNestedNumber(codexErrorInfoValue, ["httpStatusCode", "http_status_code"]),
2155+
};
2156+
return {
2157+
status,
2158+
error:
2159+
error.message || error.codexErrorInfo || error.httpStatusCode !== undefined ? error : undefined,
2160+
};
2161+
}
2162+
20252163
function mapPendingInputResponse(params: {
20262164
methodLower: string;
20272165
requestParams: unknown;
@@ -2061,6 +2199,30 @@ function mapPendingInputResponse(params: {
20612199
return response;
20622200
}
20632201

2202+
function extractApprovalDecision(value: unknown): string | undefined {
2203+
const record = asRecord(value);
2204+
return record ? pickString(record, ["decision"]) : undefined;
2205+
}
2206+
2207+
function resolveTurnStoppedReason(params: {
2208+
interrupted: boolean;
2209+
terminalStatus?: TurnResult["terminalStatus"];
2210+
approvalCancelled: boolean;
2211+
assistantText: string;
2212+
hasPlanArtifact: boolean;
2213+
}): TurnResult["stoppedReason"] | undefined {
2214+
if (params.interrupted) {
2215+
return "interrupt";
2216+
}
2217+
if (params.terminalStatus === "interrupted") {
2218+
return "cancelled";
2219+
}
2220+
if (params.approvalCancelled && !params.assistantText.trim() && !params.hasPlanArtifact) {
2221+
return "approval";
2222+
}
2223+
return undefined;
2224+
}
2225+
20642226
type PendingInputQueueEntry = {
20652227
state: PendingInputState;
20662228
options: string[];
@@ -2953,6 +3115,9 @@ export class CodexAppServerClient {
29533115
let interrupted = false;
29543116
let completed = false;
29553117
let latestContextUsage: ContextUsageSnapshot | undefined;
3118+
let terminalStatus: TurnResult["terminalStatus"] | undefined;
3119+
let terminalError: TurnTerminalError | undefined;
3120+
let approvalCancelled = false;
29563121
let notificationQueue = Promise.resolve();
29573122
const pendingInputCoordinator = createPendingInputCoordinator({
29583123
inputTimeoutMs: this.settings.inputTimeoutMs,
@@ -3069,6 +3234,9 @@ export class CodexAppServerClient {
30693234
methodLower === "turn/failed" ||
30703235
methodLower === "turn/cancelled"
30713236
) {
3237+
const terminalState = extractTurnTerminalState(method, notificationParams);
3238+
terminalStatus = terminalState?.status ?? terminalStatus;
3239+
terminalError = terminalState?.error ?? terminalError;
30723240
await fileEditNoticeBatcher.flush();
30733241
this.logger.debug(
30743242
`codex turn terminal notification run=${params.runId} thread=${threadId || "<pending>"} turn=${turnId || "<pending>"} method=${methodLower}`,
@@ -3136,6 +3304,13 @@ export class CodexAppServerClient {
31363304
actions: state.actions ?? [],
31373305
timedOut: pendingEntry.timedOut,
31383306
});
3307+
const approvalDecision = extractApprovalDecision(mappedResponse)?.toLowerCase();
3308+
if (approvalDecision === "cancel") {
3309+
approvalCancelled = true;
3310+
this.logger.debug(
3311+
`codex turn approval cancelled by user run=${params.runId} thread=${threadId || "<none>"} turn=${turnId || "<none>"} method=${methodLower}`,
3312+
);
3313+
}
31393314
const responseRecord = asRecord(response);
31403315
const steerText =
31413316
methodLower.includes("requestapproval") && typeof responseRecord?.steerText === "string"
@@ -3216,6 +3391,14 @@ export class CodexAppServerClient {
32163391
this.logger.debug(
32173392
`codex turn completion settled run=${params.runId} thread=${threadId || "<none>"} turn=${turnId || "<none>"} interrupted=${interrupted ? "yes" : "no"} assistantChars=${assistantText.length}`,
32183393
);
3394+
const stoppedReason = resolveTurnStoppedReason({
3395+
interrupted,
3396+
terminalStatus,
3397+
approvalCancelled,
3398+
assistantText,
3399+
hasPlanArtifact:
3400+
Boolean(finalPlanMarkdown) || planDraftByItemId.size > 0 || planSteps.length > 0,
3401+
});
32193402
return {
32203403
threadId,
32213404
text:
@@ -3229,7 +3412,10 @@ export class CodexAppServerClient {
32293412
markdown: finalPlanMarkdown,
32303413
}
32313414
: undefined,
3232-
aborted: interrupted,
3415+
aborted: stoppedReason === "interrupt" || stoppedReason === "cancelled",
3416+
stoppedReason,
3417+
terminalStatus,
3418+
terminalError,
32333419
usage: latestContextUsage,
32343420
} satisfies TurnResult;
32353421
} catch (error) {
@@ -3365,11 +3551,14 @@ export const __testing = {
33653551
buildTurnSteerPayloads,
33663552
createFileEditNoticeBatcher,
33673553
createPendingInputCoordinator,
3554+
extractApprovalDecision,
3555+
extractTurnTerminalState,
33683556
extractFileEditSummariesFromNotification,
33693557
extractFileChangePathsFromReadResult,
33703558
extractStartupProbeInfo,
33713559
formatFileEditNotice,
33723560
extractThreadTokenUsageSnapshot,
33733561
extractRateLimitSummaries,
33743562
formatStdioProcessLog,
3563+
resolveTurnStoppedReason,
33753564
};

0 commit comments

Comments
 (0)