Skip to content

fix(start-local): close 7 PGlite/Postgres parity-hygiene risks#943

Merged
buremba merged 7 commits into
mainfrom
feat/audit-hygiene
May 20, 2026
Merged

fix(start-local): close 7 PGlite/Postgres parity-hygiene risks#943
buremba merged 7 commits into
mainfrom
feat/audit-hygiene

Conversation

@buremba
Copy link
Copy Markdown
Member

@buremba buremba commented May 19, 2026

Summary

Closes 7 hygiene gaps where the PGlite entrypoint (start-local.ts) silently diverged from the Postgres entrypoint (server.ts) without justification. Each finding is from the parity audit, with file:line refs preserved in commit messages. All patches are 1–6 LOC; no behavior change for happy-path users.

Audit plan: /Users/burakemre/.claude/plans/pglite-postgres-parity-audit.md §3.

The 7 risks (ordered by blast radius from audit §3)

1. stopLobuGateway() missing on shutdown — commit 5bbf7fa3

Audit §2.10: "PGlite mode skips all of that on shutdown. Locally usually fine (process exits), but during a pkill -TERM test or supervisor restart, mid-flight MCP sessions / worker subprocesses leak."

Change: added stopLobuGateway import + await stopLobuGateway() in shutdown(), ordered before PGlite-owned resources (socket server, db.close) so postgres.js connections drain through the socket cleanly. Mirrors server.ts:258.

2. closeDbSingleton() missing on shutdown — commit b730f5f1

Audit §2.10: "PGlite uses postgres.js too (via the socket-server adapter) and the singleton accumulates pooled connections."

Change: import closeDbSingleton from ./db/client, call it after stopLobuGateway and before socketServer.stop(). Mirrors server.ts:259.

3. Sentry post-response 5xx + app.onError handler missing — commit 0dc5fbfb

Audit §2.2: "any handler that internally try/catches and returns c.json(..., 500) is never captured by Sentry in PGlite mode."

Change: ported the two app.use('*', …) blocks (response-status capture) and the app.onError block from server.ts:97-150. Both Sentry.captureMessage/captureException are no-ops when Sentry.init ran without DSN, so safe on no-DSN installs (the common PGlite case).

4. ./instrument import missing — commit 1777f167

Audit §2.8: "PGlite mode never calls Sentry.init(). Even with SENTRY_DSN set, no spans, no autocapture, no DB autoinstrumentation."

Change: one-line import './instrument'; immediately after assert-node-version. instrument.ts is a guarded no-op when SENTRY_DSN is unset.

5. LOBU_DEV_PROJECT_PATH default missing — commit 4e7c0d0f

Audit §2.9: "downstream buildGatewayConfig() in initLobuGateway derives worker paths from this; PGlite users running from a project subdir get a wrong cwd-relative resolve."

Change: ported the 3-line default block from server.ts:64-66 (resolves repo root from this source file, sets env var only if unset). References the existing CLAUDE.md note on lobu run from a project subdir.

6. assertExternalDepsResolvable post-listen missing — commit ec434d82

Audit §2.12: "PGlite mode lets a missing connector dep surface as a per-run 'Missing npm dependency: X' hours later, exactly the failure mode the assert was added to prevent."

Change: ported the 6-line try/catch from server.ts:319-324 into the httpServer.listen callback, before the embedded daemon starts. Uses the existing require (createRequire(import.meta.url)).

7. serializeBootError + stderr fallback missing — commit 047a8bdf

Audit §2.10: "the exact failure mode that #766 fixed (docker logs showing 'err':{}) recurs the moment PGlite mode hits a ZodError / AggregateError / wrapped-cause failure during boot."

Change: copied the serializeBootError helper and the stderr-fallback main-catch from server.ts:344-388. Same recursion behavior for cause / errors / issues.

Reproducer / verification

Smoke boot ran against dist/start-local.bundle.mjs (Node 22 from /opt/homebrew/opt/node@22/bin, ephemeral LOBU_DATA_DIR=/tmp/lobu-smoke-data, LOBU_ALLOW_EPHEMERAL_ENCRYPTION_KEY=1, free PORT=8799 + WORKER_PROXY_PORT=8129).

Happy-path boot: clean.

  • Lobu running at http://127.0.0.1:8799 reached
  • Embedded daemon started after listen
  • (Sentry visible — local env had SENTRY_DSN configured; sentry-trace baggage headers present in request logs)

