Skip to content

Commit 3ac2806

Browse files
committed
Capture approval generation at overlay start
/clear during the permission overlay bumped generation after handle started, but the TUI recaptured at deliver time so an accept still injected the approved decision into the empty session.
1 parent f6f6807 commit 3ac2806

4 files changed

Lines changed: 137 additions & 11 deletions

File tree

src/session/approval-resume.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ export function createApprovalResume(args: {
114114
getAgent: () => Pick<Agent, "deliver" | "history"> | undefined;
115115
// TUI session queue. When present, each decision is awaited through this
116116
// seam; exec omits it and uses getAgent().deliver.
117-
deliver?: (message: InboundMessage) => void | Promise<void>;
117+
deliver?: (message: InboundMessage, stillCurrent: () => boolean) => void | Promise<void>;
118+
// TUI: capture at handle() start so /clear during the overlay drops the
119+
// decision instead of delivering into the new session. Exec omits this.
120+
captureGeneration?: () => () => boolean;
118121
gate: PermissionGate;
119122
}): ApprovalResume {
120123
const { getAgent, gate } = args;
@@ -127,19 +130,21 @@ export function createApprovalResume(args: {
127130
return agent;
128131
};
129132

130-
const deliverDecision = async (message: InboundMessage): Promise<void> => {
131-
if (args.deliver !== undefined) {
132-
await args.deliver(message);
133-
return;
134-
}
135-
requireAgent().deliver(message);
136-
};
137-
138133
return {
139134
handle: async (result) => {
140135
if (result.type !== "suspended") return false;
136+
const stillCurrent = args.captureGeneration?.() ?? (() => true);
141137
const { correlationId, approvalSnapshot } = result;
142138

139+
const deliverDecision = async (message: InboundMessage): Promise<void> => {
140+
if (!stillCurrent()) return;
141+
if (args.deliver !== undefined) {
142+
await args.deliver(message, stillCurrent);
143+
return;
144+
}
145+
requireAgent().deliver(message);
146+
};
147+
143148
// Turn-count watermark for the settled guard below: a "approval timed
144149
// out" tool result appended after this point means the reactor settled
145150
// this very correlation before our decision lands.
@@ -164,6 +169,7 @@ export function createApprovalResume(args: {
164169
}
165170

166171
const outcome = await gate.resolveSuspended(request);
172+
if (!stillCurrent()) return true;
167173
if (settledAfterSuspend(await requireAgent().history(), turnsAtSuspend)) {
168174
// The reactor already answered the parked call (its approval timeout
169175
// fired while the surface was still up). Delivering now would append

src/tui/runner/session.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,8 +375,8 @@ export async function assembleTUISession(
375375
const deliveryGeneration = createDeliveryGeneration();
376376
const approvalResume = createApprovalResume({
377377
getAgent: () => state.agentProxy ?? state.currentAgent,
378-
deliver: (message) => {
379-
const stillCurrent = deliveryGeneration.capture();
378+
captureGeneration: deliveryGeneration.capture,
379+
deliver: (message, stillCurrent) => {
380380
return sessionOps.enqueue(async () => {
381381
if (!stillCurrent()) return;
382382
if (state.fatalBuildError !== null) throw state.fatalBuildError;

tests/unit/approval-resume.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { ConversationTurn, InboundMessage } from "@intx/types/runtime";
55

66
import type { PermissionGate } from "../../src/permission/gate.js";
77
import { createApprovalResume } from "../../src/session/approval-resume.js";
8+
import { createDeliveryGeneration } from "../../src/tui/queued-delivery.js";
89

910
const SUSPENDED: SendResult = {
1011
type: "suspended",
@@ -184,3 +185,55 @@ describe("approval resume late-bind", () => {
184185
expect(correlationHeaders(delivered[0]).interchangeCorrelationId).toBe("corr-1");
185186
});
186187
});
188+
189+
describe("approval resume generation capture", () => {
190+
test("overlay accept after a generation bump does not deliver", async () => {
191+
const generation = createDeliveryGeneration();
192+
const delivered: unknown[] = [];
193+
const agent = {
194+
deliver: (message: unknown) => delivered.push(message),
195+
history: async () => [userTurn()],
196+
};
197+
const resume = createApprovalResume({
198+
getAgent: () => agent,
199+
captureGeneration: generation.capture,
200+
deliver: (message, stillCurrent) => {
201+
if (!stillCurrent()) return;
202+
delivered.push(message);
203+
},
204+
gate: {
205+
resolveSuspended: async () => {
206+
generation.bump();
207+
return { allow: true };
208+
},
209+
} as unknown as PermissionGate,
210+
});
211+
212+
expect(await resume.handle(SUSPENDED)).toBe(true);
213+
expect(delivered).toEqual([]);
214+
});
215+
216+
test("a live generation still delivers after the overlay", async () => {
217+
const generation = createDeliveryGeneration();
218+
const delivered: unknown[] = [];
219+
const agent = {
220+
deliver: (message: unknown) => delivered.push(message),
221+
history: async () => [userTurn()],
222+
};
223+
const resume = createApprovalResume({
224+
getAgent: () => agent,
225+
captureGeneration: generation.capture,
226+
deliver: (message, stillCurrent) => {
227+
if (!stillCurrent()) return;
228+
delivered.push(message);
229+
},
230+
gate: { resolveSuspended: async () => ({ allow: true }) } as unknown as PermissionGate,
231+
});
232+
233+
expect(await resume.handle(SUSPENDED)).toBe(true);
234+
expect(delivered).toHaveLength(1);
235+
expect(JSON.parse((delivered[0] as { content: string }).content)).toEqual({
236+
outcome: "approved",
237+
});
238+
});
239+
});

tests/unit/tui/approval-reload-during-suspend.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { describe, expect, test } from "bun:test";
22

3+
import type { SendResult } from "@intx/agent";
4+
import type { ConversationTurn } from "@intx/types/runtime";
5+
6+
import type { PermissionGate } from "../../../src/permission/gate.js";
7+
import { createApprovalResume } from "../../../src/session/approval-resume.js";
8+
import { createSessionOperationQueue } from "../../../src/tui/session-operation-queue.js";
39
import { runWhileAgentBusy, type RunnerState } from "../../../src/tui/runner/state.js";
410

511
function stubBusyState() {
@@ -63,3 +69,64 @@ describe("runWhileAgentBusy vs pendingReload", () => {
6369
expect(state.pendingReload).toBe(true);
6470
});
6571
});
72+
73+
const SUSPENDED: SendResult = {
74+
type: "suspended",
75+
correlationId: "corr-1",
76+
approvalSnapshot: { name: "run_shell", arguments: { command: "curl -sS https://example.com" } },
77+
} as unknown as SendResult;
78+
79+
function userTurn(): ConversationTurn {
80+
return {
81+
role: "user",
82+
content: [{ type: "text", text: "go" }],
83+
timestamp: 0,
84+
} as unknown as ConversationTurn;
85+
}
86+
87+
describe("pendingReload during resolveSuspended vs deliver enqueue", () => {
88+
test("does not rebuild until handle returns and deliver has run", async () => {
89+
const events: string[] = [];
90+
const { enqueue, awaitTail } = createSessionOperationQueue();
91+
const state: Pick<RunnerState, "inFlight" | "reloadIfIdle"> & { pendingReload: boolean } = {
92+
inFlight: 0,
93+
pendingReload: false,
94+
reloadIfIdle: () => {
95+
if (!state.pendingReload || state.inFlight > 0) return;
96+
state.pendingReload = false;
97+
void enqueue(async () => {
98+
events.push("rebuild");
99+
});
100+
},
101+
};
102+
const agent = {
103+
deliver: (_message: unknown) => {
104+
events.push("deliver");
105+
},
106+
history: async () => [userTurn()],
107+
};
108+
const resume = createApprovalResume({
109+
getAgent: () => agent,
110+
deliver: (message) =>
111+
enqueue(async () => {
112+
agent.deliver(message);
113+
}),
114+
gate: {
115+
resolveSuspended: async () => {
116+
state.pendingReload = true;
117+
state.reloadIfIdle?.();
118+
expect(events).toEqual([]);
119+
return { allow: true };
120+
},
121+
} as unknown as PermissionGate,
122+
});
123+
124+
await runWhileAgentBusy(state, async () => {
125+
await resume.handle(SUSPENDED);
126+
});
127+
await awaitTail();
128+
129+
expect(events).toEqual(["deliver", "rebuild"]);
130+
expect(state.inFlight).toBe(0);
131+
});
132+
});

0 commit comments

Comments
 (0)