Files
gbrain/test/config-env.test.ts
T
9c60b3a068 v0.31.3 fix: stdio MCP graceful cleanup + engine-aware auth/admin SQL (closes #413, #446) (#801)
* fix(serve): clean up stdio MCP server on client disconnect

The PGLite write lock leaked indefinitely when the parent of `gbrain serve`
disconnected. Three root causes: serve.ts never called engine.disconnect()
after startMcpServer() resolved; cli.ts short-circuited with a "serve doesn't
disconnect" comment; and the MCP SDK's StdioServerTransport only listens for
'data'/'error' on stdin, never 'end'/'close', so even a clean stdin EOF never
reached the SDK.

Net effect: the next `gbrain serve` waited for the in-process 5-minute stale-
lock check or hung indefinitely.

stdio path now installs a unified lifecycle:
- SIGTERM/SIGINT/SIGHUP all funnel into one idempotent shutdown path
  (SIGHUP coverage matters for Claude Desktop on macOS / MCP gateway
  restarts; SIGINT for Ctrl-C; SIGTERM for daemon shutdown).
- stdin 'end' (clean EOF) and 'close' (parent SIGKILL with pipe still
  open) both trigger the same graceful path. TTY stdin skips the watchers
  so interactive `gbrain serve` is unaffected.
- Parent-process watchdog polls the live kernel parent PID via spawnSync
  ('ps','-o','ppid=','-p',PID) every 5s. process.ppid is cached at process
  creation by Bun (and Node) and never refreshes on re-parent — empirical
  evidence on macOS shows ps reports the new parent within one tick while
  process.ppid stays at the original PID indefinitely (oven-sh/bun#30305).
- Watchdog fires on `getParentPid() !== initialParentPid` (any reparent),
  not just `=== 1`. Catches launchd / systemd / tmux / parent-shell-with-
  PR_SET_CHILD_SUBREAPER cases where the kernel re-anchors us to a non-1
  subreaper PID. Codex review caught the original `=== 1` was incomplete.
- One-shot startup probe verifies `spawnSync('ps')` actually works on this
  host. If the probe fails (stripped containers / busybox without procps),
  we skip installing the watchdog interval entirely AND emit a loud stderr
  line — the operator sees "watchdog disabled" instead of an installed-
  but-never-fires phantom that silently falls back to cached process.ppid.
- 5-second cleanup deadline: if engine.disconnect() wedges (PGLite WASM
  stall, etc.), the process still calls process.exit(0). The abandoned
  lock dir is reclaimed on the next start by the existing stale-lock
  check in pglite-lock.ts.
- Optional `--stdio-idle-timeout <sec>`: default OFF safety net for
  parents that leak the pipe but never close it. Strict parsing rejects
  `abc` / `30junk` / `-1` / `1.5` / blank values explicitly so a typo
  doesn't silently disable the safety net (closes #446).

Test seam: ServeOptions { stdin, signals, exit, log, startMcpServer,
getParentPid, setInterval, clearInterval, probeWatchdog } lets the
lifecycle be unit-tested deterministically without spawning a real Bun
child or booting the MCP SDK.

22 test cases covering signals, stdin EOF, TTY skip, watchdog reparent
(both PID-1 and subreaper-PID-N cases), ps-unavailable degraded mode,
idle timeout, idempotent shutdown, and cleanup-deadline behavior.

Closes #413, #446. Supersedes #591.

Co-Authored-By: Aragorn2046 <noreply@github.com>
Co-Authored-By: seungsu-kr <noreply@github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(auth): route HTTP auth/admin SQL through active engine

`gbrain auth` and `gbrain serve --http` previously routed every SQL
through the postgres.js singleton in src/core/db.ts, which silently fell
back to a file-backed PGLite when DATABASE_URL was set but the config
file disagreed. The HTTP transport's verbatim use of the singleton also
made `gbrain serve --http` Postgres-only, even though the
`access_tokens` and `mcp_request_log` tables exist in both engine
schemas.

Auth, OAuth, admin, file uploads, and HTTP-transport SQL now run through
`engine.executeRaw` via a deliberately narrow tagged-template adapter
(`src/core/sql-query.ts`). The contract is scalar-binds-only — adding
JSONB or fragment composition would invite the adapter to drift into a
partial postgres.js clone. JSONB writes use a separate
`executeRawJsonb(engine, sql, scalarParams, jsonbParams)` helper that
composes positional `$N::jsonb` casts and passes objects through
`engine.executeRaw`. The CI guard at `scripts/check-jsonb-pattern.sh`
doesn't fire because the helper is a method call, not the banned
`${JSON.stringify(x)}::jsonb` template-literal interpolation, and the
v0.12.0 double-encode bug class doesn't apply to positional binding via
`postgres.js`'s `unsafe()` (verified by
`test/e2e/auth-permissions.test.ts:67` on Postgres and the new
`test/sql-query.test.ts` on PGLite).

Migrated call sites:
  - src/commands/auth.ts: takes-holders writes (lines 52, 86) →
    executeRawJsonb. List, revoke, register-client, revoke-client →
    SqlQuery via withConfiguredSql() helper that opens an engine, runs
    the callback, disconnects.
  - src/commands/serve-http.ts: ~25 call sites including the four
    mcp_request_log.params INSERTs (now write real JSONB objects, not
    JSON-encoded strings — the read side `params->>'op'` returns the
    operation name, closing CLAUDE.md's outstanding "JSON-string-into-
    JSONB" note as a side effect). The /admin/api/requests dynamic
    filter pattern (postgres.js fragment composition) is rewritten as
    parametrized SQL string + params array.
  - src/mcp/http-transport.ts: legacy bearer-auth path. The
    Postgres-only fail-fast at startup is removed because both schemas
    now carry access_tokens + mcp_request_log.
  - src/core/oauth-provider.ts: SqlQuery / SqlValue types relocated
    from here to sql-query.ts as the canonical home (Codex finding #8).
  - src/commands/files.ts: all 5 db.getConnection() sites (lines 104,
    139, 252, 326, 355). The line-256 INSERT into files.metadata uses
    executeRawJsonb; the other four are scalar-only SqlQuery (Codex
    finding #6 — scope was bigger than the plan's "lone INSERT" framing).
  - src/core/config.ts: env-var DATABASE_URL inference. When dbUrl is
    set, infer Postgres engine and clear the stale database_path.

Engine-internal sql.json() sites in src/core/postgres-engine.ts (5
sites: lines 520, 1689, 1728, 1790, 2313) STAY UNCHANGED. They live
inside PostgresEngine itself, where the postgres.js template-tag
sql.json() pattern is correct — those methods are only loaded when
Postgres is the active engine, so there's no PGLite-routing concern.

Migration v45 (mcp_request_log_params_jsonb_normalize): one-shot UPDATE
that lifts pre-v0.31 string-shaped JSONB rows to objects so the
/admin/api/requests endpoint at serve-http.ts:605 returns one
consistent shape to the admin SPA. Idempotent (subsequent runs find no
rows where jsonb_typeof = 'string'). Closes the mixed-shape window
that would otherwise have made post-deploy admin reads break.

Tests:
  - test/sql-query.test.ts: 7 cases covering scalar binds, the
    .json() rejection (defense in depth — SqlQuery is scalar-only),
    JSONB round-trip with `jsonb_typeof = 'object'` and `->>`
    semantics, the v0.12.0 double-encode regression guard, null
    JSONB handling, and the scalars-then-jsonb call shape.
  - test/config-env.test.ts: migrated from PR's manual `restoreEnv()`
    in afterEach to the canonical `withEnv()` helper at
    test/helpers/with-env.ts (CLAUDE.md R1 / codex finding D3).
    Five cases covering DATABASE_URL precedence, GBRAIN_DATABASE_URL
    operator override, file-only config, env-only config, and the
    no-config null path.
  - test/e2e/auth-takes-holders-pglite.test.ts: 6 cases against
    in-memory PGLite (no DATABASE_URL gate). Covers create / update /
    read of access_tokens.permissions, mcp_request_log.params object
    + null writes, and the migration v45 normalizer (seed
    string-shaped row, run UPDATE, assert object shape; second-run
    no-op for idempotency).
  - test/http-transport.test.ts: mock updated to intercept
    engine.executeRaw (the new code path) instead of the postgres.js
    template tag. 24 cases pass.

Plan reference: ~/.claude/plans/system-instruction-you-are-working-peppy-moore.md.
Codex outside-voice review applied: D-codex-1, D-codex-2, D-codex-5,
D-codex-8, D-codex-9, D-codex-10 (and D1, D5 reversed by codex).

Closes the architectural intent of #681. Supersedes its branch.

Co-Authored-By: codex-bot <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: update CLAUDE.md key files for v0.31.3

Annotate the v0.31.3 changes in the canonical Key Files section:
new src/core/sql-query.ts adapter (#681), src/commands/serve.ts stdio
cleanup (#676), v0.31.3 amendments to auth.ts / serve-http.ts /
oauth-provider.ts surfaces, and migration v46 normalizer in migrate.ts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* chore: regenerate llms-full.txt for v0.31.3 docs sync

CI's build-llms test asserts the committed llms.txt + llms-full.txt
match what scripts/build-llms.ts produces from current source state.
CLAUDE.md was amended by /document-release post-merge (new entries for
src/core/sql-query.ts and src/commands/serve.ts; amended notes on
auth.ts / serve-http.ts / migrate.ts), so the inlined-bundle fell out
of sync. Regenerated via `bun run build:llms`.

llms.txt unchanged (curated index — no new web URLs added).
llms-full.txt updated to inline the new CLAUDE.md content.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Aragorn2046 <noreply@github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 22:58:19 -07:00

132 lines
4.7 KiB
TypeScript

import { mkdtempSync, rmSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import { describe, expect, test } from 'bun:test';
import { loadConfig, saveConfig } from '../src/core/config.ts';
import { withEnv } from './helpers/with-env.ts';
// PR #681 originally shipped a manual `restoreEnv()` in afterEach for these
// tests. CLAUDE.md R1 (test-isolation lint) and the codex outside-voice
// review (D3 in the v0.31 plan) called that out — manual try/restore patterns
// don't survive an assertion failure mid-test, the codemod for parallel
// test execution can't whitelist them, and the canonical helper at
// test/helpers/with-env.ts already exists for this. Migrating here.
//
// withEnv()'s try/finally restores even when the callback throws, so a
// failed expect() inside the block leaves the process env clean for the
// next file in the shard.
describe('loadConfig env database URL precedence', () => {
test('DATABASE_URL switches an existing PGLite file config to Postgres', async () => {
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-env-'));
try {
// Pre-seed: PGLite file config in this isolated GBRAIN_HOME.
await withEnv(
{ GBRAIN_HOME: home, GBRAIN_DATABASE_URL: undefined, DATABASE_URL: undefined },
() => {
saveConfig({ engine: 'pglite', database_path: '/tmp/local-brain.pglite' });
},
);
// DATABASE_URL set: loadConfig must override PGLite selection,
// pick Postgres, and clear the stale database_path so toEngineConfig
// doesn't try to use both.
await withEnv(
{
GBRAIN_HOME: home,
GBRAIN_DATABASE_URL: undefined,
DATABASE_URL: 'postgres://user:pass@example.test:5432/gbrain',
},
() => {
const cfg = loadConfig();
expect(cfg?.engine).toBe('postgres');
expect(cfg?.database_url).toBe('postgres://user:pass@example.test:5432/gbrain');
expect(cfg?.database_path).toBeUndefined();
},
);
} finally {
rmSync(home, { recursive: true, force: true });
}
});
test('GBRAIN_DATABASE_URL beats DATABASE_URL (operator override)', async () => {
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-env-'));
try {
await withEnv(
{ GBRAIN_HOME: home, GBRAIN_DATABASE_URL: undefined, DATABASE_URL: undefined },
() => {
saveConfig({ engine: 'pglite', database_path: '/tmp/local-brain.pglite' });
},
);
await withEnv(
{
GBRAIN_HOME: home,
GBRAIN_DATABASE_URL: 'postgres://win:win@gbrain.test:5432/db',
DATABASE_URL: 'postgres://lose:lose@other.test:5432/db',
},
() => {
const cfg = loadConfig();
expect(cfg?.engine).toBe('postgres');
expect(cfg?.database_url).toBe('postgres://win:win@gbrain.test:5432/db');
},
);
} finally {
rmSync(home, { recursive: true, force: true });
}
});
test('No env DB URL → existing PGLite file config is honored', async () => {
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-env-'));
try {
await withEnv(
{ GBRAIN_HOME: home, GBRAIN_DATABASE_URL: undefined, DATABASE_URL: undefined },
() => {
saveConfig({ engine: 'pglite', database_path: '/tmp/local-brain.pglite' });
const cfg = loadConfig();
expect(cfg?.engine).toBe('pglite');
expect(cfg?.database_path).toBe('/tmp/local-brain.pglite');
expect(cfg?.database_url).toBeUndefined();
},
);
} finally {
rmSync(home, { recursive: true, force: true });
}
});
test('No file config + DATABASE_URL → infers Postgres', async () => {
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-env-'));
try {
await withEnv(
{
GBRAIN_HOME: home,
GBRAIN_DATABASE_URL: undefined,
DATABASE_URL: 'postgres://only:env@gbrain.test:5432/db',
},
() => {
// No saveConfig() — no file present at all.
const cfg = loadConfig();
expect(cfg?.engine).toBe('postgres');
expect(cfg?.database_url).toBe('postgres://only:env@gbrain.test:5432/db');
},
);
} finally {
rmSync(home, { recursive: true, force: true });
}
});
test('No file config + no env DB URL → loadConfig returns null', async () => {
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-env-'));
try {
await withEnv(
{ GBRAIN_HOME: home, GBRAIN_DATABASE_URL: undefined, DATABASE_URL: undefined },
() => {
expect(loadConfig()).toBeNull();
},
);
} finally {
rmSync(home, { recursive: true, force: true });
}
});
});