mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix(server): share health-check batches across due companies
A real database regression reproduces a quiet company's delayed check behind another company's unchecked inventory. Rotate company turns and preserve each company's deadline ordering. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
097f5c8aff
commit
6742665ecf
4 files changed
+56
-3
No files matched your search
@@ -23,7 +23,9 @@ 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
|
||||
there are no eligible rows or unresolved changes. Future checks cause no SQL
|
||||
before their deadline. Each pass checks at most 25 due connections.
|
||||
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
|
||||
rotates across passes when more companies are due than fit in one batch.
|
||||
|
||||
Before a provider check, the service reloads current eligibility and deadline.
|
||||
A saved health result, including a failed health observation, owns the next
|
||||
|
||||
@@ -1022,3 +1022,31 @@ shared-DB attempt produced cleanup foreign-key failures after email; those are
|
||||
recorded, not counted as passes. Total selected coverage is 770 cases. The server
|
||||
build passes. The inventory SQL uses the existing company/enabled index with
|
||||
40,000 retained rows (13 buffers, 0.022 ms). Full workspace and remote gates remain.
|
||||
|
||||
|
||||
## Discord publication and health fairness repair
|
||||
|
||||
Discord registration is [#17557](https://github.com/paperclipai/paperclip/pull/17557),
|
||||
head `3755d7caced67589de474e0ca3e1b8b5b4e659c4`, based on #17554 and attached.
|
||||
It has 639 selected passing tests, workspace typecheck/build, and final server
|
||||
verification. CI run **40168371442** and review run **40168371311** are confirmed
|
||||
live on that head. Fresh CI and review remain required; no merge is authorized.
|
||||
The source and detailed producer audit are on the Discord child branch.
|
||||
|
||||
The health parent Paperclip Review completed neutral with 4/5 at `da40899606`.
|
||||
It requests fair company batching and direct real-service producer coverage.
|
||||
The existing probes are already scoped by connection ID, so the review's concern
|
||||
about full inventory scans around each probe needs clarification rather than
|
||||
removing current-state eligibility checks. Self-generated observation hints still
|
||||
cause a bounded coalesced company refresh; avoid muting correctness signals merely
|
||||
to reduce that bounded work without proving equivalent recovery.
|
||||
|
||||
Fairness was reproduced with 30 unchecked connections in one company and one due
|
||||
connection in another. The quiet company was omitted from the first batch. The
|
||||
shared selector from Discord commit `e77979d4fc` was applied to the health branch
|
||||
as `097f5c8aff`; the health worker now rotates companies. All 22 health/selection
|
||||
tests pass (`/tmp/sleep18-fairness-after.log`), after the recorded failing baseline
|
||||
(`/tmp/sleep18-fairness-before.log`). The fix is local until the additional
|
||||
AgentMail activation/pause, GitHub installation event, and access-removal producer
|
||||
tests are complete. Then push the owning PR, request fresh review, and merge it
|
||||
into the Discord branch. Full original inventory and earlier unresolved CI remain.
|
||||
@@ -164,6 +164,25 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu
|
||||
await tools.archiveConnection(row.id, companyId); await quiet();
|
||||
});
|
||||
|
||||
it("checks a quiet company in the first batch despite another company's unchecked backlog", async () => {
|
||||
for (let index = 0; index < 30; index++) await connection({}, false);
|
||||
const otherId = randomUUID();
|
||||
await db.insert(companies).values({ id: otherId, name: "Quiet health", issuePrefix: `Q${otherId.slice(0, 6)}` });
|
||||
const [app] = await db.insert(toolApplications).values({ companyId: otherId, name: "Quiet health", type: "mcp_http" }).returning();
|
||||
try {
|
||||
const [other] = await db.insert(toolConnections).values({
|
||||
companyId: otherId, applicationId: app.id, name: "Quiet", uid: randomUUID(), transport: "mcp_remote",
|
||||
status: "active", enabled: true, healthCheckedAt: new Date(Date.now() - 901_000),
|
||||
}).returning();
|
||||
await start();
|
||||
expect(probe).toHaveBeenCalledTimes(25);
|
||||
expect(probe.mock.calls.map(([id]) => id)).toContain(other.id);
|
||||
} finally {
|
||||
await coordinator?.stop(); coordinator = undefined;
|
||||
await db.delete(companies).where(eq(companies.id, otherId));
|
||||
}
|
||||
});
|
||||
|
||||
it("continues another company's checks when one scope cannot refresh", async () => {
|
||||
const service = connectionHealthSchedule(db, probe), load = service.load;
|
||||
await start(service); await quiet();
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { takeCompanyWorkBatch } from "./company-work-batch.js";
|
||||
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
||||
import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
|
||||
import type { ConnectionHealthDeadline } from "./tool-access/catalog/health-schedule.js";
|
||||
@@ -15,6 +16,7 @@ export function registerToolConnectionHealthWork(
|
||||
const failures = new Map<string, { count: number; retryAt: number }>();
|
||||
const retryMs = 30_000;
|
||||
let earliest = 0;
|
||||
let lastCompany: string | null = null;
|
||||
function request(hints?: ReadonlySet<string>) {
|
||||
for (const hint of hints ?? ["*"]) dirty.add(hint);
|
||||
}
|
||||
@@ -52,8 +54,10 @@ export function registerToolConnectionHealthWork(
|
||||
}
|
||||
const due = [...deadlines.values()].filter(entry => !unavailable.has("*") && !unavailable.has(entry.companyId)
|
||||
&& Math.max(entry.dueAt, failures.get(entry.id)?.retryAt ?? 0) <= Date.now())
|
||||
.sort((a, b) => a.dueAt - b.dueAt || a.id.localeCompare(b.id)).slice(0, 25);
|
||||
for (const entry of due) {
|
||||
.sort((a, b) => a.dueAt - b.dueAt || a.id.localeCompare(b.id));
|
||||
const batch = takeCompanyWorkBatch(due, 25, lastCompany);
|
||||
lastCompany = batch.lastCompany;
|
||||
for (const entry of batch.entries) {
|
||||
if (signal.aborted || !canRun()) break;
|
||||
try {
|
||||
const current = await service.check(entry.id);
|
||||
|
||||
Reference in new issue
Block a user