diff --git a/doc/plans/2026-10-25-configuration-worker-drain-repair.md b/doc/plans/2026-10-25-configuration-worker-drain-repair.md new file mode 100644 index 00000000000..03501ef329e --- /dev/null +++ b/doc/plans/2026-10-25-configuration-worker-drain-repair.md @@ -0,0 +1,61 @@ +# Configuration worker drain repairs + +The voice drain audit exposed a pattern in three earlier independent workers: +retained dirty scopes reported as pending work keep the coordinator's intent +activity hold alive after the write has settled. No recurring SQL is necessary +to block idle admission this way. The earlier tests checked traffic but did not +check the idle counter or generation while paused. + +Each owner must retain its hints and next deadline for resume, report dirty work +as pending only while dispatch is allowed, and leave unresolved transaction +fencing to the shared coordinator. No hint is acknowledged or discarded during +drain. Business health/registration/lease deadlines and provider behavior stay +unchanged. This is an idle-work boundary repair, not provider sleep qualification. + +## Tool health — slice 18 / #17554 + +Baseline 9ef6e4f4c8 reproduces the bug: the strengthened existing drain case fails +with active=1 after commit, recorded in /tmp/sleep18-drain-before.log. The repaired +case asserts active=0, then a stable idle snapshot and no SQL/provider calls for a +minute, then the saved work runs on resume without another producer. + +All 23 lifecycle cases pass in /tmp/sleep18-drain-tests.log; production server +typecheck passes in /tmp/sleep18-drain-typecheck.log. The test database is the +fresh migrated paperclip_sleep18_drain_20261025_test. No test failures were retried +blindly. Current master bf9a8551c3 has no changes to these three worker owners +since the preceding aec86c794b inspection; full stack reconciliation remains open. + +Discord command registration and receiver discovery require the same causal +check at their own parents, then propagation through every dependent stack PR. +The full original recurring-work inventory, milestone producer audit, older E2E +failure and inbound cleanup/deletion policy finding remain outstanding. + +## Discord command registration — slice 19 / #17557 + +The strengthened outer-rollback/drain case reproduces active=1 after commit in +/tmp/sleep19-drain-before.log. The repair preserves the dirty scope and saved +deadlines, but reports pending dirty work only while dispatch is allowed. The +shared coordinator still holds unresolved transactions independently. + +All 67 registration/store cases pass in /tmp/sleep19-drain-tests.log, including +90 seconds of no SQL/provider calls and an unchanged idle snapshot during drain, +followed by registration on resume without another producer. Production server +typecheck passes in /tmp/sleep19-drain-typecheck.log. These tests use the migrated +paperclip_sleep19_drain_20261025_test database. Remote CI/review must run on the +new head; this is not evidence of provider suspension. + +## Receiver discovery — slice 20 / #17572 + +The strengthened first-receiver drain case fails with active=1 after commit in +/tmp/sleep20-drain-before.log. Apply the same owner-level pending predicate while +retaining all dirty scopes and lease/retry deadlines. All 19 receiver lifecycle +cases pass in /tmp/sleep20-drain-tests.log, including no SQL and an unchanged idle +snapshot for a minute after settlement, then receiver acquisition on resume. +Production server typecheck passes in /tmp/sleep20-drain-typecheck.log. The fresh +migrated database is paperclip_sleep20_drain_20261025_test. + +The three configuration owners now have measured drain activity as well as traffic +checks. Their retained timers remain memory-only while paused; they do not provide +external wake coverage. Fresh remote CI/review and downstream propagation remain +required. Origin/master was refreshed to e335265568; the latest change removes +idle task-run UI polling and does not replace these worker repairs. diff --git a/server/src/__tests__/chat-receiver-work.test.ts b/server/src/__tests__/chat-receiver-work.test.ts index 8cbfe0e3577..83aff022265 100644 --- a/server/src/__tests__/chat-receiver-work.test.ts +++ b/server/src/__tests__/chat-receiver-work.test.ts @@ -96,7 +96,12 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu }); it("preserves the first receiver written during drain without passive SQL", async () => { await start(); await settled(); enabled = false; const row = await endpoint(); - await quiet(60_000); expect(reconcile).not.toHaveBeenCalled(); + await vi.waitFor(async () => { + await vi.advanceTimersByTimeAsync(1000); + expect(idleWorkSnapshot().active).toBe(0); + }); + const drained = idleWorkSnapshot(); + await quiet(60_000); expect(idleWorkSnapshot()).toEqual(drained); expect(reconcile).not.toHaveBeenCalled(); enabled = true; await settled(); expect(reconcile).toHaveBeenCalledWith(row.id); }); it("reconstructs after standby becomes primary without restarting the worker", async () => { diff --git a/server/src/__tests__/tool-connection-health-work.test.ts b/server/src/__tests__/tool-connection-health-work.test.ts index 247f3c36873..da7b53eacbe 100644 --- a/server/src/__tests__/tool-connection-health-work.test.ts +++ b/server/src/__tests__/tool-connection-health-work.test.ts @@ -185,8 +185,11 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu }); it("retains first writes through drain without querying", async () => { await start(); await quiet(); draining = true; await connection(); + await vi.waitFor(async () => { await vi.advanceTimersByTimeAsync(1000); expect(idleWorkSnapshot().active).toBe(0); }); + const drained = idleWorkSnapshot(); const reads = vi.spyOn(db, "select"); await vi.advanceTimersByTimeAsync(60_000); - expect(reads).not.toHaveBeenCalled(); reads.mockRestore(); expect(probe).not.toHaveBeenCalled(); + expect(reads).not.toHaveBeenCalled(); expect(idleWorkSnapshot()).toEqual(drained); + reads.mockRestore(); expect(probe).not.toHaveBeenCalled(); draining = false; await checked(); }); it("moves the deadline when an actual-use health observation commits", async () => { diff --git a/server/src/services/chat-channels/discord/command-work.ts b/server/src/services/chat-channels/discord/command-work.ts index 28169037824..af826d9cf1b 100644 --- a/server/src/services/chat-channels/discord/command-work.ts +++ b/server/src/services/chat-channels/discord/command-work.ts @@ -88,7 +88,9 @@ export function registerDiscordCommandWork( if (due.length > batch.entries.length) earliest = Date.now() + 1_000; if (errors.length) throw new AggregateError(errors, "Discord command registration could not settle"); }, - hasPending: async () => dirty.size > 0, + // Preserve hints/deadlines for resume without holding settled intent during drain. + // The coordinator independently fences unresolved database transactions. + hasPending: async () => canRun() && dirty.size > 0, nextRunAt: next, }); } diff --git a/server/src/services/chat-channels/runtime/receiver-work.ts b/server/src/services/chat-channels/runtime/receiver-work.ts index 5cb664d87ab..ad62f0baa06 100644 --- a/server/src/services/chat-channels/runtime/receiver-work.ts +++ b/server/src/services/chat-channels/runtime/receiver-work.ts @@ -79,7 +79,9 @@ export function registerChatReceiverWork( } if (errors.length) throw new AggregateError(errors, "Chat receiver discovery could not settle"); }, - hasPending: async () => dirty.size > 0, + // Preserve hints/deadlines for resume without holding settled intent during drain. + // The coordinator independently fences unresolved database transactions. + hasPending: async () => canRun() && dirty.size > 0, nextRunAt: next, }); return { ready: worker.ready, changed(id: string) { diff --git a/server/src/services/chat-discord-command-registration-store.test.ts b/server/src/services/chat-discord-command-registration-store.test.ts index 71660aaeae6..44809957ca4 100644 --- a/server/src/services/chat-discord-command-registration-store.test.ts +++ b/server/src/services/chat-discord-command-registration-store.test.ts @@ -765,7 +765,9 @@ suite("Discord command ownership store (real PostgreSQL, no network)", () => { await sqlQuiet(60_000); draining = true; await db.transaction(async tx => activate(tx as unknown as typeof db)); - await sqlQuiet(90_000); expect(f.calls).toEqual([]); + await vi.waitFor(async () => { await vi.advanceTimersByTimeAsync(1000); expect(idleWorkSnapshot().active).toBe(0); }); + const drained = idleWorkSnapshot(); + await sqlQuiet(90_000); expect(idleWorkSnapshot()).toEqual(drained); expect(f.calls).toEqual([]); draining = false; await vi.advanceTimersByTimeAsync(30_000); await vi.waitFor(() => expect(f.calls).toEqual(["GET", "POST"])); diff --git a/server/src/services/tool-connection-health-work.ts b/server/src/services/tool-connection-health-work.ts index 6d684ba4a42..7259baca19e 100644 --- a/server/src/services/tool-connection-health-work.ts +++ b/server/src/services/tool-connection-health-work.ts @@ -82,7 +82,9 @@ export function registerToolConnectionHealthWork( if (due.length > batch.entries.length) earliest = Date.now() + 1_000; if (errors.length) throw new AggregateError(errors, "Tool connection health checks could not settle"); }, - hasPending: async () => dirty.size > 0, + // Keep the hints/deadlines for resume, but release settled intent while + // paused. The coordinator independently fences unresolved transactions. + hasPending: async () => canRun() && dirty.size > 0, nextRunAt: next, }); }