Skip to content

Commit de08ca8

Browse files
committed
fix(session): drop a parked call when its surface fails stale
1 parent 9a3d500 commit de08ca8

2 files changed

Lines changed: 80 additions & 0 deletions

File tree

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,78 @@ describe("approval identity ordering", () => {
938938
expect(decisionBody(message).outcome).toBe("rejected");
939939
});
940940

941+
test("generation change during catch history drops the parked call once", async () => {
942+
const ctx = deferredResume();
943+
const failure = new Error("gate exploded");
944+
ctx.resolveSuspended.mockImplementation(async () => {
945+
throw failure;
946+
});
947+
ctx.history.mockResolvedValueOnce([]).mockImplementationOnce(async () => {
948+
ctx.invalidate();
949+
return [];
950+
});
951+
const handling = ctx.resume.handle(suspension("corr-A", "echo same"));
952+
ctx.lookup.resolve("call-A");
953+
await expect(handling).rejects.toBe(failure);
954+
expect(ctx.onDropped).toHaveBeenCalledTimes(1);
955+
expect(ctx.onDropped).toHaveBeenCalledWith(APPROVAL_DROPPED_NOTICE);
956+
expect(ctx.deliver).toHaveBeenCalledTimes(1);
957+
const message = ctx.deliver.mock.calls[0]?.[0];
958+
if (message === undefined)
959+
throw new Error("expected cancellation rejection");
960+
expect(deliveredCorrelationId(message)).toBe("corr-A");
961+
expect(decisionBody(message)).toEqual({
962+
outcome: "rejected",
963+
message: APPROVAL_DROPPED_NOTICE,
964+
});
965+
});
966+
967+
test("a gate throw with no generation change rejects once as approval surface failed", async () => {
968+
const ctx = deferredResume();
969+
const failure = new Error("gate exploded");
970+
ctx.resolveSuspended.mockImplementation(async () => {
971+
throw failure;
972+
});
973+
const handling = ctx.resume.handle(suspension("corr-A", "echo same"));
974+
ctx.lookup.resolve("call-A");
975+
await expect(handling).rejects.toBe(failure);
976+
expect(ctx.onDropped).not.toHaveBeenCalled();
977+
expect(ctx.deliver).toHaveBeenCalledTimes(1);
978+
const message = ctx.deliver.mock.calls[0]?.[0];
979+
if (message === undefined) throw new Error("expected surface rejection");
980+
expect(deliveredCorrelationId(message)).toBe("corr-A");
981+
expect(decisionBody(message)).toEqual({
982+
outcome: "rejected",
983+
message: "approval surface failed",
984+
});
985+
});
986+
987+
test("generation change during catch history after a timeout does not reject again", async () => {
988+
const ctx = deferredResume();
989+
const failure = new Error("gate exploded");
990+
ctx.resolveSuspended.mockImplementation(async () => {
991+
throw failure;
992+
});
993+
const reading = Promise.withResolvers<undefined>();
994+
const history = Promise.withResolvers<ConversationTurn[]>();
995+
ctx.history.mockResolvedValueOnce([]).mockImplementationOnce(() => {
996+
reading.resolve(undefined);
997+
return history.promise;
998+
});
999+
const handling = ctx.resume.handle(suspension("corr-A", "echo same"));
1000+
ctx.lookup.resolve("call-A");
1001+
await reading.promise;
1002+
expect(ctx.resolveSuspended).toHaveBeenCalledTimes(1);
1003+
expect(ctx.registered()).toBeUndefined();
1004+
ctx.invalidate();
1005+
history.resolve([timeoutTurn("call-A")]);
1006+
await expect(handling).rejects.toBe(failure);
1007+
expect(ctx.onDropped).toHaveBeenCalledTimes(1);
1008+
expect(ctx.onDropped).toHaveBeenCalledWith(APPROVAL_DROPPED_NOTICE);
1009+
expect(ctx.deliver).not.toHaveBeenCalled();
1010+
expect(ctx.registered()).toBeUndefined();
1011+
});
1012+
9411013
for (const timedOutCallId of ["call-A", "call-B"]) {
9421014
test(`generation change during post-gate history with ${timedOutCallId} timeout preserves exact-call cancellation`, async () => {
9431015
const ctx = deferredResume();

‎src/session/approval-resume.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,14 @@ export function createApprovalResume(args: {
561561
() => parkedAgent.history(),
562562
parkedCallId,
563563
));
564+
// A bump during that history read already cleared this cancel, so
565+
// deliver the drop here. A call the reactor already timed out is not
566+
// rejected again.
567+
if (timedOut) canReject = false;
568+
if (!stillCurrent()) {
569+
dropParked();
570+
throw err;
571+
}
564572
if (!timedOut) {
565573
await deliverDecision(
566574
decisionMessage(

0 commit comments

Comments
 (0)