Archived
fix(sessions): settle run-scoped extension dialogs at abort-request time
A user abort while a tool_call dialog is parked deadlocked the dialog until its timeout: pi's agent loop waits for the parked dialog handler before emitting agent_end, and run-scoped dialogs were settled only on agent_end. Settle them synchronously at abort-request time, before awaiting the runtime abort, so a hung or failing abort cannot strand the parked waiter. Keep the agent_end settlement as the run-crash backstop; the store makes the double settlement a stale no-op.
This commit is contained in:
@@ -403,6 +403,105 @@ describe("PiSessionService extension dialog run end and teardown", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("PiSessionService extension dialog abort request", () => {
|
||||
it("settles a parked run-scoped dialog as aborted when an abort is requested", async () => {
|
||||
const { service, store, events, fake } = dialogService();
|
||||
const ui = await boundUiContext(service, fake);
|
||||
fake.session.isStreaming = true;
|
||||
const consent = ui.confirm("Run consent", "Allow this tool call?");
|
||||
|
||||
await service.abort(sessionRef(ACTIVE_SESSION_ID));
|
||||
|
||||
await expect(consent).resolves.toBe(false);
|
||||
expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toEqual([]);
|
||||
expect(dialogEvents(events).map(({ event }) => event)).toEqual([
|
||||
{ type: "dialog.opened", dialog: openDialog(events) },
|
||||
{ type: "dialog.closed", dialogId: "dialog-1", reason: "aborted" },
|
||||
]);
|
||||
const statuses = events.sessionEvents.flatMap(({ event }) => (event.type === "status.update" ? [event.status] : []));
|
||||
expect(statuses.at(-1)?.pendingDialogs).toBeUndefined();
|
||||
expect(fake.calls.abort).toBe(1);
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("settles the dialog before the runtime abort completes, so a parked handler cannot deadlock it", async () => {
|
||||
const { service, store, fake } = dialogService();
|
||||
const ui = await boundUiContext(service, fake);
|
||||
fake.session.isStreaming = true;
|
||||
// Model pi's agent loop parked behind the dialog handler: the runtime
|
||||
// abort can only finish once the handler (and so the dialog) has ended.
|
||||
const healthyAbort: typeof fake.session.abort = () => Promise.resolve();
|
||||
let releaseAbort: (() => void) | undefined;
|
||||
fake.session.abort = () =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseAbort = resolve;
|
||||
});
|
||||
const consent = ui.confirm("Run consent", "Allow this tool call?");
|
||||
|
||||
const aborting = service.abort(sessionRef(ACTIVE_SESSION_ID));
|
||||
|
||||
await expect(consent).resolves.toBe(false);
|
||||
expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toEqual([]);
|
||||
if (releaseAbort === undefined) throw new Error("runtime abort was not requested");
|
||||
releaseAbort();
|
||||
await aborting;
|
||||
fake.session.abort = healthyAbort;
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("settles the dialog even when the runtime abort itself fails", async () => {
|
||||
const { service, store, events, fake } = dialogService();
|
||||
const ui = await boundUiContext(service, fake);
|
||||
fake.session.isStreaming = true;
|
||||
const healthyAbort: typeof fake.session.abort = () => Promise.resolve();
|
||||
fake.session.abort = () => Promise.reject(new Error("abort blew up"));
|
||||
const consent = ui.confirm("Run consent", "Allow this tool call?");
|
||||
|
||||
await expect(service.abort(sessionRef(ACTIVE_SESSION_ID))).rejects.toThrow("abort blew up");
|
||||
|
||||
await expect(consent).resolves.toBe(false);
|
||||
expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toEqual([]);
|
||||
expect(dialogEvents(events).map(({ event }) => event)).toEqual([
|
||||
{ type: "dialog.opened", dialog: openDialog(events) },
|
||||
{ type: "dialog.closed", dialogId: "dialog-1", reason: "aborted" },
|
||||
]);
|
||||
fake.session.abort = healthyAbort;
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("leaves idle-opened dialogs parked across an abort request", async () => {
|
||||
const { service, store, events, fake } = dialogService();
|
||||
const ui = await boundUiContext(service, fake);
|
||||
const idle = ui.input("Session note?");
|
||||
|
||||
await service.abort(sessionRef(ACTIVE_SESSION_ID));
|
||||
|
||||
await expect(settledValue(idle)).resolves.toEqual({ settled: false });
|
||||
expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toEqual([expect.objectContaining({ dialogId: "dialog-1" })]);
|
||||
expect(dialogEvents(events).map(({ event }) => event)).toEqual([
|
||||
{ type: "dialog.opened", dialog: openDialog(events) },
|
||||
]);
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("does not close the dialog a second time when agent_end arrives after the abort", async () => {
|
||||
const { service, events, fake } = dialogService();
|
||||
const ui = await boundUiContext(service, fake);
|
||||
fake.session.isStreaming = true;
|
||||
const consent = ui.confirm("Run consent", "Allow this tool call?");
|
||||
|
||||
await service.abort(sessionRef(ACTIVE_SESSION_ID));
|
||||
fake.emit({ type: "agent_end" });
|
||||
|
||||
await expect(consent).resolves.toBe(false);
|
||||
expect(dialogEvents(events).map(({ event }) => event)).toEqual([
|
||||
{ type: "dialog.opened", dialog: openDialog(events) },
|
||||
{ type: "dialog.closed", dialogId: "dialog-1", reason: "aborted" },
|
||||
]);
|
||||
await service.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
describe("PiSessionService extension dialog status projection", () => {
|
||||
it("reports open dialogs oldest first so a reloading browser rehydrates them", async () => {
|
||||
const { service, fake } = dialogService();
|
||||
|
||||
@@ -1379,10 +1379,13 @@ export class PiSessionService implements SessionRouteService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Settle the session's run-scoped dialogs as `"aborted"` when its run ends.
|
||||
* Covers user-abort mid-dialog and run crashes; idle-opened dialogs (a
|
||||
* `session_start` probe, say) are not run-scoped and survive, because their
|
||||
* waiter is still alive after `agent_end`.
|
||||
* Settle the session's run-scoped dialogs as `"aborted"`. Runs at
|
||||
* abort-request time (a user abort parks the agent loop behind the dialog
|
||||
* handler, so `agent_end` would never arrive on its own) and again from
|
||||
* the `agent_end` observer as the run-crash backstop — the store makes the
|
||||
* second settlement a stale no-op. Idle-opened dialogs (a `session_start`
|
||||
* probe, say) are not run-scoped and survive, because their waiter
|
||||
* outlives the run.
|
||||
*/
|
||||
private abortRunScopedExtensionDialogs(sessionId: string): void {
|
||||
let closedAny = false;
|
||||
@@ -2361,6 +2364,12 @@ export class PiSessionService implements SessionRouteService {
|
||||
const sessionId = active.runtime.session.sessionId;
|
||||
this.clearCompactionPromptQueue(sessionId);
|
||||
clearSessionQueue(active.runtime.session);
|
||||
// Settle run-scoped dialogs now, at abort-request time: pi's agent loop
|
||||
// waits for a parked `tool_call` dialog handler before it can emit
|
||||
// `agent_end`, so leaving settlement to the `agent_end` observer would
|
||||
// strand the dialog until its timeout. Settling before the runtime abort
|
||||
// also means a failing or hung abort cannot strand the parked waiter.
|
||||
this.abortRunScopedExtensionDialogs(sessionId);
|
||||
try {
|
||||
await this.abortSessionOperations(active.runtime.session);
|
||||
this.publishActivity(active.runtime.session, "stopped", "idle");
|
||||
|
||||
Reference in New Issue
Block a user