diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 0e490c482b..855bd36bbc 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -3366,6 +3366,22 @@ export async function createHub(config: HubConfig) { if (sidecarAllocationReconciliationTimer !== undefined) { clearTimeout(sidecarAllocationReconciliationTimer); } + // Retire the relaunch sweep's series so any in-flight pass's + // `.finally` reschedule is a no-op, and cancel whatever pass is + // currently pending. Without this the sweep outlives `close()` + // entirely (it's only ever re-armed, never torn down) and keeps + // querying `chat.workbench_launch` on a timer this function is + // about to end — including, once `close()` below tears down the + // db pool, querying a pool that's already shut down. In a test + // suite that boots many hubs back to back (e.g. + // slack-tag-mount.test.ts, CL-7453) those leaked timers pile up + // across the whole `bun test` process and contend with later + // tests' own boots for Postgres connections, which is what + // surfaced as `chat·relaunch-sweep: relaunch sweep pass failed: + // Failed query: select ... from chat.workbench_launch` and an + // intermittent test timeout. + relaunchSweepSeries += 1; + clearTimeout(relaunchSweepTimer); envCredentialPlant.stop(); chatOrchestrator.dispose(); workflowScheduler.stop(); diff --git a/apps/hub/test/relaunch-sweep-close.test.ts b/apps/hub/test/relaunch-sweep-close.test.ts new file mode 100644 index 0000000000..5aa76cf37c --- /dev/null +++ b/apps/hub/test/relaunch-sweep-close.test.ts @@ -0,0 +1,84 @@ +// Regression for CL-7453: createHub's relaunch sweep re-arms itself on +// a fixed [0, 2_000, 5_000, 15_000, 45_000] ms schedule (see +// `scheduleRelaunchSweep` in ../src/index.ts) and is never otherwise +// torn down. Before this fix, `hub.close()` stopped every other +// background loop (workflow scheduler, credential-expiry sweep, +// inbox-unsnooze sweep, bench provisioner, sidecar-allocation +// reconciliation) but left the relaunch sweep's timer running, so it +// kept firing — and logging `relaunch sweep pass failed: ...` when it +// queried a pool `close()` had already shut down — for the rest of the +// `bun test` process. A suite that boots several hubs back to back +// (slack-tag-mount.test.ts's env-gate suite among them) accumulated +// these leaked loops, contending with later tests' own boots for +// Postgres connections and surfacing as an intermittent test timeout. +import { afterAll, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import type { HubConfig } from "../src/config.ts"; +import { createHub } from "../src/index.ts"; +import { dbGate } from "../../../scripts/e2e/db-gate"; + +const DATABASE_URL = process.env["DATABASE_URL"] ?? ""; +const describeIfDb = dbGate(DATABASE_URL, import.meta.path); + +const root = mkdtempSync(path.join(tmpdir(), "hub-relaunch-sweep-close-")); +const staticDir = path.join(root, "static"); +mkdirSync(staticDir, { recursive: true }); +writeFileSync(path.join(staticDir, "index.html"), "shell"); +mkdirSync(path.join(root, "data"), { recursive: true }); + +const config: HubConfig = { + databaseUrl: DATABASE_URL, + baseUrl: "http://localhost:3000", + sessionSecret: "insecure-test-only-session-secret-0000", + hubDataDir: path.join(root, "data"), + hubStaticDir: staticDir, + defaultTenantSlug: "workbench", + signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, + socialProviders: {}, + allowUnverifiedEmails: true, + sidecarProvisioners: [], + envProviderKeys: {}, + envProviderBaseUrls: {}, + chatIdleReapMs: 30 * 60_000, + envCredentialPlantAdmin: { + email: "alice@example.com", + password: "password123", + orgSlug: "workbench", + }, + signupMode: "closed", + allowedEmailDomains: [], + allowPlaintextSecrets: true, +}; + +afterAll(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describeIfDb("relaunch sweep teardown", () => { + test("does not run after hub.close()", async () => { + const hub = await createHub(config); + await hub.close(); + + const errors: string[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => { + errors.push(args.map(String).join(" ")); + }; + try { + // The sweep's own schedule reschedules at 0ms then 2_000ms; wait + // past both so a leaked timer would have fired (and, querying a + // pool `close()` already shut down, logged) at least once. + await new Promise((resolve) => setTimeout(resolve, 2_200)); + } finally { + console.error = originalError; + } + + const leaked = errors.filter((line) => + line.includes("relaunch sweep pass failed"), + ); + expect(leaked).toEqual([]); + }, 10_000); +}); diff --git a/apps/hub/test/slack-tag-mount.test.ts b/apps/hub/test/slack-tag-mount.test.ts index 2b5bdd9843..c0036922bb 100644 --- a/apps/hub/test/slack-tag-mount.test.ts +++ b/apps/hub/test/slack-tag-mount.test.ts @@ -97,12 +97,20 @@ const SLACK_ENV_KEYS = [ ] as const; const savedEnv = new Map(SLACK_ENV_KEYS.map((key) => [key, process.env[key]])); -afterEach(() => { +afterEach(async () => { for (const key of SLACK_ENV_KEYS) { const saved = savedEnv.get(key); if (saved === undefined) Reflect.deleteProperty(process.env, key); else process.env[key] = saved; } + // Close each test's hub as soon as its test ends rather than letting + // every hub booted in this file stay live (and its background loops + // — relaunch sweep among them — keep running) until `afterAll`. Three + // live hubs contending for the same Postgres connections is exactly + // the kind of self-inflicted race this file should not add on top of + // CL-7453's fix in `apps/hub/src/index.ts`. + let closer: (() => Promise) | undefined; + while ((closer = closers.pop()) !== undefined) await closer(); }); async function bootHub(): Promise>> {