SIGTERM: clean shutdown.

  • Shutting down log fires
  • [gateway] Stopping platform: … for every platform → proves stopLobuGateway() ran (risks 1)
  • Embedded gateway stopped → confirms gateway awaited
  • PGlite socket server closed → confirms ordering held (risks 2)
  • Process exited cleanly

Boot-error path: LOBU_DATA_DIR=/dev/null/cant-exist triggered ENOTDIR from mkdirSync. Output:

Validation

  • make typecheck: clean
  • make build-packages: clean (both start-local.bundle.mjs and server.bundle.mjs rebuild without warnings)

Notes

  • Item 6 in the request prompt was loosely described as "env-injection middleware divergence"; the audit plan classified that as defer / document as intentional and listed assertExternalDepsResolvable as the actual 6th hygiene patch. Patch set follows the plan ordering.
  • Audit §4 refactor to a shared buildApp(deps) is intentionally not included — separate decision per audit author.
  • packages/owletto untouched.

Summary by CodeRabbit

  • Bug Fixes

    • Improved server stability during graceful shutdown by ensuring services and DB connections are closed in the correct order.
    • Unified handling of unhandled handler errors to return a consistent generic 500 response and ensure errors are reported to monitoring.
  • Chores

    • Integrated automatic error-monitoring and de-duplication during startup and request handling.
    • Added structured error serialization, enhanced logging, and an early external-dependency validation at boot.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d6e65344-07b6-4256-a678-9d9fd68657e3

📥 Commits

Reviewing files that changed from the base of the PR and between 047a8bd and 1c6d720.

📒 Files selected for processing (1)
  • packages/server/src/start-local.ts

📝 Walkthrough

Walkthrough

The PR hardens local server boot in start-local.ts: it adds early Sentry instrumentation and runtime imports, sets a repo-root dev path fallback, captures gateway stop for shutdown, installs HTTP 5xx/error reporting with request-scoped de-dup, performs a post-listen external-deps check, and replaces boot error logging with structured serialization.

Changes

Server Boot Hardening with Sentry and Graceful Shutdown

Layer / File(s) Summary
Sentry Auto-Instrumentation Setup
packages/server/src/start-local.ts
Early import of ./instrument placed after Node version assertion, and expanded imports to include Sentry runtime, assertExternalDepsResolvable, DB/migration close helpers, and Sentry request de-dup utilities.
Repo-root Fallback, Gateway Wiring, and Shutdown Ordering
packages/server/src/start-local.ts
Adds PACKAGE_REPO_ROOT fallback and sets LOBU_DEV_PROJECT_PATH when unset; captures stopLobuGateway from initLobuGateway(); reorders graceful shutdown to call stopLobuGateway() and closeDbSingleton() before closing the HTTP server.
HTTP Error Capture and wrapper.onError
packages/server/src/start-local.ts
Adds Hono middleware to parse JSON bodies for 5xx responses and report errors to Sentry; adds wrapper.onError to capture handler exceptions with request-scoped de-duplication and return a generic 500 { error: "Internal server error" }.
Post-listen External-Dependency Validation
packages/server/src/start-local.ts
After listen(), runs assertExternalDepsResolvable(require.resolve) to fail fast on missing connector external dependencies and exit with error logging if unresolved.
Boot Error Serialization and Exit
packages/server/src/start-local.ts
Replaces the previous top-level catch logger with serializeBootError, which recursively serializes error/cause chains for structured logs and emits plain-text stderr (including stack) before exiting with code 1.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • lobu-ai/lobu#943: The current PR modifies packages/server/src/start-local.ts for Sentry init, shutdown ordering, 5xx/error capture, external-deps check, and boot error serialization (same PR identifier retrieved by vector search).
  • lobu-ai/lobu#775: Related changes coordinating Sentry forwarding and de-duplication markers across logging utilities and server error reporting.
  • lobu-ai/lobu#940: Modifies initLobuGateway() usage and overlaps with gateway initialization/shutdown wiring touched here.

Poem

🐰 I hopped where startup lights ignite,
Sentry listened through the night,
Gateway sleeps, DB softly closed,
Errors logged and loudly posed—
Booted safe, the rabbit smiles bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: closing 7 parity-hygiene risks between PGlite and Postgres entrypoints.
Description check ✅ Passed The PR description is comprehensive, structured, and follows the template with Summary, Test plan, and Notes sections all completed with detailed information.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/audit-hygiene

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants