diff --git a/.superpowers/sdd/2026-08-22-notification-center-v2/inflight-fix-report.md b/.superpowers/sdd/2026-08-22-notification-center-v2/inflight-fix-report.md new file mode 100644 index 0000000..44c94a3 --- /dev/null +++ b/.superpowers/sdd/2026-08-22-notification-center-v2/inflight-fix-report.md @@ -0,0 +1,68 @@ +# Notification Outbox In-Flight Fix + +## Status + +Abgeschlossen auf `feature/notification-center-v2`. + +Commit-Betreff: `fix(notifications): preserve in-flight outbox events` + +## Ursache + +`NotificationOutbox.performDrain()` wählte das nächste Event unter der Exclusivkette aus, führte `sendEvent(current)` danach aber außerhalb dieser Kette aus. Während der Send wartete, konnte ein paralleles `enqueue()` über `persist()` und `enforceLimits()` genau dieses Event wegen Ablauf oder Kapazitätsgrenze aus der Queue entfernen. Die anschließende Ergebnisverarbeitung fand die Objekt-ID nicht mehr. Dadurch gingen bei einem Fehler Retry-Zustand und Fehlerzeit verloren; bei einem Erfolg entfielen Erfolgszeit, `onDelivered` und der daran gekoppelte Cooldown. + +## TDD RED + +Command: + +```text +node_modules\.bin\vitest.cmd run tests\notification-outbox.test.ts +``` + +Result: Exit 1. Zwei neue Regressionstests schlugen erwartungsgemäß fehl, 22 bestehende Tests bestanden. + +- Ein langsamer erfolgreicher Send wurde während eines parallelen Enqueue ablaufbedingt entfernt; der Delivery-Ack blieb aus. +- Ein langsamer fehlgeschlagener Send wurde während eines parallelen Enqueue durch das 250er-Limit entfernt; der Retry wurde nicht gespeichert. + +## Umsetzung + +- Das exakt ausgewählte Event wird während des externen Sends als In-Flight-Objekt reserviert. +- Ablaufbereinigung und Kapazitäts-Eviction überspringen ausschließlich dieses Objekt. +- Die Ergebnisverarbeitung findet das reservierte Objekt über Identität, verarbeitet Erfolg oder Fehler genau einmal und löst den Schutz innerhalb derselben Exclusivoperation vor dem abschließenden Persistieren. +- Nach einem Erfolg wird das Event entfernt und `onDelivered` mit der tatsächlichen Ergebniszeit ausgeführt. +- Nach einem Fehler bleiben Versuchszähler, Retry-Termin und Fehlerzeit erhalten, sofern das Event nach der Ergebniszeit noch gültig ist. + +## GREEN und Verifikation + +Command: + +```text +node_modules\.bin\vitest.cmd run tests\notification-outbox.test.ts tests\download-health-monitor.test.ts +``` + +Result: Exit 0. Zwei Testdateien mit 58 von 58 Tests bestanden. + +Command: + +```text +node_modules\.bin\tsc.cmd --noEmit +``` + +Result: Exit 0, keine Ausgabe. + +Command: + +```text +git diff --check -- src/main/notification-outbox.ts tests/notification-outbox.test.ts +``` + +Result: Exit 0, keine Ausgabe. + +## Geänderte Dateien + +- `src/main/notification-outbox.ts` +- `tests/notification-outbox.test.ts` +- `.superpowers/sdd/2026-08-22-notification-center-v2/inflight-fix-report.md` + +## Bedenken + +Keine offenen funktionalen Bedenken. Vitest zeigt ausschließlich die bereits vorhandene Deprecation-Warnung für die CJS-Ausgabe der Vite Node API; die Tests selbst sind vollständig grün. diff --git a/src/main/notification-outbox.ts b/src/main/notification-outbox.ts index 1af326f..1cd4338 100644 --- a/src/main/notification-outbox.ts +++ b/src/main/notification-outbox.ts @@ -173,6 +173,7 @@ export class NotificationOutbox { private retryTimer: NodeJS.Timeout | null = null; private shutdownRequested = false; private drainOperation: Promise | null = null; + private inFlightEvent: NotificationEvent | null = null; public constructor(options: NotificationOutboxOptions) { this.filePath = options.filePath; @@ -257,6 +258,7 @@ export class NotificationOutbox { } return null; } + this.inFlightEvent = next; return next; }); if (!current) { @@ -270,7 +272,8 @@ export class NotificationOutbox { } const outcomeAt = finiteInteger(this.clock(), currentNow); const delivered = await this.runExclusive(async () => { - const index = this.events.findIndex((event) => event.id === current.id); + const index = this.events.indexOf(current); + this.inFlightEvent = null; if (index < 0) { return false; } @@ -346,10 +349,15 @@ export class NotificationOutbox { } private enforceLimits(now: number): void { - this.events = this.events.filter((event) => event.expiresAt > now); + this.events = this.events.filter((event) => event === this.inFlightEvent || event.expiresAt > now); while (this.events.length > MAX_EVENTS) { - const successIndex = oldestIndex(this.events, (event) => event.priority === "success"); - const removeIndex = successIndex >= 0 ? successIndex : oldestIndex(this.events, () => true); + const successIndex = oldestIndex(this.events, (event) => event !== this.inFlightEvent && event.priority === "success"); + const removeIndex = successIndex >= 0 + ? successIndex + : oldestIndex(this.events, (event) => event !== this.inFlightEvent); + if (removeIndex < 0) { + break; + } this.events.splice(removeIndex, 1); } } diff --git a/tests/notification-outbox.test.ts b/tests/notification-outbox.test.ts index 2001370..8a6802d 100644 --- a/tests/notification-outbox.test.ts +++ b/tests/notification-outbox.test.ts @@ -579,6 +579,78 @@ describe("NotificationOutbox", () => { expect(stateBeforeRelease.events.map((queuedEvent) => queuedEvent.id)).toContain("late-digest"); }); + it("acknowledges a successful in-flight delivery after a parallel enqueue expires it", async () => { + const filePath = createOutboxFile(); + let now = 1000; + let releaseSend = (_sent: boolean) => {}; + let markSendStarted = () => {}; + const sendStarted = new Promise((resolve) => { markSendStarted = resolve; }); + const sendResult = new Promise((resolve) => { releaseSend = resolve; }); + const delivered: Array<{ id: string; deliveredAt: number }> = []; + const outbox = new NotificationOutbox({ + filePath, + now: () => now, + send: async () => { + markSendStarted(); + return sendResult; + }, + onDelivered: (queuedEvent, deliveredAt) => { + delivered.push({ id: queuedEvent.id, deliveredAt }); + } + }); + await outbox.enqueue(event("expires-in-flight", { expiresAt: 1500 })); + const draining = outbox.drain(); + await sendStarted; + + now = 2000; + await outbox.enqueue(event("late", { createdAt: 2000, nextAttemptAt: 3000 })); + expect(persisted(filePath).events.map((queuedEvent) => queuedEvent.id)).toContain("expires-in-flight"); + releaseSend(true); + await draining; + + expect(delivered).toEqual([{ id: "expires-in-flight", deliveredAt: 2000 }]); + expect(outbox.getStatus()).toEqual({ queued: 1, lastSuccessAt: 2000, lastFailureAt: 0 }); + expect(persisted(filePath).events.map((queuedEvent) => queuedEvent.id)).toEqual(["late"]); + }); + + it("keeps a failed in-flight delivery queued for retry during a capacity-enforcing enqueue", async () => { + const filePath = createOutboxFile(); + const queuedEvents = [ + event("fails-in-flight", { createdAt: 1 }), + ...Array.from({ length: 249 }, (_, index) => event(`queued-${index}`, { createdAt: index + 2 })) + ]; + fs.writeFileSync(filePath, JSON.stringify({ + version: 1, + events: queuedEvents, + lastSuccessAt: 0, + lastFailureAt: 0 + }), "utf8"); + let releaseSend = (_sent: boolean) => {}; + let markSendStarted = () => {}; + const sendStarted = new Promise((resolve) => { markSendStarted = resolve; }); + const sendResult = new Promise((resolve) => { releaseSend = resolve; }); + const outbox = new NotificationOutbox({ + filePath, + now: () => 1000, + send: async () => { + markSendStarted(); + return sendResult; + } + }); + const draining = outbox.drain(); + await sendStarted; + + await outbox.enqueue(event("late-capacity", { createdAt: 10000 })); + expect(persisted(filePath).events.map((queuedEvent) => queuedEvent.id)).toContain("fails-in-flight"); + releaseSend(false); + await draining; + + const state = persisted(filePath); + expect(state.events).toHaveLength(250); + expect(state.events[0]).toMatchObject({ id: "fails-in-flight", attempts: 1, nextAttemptAt: 2000 }); + expect(state.lastFailureAt).toBe(1000); + }); + it("acknowledges only successful delivery with its actual completion time", async () => { const filePath = createOutboxFile(); let now = 1000;