From 6c40618ec1bb098e1c7fe4098133d1de71d287ee Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 25 Oct 2026 12:49:48 -0500 Subject: [PATCH] fix(server): load task bindings only for progress events Co-Authored-By: Paperclip --- ...-10-25-native-chat-progress-sleep-audit.md | 22 +++++++++++++ .../heartbeat-run-event-atomic-append.test.ts | 31 ++++++++++++++----- server/src/services/heartbeat-run-events.ts | 4 ++- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/doc/plans/2026-10-25-native-chat-progress-sleep-audit.md b/doc/plans/2026-10-25-native-chat-progress-sleep-audit.md index 9ead782f720..520e11c6188 100644 --- a/doc/plans/2026-10-25-native-chat-progress-sleep-audit.md +++ b/doc/plans/2026-10-25-native-chat-progress-sleep-audit.md @@ -172,3 +172,25 @@ evidence. Fresh CI must verify those native consumers. Before/after logs: `/tmp/sleep29-notification-fix-workers.log`, `/tmp/sleep29-notification-fix-typecheck.log`, and `/tmp/sleep29-native-finalization-acceptance.log`. + +## CI repair: keep non-progress event admission narrow (2026-10-25) + +CI at `3447973f6f` clears the prior finalization failures. Its sole failing test +file is `heartbeat-run-event-atomic-append.test.ts`: native lifecycle and retry +exhaustion events unnecessarily select runtime mode and the task-binding SQL, +which can read a large context snapshot. Those events do not project progress. +The lock query now selects the extra binding only for allowlisted progress +kinds. Keep the existing strict SQL/payload regression assertions unchanged. + +The atomic-event suite now supports the repository's guarded external test DB +convention, allowing its actual PostgreSQL SQL assertions to run locally despite +embedded bootstrap failure. The original implementation reproduces both CI +failures in `/tmp/sleep29-atomic-events-before.log`. That first external run also +exposed fixture cleanup ordering; cleanup now removes event/run/agent rows before +companies and always closes its pools. The repaired atomic-event and progress +worker suites pass all 25 cases in `/tmp/sleep29-atomic-progress-fixed.log`; +server typecheck passes in `/tmp/sleep29-atomic-progress-typecheck.log`. + +Greptile reviewed `3447973f6f` at 5/5 with no threads. A new full CI/review pass +is required for this second repair; do not carry forward readiness from its +parent head. diff --git a/server/src/__tests__/heartbeat-run-event-atomic-append.test.ts b/server/src/__tests__/heartbeat-run-event-atomic-append.test.ts index dc15a57e6c5..3d4b55b76cb 100644 --- a/server/src/__tests__/heartbeat-run-event-atomic-append.test.ts +++ b/server/src/__tests__/heartbeat-run-event-atomic-append.test.ts @@ -1,16 +1,18 @@ +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import { randomBytes } from "node:crypto"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { eq, sql } from "drizzle-orm"; +import { eq, inArray, sql } from "drizzle-orm"; import { drizzle } from "drizzle-orm/postgres-js"; import { agents, companies, createDb, heartbeatRunEvents, heartbeatRuns } from "@paperclipai/db"; import { appendHeartbeatRunEvent } from "../services/heartbeat-run-events.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; -const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const externalTestDatabaseUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_ATOMIC_EVENTS_TEST_DATABASE_URL"); +const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; describeEmbeddedPostgres("atomic ordinary run events", () => { - let temporary: Awaited>; + let temporary: Awaited> | undefined; let db: ReturnType; let otherDb: ReturnType; const companyId = "30000000-0000-4000-8000-000000000001"; @@ -30,9 +32,10 @@ describeEmbeddedPostgres("atomic ordinary run events", () => { .where(eq(heartbeatRunEvents.runId, id)).orderBy(heartbeatRunEvents.seq); beforeAll(async () => { - temporary = await startEmbeddedPostgresTestDatabase("paperclip-atomic-events-"); - db = createDb(temporary.connectionString); - otherDb = createDb(temporary.connectionString); + if (!externalTestDatabaseUrl) temporary = await startEmbeddedPostgresTestDatabase("paperclip-atomic-events-"); + const connectionString = externalTestDatabaseUrl ?? temporary!.connectionString; + db = createDb(connectionString); + otherDb = createDb(connectionString); await db.insert(companies).values([ { id: companyId, name: "Atomic events", issuePrefix: "ATM" }, { id: otherCompanyId, name: "Other events", issuePrefix: "OTH" }, @@ -42,7 +45,21 @@ describeEmbeddedPostgres("atomic ordinary run events", () => { { id: otherAgentId, companyId: otherCompanyId, name: "Other agent" }, ]); }, 60_000); - afterAll(async () => { await temporary?.cleanup(); }); + afterAll(async () => { + try { + if (externalTestDatabaseUrl && db) { + const fixtureCompanies = [companyId, otherCompanyId]; + await db.delete(heartbeatRunEvents).where(inArray(heartbeatRunEvents.companyId, fixtureCompanies)); + await db.delete(heartbeatRuns).where(inArray(heartbeatRuns.companyId, fixtureCompanies)); + await db.delete(agents).where(inArray(agents.companyId, fixtureCompanies)); + await db.delete(companies).where(inArray(companies.id, fixtureCompanies)); + } + } finally { + await otherDb?.$client.end({ timeout: 1 }); + await db?.$client.end({ timeout: 1 }); + await temporary?.cleanup(); + } + }); it("persists a decoded receipt in one executed statement", async () => { const runId = await makeRun(); diff --git a/server/src/services/heartbeat-run-events.ts b/server/src/services/heartbeat-run-events.ts index c11687c9045..66a7839e25f 100644 --- a/server/src/services/heartbeat-run-events.ts +++ b/server/src/services/heartbeat-run-events.ts @@ -99,7 +99,9 @@ export async function appendHeartbeatRunEvent( return db.transaction(async (tx) => { const run = await tx .select({ companyId: heartbeatRuns.companyId, agentId: heartbeatRuns.agentId, - runtimeMode: heartbeatRuns.runtimeMode, issueId: boundRunIssueIdSql() }) + // Only progress needs the task binding. Other native/recovery events + // must not read a potentially large context snapshot to append a log. + ...(projectsProgress ? { runtimeMode: heartbeatRuns.runtimeMode, issueId: boundRunIssueIdSql() } : {}) }) .from(heartbeatRuns) .where(eq(heartbeatRuns.id, input.runId)) .for("update")