diff --git a/doc/idle-tool-connection-health.md b/doc/idle-tool-connection-health.md index 148aa7ed808..6434343c244 100644 --- a/doc/idle-tool-connection-health.md +++ b/doc/idle-tool-connection-health.md @@ -21,7 +21,9 @@ company status are not new filters: this migration preserves the old policy. An unchecked row is due immediately. Otherwise its deadline is `healthCheckedAt` plus 15 minutes. A company hint refreshes that company's cache. Startup and an instance feature-settings change can refresh the whole instance. Refreshes are -coalesced with a 30-second minimum spacing. There is no scheduled refresh when +coalesced with a 30-second minimum spacing when no due backlog remains. +Batches retain at most 25 checks and continue after one second while already due +rows remain, so the batch limit does not cap dispatch at 50 checks per minute. There is no scheduled refresh when there are no eligible rows or unresolved changes. Future checks cause no SQL before their deadline. Each pass checks at most 25 due connections, taking turns across companies and preserving due order within each company. The company cursor @@ -152,3 +154,13 @@ changing application behavior or weakening assertions. All 66 access/email/GitHu cases pass on the final run. Server typecheck/build pass. Cumulative selected coverage is 776 unique tests; full-suite and fresh remote review/checks remain acceptance gates. No whole-instance sleep or provider suspension is claimed. + + +The Discord child review also exposed fixed-batch throughput as a risk for health. +A probe-timestamp regression measured a 30.7-second gap before the 26th already +owed check. The worker now schedules a one-second continuation only when the due +selection exceeds its bounded batch. Failed work retains backoff; an empty or +fully future-dated inventory has no continuation. This is scheduling concrete +backlog, not a periodic idle scan. The initial deadline-only assertion passed +incorrectly because a pending transaction notification temporarily reported an +immediate coordinator wake; acceptance measures actual probe timestamps instead. diff --git a/doc/plans/2026-10-24-recurring-server-sleep-stack.md b/doc/plans/2026-10-24-recurring-server-sleep-stack.md index 161012742d6..0702bb0dd95 100644 --- a/doc/plans/2026-10-24-recurring-server-sleep-stack.md +++ b/doc/plans/2026-10-24-recurring-server-sleep-stack.md @@ -1062,3 +1062,13 @@ the fair company selector, request fresh review, and propagate to Discord #17557 The Discord initial head has Greptile 5/5; its CI and Paperclip review were still running when inspected. The remaining original inventory and #17490 failure remain open; no stack PR is merged or declared fully accepted. + + +Discord's first Paperclip review (4/5) identified a dispatch-capacity issue shared +by health: 25 checks per 30 seconds delays a large already-due backlog. Health's +actual-probe timestamp regression failed with a 30.7-second gap, then passed when +only a remaining due batch receives a one-second continuation. All 22 health and +batching cases pass; server types pass. Propagate the parent and apply the same +bounded continuation to Discord, with its own regression. The Discord review also +requests preserving queued scopes after malformed hints and broader producer +coverage. Its initial head has Greptile 5/5 but these findings remain open. diff --git a/server/src/__tests__/tool-connection-health-work.test.ts b/server/src/__tests__/tool-connection-health-work.test.ts index cce40d3f748..5f2d1caa4b2 100644 --- a/server/src/__tests__/tool-connection-health-work.test.ts +++ b/server/src/__tests__/tool-connection-health-work.test.ts @@ -136,7 +136,10 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu }); it("drains more than one due page without repeatedly checking completed rows", async () => { for (let i = 0; i < 30; i++) await connection(); + const probedAt: number[] = [], performProbe = probe.getMockImplementation()!; + probe.mockImplementation(async (id: string) => { probedAt.push(Date.now()); return performProbe(id); }); await start(); await checked(30); + expect(probedAt[25] - probedAt[24]).toBeLessThan(5_000); expect(new Set(probe.mock.calls.map(([id]) => id)).size).toBe(30); }); it("checks the first connection created by the real API service after setup commits", async () => { diff --git a/server/src/services/tool-connection-health-work.ts b/server/src/services/tool-connection-health-work.ts index 8754d1a67a7..6743dd78dfe 100644 --- a/server/src/services/tool-connection-health-work.ts +++ b/server/src/services/tool-connection-health-work.ts @@ -69,6 +69,7 @@ export function registerToolConnectionHealthWork( errors.push(error); } } + 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,