Skip to content

Commit 0ed4cb6

Browse files
committed
fix(tui): drop interrupt-stale continuation notices
1 parent 825085f commit 0ed4cb6

2 files changed

Lines changed: 82 additions & 12 deletions

File tree

‎src/tui/deliver-agent-message.test.ts‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ describe("runGenerationGuardedDeliver", () => {
131131
});
132132

133133
describe("settleCompactionContinuationHop", () => {
134-
test("a superseded hop requeues instead of delivering to the outgoing agent", async () => {
134+
test("a superseded hop does not deliver to the outgoing agent", async () => {
135135
let generation = 1;
136136
const stillCurrent = () => generation === 1;
137137
let delivered = 0;
@@ -195,6 +195,45 @@ describe("settleCompactionContinuationHop", () => {
195195
expect(runs).toBe(1);
196196
});
197197

198+
test("an accepted hop stays accepted if generation flips after deliver", async () => {
199+
let generation = 1;
200+
const result = await settleCompactionContinuationHop({
201+
stillCurrent: () => generation === 1,
202+
deliver: async () => {
203+
generation = 2;
204+
return { status: "accepted" as const };
205+
},
206+
onSuperseded: () => {
207+
throw new Error("accepted hop must not be relabeled superseded");
208+
},
209+
});
210+
expect(result).toEqual({ status: "accepted" });
211+
});
212+
213+
test("a closed hop that goes stale does not retry deliver", async () => {
214+
let generation = 1;
215+
let attempts = 0;
216+
const result = await settleCompactionContinuationHop({
217+
stillCurrent: () => generation === 1,
218+
deliver: async () => {
219+
attempts += 1;
220+
generation = 2;
221+
return {
222+
status: "not-delivered" as const,
223+
reason: "agent-closed" as const,
224+
detail: "agent is closed",
225+
};
226+
},
227+
onSuperseded: () => undefined,
228+
});
229+
expect(result).toEqual({
230+
status: "not-delivered",
231+
reason: "superseded",
232+
detail: "session identity changed before delivery",
233+
});
234+
expect(attempts).toBe(1);
235+
});
236+
198237
test("continuation enqueued then interrupt rebuild queued does not auto-deliver to the replacement agent", async () => {
199238
const { enqueue, enqueuePreemptible, abortInFlight, awaitTail } =
200239
createSessionOperationQueue();
@@ -393,6 +432,34 @@ describe("settleCompactionContinuationHop", () => {
393432
await awaitTail();
394433
expect(delivered).toEqual([1, 2]);
395434
});
435+
436+
test("an interrupt-stale hop does not report not-delivered", async () => {
437+
const { enqueue, enqueuePreemptible, abortInFlight, awaitTail } =
438+
createSessionOperationQueue();
439+
const deliveryGeneration = createDeliveryGeneration();
440+
const notices: string[] = [];
441+
442+
enqueueCompactionContinuationHop({
443+
enqueue: enqueuePreemptible,
444+
captureGeneration: () => deliveryGeneration.capture(),
445+
deliver: async () => ({ status: "accepted" as const }),
446+
onResult: (result) => {
447+
if (result.status === "accepted") return;
448+
notices.push(result.detail);
449+
},
450+
});
451+
452+
startInterruptRebuild({
453+
deliveryGeneration,
454+
markSendAborted: () => undefined,
455+
abortInFlight,
456+
enqueue,
457+
rebuild: async () => undefined,
458+
});
459+
460+
await awaitTail();
461+
expect(notices).toEqual([]);
462+
});
396463
});
397464

398465
describe("deliveryResultNotice", () => {

‎src/tui/delivery-queue.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ export async function deliverAgentMessage(
8484

8585
/**
8686
* Settles a deliver that was enqueued on the serial operation queue against
87-
* the shoot generation captured at enqueue time. The queue is FIFO with no
88-
* preemption, so a deliver queued ahead of a reload still executes after the
89-
* reload has replaced the agent — the generation must be re-checked when the
87+
* the shoot generation captured at enqueue time. Serial ops stay FIFO with
88+
* no preemption; continuation hops are preemptible, so a hung compact-continue
89+
* cannot park interrupt. Either way the generation must be re-checked when the
9090
* queued closure runs, not just when it enqueues. A stale deliver takes the
9191
* `onStale` path (the caller reports `not-delivered`); a current deliver runs
9292
* the real settle. This is what closes the reload-vs-async-deliver race: a
@@ -134,14 +134,14 @@ export async function settleCompactionContinuationHop(options: {
134134
options.onSuperseded();
135135
return first;
136136
}
137-
if (!options.stillCurrent()) {
138-
return {
139-
status: "not-delivered",
140-
reason: "superseded",
141-
detail: "session identity changed before delivery",
142-
};
143-
}
144137
if (first.status === "not-delivered" && first.reason === "agent-closed") {
138+
if (!options.stillCurrent()) {
139+
return {
140+
status: "not-delivered",
141+
reason: "superseded",
142+
detail: "session identity changed before delivery",
143+
};
144+
}
145145
return options.deliver();
146146
}
147147
return first;
@@ -164,6 +164,8 @@ export function enqueueCompactionContinuationHop(options: {
164164
deliver: options.deliver,
165165
onSuperseded: () => undefined,
166166
});
167+
if (!stillCurrent()) return;
168+
if (compactionContinuationIsSuperseded(result)) return;
167169
options.onResult(result);
168170
});
169171
}
@@ -420,7 +422,8 @@ export interface SessionOperationQueue {
420422
/**
421423
* Same tail as `enqueue`, marked preemptible. `abortInFlight` races the
422424
* current op against a captured abort signal so a hung continuation cannot
423-
* park rebuild, send, or quit.
425+
* park rebuild or send. Quit still starts shutdown first, then awaits the
426+
* tail — it does not call `abortInFlight`.
424427
*/
425428
enqueuePreemptible: (op: () => Promise<void>) => Promise<void>;
426429
/**

0 commit comments

Comments
 (0)