fix: make heartbeat reporting and Prisma init build-safe#371
Conversation
Superseded by a newer automated review for this pull request.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
Recommendation: Approve
This PR fixes two production-impacting bugs with minimal, well-tested changes. The implementation is sound and follows the repository's documented conventions.
Change-by-Change Findings
src/lib/prisma.ts — Lazy Prisma initialization
The core fix: replaces eager PrismaPg/PrismaClient construction at module import with a Proxy that defers initClient() until first property access.
- Proxy traps:
get,has,ownKeys,getOwnPropertyDescriptorare all implemented. Method calls viagetare correctly.bind(client)to preservethiscontext — important for Prisma's method chaining. - Error handling:
initClient()throws with a clear message if neitherDATABASE_URLnorDISPATCH_DATABASE_URLis set — matches the build-time contract documented in AGENTS.md: "The first actual use of the client at runtime should still surface a clear error." - Alias support:
databaseUrl()checksDATABASE_URLfirst, thenDISPATCH_DATABASE_URL, matching the resolution order in AGENTS.md. - Test helper:
__resetPrismaClientForTests()is exported with a__prefix and docstring marking it as internal — appropriate for a non-public test escape hatch.
src/lib/heartbeat.ts — Real heartbeat counts
Two specific fixes:
- runSyncBestEffort (line 98):
reposProcessedis now assigned fromresult.reposinstead of the hardcoded0. This correctly reflects how many repos were processed during sync. - runReconcileBestEffort (lines 168-177): Returns actual
issuesReconciled,issuesChecked, andreposProcessedfrom thereconcileClosedIssuesresult — previously it was swallowing these values and returning an incomplete object.
Test coverage
Both new test files (prisma.test.ts, heartbeat.test.ts) use vi.hoisted and vi.mock patterns consistent with the existing test suite. The prisma.test.ts tests the lazy import contract directly, including:
- No throw on import without
DATABASE_URL - Throw on first property access
- Environment alias (
DISPATCH_DATABASE_URL) works - No error leak across reset cycles
Standards Compliance
- AGENTS.md §Code Standards: Error handling uses
error instanceof Errorpattern where applicable; the new code correctly surfaces meaningful messages. - AGENTS.md §Prisma Notes: Lazy initialization resolves the documented build-time issue: "Next.js static generation runs at build time; Prisma initializes lazily."
- AGENTS.md §Environment Variables: Resolution order (
DATABASE_URL>DISPATCH_DATABASE_URL) is correctly implemented.
Linked Issue Fit
No linked issue. The PR is self-describing: it addresses a build breakage where npm run build failed without DATABASE_URL due to Next.js stripping the process.env.NODE_ENV === "production" guard during bundling, and incorrect heartbeat telemetry.
Evidence Provider Findings
No evidence providers configured for this PR.
Tool Harness Findings
No tool harness output present.
CI Check Results
CI reached terminal state with success for both "Validate" and "Docker Build" checks. The test suite and build validation completed successfully.
Unknowns / Needs Verification
- Test suite execution: The CI result shows
Validatecompleted but does not enumerate which commands ran. The PR body commits tonpm run typecheck,npm run lint,npm run test, andenv -u DATABASE_URL -u DISPATCH_DATABASE_URL npm run build. The corpus does not contain the actual test output, but the passing CI status is the authoritative signal.
Open Findings From Previous Review
None.
Summary
Validation