diff --git a/src/lib/heartbeat.test.ts b/src/lib/heartbeat.test.ts new file mode 100644 index 0000000..ed04cd6 --- /dev/null +++ b/src/lib/heartbeat.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +const { mocks } = vi.hoisted(() => ({ + mocks: { + reconcileClosedIssues: vi.fn(), + getSyncRepos: vi.fn(), + syncIssuesForRepos: vi.fn(), + parseExcludedLabels: vi.fn(), + }, +})); + +vi.mock("@/lib/issue-sync", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + reconcileClosedIssues: mocks.reconcileClosedIssues, + syncIssuesForRepos: mocks.syncIssuesForRepos, + mergeLabels: actual.mergeLabels, + }; +}); + +vi.mock("@/lib/config", () => ({ + getSyncRepos: mocks.getSyncRepos, + parseExcludedLabels: mocks.parseExcludedLabels, +})); + +vi.mock("@/lib/github", () => ({ + fetchIssues: vi.fn(), +})); + +vi.mock("@/lib/prisma", () => ({ + prisma: {}, +})); + +import { runSyncBestEffort, runReconcileBestEffort } from "@/lib/heartbeat"; + +describe("runReconcileBestEffort", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("reports actual counts from reconcileClosedIssues", async () => { + mocks.getSyncRepos.mockResolvedValue([{ id: "repo-1", fullName: "org/repo" }]); + mocks.reconcileClosedIssues.mockResolvedValue({ + success: true, + reposProcessed: 1, + issuesChecked: 5, + issuesReconciled: 3, + results: [], + }); + + const result = await runReconcileBestEffort(); + + expect(result.issuesReconciled).toBe(3); + expect(result.issuesChecked).toBe(5); + expect(result.reposProcessed).toBe(1); + }); +}); + +describe("runSyncBestEffort", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("reports actual reposProcessed from syncIssuesForRepos", async () => { + mocks.getSyncRepos.mockResolvedValue([ + { id: "repo-1", fullName: "org/repo" }, + { id: "repo-2", fullName: "org/repo2" }, + ]); + mocks.parseExcludedLabels.mockReturnValue([]); + mocks.syncIssuesForRepos.mockResolvedValue({ + success: true, + repos: 2, + syncedCount: 10, + results: [], + }); + + const result = await runSyncBestEffort(); + + expect(result.reposProcessed).toBe(2); + }); +}); diff --git a/src/lib/heartbeat.ts b/src/lib/heartbeat.ts index a6de0d2..1fa7550 100644 --- a/src/lib/heartbeat.ts +++ b/src/lib/heartbeat.ts @@ -42,6 +42,7 @@ export async function runSyncBestEffort( const errors: string[] = []; const touchedIssueUrls: string[] = []; let syncedCount = 0; + let reposProcessed = 0; try { const repos = await getSyncRepos(); @@ -77,6 +78,7 @@ export async function runSyncBestEffort( }, excludedLabels); syncedCount = result.syncedCount; + reposProcessed = result.repos; for (const r of result.results) { if (r.error) { @@ -96,7 +98,7 @@ export async function runSyncBestEffort( errors.push(`Sync failed: ${message}`); } - return { synced: syncedCount, reposProcessed: 0, warnings, errors, touchedIssueUrls }; + return { synced: syncedCount, reposProcessed, warnings, errors, touchedIssueUrls }; } // --------------------------------------------------------------------------- @@ -166,6 +168,13 @@ export async function runReconcileBestEffort(): Promise { if (!result.success) { errors.push("Reconciliation completed with one or more failures"); } + return { + issuesReconciled: result.issuesReconciled, + issuesChecked: result.issuesChecked, + reposProcessed: result.reposProcessed, + warnings, + errors, + }; } catch (error) { const message = error instanceof Error ? error.message : "Unknown reconcile error"; errors.push(`Reconciliation failed: ${message}`); diff --git a/src/lib/prisma.test.ts b/src/lib/prisma.test.ts new file mode 100644 index 0000000..62b7e6c --- /dev/null +++ b/src/lib/prisma.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; + +/** + * Build-time contract for the prisma client. + * + * `next build` performs page-data collection that imports every route handler + * and page module. Importing `@/lib/prisma` must NOT throw, even when + * `DATABASE_URL` is intentionally absent (CI / local dev / static export + * steps). The first actual *use* of the client at runtime should still + * surface a clear error if the env var is missing. + * + * The previous eager implementation threw at module load (and Next.js's + * page-data collection triggered it because the bundler inlines + * `process.env.NODE_ENV === "production"` to `true` during `next build`, + * stripping the guard). The fix is a lazy Proxy that defers + * `PrismaPg`/`PrismaClient` construction until first property access. + */ + +const ORIGINAL_DATABASE_URL = process.env.DATABASE_URL; +const ORIGINAL_DISPATCH_DATABASE_URL = process.env.DISPATCH_DATABASE_URL; +const ORIGINAL_NODE_ENV = process.env.NODE_ENV; + +describe("prisma module lazy initialization", () => { + beforeEach(() => { + vi.resetModules(); + const env = process.env as Record; + delete env.DATABASE_URL; + delete env.DISPATCH_DATABASE_URL; + delete env.NODE_ENV; + }); + + afterEach(() => { + // Restore a working DATABASE_URL for downstream tests that may share + // the worker; vitest.setup.ts also sets a default. + if (ORIGINAL_DATABASE_URL !== undefined) { + process.env.DATABASE_URL = ORIGINAL_DATABASE_URL; + } else { + process.env.DATABASE_URL = "postgresql://test:test@localhost:5432/dispatch_test"; + } + if (ORIGINAL_DISPATCH_DATABASE_URL !== undefined) { + process.env.DISPATCH_DATABASE_URL = ORIGINAL_DISPATCH_DATABASE_URL; + } + if (ORIGINAL_NODE_ENV !== undefined) { + (process.env as Record).NODE_ENV = ORIGINAL_NODE_ENV; + } + }); + + it("does not throw on import when DATABASE_URL is unset", async () => { + await expect(import("./prisma")).resolves.toBeDefined(); + }); + + it("does not construct a PrismaClient on import when DATABASE_URL is unset", async () => { + // We can't directly observe the PrismaPg/PrismaClient constructors + // without mocking, but we can assert that `initClient` is not + // exercised by ensuring no delegated call yet exists: the proxy is + // present, but accessing a non-existent property returns undefined + // (not throw) because initClient throws only on first call. + // + // If prisma were eager, this import would have thrown before we got + // here, failing the previous test. + const mod = await import("./prisma"); + expect(typeof mod.prisma).toBe("object"); + // Reading symbols that the lazy proxy would resolve by calling + // initClient() should throw (no DATABASE_URL). + expect(() => (mod.prisma as any).repository).toThrow(/DATABASE_URL/); + }); + + it("throws with a clear message when DATABASE_URL is missing on first access", async () => { + const mod = await import("./prisma"); + expect(() => (mod.prisma as any).issue).toThrow(/DATABASE_URL is not set/); + expect(() => (mod.prisma as any).$transaction).toThrow(/DATABASE_URL is not set/); + }); + + it("does not leak the missing-env error across reset cycles", async () => { + const mod = await import("./prisma"); + // First access throws. + expect(() => (mod.prisma as any).repository).toThrow(/DATABASE_URL/); + // After we set DATABASE_URL and reset the cached client, next access + // should NOT throw the missing-env error. (We don't actually connect, + // we only verify the env gate is past.) + process.env.DATABASE_URL = "postgresql://user:pass@localhost:5432/db"; + mod.__resetPrismaClientForTests(); + expect(() => (mod.prisma as any).repository).not.toThrow(/DATABASE_URL/); + }); + + it("accepts DISPATCH_DATABASE_URL as the documented alias", async () => { + process.env.DISPATCH_DATABASE_URL = "postgresql://user:pass@localhost:5432/db"; + const mod = await import("./prisma"); + expect(() => (mod.prisma as any).repository).not.toThrow(/DATABASE_URL/); + }); +}); diff --git a/src/lib/prisma.ts b/src/lib/prisma.ts index 89a85e9..00a3969 100644 --- a/src/lib/prisma.ts +++ b/src/lib/prisma.ts @@ -3,26 +3,97 @@ import { PrismaPg } from "@prisma/adapter-pg"; import { PrFixQueueClient } from "@/lib/pr-fix-queue"; import { AgentWorkClient } from "@/lib/agent-work"; -if (process.env.NODE_ENV === "production" && !process.env.DATABASE_URL) { - throw new Error( - "DATABASE_URL is not set. Please set the DATABASE_URL environment variable before starting the application.", - ); -} - -const adapter = new PrismaPg(process.env.DATABASE_URL!); - const globalForPrisma = globalThis as unknown as { prisma: PrismaClient | undefined; }; -export const prisma = - globalForPrisma.prisma ?? - new PrismaClient({ +let _client: PrismaClient | undefined; + +function databaseUrl(): string | undefined { + return process.env.DATABASE_URL ?? process.env.DISPATCH_DATABASE_URL; +} + +function initClient(): PrismaClient { + if (_client) return _client; + if (globalForPrisma.prisma) { + _client = globalForPrisma.prisma; + return _client; + } + + const url = databaseUrl(); + if (!url) { + throw new Error( + "DATABASE_URL is not set. Please set DATABASE_URL or DISPATCH_DATABASE_URL before starting the application.", + ); + } + + const adapter = new PrismaPg(url); + const client = new PrismaClient({ adapter, log: process.env.NODE_ENV === "development" ? ["error", "warn"] : ["error"], }); -if (process.env.NODE_ENV !== "production") globalForPrisma.prisma = prisma; + // Cache on globalThis in non-production to preserve connection pooling + // across HMR reloads (Next.js dev server) and to keep the previously + // created instance visible to vitest workers that re-evaluate this module. + if (process.env.NODE_ENV !== "production") { + globalForPrisma.prisma = client; + } + _client = client; + return client; +} + +/** + * Lazy Prisma client. + * + * Defers `PrismaPg`/`PrismaClient` construction (and the `DATABASE_URL` + * presence check) until the first property access on the client. This lets + * route handlers, pages, and other modules `import { prisma } from + * "@/lib/prisma"` at build time without throwing when `DATABASE_URL` is + * intentionally absent - for example, during `next build` page-data + * collection in CI or local dev. + * + * The first call to any model delegate or `$transaction`/`$queryRaw`/etc. + * during a real request will still surface a clear error if the env var is + * missing. This matches the build-time contract documented in AGENTS.md. + * + * The Proxy is typed as `PrismaClient` and forwards every property access + * to the lazily-constructed underlying client, binding methods so `this` + * stays correct. + */ +export const prisma = new Proxy({} as PrismaClient, { + get(_target, prop) { + const client = initClient(); + const value = (client as unknown as Record)[ + prop as PropertyKey + ]; + return typeof value === "function" + ? (value as (...args: unknown[]) => unknown).bind(client) + : value; + }, + has(_target, prop) { + return prop in (initClient() as unknown as object); + }, + ownKeys() { + return Reflect.ownKeys(initClient() as unknown as object); + }, + getOwnPropertyDescriptor(_target, prop) { + return Reflect.getOwnPropertyDescriptor( + initClient() as unknown as object, + prop, + ); + }, +}); + +/** + * Internal: reset the cached client. Intended for tests that need to + * re-evaluate the env between cases. Not part of the stable surface used + * by route handlers. + */ +export function __resetPrismaClientForTests(): void { + _client = undefined; + globalForPrisma.prisma = undefined; +} export function asPrFixQueueClient(client: PrismaClient): PrFixQueueClient { return {