Skip to content

Commit 014c058

Browse files
committed
test(tui): cover a lost decision's rejection and the queued run state
1 parent 93e7ca0 commit 014c058

2 files changed

Lines changed: 63 additions & 14 deletions

File tree

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

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
InboundMessage,
1313
} from "@intx/types/runtime";
1414

15+
import { APPROVAL_TIMEOUT_RESULT_TEXT } from "../permission/decline-markers.js";
1516
import type { PermissionGate } from "../permission/gate.js";
1617
import { createExtraDeniedPathMatcher } from "../plugins/secret-guard-plugin.js";
1718
import {
@@ -1193,10 +1194,9 @@ describe("approval resume retry re-await", () => {
11931194
expect(delivered).toHaveLength(1);
11941195
});
11951196

1196-
// Concurrent duplicates share one in-flight rejection (one gate, zero
1197-
// deliveries), then a retry re-gates and delivers exactly once, and later
1198-
// retries memoize.
1199-
test("a shared rejection re-gates once, delivers once, then memoizes", async () => {
1197+
// Concurrent duplicates share one in-flight gate failure (one gate, one
1198+
// rejection delivered), and later retries memoize.
1199+
test("a shared gate failure rejects the parked call once, then memoizes", async () => {
12001200
const gate = Promise.withResolvers<{ allow: boolean }>();
12011201
const { resume, delivered, resolveSuspended } = retryHarness({
12021202
allow: true,
@@ -1211,17 +1211,54 @@ describe("approval resume retry re-await", () => {
12111211
await expect(first).rejects.toBe(failure);
12121212
await expect(second).rejects.toBe(failure);
12131213
expect(resolveSuspended).toHaveBeenCalledTimes(1);
1214-
expect(delivered).toHaveLength(0);
1215-
1216-
expect(await resume.handle(suspension("corr-A", "echo alpha"))).toBe(true);
1217-
expect(resolveSuspended).toHaveBeenCalledTimes(2);
1214+
// Nothing retries a parked call, so the failure answers it.
12181215
expect(delivered).toHaveLength(1);
1219-
expect(decisionBody(onlyDecision(delivered)).outcome).toBe("approved");
1216+
expect(decisionBody(onlyDecision(delivered)).outcome).toBe("rejected");
12201217

12211218
expect(await resume.handle(suspension("corr-A", "echo alpha"))).toBe(true);
1222-
expect(resolveSuspended).toHaveBeenCalledTimes(2);
1219+
expect(resolveSuspended).toHaveBeenCalledTimes(1);
12231220
expect(delivered).toHaveLength(1);
12241221
});
1222+
test("a gate failure after the reactor timed the call out delivers nothing", async () => {
1223+
const delivered: InboundMessage[] = [];
1224+
let timedOut = false;
1225+
const failure = new Error("gate exploded");
1226+
const resume = createApprovalResume({
1227+
getAgent: () => ({
1228+
deliver: (message: InboundMessage) => {
1229+
delivered.push(message);
1230+
},
1231+
history: async () =>
1232+
timedOut
1233+
? ([
1234+
{
1235+
role: "user",
1236+
content: [
1237+
{
1238+
type: "tool_result",
1239+
callId: "call-A",
1240+
content: [
1241+
{ type: "text", text: APPROVAL_TIMEOUT_RESULT_TEXT },
1242+
],
1243+
},
1244+
],
1245+
},
1246+
] as unknown as Awaited<ReturnType<Agent["history"]>>)
1247+
: [],
1248+
}),
1249+
resolveParkedCallId: () => "call-A",
1250+
gate: {
1251+
resolveSuspended: async () => {
1252+
timedOut = true;
1253+
throw failure;
1254+
},
1255+
} as unknown as PermissionGate,
1256+
});
1257+
await expect(
1258+
resume.handle(suspension("corr-A", "echo alpha")),
1259+
).rejects.toBe(failure);
1260+
expect(delivered).toEqual([]);
1261+
});
12251262
});
12261263

12271264
describe("parked overlay registration after the operator answers", () => {

‎src/tui/runner/queued-approval-delivery.test.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import type { PermissionGate } from "../../permission/gate.js";
3232
import { ASK_DIRECTOR_WAKE_PREFIX } from "../../subagent/fleet-report.js";
3333
import { withTestRenderer } from "../harness.js";
34+
import { setShellRunState } from "../shell/chrome.js";
3435
import { createAppShell } from "../shell/index.js";
3536
import { shellInternals, type AppShell } from "../shell/internals.js";
3637
import { createDeliverRouting, createSubmitPath } from "./submit.js";
@@ -244,16 +245,23 @@ const drainWake = (h: ReturnType<typeof harness>): (() => unknown) => {
244245

245246
// A decision that fails after the drained send returned cannot change the
246247
// drain's result: the wake already reached the agent, and restoring it would
247-
// resend it. It must be surfaced the way a failed send is instead.
248-
for (const [name, opts] of [
248+
// resend it. The failure is surfaced as a notice, the parked call is answered
249+
// rather than left to the reactor's timeout, and the shell keeps its run state
250+
// so follow-ups queued behind this turn still drain when it ends.
251+
for (const [name, opts, decisions] of [
249252
[
250253
"the approval surface throws",
251254
{
252255
resolveSuspended: () =>
253256
Promise.reject(new Error("approval overlay failed")),
254257
},
258+
["approval.denied"],
259+
],
260+
[
261+
"the reactor never accepts the decision",
262+
{ acceptsDecisions: false },
263+
["approval.granted"],
255264
],
256-
["the reactor never accepts the decision", { acceptsDecisions: false }],
257265
] as const) {
258266
test(`wake-drained turn: surfaces a lost decision when ${name}`, async () => {
259267
await withTestRenderer(async (r) => {
@@ -265,12 +273,16 @@ for (const [name, opts] of [
265273
});
266274
try {
267275
const h = harness({ ...opts, shell });
276+
setShellRunState(shell, "busy");
268277
const settled = drainWake(h);
269278
await new Promise((resolve) => setTimeout(resolve, 200));
270279
expect(h.gateAsked()).toBe(1);
271280
expect(settled()).toEqual({ status: "accepted" });
272-
expect(h.state.runError).toBeDefined();
281+
expect(h.delivered.map((m) => m.headers.interchangeType)).toEqual([
282+
...decisions,
283+
]);
273284
expect(systemRows(shell).length).toBe(1);
285+
expect(shell.session.run).toBe("busy");
274286
expect(await settleWithin(h.sessionOps.awaitTail(), 200)).toBe(
275287
"settled",
276288
);

0 commit comments

Comments
 (0)