Skip to content

fix(serve): boot probe uses resolvePgserveTransport (matches connection probe) - #1672

Merged
namastex888 merged 1 commit into
devfrom
fix/serve-boot-probe-uses-new-resolver
May 6, 2026
Merged

namastex888 merged 1 commit into
devfrom
fix/serve-boot-probe-uses-new-resolver

Conversation

@namastex888

Copy link
Copy Markdown
Contributor

Summary

Cosmetic followup to #1667. Pre-this-fix, genie serve boot called requirePgserveDaemon() (UDS-only) and printed a misleading "pgserve unreachable" warning on hosts where pgserve install registered foreground TCP mode — even though real connections then succeeded via the resolver's TCP fallback. Operators read the warning, assumed something was broken, wasted time on the wrong fix.

What changed

-async function requirePgserveReady(): Promise<void> {
-  console.log('  Probing canonical pgserve daemon...');
-  try {
-    const { requirePgserveDaemon, resolvePgserveSocketDir } = await import('../lib/db.js');
-    await requirePgserveDaemon();
-    console.log(`  pgserve daemon ready (canonical, pm2-supervised) on ${resolvePgserveSocketDir()}`);
+async function requirePgserveReady(): Promise<void> {
+  console.log('  Probing pgserve transport...');
+  try {
+    const { resolvePgserveTransport } = await import('../lib/db.js');
+    const transport = await resolvePgserveTransport();
+    if (transport.kind === 'unix') {
+      console.log(`  pgserve ready: unix socket ${transport.socketDir}/.s.PGSQL.${transport.port}`);
+    } else {
+      console.log(`  pgserve ready: tcp ${transport.host}:${transport.port}`);
+    }

Same retry-guard semantics on failure (both GENIE_PG_NO_AUTOSTART env vars set, no exit). The misleading legacy "Recovery: pm2 status / pm2 restart pgserve / pgserve install" lines drop because the resolver throws a richer message that already mentions both probe attempts and recovery steps.

After this ships

Fresh genie update + pm2 restart genie-serve on a TCP-only host will show:

  Probing pgserve transport...
  pgserve ready: tcp 127.0.0.1:8432

…instead of the noisy "unreachable" warning. No operator action required.

Diagnostic context

While the user was dogfooding the post-#1667 binary, their ~/.genie/logs/update-diagnostics-2026-05-06T15-20-52-347Z.json showed 141 scheduler errors over 30 min: 7 distinct event types (process_cycle_error, agent_resume_timer_error, mailbox_retry_error, heartbeat_error, lease_recovery_error, orphan_reconciliation_error, retention_error), each with the legacy "pgserve canonical daemon is not reachable" hint. Investigation showed those errors were all from the pre-#1667 daemon process still running pre-restart — once the daemon picks up the new code via pm2 restart genie-serve, every error path goes through _buildConnection() → resolvePgserveTransport() and succeeds via TCP fallback. No scheduler-side change needed in this PR.

Validation

  • ✅ bun run typecheck
  • ✅ bun run lint (2 pre-existing test-fixture symlink warnings)
  • ✅ bun test src/term-commands/serve.test.ts — 19 pass / 0 fail / 71 expects

Test plan

  • CI: typecheck + lint + dead-code + tests on dev
  • On a TCP-only pgserve host, run genie serve → confirm pgserve ready: tcp ... banner instead of legacy "unreachable" warning
  • On a UDS host (running pgserve daemon standalone), confirm pgserve ready: unix socket ... banner
  • On a host with neither, confirm retry-guard still fires (GENIE_PG_NO_AUTOSTART=1 set)

Related

  • #1665 — update-unify-stages G1-G5 (merged)
  • #1667 — pgserve transport discovery (merged)
  • #1668 — SHARED-DESIGN.md §4.6 sync (merged)
  • #1671 — skills .orphaned_at regression fix
  • THIS PR — boot-probe migration

🤖 Generated with Claude Code

…on probe)

Cosmetic followup to #1667. Pre-this-fix, `genie serve` boot called
`requirePgserveDaemon()` which only accepted the canonical Unix socket.
On hosts where `pgserve install` registered foreground TCP mode (the
supported install path post pgserve@^2.2 — see
pgserve/src/cli-install.cjs:225-249), the boot probe printed:

  Probing canonical pgserve daemon...
  pgserve unreachable: pgserve canonical daemon is not reachable (no daemon).
  Recovery:
    pm2 status              # is pgserve registered?
    pm2 restart pgserve     # OR: autopg restart
    pgserve install         # if not registered yet

…AND THEN real connections succeeded via the resolver's TCP fallback.
Operators read the warning, assumed something was broken, and wasted
time on the wrong fix. Diagnosed live during 2026-05-06 dogfood after
the user ran `genie update` and saw 141 scheduler errors / 30 min
in the diagnostic file (all the pre-#1667 binary's UDS-only probes).

Fix: replace the boot probe with the same `resolvePgserveTransport()`
the connection layer uses. UDS-first, TCP-fallback. Banner now reports
the truth:
- UDS reachable: `pgserve ready: unix socket <socketDir>/.s.PGSQL.<port>`
- TCP only:     `pgserve ready: tcp <host>:<port>`
- Both down:    `pgserve unreachable: <both-transports hint>` + retry-guard

Tests (src/term-commands/serve.test.ts) updated accordingly:
- 'boot probe uses the unified resolvePgserveTransport resolver' replaces
  the legacy `requirePgserveDaemon` source-string lock.
- 'boot probe branches on transport.kind for the ready banner' locks the
  UDS-vs-TCP banner branching.
- 'boot probe disables in-process pgserve retries' (renamed from
  'serve disables in-process pgserve retries') unchanged in spirit.
- Removed 'serve emits the canonical-cutover ready banner on success' —
  the wording changed; new banner asserted in the kind-branching test.

Validation: bun run typecheck ok, bun run lint ok, bun test
src/term-commands/serve.test.ts — 19 pass / 0 fail / 71 expects.

After this ships, fresh `genie update` + `pm2 restart genie-serve` on
a host with foreground-TCP pgserve will show:

  Probing pgserve transport...
  pgserve ready: tcp 127.0.0.1:8432

…instead of the misleading "unreachable" warning, with no operator
action required. The 141-errors-per-30min scheduler storm in the
diagnostic file was already from the pre-#1667 daemon and stops on
restart — no scheduler-side change needed in this PR.
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 478f7ccf-cca2-4ba0-bc02-d1d8a407e66f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/serve-boot-probe-uses-new-resolver

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the requirePgserveReady function to utilize the unified resolvePgserveTransport resolver, which supports both Unix Domain Sockets and TCP fallbacks. This change fixes a bug where TCP-only installations would trigger a misleading 'unreachable' error message. The associated tests have been updated to reflect this new transport discovery logic. A review comment suggests improving the formatting of multi-line error messages by ensuring consistent indentation when they are printed to the console.

I am having trouble creating individual review comments. Click here to see my feedback.

src/term-commands/serve.ts (635)

medium

The error message returned by resolvePgserveTransport (via buildBothTransportsUnavailableHint) is a multi-line string containing bullet points and recovery steps. When printed directly with a single prefix, subsequent lines will lose their indentation, making the output harder to read and inconsistent with the rest of the boot logs. Indenting all lines of the message ensures the visual structure is preserved.

    console.error("  pgserve unreachable: " + msg.replace(/\n/g, "\n  "));

@namastex888
namastex888 merged commit 9eb0537 into dev May 6, 2026
15 checks passed
@automagik-genie
automagik-genie deleted the fix/serve-boot-probe-uses-new-resolver branch September 25, 2026 04:50
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.

1 participant