Skip to content
23 changes: 20 additions & 3 deletions src/lib/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { join } from 'node:path';
import type postgres from 'postgres';
import { runMigrations } from './db-migrations.js';
import { needsSeed, runSeed } from './pg-seed.js';
import { isWSL2 } from './wsl2-detect.js';

/**
* Re-export Sql type for callers that need to annotate sql connection parameters.
Expand Down Expand Up @@ -327,10 +328,16 @@ function findPgserveBin(): string {
* Avoids the self-referencing proxy deadlock that occurs when the
* MultiTenantRouter Bun TCP proxy runs in the same event loop as
* the daemon that also connects to it.
*
* On WSL2, uses an extended timeout (30s) due to slower I/O performance.
*/
async function startPgserveOnPort(port: number): Promise<number> {
mkdirSync(DATA_DIR, { recursive: true });

const isWsl = isWSL2();
const timeoutMs = isWsl ? 30000 : 15000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Align client wait window with extended WSL startup timeout

Increasing daemon-side startup timeout to 30s on WSL2 here creates a mismatch with the CLI path in _ensurePgserve() that still gives up after 16s (const deadline = Date.now() + 16000). On slower WSL2 setups, the daemon can now succeed between 16–30s, but the initiating CLI command will still throw a timeout error even though pgserve comes up shortly after, which makes normal command execution fail spuriously in the target environment.

Useful? React with 👍 / 👎.

const timeoutSec = timeoutMs / 1000;

Comment on lines +337 to +340

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

WSL2 gets a longer pgserve startup timeout here, but the CLI path (_ensurePgserve step 4b) still times out waiting for the lockfile after 16s. If pgserve initialization ever takes >16s on WSL2, CLI commands can still fail even though the daemon continues starting successfully. Consider applying the same WSL2-aware timeout to the lockfile wait path as well.

Copilot uses AI. Check for mistakes.
const child = spawn(
findPgserveBin(),
[
Expand All @@ -352,7 +359,13 @@ async function startPgserveOnPort(port: number): Promise<number> {
child.unref();
pgserveChild = child;

const deadline = Date.now() + 15000;
const initialBootstrapMs = isWsl ? 2000 : 100;
await new Promise((r) => setTimeout(r, initialBootstrapMs));

Comment on lines +362 to +364

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

This introduces a 100ms bootstrap delay on non-WSL platforms (initialBootstrapMs = ... : 100), which is a behavior change (previously health checks started immediately). If the intent is “Non-WSL2 systems unchanged” per the PR description, consider using 0 (or leaving the old behavior) for the non-WSL branch.

Copilot uses AI. Check for mistakes.
const deadline = Date.now() + timeoutMs;
let retryDelayMs = 100;
Comment on lines +362 to +366

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

deadline is computed after the initial bootstrap sleep, so the total wait from spawn → timeout becomes initialBootstrapMs + timeoutMs, but the thrown error reports only timeoutMs seconds. Consider starting the deadline before the bootstrap delay, or include the bootstrap delay in the reported timeout to keep the error message accurate.

Copilot uses AI. Check for mistakes.
const maxRetryDelayMs = 1000;

while (Date.now() < deadline) {
if (await isPostgresHealthy(port)) {
activePort = port;
Expand All @@ -361,15 +374,19 @@ async function startPgserveOnPort(port: number): Promise<number> {
writeLockfile(port);
return port;
}
await new Promise((r) => setTimeout(r, 500));

await new Promise((r) => setTimeout(r, retryDelayMs));
retryDelayMs = Math.min(Math.floor(retryDelayMs * 1.5), maxRetryDelayMs);
}

try {
child.kill('SIGTERM');
} catch {
/* dead */
}
throw new Error(`pgserve failed to start on port ${port} (timeout after 15s)`);

process.env.GENIE_PG_AVAILABLE = 'false';
throw new Error(`pgserve failed to start on port ${port} (timeout after ${timeoutSec}s)`);
}

/** Register process exit handler to clean up lockfile (once). */
Expand Down
21 changes: 19 additions & 2 deletions src/lib/mailbox.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,30 @@
/**
* Mailbox — Durable message store with unread/read semantics.
*
* ## Migration note (commit 44a153a3)
*
* Previously messages were written to `.genie/mailbox/<worker>.json` files and
* delivered by a polling loop that woke up every few seconds to check for new
* entries. This introduced latency proportional to the poll interval and made
* cross-process coordination fragile.
*
* The current implementation replaced file-based polling with PostgreSQL:
* - Messages are persisted to the `mailbox` table (durable, queryable).
* - A `AFTER INSERT` trigger fires `pg_notify('genie_mailbox_delivery', …)`

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

Grammar: “A AFTER INSERT trigger” should be “An AFTER INSERT trigger”.

Suggested change
* - A `AFTER INSERT` trigger fires `pg_notify('genie_mailbox_delivery', …)`
* - An `AFTER INSERT` trigger fires `pg_notify('genie_mailbox_delivery', …)`

Copilot uses AI. Check for mistakes.
* with payload `<to_worker>:<message_id>`.
* - `subscribeDelivery()` calls `sql.listen('genie_mailbox_delivery', …)` so
* the scheduler daemon receives the notification instantly — no polling.
* - A 30-second fallback poll catches any notifications missed during
* reconnects or daemon restarts.
Comment on lines +17 to +18

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

The header comment claims “A 30-second fallback poll catches any notifications missed…”, but this module doesn’t implement a fallback poll (subscribeDelivery only registers LISTEN/NOTIFY). Either remove that statement or move/clarify it to wherever the fallback polling is actually implemented so the docs match the code.

Suggested change
* - A 30-second fallback poll catches any notifications missed during
* reconnects or daemon restarts.

Copilot uses AI. Check for mistakes.
*
* `.genie/mailbox/` JSON files are no longer written or read; references to
* that path in older docs are outdated.
*
* Messages persist to PostgreSQL `mailbox` table before any push delivery
* attempt. This ensures durability (DEC-7).
*
* Delivery is state-aware: messages are queued and pushed to tmux
* panes only when the worker is idle (not mid-turn).
*
* PG LISTEN/NOTIFY triggers instant delivery notification on new inserts.
*/

import { v4 as uuidv4 } from 'uuid';
Expand Down
31 changes: 31 additions & 0 deletions src/lib/wsl2-detect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* WSL2 environment detection.
*
* Genie on WSL2 experiences slower pgserve startup due to I/O characteristics.
* This utility detects WSL2 and allows callers to adjust timeouts accordingly.
*/

import { readFileSync } from 'node:fs';

let memoized: boolean | null = null;

/**
* Detect if running on WSL2 by checking /proc/version for "microsoft" or "wsl".
* Result is memoized for performance.
*
* @returns true if running on WSL2, false otherwise
*/
export function isWSL2(): boolean {
// Return memoized result if already computed
if (memoized !== null) return memoized;

try {
const procVersion = readFileSync('/proc/version', 'utf-8');
memoized = procVersion.toLowerCase().includes('microsoft') || procVersion.toLowerCase().includes('wsl');
return memoized;
Comment on lines +12 to +25

Copilot AI Apr 4, 2026

Copy link

Choose a reason for hiding this comment

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

isWSL2() currently checks /proc/version for "microsoft" or "wsl", which identifies WSL in general but does not distinguish WSL2 vs WSL1. Since the function name/docs are WSL2-specific, this can return true on WSL1 as well. Consider switching to a WSL2-specific signal (e.g., /proc/sys/kernel/osrelease containing "WSL2") or renaming the API to reflect what it actually detects.

Copilot uses AI. Check for mistakes.
} catch {
// /proc/version not readable (non-Linux or permission issue) — assume not WSL2
memoized = false;
return false;
}
Comment on lines +22 to +30

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.

medium

For improved readability and slightly better performance, you can refactor this block. Calling toLowerCase() once is more efficient, and the catch block can be made more concise.

  try {
    const procVersion = readFileSync('/proc/version', 'utf-8').toLowerCase();
    memoized = procVersion.includes('microsoft') || procVersion.includes('wsl');
    return memoized;
  } catch {
    // /proc/version not readable (non-Linux or permission issue) — assume not WSL2
    return (memoized = false);
  }

}
Loading