Skip to content

Commit efe2bb3

Browse files
committed
fix(approval): recover verified suspended calls from watchdog
1 parent 4f3aaa6 commit efe2bb3

6 files changed

Lines changed: 209 additions & 10 deletions

File tree

‎src/session/approval-resume.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
createApprovalResume,
2020
requestFromApprovalSnapshot,
2121
resolveParkedCallIdFromStore,
22+
resolveSuspendedApprovalFromStore,
2223
} from "./approval-resume.js";
2324
import { createSessionOperationQueue } from "../tui/delivery-queue.js";
2425
import {
@@ -419,6 +420,12 @@ const operation: PendingOperation = {
419420
name: "run_shell",
420421
arguments: { command: "echo same" },
421422
},
423+
approvalSnapshot: {
424+
name: "run_shell",
425+
description: "run shell",
426+
inputSchema: {},
427+
arguments: { command: "echo same" },
428+
},
422429
};
423430

424431
describe("persisted approval identity", () => {
@@ -451,6 +458,37 @@ describe("persisted approval identity", () => {
451458
});
452459
});
453460

461+
describe("persisted suspended approval proof", () => {
462+
test("requires an exact persisted snapshot and parked call", async () => {
463+
const result = suspension("corr-A", "echo same");
464+
expect(
465+
await resolveSuspendedApprovalFromStore(storeWith([operation]), result),
466+
).toEqual({
467+
ok: true,
468+
parkedCallId: "call-A",
469+
});
470+
471+
const mismatch: PendingOperation = {
472+
...operation,
473+
approvalSnapshot: {
474+
name: "run_shell",
475+
description: "run shell",
476+
inputSchema: {},
477+
arguments: { command: "secret" },
478+
},
479+
};
480+
expect(
481+
await resolveSuspendedApprovalFromStore(storeWith([mismatch]), result),
482+
).toEqual({ ok: false, code: "snapshot-mismatch" });
483+
expect(
484+
await resolveSuspendedApprovalFromStore(
485+
storeWith([operation, operation]),
486+
result,
487+
),
488+
).toEqual({ ok: false, code: "ambiguous-pending" });
489+
});
490+
});
491+
454492
test("missing or ambiguous stored identity never falls back to an identical history call", async () => {
455493
for (const pending of [[], [operation, operation]]) {
456494
const deliver = mock((_message: InboundMessage): void => undefined);

‎src/session/approval-resume.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
ContextStore,
1818
InboundMessage,
1919
} from "@intx/types/runtime";
20+
import { isDeepStrictEqual } from "node:util";
2021
import { type } from "arktype";
2122

2223
import { getLogger } from "@intx/log";
@@ -54,6 +55,60 @@ export interface ApprovalResume {
5455
handle: (result: SendResult) => Promise<boolean>;
5556
}
5657

58+
export interface SuspendedApprovalRecovery {
59+
capture: (
60+
result: Extract<SendResult, { type: "suspended" }>,
61+
stillCurrent: () => boolean,
62+
) => void;
63+
clear: () => void;
64+
tryResumeOnce: () => Promise<boolean>;
65+
}
66+
67+
export function createSuspendedApprovalRecovery(args: {
68+
storage: () => Pick<ContextStore, "load"> | undefined;
69+
resume: ApprovalResume;
70+
}): SuspendedApprovalRecovery {
71+
let candidate:
72+
| {
73+
readonly result: Extract<SendResult, { type: "suspended" }>;
74+
readonly stillCurrent: () => boolean;
75+
attempted: boolean;
76+
}
77+
| undefined;
78+
return {
79+
capture: (result, stillCurrent) => {
80+
candidate = { result, stillCurrent, attempted: false };
81+
},
82+
clear: () => {
83+
candidate = undefined;
84+
},
85+
tryResumeOnce: async () => {
86+
const current = candidate;
87+
if (
88+
current === undefined ||
89+
current.attempted ||
90+
!current.stillCurrent()
91+
) {
92+
candidate = undefined;
93+
return false;
94+
}
95+
current.attempted = true;
96+
const storage = args.storage();
97+
if (storage === undefined) return false;
98+
try {
99+
const verified = await resolveSuspendedApprovalFromStore(
100+
storage,
101+
current.result,
102+
);
103+
if (!verified.ok || !current.stillCurrent()) return false;
104+
return await args.resume.handle(current.result);
105+
} catch {
106+
return false;
107+
}
108+
},
109+
};
110+
}
111+
57112
// Rebuild the operator-facing request from the persisted snapshot. The parked
58113
// name is canonicalized first so resume matches live decide() (file scopes for
59114
// write_file, not a generic tool grant for default.write_file). Scopes come
@@ -103,6 +158,62 @@ export async function resolveParkedCallIdFromStore(
103158
return matches.length === 1 ? matches[0]?.suspendedCall?.id : undefined;
104159
}
105160

161+
export type SuspendedApprovalResolution =
162+
| { readonly ok: true; readonly parkedCallId: string }
163+
| {
164+
readonly ok: false;
165+
readonly code:
166+
| "not-pending"
167+
| "ambiguous-pending"
168+
| "snapshot-invalid"
169+
| "snapshot-mismatch"
170+
| "settled";
171+
};
172+
173+
/**
174+
* Proves that the suspended result still identifies the one persisted call.
175+
* Failure codes are intentionally content-free so watchdog diagnostics cannot
176+
* disclose approval or tool payloads.
177+
*/
178+
export async function resolveSuspendedApprovalFromStore(
179+
storage: Pick<ContextStore, "load">,
180+
result: Extract<SendResult, { type: "suspended" }>,
181+
): Promise<SuspendedApprovalResolution> {
182+
const { pendingOperations } = await storage.load();
183+
const matches = pendingOperations.filter(
184+
(operation) =>
185+
operation.kind === "approval" &&
186+
operation.correlationId === result.correlationId,
187+
);
188+
if (matches.length === 0) return { ok: false, code: "not-pending" };
189+
if (matches.length !== 1) return { ok: false, code: "ambiguous-pending" };
190+
const operation = matches[0];
191+
if (operation === undefined) return { ok: false, code: "not-pending" };
192+
if (operation.timeoutAt !== undefined && operation.timeoutAt <= Date.now())
193+
return { ok: false, code: "settled" };
194+
const parked = operation.suspendedCall;
195+
const persisted = operation.approvalSnapshot;
196+
const captured = result.approvalSnapshot;
197+
const parsedPersisted = persisted && ApprovalSnapshotShape(persisted);
198+
const parsedCaptured = ApprovalSnapshotShape(captured);
199+
if (
200+
parked === undefined ||
201+
parked.id.length === 0 ||
202+
!parsedPersisted ||
203+
parsedPersisted instanceof type.errors ||
204+
parsedCaptured instanceof type.errors
205+
)
206+
return { ok: false, code: "snapshot-invalid" };
207+
if (
208+
parked.name !== parsedPersisted.name ||
209+
parked.name !== parsedCaptured.name ||
210+
!isDeepStrictEqual(parked.arguments, parsedPersisted.arguments ?? {}) ||
211+
!isDeepStrictEqual(parked.arguments, parsedCaptured.arguments ?? {})
212+
)
213+
return { ok: false, code: "snapshot-mismatch" };
214+
return { ok: true, parkedCallId: parked.id };
215+
}
216+
106217
function timeoutResult(
107218
turns: Awaited<ReturnType<Agent["history"]>>,
108219
parkedCallId: string,

‎src/tui/runner/session.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ import type { CompactionArchive } from "../../session/compaction-archive.js";
6161
import { tryReadPriorHandoffFile } from "../../session/compaction-handoff.js";
6262
import {
6363
createApprovalResume,
64+
createSuspendedApprovalRecovery,
6465
resolveParkedCallIdFromStore,
66+
type SuspendedApprovalRecovery,
6567
} from "../../session/approval-resume.js";
6668
import { createReactorAuthorize } from "../../permission/reactor-authorize.js";
6769
import {
@@ -185,7 +187,9 @@ export async function assembleTUISession(
185187
const correlationAcceptance = createCorrelationAcceptance();
186188
const parkedApprovalCancel = { fn: undefined as (() => void) | undefined };
187189
const parkedOverlay = createParkedOverlayAbortBinding();
190+
let suspendedApprovalRecovery: SuspendedApprovalRecovery | undefined;
188191
const deliveryGeneration = createDeliveryGeneration(() => {
192+
suspendedApprovalRecovery?.clear();
189193
parkedApprovalCancel.fn?.();
190194
correlationAcceptance.settleAll();
191195
});
@@ -550,6 +554,10 @@ export async function assembleTUISession(
550554
cwd: config.cwd,
551555
extraDeniedPaths: [config.globalSettingsPath],
552556
});
557+
suspendedApprovalRecovery = createSuspendedApprovalRecovery({
558+
storage: () => state.currentStorage ?? undefined,
559+
resume: approvalResume,
560+
});
553561
state.enqueueAgentDeliver = (
554562
deliverToLiveAgent: () => void,
555563
onSettle?: (result: AgentDeliveryResult) => void,
@@ -788,6 +796,7 @@ export async function assembleTUISession(
788796
inferenceDeps: start.inferenceDeps,
789797
permissionGate,
790798
approvalResume,
799+
suspendedApprovalRecovery,
791800
permissionsAdmin,
792801
liveSubAgent,
793802
subAgentSessions,

‎src/tui/runner/state.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ export interface RunnerServices {
118118
approvalResume: ReturnType<
119119
typeof import("../../session/approval-resume.js").createApprovalResume
120120
>;
121+
suspendedApprovalRecovery: ReturnType<
122+
typeof import("../../session/approval-resume.js").createSuspendedApprovalRecovery
123+
>;
121124
permissionsAdmin: ReturnType<
122125
typeof import("../../permission/admin.js").createPermissionsAdmin
123126
>;

‎src/tui/runner/submit.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,9 @@ export function createSubmitPath(
366366
live.agentProxy.send(next),
367367
presentFailureNotice = true,
368368
): Promise<AgentDeliveryResult> => {
369+
hostOf(state).bridge.setSuspendedApprovalRecovery(
370+
services.suspendedApprovalRecovery.tryResumeOnce,
371+
);
369372
const attempt = live.attemptIdentity();
370373
const providerFailure = services.providerFailureAttempts.begin(attempt);
371374
const recoveryAttempt = state.credentialRecovery.begin(
@@ -385,6 +388,12 @@ export function createSubmitPath(
385388
// send early; resolve the operator surface here and deliver the
386389
// decision on the correlationId signal channel so the parked run
387390
// resumes.
391+
if (result.type === "suspended") {
392+
services.suspendedApprovalRecovery.capture(
393+
result,
394+
services.deliveryGeneration.capture(),
395+
);
396+
}
388397
await services.approvalResume.handle(result);
389398
});
390399
return { status: "accepted" };

‎src/tui/runtime-bridge.ts‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,12 @@ export interface TurnMonitorOptions {
178178
readonly stallNoticeMs?: number;
179179
/** Registers the periodic tick; returns an unsubscribe. */
180180
readonly schedule?: (tick: () => void, intervalMs: number) => () => void;
181+
/**
182+
* Attempts to re-present one already persisted approval suspension. This
183+
* deliberately receives no agent, storage, grant, or tool capability: a
184+
* true result means the correlated approval path owns the continuation.
185+
*/
186+
readonly tryResumeSuspendedApproval?: () => boolean | Promise<boolean>;
181187
}
182188

183189
const DEFAULT_TICK_MS = 250;
@@ -235,6 +241,9 @@ export interface SessionBridge {
235241
gateOpened: () => void;
236242
/** A previously raised gate resolved. */
237243
gateClosed: () => void;
244+
setSuspendedApprovalRecovery: (
245+
recovery: (() => boolean | Promise<boolean>) | undefined,
246+
) => void;
238247
dispose: () => void;
239248
/** Current derived turn phase (progress label, stall clock, quota window). */
240249
readonly turn: TurnState;
@@ -510,6 +519,7 @@ export interface BridgeBag {
510519
/** callId→name / delta bookkeeping for production-shaped events. */
511520
mapCtx: StreamMapContext;
512521
disposed: boolean;
522+
suspendedApprovalRecovery: (() => boolean | Promise<boolean>) | undefined;
513523
turn: TurnState;
514524
/**
515525
* Live fleet-lane count from the last `fleet` event (idle-with-fleet).
@@ -1581,6 +1591,7 @@ export function attachSessionBridge(
15811591
pendingPromptRecoveries: [],
15821592
mapCtx: createStreamMapContext(),
15831593
disposed: false,
1594+
suspendedApprovalRecovery: undefined,
15841595
turn: initialTurnState(now()),
15851596
liveFleet: 0,
15861597
pendingAskWake: new Map(),
@@ -2201,17 +2212,32 @@ export function attachSessionBridge(
22012212
const stallArgs = stallArgsFor(nowMs);
22022213

22032214
if (shouldAbortForStall(stallArgs)) {
2204-
applyStallRecovery(
2205-
{
2206-
abort: () => {
2207-
if (abortStalledWakeTurn()) return;
2208-
doInterrupt();
2215+
const recover = (): void =>
2216+
applyStallRecovery(
2217+
{
2218+
abort: () => {
2219+
if (abortStalledWakeTurn()) return;
2220+
doInterrupt();
2221+
},
2222+
notify: (message) =>
2223+
setStatusFlash(shell, message, { ttlMs: RUNTIME_FLASH_MS }),
22092224
},
2210-
notify: (message) =>
2211-
setStatusFlash(shell, message, { ttlMs: RUNTIME_FLASH_MS }),
2212-
},
2213-
STALL_RECOVERY_MESSAGE,
2214-
);
2225+
STALL_RECOVERY_MESSAGE,
2226+
);
2227+
const resumed =
2228+
bag.suspendedApprovalRecovery?.() ??
2229+
monitor?.tryResumeSuspendedApproval?.();
2230+
if (resumed instanceof Promise) {
2231+
void resumed.then(
2232+
(handled) => {
2233+
if (!handled) recover();
2234+
},
2235+
() => recover(),
2236+
);
2237+
return;
2238+
}
2239+
if (resumed) return;
2240+
recover();
22152241
return;
22162242
}
22172243

@@ -2258,6 +2284,9 @@ export function attachSessionBridge(
22582284
submit,
22592285
interrupt: doInterrupt,
22602286
clearQueuedDelivery,
2287+
setSuspendedApprovalRecovery: (recovery) => {
2288+
bag.suspendedApprovalRecovery = recovery;
2289+
},
22612290
get parentCycleLive() {
22622291
return bag.liveSteerInject;
22632292
},

0 commit comments

Comments
 (0)