Files
gbrain/src/commands/serve.ts
T
488e4824e8 v0.34.1.0 fix(mcp): MCP fix wave — source-isolation P0 + PKCE DCR + federated_read + 3 more (#996)
* fix(mcp): skip stdin EOF handlers when MCP_STDIO=1

OpenClaw's bundle-mcp gateway and similar wrappers pipe the JSON-RPC
handshake on stdin then close their stdin half. Pre-fix, both stdin
'end' and 'close' listeners (server.ts:65-66 and serve.ts:204-206)
treated this as a permanent disconnect and shut the server down before
the first tool call arrived.

Guard both sites with `process.env.MCP_STDIO !== '1'`. Signal handlers
(SIGTERM/SIGINT/SIGHUP), transport.onclose, and the parent-process
watchdog still cover legitimate shutdown paths. The serve.ts site
threads the env read through an injectable `mcpStdio?: boolean` on
ServeOptions so tests stay isolated (no process.env mutation per
scripts/check-test-isolation.sh R1).

Tests: 3 new cases in test/serve-stdio-lifecycle.test.ts pin the
guard's invariants — mcpStdio=true must NOT trigger shutdown on stdin
EOF, signals must still drive shutdown with mcpStdio=true, and
mcpStdio=false (default) preserves existing CLI behavior. 25/25 pass.

Origin: PR #870.

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

* fix(oauth): honor token_endpoint_auth_method=none for PKCE public clients

RFC 7591 §3.2.1: when a DCR client declares
token_endpoint_auth_method="none" (PKCE-only public clients like Claude
Code, Cursor), the authorization server MUST NOT issue a client_secret.
Pre-fix, registerClient unconditionally minted a secret, and the MCP
SDK's clientAuth middleware then rejected valid public-client flows on
/token because it expected client.client_secret to match.

Three changes to src/core/oauth-provider.ts:registerClient:

  - Gate clientSecret generation on isPublicClient = (auth_method === 'none').
    Public clients store client_secret_hash = NULL.
  - Omit client_secret from the response payload for public clients.
    Confidential clients (default client_secret_post and explicit
    client_secret_basic) keep their existing one-time-reveal shape.
  - Normalize NULL secret_hash to JS undefined in getClient so SDK
    middleware (which checks client.client_secret === undefined, not
    === null) correctly identifies public clients and skips the
    secret-comparison branch on /token.

Schema is already permissive (client_secret_hash TEXT, no NOT NULL on
both src/schema.sql and src/core/pglite-schema.ts) — no migration
needed.

Tests: 5 new cases in test/oauth.test.ts pin:
  - public client → no client_secret in response (#11 from plan)
  - default auth_method → secret unchanged (regression guard)
  - explicit client_secret_post → secret unchanged
  - getClient NULL→undefined normalization
  - PKCE full /authorize → /token end-to-end with no secret (#15 from plan)

69/69 oauth.test.ts cases pass. typecheck clean.

Origin: PR #909.

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

* feat(serve-http): --bind HOST, default to loopback (127.0.0.1)

Adds `gbrain serve --http --bind <interface>` to control which network
interface the HTTP MCP server listens on. Default flipped from
`0.0.0.0` (pre-v0.34) to `127.0.0.1` (v0.34.0+).

Why the flip: gbrain's primary use case is a personal-knowledge brain on
a laptop. The previous default exposed brains on every interface — one
accidental `--http` invocation away from publishing the brain to a LAN.
Server operators who need remote access pass `--bind 0.0.0.0` (or a
specific interface). Codex's outside-voice on the original PR #864
correctly flagged that the additive flag wasn't actually the fix; the
default needed to change for the safety claim to hold.

If `--public-url` is set but `--bind` is unset, runServeHttp prints a
loud stderr WARN at startup recommending `--bind 0.0.0.0`. Declaring a
public URL while quietly binding loopback is almost always a
misconfiguration; we want the operator to see it on first start, not
silently fail remote requests.

Startup banner now includes a `Bind:` row so the listening interface is
visible alongside Port / Engine / Issuer.

Origin: PR #864, extended with D11 (default flip) per /plan-eng-review
codex outside-voice review.

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

* fix(mcp): seal source-isolation leak on read path (P0)

Pre-fix, an authenticated OAuth MCP client scoped to source-A could
enumerate source-B pages via six read-side ops: search, query (text
AND image paths), list_pages, traverse_graph, and find_experts. The
v0.31.8 source-scoping pattern shipped through dispatch.ts but the op
handlers never threaded ctx.sourceId into their engine calls, and
hybridSearch.ts:223's explicit SearchOpts rebuild dropped sourceId
even when callers passed it.

Sealing the leak:

  - src/core/operations.ts adds sourceScopeOpts(ctx), the canonical
    precedence ladder: ctx.auth.allowedSources (federated) wins over
    ctx.sourceId (scalar) wins over nothing. Threaded into all 5
    read-side op handlers + the query-image-path searchVector call
    (the 6th leak surface codex caught in plan review).

  - src/core/search/hybrid.ts:223 now threads sourceId + sourceIds
    fields through the inner SearchOpts rebuild. The explicit pick
    shape is preserved (HNSW inner-CTE ordering depends on it) but
    extended.

  - src/core/types.ts adds sourceIds?: string[] to SearchOpts +
    PageFilters (D9: federated read needs array-shaped engine filter
    or fan-out; array wins for hot retrieval).

  - src/core/operations.ts AuthInfo gains sourceId + allowedSources
    (D2: identity surface symmetric with the federated_read column
    #876 will add).

  - Both engines now apply WHERE source_id = $N (scalar) or = ANY($N::text[])
    (array) at the SQL layer for searchKeyword, searchKeywordChunks,
    searchVector, listPages, traverseGraph, traversePaths. Array form
    wins when both are set. The searchVector filter pushes into the
    inner HNSW CTE (codex flagged this placement during plan review).

  - traverseGraph + traversePaths signatures gain opts.sourceId +
    opts.sourceIds; engine.ts interface updated.

  - findExperts (the whoknows op, D3 5th leak surface) accepts
    sourceId + sourceIds and threads them into its internal
    hybridSearch call. PR #861 was authored before v0.33 shipped so
    this op wasn't covered in the original PR.

Auth wiring:

  - GBrainOAuthProvider.verifyAccessToken populates AuthInfo.sourceId
    from oauth_clients.source_id. JOIN guarded by isUndefinedColumnError
    so pre-v55 brains degrade to legacy projection rather than refusing
    every token verification.

  - GBrainOAuthProvider.registerClientManual gains a sourceId
    parameter (defaults to 'default'). DCR registerClient also sets
    source_id='default' on the inserted row.

  - serve-http.ts:929 cleanup: AuthInfo.sourceId is now a real typed
    field. The cast + GBRAIN_SOURCE env fallback chain is gone (D13).
    Legacy bearer tokens default to 'default' source in
    verifyAccessToken.

  - http-transport.ts (legacy access_tokens path) threads
    sourceId='default' through DispatchOpts so v0.22.7 callers stay
    source-scoped.

  - auth.ts CLI adds --source flag to gbrain auth register-client.

Migration v55 (D10 + D13):

  - ALTER TABLE oauth_clients ADD COLUMN source_id TEXT (nullable).
  - Backfill UPDATE source_id = 'default' WHERE source_id IS NULL —
    preserves v0.33 effective behavior verbatim for legacy clients.
  - ADD CONSTRAINT FK ... REFERENCES sources(id) ON DELETE SET NULL,
    wrapped in DO block so re-runs against fresh-install brains (where
    the FK already lives inline in SCHEMA_SQL) no-op cleanly.
  - CREATE INDEX idx_oauth_clients_source_id WHERE source_id IS NOT NULL
    for the verifyAccessToken JOIN.
  - GBRAIN_ACCEPT_SILENT_WIDEN env-flag wired through the runner via
    SET LOCAL gbrain.accept_silent_widen — reserved for future migrations
    that hit the silent-widen footgun codex flagged. This migration
    doesn't need it (column is brand new; no pre-existing stale values
    possible by definition).
  - src/core/pglite-schema.ts + src/schema.sql include the column +
    FK + index inline for fresh installs.

Tests: new test/e2e/source-isolation-pglite.test.ts with 13 regression
cases — one per leak surface (search/list_pages/traverse/etc.) plus
explicit AuthInfo.sourceId and AuthInfo.allowedSources op-handler
threading checks. Full unit suite: 6034 pass / 0 fail. PGLite
initSchema time dropped from 2.4s to 850ms after consolidating v55's
DO blocks (multiple DO blocks were slow on PGLite; one DO block for
the FK install only is fine).

Origin: PR #861 + plan-eng-review decisions D2/D3/D4/D9/D10/D13 + F2.

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

* feat(gateway): multimodal embedding for openai-compatible providers

Pre-fix, embedMultimodal hardcoded a recipe.id === 'voyage' branch and
threw AIConfigError for every other recipe. Multimodal-capable providers
fronted by LiteLLM (or any openai-compatible proxy) were unreachable
even when the operator had wired up the model.

The fix:

  - src/core/ai/gateway.ts adds embedMultimodalOpenAICompat() that
    POSTs to the standard /embeddings endpoint with content arrays
    carrying image_url entries. Routing comes from the existing
    recipe.implementation switch — Voyage stays on its own
    /multimodalembeddings path; every other openai-compatible recipe
    flows through the new helper.

  - src/core/ai/recipes/litellm-proxy.ts declares
    supports_multimodal: true so embedMultimodal accepts the recipe.
    No multimodal_models allow-list: LiteLLM is a passthrough proxy
    and the user owns model-id selection; provider rejection (400 from
    upstream) is the right enforcement layer there. Voyage's static
    allow-list shape stays unchanged (its 12 models share
    supports_multimodal but only one is multimodal-capable).

  - D12 runtime dimension validation: the new helper checks the
    returned vector length against the recipe's declared default_dims
    (preferred) or the brain's embedding_dimensions config. Mismatch
    throws AIConfigError with model id + observed + expected so the
    operator can swap models or rebuild the column. Pre-fix, a
    wrong-dim response would surface as a cryptic pgvector
    "vector dimension mismatch" at INSERT time.

  - Auth resolution routes through the existing defaultResolveAuth
    helper so optional-auth recipes (LiteLLM proxy with no
    LITELLM_API_KEY) and required-auth recipes both share one code
    path. Optional-auth sends "Authorization: Bearer unauthenticated"
    which servers like Ollama / llama-server ignore but the SDK
    contract requires.

Tests: 11 new cases in test/openai-compat-multimodal.test.ts cover
happy-path, multi-input batching, unauthenticated proxy, D12 dim
mismatch + default-dim fallback, 401 / 400 / malformed-JSON / non-array
error paths, and an explicit Voyage-regression test pinning that the
new openai-compat route doesn't accidentally hijack the Voyage path.
All 41 multimodal-related tests pass (existing voyage suite + new).
typecheck clean.

Origin: PR #875 + plan-eng-review D12 (runtime dim validation).

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

* feat(oauth): federated_read read scope (#876)

Pre-fix, OAuth clients had a single source-scope axis (source_id, added
in v55). A client could either write+read one source OR be a super-reader
across all sources (via NULL source_id). There was no middle ground —
WeCare-style L3 dept clients that need to write to dept-x but read
dept-x + parent canon + shared canon had no expression.

#876 adds federated_read TEXT[] as an orthogonal read-scope axis. source_id
is the WRITE authority; federated_read is the READ authority. They default
to matching values (read scope == write scope, the pre-v0.34 default)
when a client is registered without an explicit federated read list.

Migrations v56-v60 (six new migrations on top of v55):

  - v56: ALTER TABLE ... ADD COLUMN federated_read TEXT[] NOT NULL DEFAULT '{}'.
  - v57 (F5): explicit CASE backfill so source_id IS NULL → '{}' (not an
    array containing NULL — codex caught this ambiguity during plan review).
  - v58: post-backfill validation. Fails loud if any row's source_id isn't
    in its federated_read array, pointing at a logic bug in v57 if fired.
  - v59: flip the source_id FK from ON DELETE SET NULL to ON DELETE
    RESTRICT now that federated_read provides the alternative scope-loss
    path. Pre-flip, deleting a source could silently widen any oauth_client
    to super-reader; post-flip, source delete is refused if any client
    references it (operator must revoke/re-scope first).
  - v60: GIN index on federated_read for array-containment queries.

Auth wiring:

  - GBrainOAuthProvider.verifyAccessToken JOINs c.federated_read and
    populates AuthInfo.allowedSources. Pre-v56 / pre-v55 brains degrade
    via the existing isUndefinedColumnError fallback chain.
  - registerClientManual gains a federatedRead?: string[] parameter
    (defaults to [sourceId]).
  - DCR registerClient sets source_id='default' + federated_read=['default']
    on the inserted row.
  - auth.ts CLI adds --federated-read SRC1,SRC2,... flag. The
    register-client output now prints "Federated reads:" so operators
    confirm the scope they set.

Engines consume the federated array through the SearchOpts.sourceIds /
PageFilters.sourceIds field that #861 added (no engine changes here — the
plumbing was D9). sourceScopeOpts in operations.ts already prefers the
auth.allowedSources array over scalar ctx.sourceId when set.

Test seam:
  - test/book-mirror.test.ts now spawns the CLI with GBRAIN_HOME pointed
    at a tempdir so the test isn't sensitive to the developer's local
    ~/.gbrain/config.json. Pre-fix the test could silently inherit a real
    Postgres connection and hang past the default 5s test timeout. Fresh
    GBRAIN_HOME → "No brain configured" → exit 1 in <1s.
  - test/e2e/source-isolation-pglite.test.ts gains one more regression
    case: AuthInfo.allowedSources = [] (explicit empty) MUST NOT widen
    scope to "all sources" — the silent-widen footgun precedence ladder.
  - test/openai-compat-multimodal.test.ts is part of the wave's commits
    via the migrate.ts changes that bump the schema chain. typecheck-only
    fix on a captured-auth type was already in #875's tree.

6045 unit tests pass / 0 fail. typecheck clean. PGLite initSchema runs
v55-v60 in ~786ms total (within the test-harness budget for tests using
the canonical beforeAll engine pattern).

Origin: PR #876 + plan-eng-review F5 (CASE backfill).

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

* v0.34.0.0: MCP fix wave (#870 #909 #864 #861 #875 #876)

VERSION + package.json + CHANGELOG bump for the six-PR MCP fix wave.
Schema chain extends from v54 → v60; oauth_clients gains source_id +
federated_read columns; auth'd MCP clients now stay inside their scope
across all read-side ops; PKCE-only DCR works; --bind defaults to
loopback; LiteLLM multimodal embedding ships.

Contributed by @Hansen1018 (#870), @ding-modding (#909), @DukeDawg
(#864), @toilalesondev (#861 + #876), @yoelgal (#875).

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

* docs: update project documentation for v0.34.0.0

Sync README, CLAUDE.md, SECURITY.md, docs/architecture/topologies.md,
and docs/mcp/DEPLOY.md to reflect the v0.34.0.0 MCP fix wave:

- README: document --bind HOST default (loopback), --source +
  --federated-read register-client flags, PKCE public-client gate
- SECURITY.md: note loopback-by-default for serve --http, update the
  trust-proxy contract to point at the new default
- CLAUDE.md: annotate operations.ts (sourceScopeOpts helper),
  oauth-provider.ts (verifyAccessToken JOIN + PKCE public clients),
  serve-http.ts (--bind flag), gateway.ts (openai-compat multimodal +
  dim validation), mcp/server.ts (MCP_STDIO guard), auth.ts (--source
  + --federated-read), migrate.ts (v58-v63 chain), engine.ts
  (sourceIds field). Add 4 new test-file entries for
  source-isolation-pglite, openai-compat-multimodal,
  serve-stdio-lifecycle, oauth.test.ts PKCE cases
- docs/architecture/topologies.md: source-scoped register-client
  example, --bind 0.0.0.0 for thin-client host setup
- docs/mcp/DEPLOY.md: --bind explanation in the ngrok section,
  source-scoped client recipe
- llms-full.txt: regenerated per the CLAUDE.md-edit chaser rule

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

* chore: bump v0.34.0.0 → v0.34.1.0

Renumbering the MCP fix wave from v0.34.0.0 to v0.34.1.0 so the
release slot lands between master's v0.33.2.1 and the next minor.

Touches every release-artifact mention:
- VERSION: 0.34.0.0 → 0.34.1.0
- package.json: same
- CHANGELOG.md header + "To take advantage" block
- CLAUDE.md key-files annotations (8 entries that document this wave)
- llms-full.txt (regen from CLAUDE.md)
- README.md / SECURITY.md / docs/architecture/topologies.md / docs/mcp/DEPLOY.md
- Wave code-comment markers ("// v0.34.0 (#NNN):" → "// v0.34.1 (#NNN):")

Test files renamed alongside since they were committed with the wave.

Commit subjects on the original 6 PR commits + the v0.34.0.0 bump
commit (4f533c726b47db7e) intentionally NOT rewritten — those are
history. `git log` finds the implementation by message subject, not by
version tag.

6275 unit tests pass, typecheck clean, migration chain v58-v63 unchanged.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 20:15:29 -07:00

396 lines
18 KiB
TypeScript

import { spawnSync } from 'node:child_process';
import type { BrainEngine } from '../core/engine.ts';
import { startMcpServer } from '../mcp/server.ts';
// Maximum time the stdio path will wait for engine.disconnect() (PGLite
// close + advisory lock release) before forcing exit. Keeps a wedged
// disconnect from trapping the process forever; the abandoned lock dir is
// already covered by the in-process stale-lock check (acquireLock walks
// the dir, sees a dead PID, and removes it).
const CLEANUP_DEADLINE_MS = 5_000;
// How often the parent-process watchdog polls the live kernel parent PID
// (via `readLiveParentPid`, NOT the cached `process.ppid` — see that
// helper's comment). We don't receive a signal when our parent dies (the
// kernel just re-parents us to init / launchd / a subreaper-PID), so
// polling is the only reliable way to detect "parent went away without
// closing stdin". 5s matches the cadence in the concurrent #591 PR;
// faster polling has no benefit, slower would extend the lock-leak window.
const PARENT_WATCHDOG_INTERVAL_MS = 5_000;
export interface ServeOptions {
// Test seam — defaults to the live process. The lifecycle plumbing reads
// these for stdin EOF detection, signal handlers, and exit, so unit
// tests can drive end-to-end shutdown via mocked streams without
// spawning a real Bun process. `exit` is typed as `void` (not `never`)
// so test stubs that record + return are accepted without casts;
// `process.exit`'s `never` return is assignable to `void`.
stdin?: NodeJS.ReadableStream & { isTTY?: boolean };
signals?: Pick<NodeJS.Process, 'on'>;
exit?: (code?: number) => void;
log?: (msg: string) => void;
// Test seam: replace startMcpServer to avoid booting the real MCP SDK
// (which unconditionally attaches a 'data' listener to real
// process.stdin and would pollute the test runner's stdin handle).
// Defaults to the real implementation when omitted.
startMcpServer?: (engine: BrainEngine) => Promise<void>;
// Test seam for the parent-process watchdog. The default
// (`readLiveParentPid`) reads the live kernel PPID via `ps` because
// `process.ppid` is captured at process creation and does not refresh
// on re-parent (Node/Bun parity). Tests inject a stub so they can
// simulate the parent dying without spawning ps or re-parenting any
// real process.
getParentPid?: () => number;
// Test seam: replace setInterval/clearInterval so the watchdog can
// fire deterministically in tests instead of waiting 5s. Defaults to
// the global timer functions.
setInterval?: (fn: () => void, ms: number) => unknown;
clearInterval?: (handle: unknown) => void;
// Test seam for the one-shot watchdog readiness probe. The default
// runs `spawnSync('ps', ['-o','ppid=','-p',PID])` and returns true on
// success. Tests inject a stub to simulate ps unavailability (e.g.
// stripped containers, busybox without procps) without modifying PATH.
// When the probe returns false, `installStdioLifecycle` skips the
// watchdog interval entirely and emits a loud stderr line. Without
// the probe, the original PR's behavior was a silent no-op: every
// tick fell through to the cached `process.ppid` and the watchdog
// never fired, while still claiming to be installed.
probeWatchdog?: () => boolean;
// v0.34.1 (#870): test seam for the MCP_STDIO=1 piped-stdin guard.
// When true, runServe skips the stdin 'end'/'close' shutdown hooks
// because the wrapping gateway (OpenClaw bundle-mcp, others) pipes the
// JSON-RPC handshake and closes stdin immediately. Signal handlers and
// transport.onclose still cover legitimate shutdown.
// Defaults to `process.env.MCP_STDIO === '1'` when omitted.
mcpStdio?: boolean;
}
export async function runServe(
engine: BrainEngine,
args: string[] = [],
opts: ServeOptions = {},
) {
// v0.26+: --http dispatches to the full OAuth 2.1 server (serve-http.ts)
// with admin dashboard, scope enforcement, SSE feed, and the requireBearerAuth
// middleware. Master's simpler startHttpTransport from v0.22.7 is superseded
// — the OAuth provider in serve-http.ts handles bearer auth via
// verifyAccessToken with legacy access_tokens fallback (so v0.22.7 callers
// that used `gbrain auth create` keep working unchanged).
const isHttp = args.includes('--http');
if (isHttp) {
const portIdx = args.indexOf('--port');
const port = portIdx >= 0 ? parseInt(args[portIdx + 1]) || 3131 : 3131;
const ttlIdx = args.indexOf('--token-ttl');
const tokenTtl = ttlIdx >= 0 ? parseInt(args[ttlIdx + 1]) || 3600 : 3600;
const enableDcr = args.includes('--enable-dcr');
const publicUrlIdx = args.indexOf('--public-url');
const publicUrl = publicUrlIdx >= 0 ? args[publicUrlIdx + 1] : undefined;
// F8 escape hatch: --log-full-params writes raw payloads to mcp_request_log
// and the admin SSE feed instead of redacted summaries. Off by default
// (privacy-first); operators running gbrain on their own laptop can flip
// it on for debug visibility. Loud startup warning fires in serve-http.ts
// when set so the posture change is visible in stderr.
const logFullParams = args.includes('--log-full-params');
// v0.34.1 (#864, D11): `--bind HOST` lets operators choose the network
// interface to listen on. When unset, runServeHttp defaults to 127.0.0.1
// (loopback) — server operators who need remote access pass
// `--bind 0.0.0.0` (or a specific interface IP). `bind` is intentionally
// left undefined here when the flag is absent so the WARN-on-public-url
// path in serve-http can distinguish "operator chose loopback explicitly"
// from "operator didn't set the flag at all."
const bindIdx = args.indexOf('--bind');
const bind = bindIdx >= 0 ? args[bindIdx + 1] : undefined;
const { runServeHttp } = await import('./serve-http.ts');
await runServeHttp(engine, { port, tokenTtl, enableDcr, publicUrl, logFullParams, bind });
return;
}
// stdio path — install lifecycle handlers BEFORE startMcpServer so that
// an early stdin EOF (parent died before our first read) can still
// trigger graceful release of the PGLite write lock held by `engine`.
// The HTTP / OAuth path above has its own lifecycle in serve-http.ts
// and is intentionally NOT wired into this stdio plumbing.
console.error('Starting GBrain MCP server (stdio)...');
installStdioLifecycle(engine, args, opts);
const start = opts.startMcpServer ?? startMcpServer;
await start(engine);
// startMcpServer's `await server.connect(transport)` resolves once the
// SDK has wired up its stdin 'data' listener; that listener keeps the
// event loop alive. We deliberately do NOT add `await new Promise(() =>
// {})` here — it would block this async frame and stop the lifecycle
// hooks from being able to call process.exit() cleanly.
}
interface StdioLifecycleDeps {
stdin: NodeJS.ReadableStream & { isTTY?: boolean };
signals: Pick<NodeJS.Process, 'on'>;
exit: (code?: number) => void;
log: (msg: string) => void;
getParentPid: () => number;
setInterval: (fn: () => void, ms: number) => unknown;
clearInterval: (handle: unknown) => void;
probeWatchdog: () => boolean;
}
function installStdioLifecycle(
engine: BrainEngine,
args: string[],
opts: ServeOptions,
): void {
const deps: StdioLifecycleDeps = {
stdin: opts.stdin ?? process.stdin,
signals: opts.signals ?? process,
exit: opts.exit ?? ((code?: number) => { process.exit(code); }),
log: opts.log ?? ((msg: string) => console.error(msg)),
getParentPid: opts.getParentPid ?? readLiveParentPid,
setInterval: opts.setInterval ?? ((fn, ms) => setInterval(fn, ms)),
clearInterval: opts.clearInterval ?? ((h) => clearInterval(h as ReturnType<typeof setInterval>)),
probeWatchdog: opts.probeWatchdog ?? probeWatchdogAvailable,
};
let shuttingDown = false;
let parentWatchdog: unknown = null;
const beginShutdown = (reason: string): void => {
if (shuttingDown) return;
shuttingDown = true;
// Stop the parent-watchdog interval as soon as a shutdown begins so
// it cannot fire a redundant 'parent-died' shutdown while the first
// one is still draining the cleanup chain.
if (parentWatchdog !== null) {
deps.clearInterval(parentWatchdog);
parentWatchdog = null;
}
deps.log(`GBrain MCP server: graceful exit (${reason})`);
// Race the cleanup against a deadline. engine.disconnect() does a
// PGLite WASM close + a synchronous rmSync on the lock dir; both
// should be sub-second, but a wedged WASM runtime shouldn't be able
// to trap us forever. If we hit the deadline we still exit; the
// lock dir is advisory and the next process's stale-lock check
// (process.kill(pid, 0) → ESRCH) will reclaim it.
const deadline = setTimeout(() => {
deps.log(
`GBrain MCP server: cleanup deadline (${CLEANUP_DEADLINE_MS}ms) exceeded — forcing exit`,
);
deps.exit(0);
}, CLEANUP_DEADLINE_MS);
deadline.unref?.();
Promise.resolve()
.then(() => engine.disconnect())
.catch((err: unknown) => {
const msg = err instanceof Error ? err.message : String(err);
deps.log(`GBrain MCP server: cleanup error: ${msg}`);
})
.finally(() => {
clearTimeout(deadline);
deps.exit(0);
});
};
// Signal-based termination. SIGTERM: daemon ask. SIGINT: user Ctrl-C.
// SIGHUP: terminal disconnect / daemon-style "reload" channels — Aragorn
// observed real-world hosts (Claude Desktop on macOS, hermes-agent
// restart) send these instead of closing stdin. All three get the same
// graceful path; the idempotency guard absorbs duplicate signals.
deps.signals.on('SIGTERM', () => beginShutdown('SIGTERM'));
deps.signals.on('SIGINT', () => beginShutdown('SIGINT'));
deps.signals.on('SIGHUP', () => beginShutdown('SIGHUP'));
// Stdin EOF — the parent closes the pipe but the MCP SDK's
// StdioServerTransport only listens for 'data'/'error', not 'end' or
// 'close', so without these hooks the process keeps the engine (and its
// PGLite write lock) live indefinitely after the parent disconnects.
// 'end' fires on a clean EOF; 'close' fires when the underlying handle
// is destroyed (e.g. parent SIGKILL'd while pipe still open). Both
// converge on the same idempotent shutdown.
// Skip when stdin is a TTY: interactive `gbrain serve` use shouldn't
// terminate just because the user hasn't typed anything. Signal /
// watchdog paths still cover that case if needed.
// v0.34.1 (#870): when MCP_STDIO=1, the wrapping gateway pipes the
// JSON-RPC handshake then closes its stdin half. Treating that as a
// permanent disconnect kills the server before the first tool call.
// Signal handlers (SIGTERM/SIGINT/SIGHUP), transport.onclose, and the
// parent-process watchdog below still cover legitimate shutdown paths.
// `mcpStdio` is the injectable form; default reads the env once at
// install time so tests stay isolated (no process.env mutation).
const mcpStdioMode = opts.mcpStdio ?? (process.env.MCP_STDIO === '1');
if (!deps.stdin.isTTY && !mcpStdioMode) {
deps.stdin.once('end', () => beginShutdown('stdin-end'));
deps.stdin.once('close', () => beginShutdown('stdin-close'));
}
// Parent-process watchdog. Some hosts (launchd, cron, certain MCP
// gateways) terminate without closing stdin and without sending a
// signal — the kernel just re-parents us to whichever ancestor is
// still alive (PID 1, or any closer subreaper such as launchd, systemd,
// tmux, or a parent shell with PR_SET_CHILD_SUBREAPER). Polling is the
// only portable way to notice; see `readLiveParentPid` for why we
// cannot rely on `process.ppid` (cached at process creation and never
// refreshed on re-parent in Node or Bun).
//
// We capture the initial parent PID once at install time and fire on
// ANY change, not just reparent-to-PID-1. The PR-#676 author's original
// `=== 1` check missed reparent-to-subreaper-PID-N, which is the actual
// observed behavior under launchd / systemd subreapers (codex review
// finding #3). A process legitimately started under PID 1 (e.g. a
// systemd service) skips the watchdog: there's no parent-death event
// to detect, and any reparent FROM 1 doesn't happen. `unref()` keeps
// the interval from blocking other exit paths.
//
// A one-shot startup probe (D2-revisited per codex finding #4) verifies
// that the underlying mechanism (`spawnSync('ps')`) actually works on
// this host. Stripped containers / busybox-without-procps environments
// would silently fall back to the cached `process.ppid` on every tick
// — the watchdog claims to be installed but never fires. When the probe
// fails, we skip installing the interval entirely and log loudly so the
// operator sees the degraded mode instead of a phantom watchdog.
const initialParentPid = deps.getParentPid();
if (initialParentPid !== 1) {
if (!deps.probeWatchdog()) {
deps.log(
'[gbrain serve] watchdog disabled: ps unavailable, parent-death detection unavailable — child will rely on stdin EOF / signals only',
);
} else {
parentWatchdog = deps.setInterval(() => {
if (deps.getParentPid() !== initialParentPid) {
beginShutdown('parent-died');
}
}, PARENT_WATCHDOG_INTERVAL_MS);
(parentWatchdog as { unref?: () => void } | null)?.unref?.();
}
}
// Optional idle-timeout safety net. Default OFF; opt-in via
// `--stdio-idle-timeout <seconds>`. The flag is for the rare case where
// the parent leaks the stdin pipe but never closes it (so 'end' never
// fires) and never sends another message — we'd otherwise sit on the
// PGLite lock forever. Off by default because most parents close
// properly and an over-eager idle timeout would surprise long-poll
// workloads.
const idleTimeoutSec = parseStdioIdleTimeout(args);
if (idleTimeoutSec > 0) {
let idleTimer: ReturnType<typeof setTimeout> | null = null;
const armIdle = (): void => {
if (idleTimer) clearTimeout(idleTimer);
idleTimer = setTimeout(
() => beginShutdown(`stdio-idle-timeout (${idleTimeoutSec}s)`),
idleTimeoutSec * 1000,
);
idleTimer.unref?.();
};
armIdle();
// Reset on every chunk. We can't observe SDK-parsed messages from
// here, but every JSON-RPC frame causes a 'data' event on stdin, so
// chunk-level granularity is sufficient.
deps.stdin.on('data', armIdle);
deps.log(`GBrain MCP server: stdio idle timeout = ${idleTimeoutSec}s`);
}
}
/**
* Resolve the live parent PID from the kernel (not the cached startup
* value). Both Node and Bun expose `process.ppid` as a property captured
* at process creation, so it does NOT update when the kernel re-parents
* us to a new ancestor after the original parent dies — which is the
* exact event the watchdog needs to detect. Empirical evidence on
* macOS / Bun 1.3.12: `process.ppid` stays at the original parent ID
* indefinitely while `ps -o ppid= -p $$` reports the new parent within
* one tick.
*
* Cost: ~10ms per spawn. Called every 5s (PARENT_WATCHDOG_INTERVAL_MS),
* so amortized < 0.5% CPU. Falls back to `process.ppid` if `ps` fails
* (best-effort safety net for stripped-down containers, etc.); the
* startup probe at watchdog-install time loud-logs and skips the
* interval entirely when ps is unavailable, so a per-tick fallback is
* a redundant safety net rather than a primary mechanism.
*/
function readLiveParentPid(): number {
try {
const r = spawnSync('ps', ['-o', 'ppid=', '-p', String(process.pid)], {
encoding: 'utf8',
timeout: 1000,
});
if (r.status === 0 && typeof r.stdout === 'string') {
const n = parseInt(r.stdout.trim(), 10);
if (Number.isInteger(n) && n >= 0) return n;
}
} catch {
/* fall through */
}
return process.ppid;
}
/**
* One-shot probe at watchdog-install time to confirm ps actually works
* on this host. Returns true iff `spawnSync('ps','-o','ppid=','-p',PID)`
* exits 0 with a parseable integer. When it returns false, the caller
* skips installing the watchdog and emits a loud stderr line — the
* operator sees "watchdog disabled" instead of an installed-but-never-
* fires phantom.
*
* Why a separate probe rather than relying on the per-tick fallback in
* `readLiveParentPid`: the per-tick fallback returns the cached
* `process.ppid` silently, so the watchdog runs every 5s, compares
* cached PPID to itself, never detects a change, and never fires —
* while still claiming to be active. The probe surfaces the gap once
* at install time and lets the caller short-circuit cleanly.
*/
function probeWatchdogAvailable(): boolean {
try {
const r = spawnSync('ps', ['-o', 'ppid=', '-p', String(process.pid)], {
encoding: 'utf8',
timeout: 1000,
});
if (r.status !== 0 || typeof r.stdout !== 'string') return false;
const n = parseInt(r.stdout.trim(), 10);
return Number.isInteger(n) && n >= 0;
} catch {
return false;
}
}
function parseStdioIdleTimeout(args: string[]): number {
const idx = args.indexOf('--stdio-idle-timeout');
if (idx < 0) return 0;
const raw = args[idx + 1];
// Strict parsing — silent fallback to 0 turns an opt-in safety net into
// a no-op when an operator typos the value (e.g. `--stdio-idle-timeout
// 30s`). `Number()` rejects partial parses like `30junk` (returns NaN),
// unlike `parseInt` which would silently accept it. A missing value
// (`--stdio-idle-timeout` at end of args) and any non-integer / negative
// value are surfaced as a CLI error before we install the timer.
if (raw === undefined) {
throw new Error(
'--stdio-idle-timeout requires a non-negative integer (seconds). Got: (missing value)',
);
}
// Reject empty / whitespace-only explicitly: `Number('')` is 0 in JS,
// which would silently turn `--stdio-idle-timeout ""` into the
// documented opt-out — the exact silent-fallback failure mode this
// strict parser exists to prevent.
if (raw.trim() === '') {
throw new Error(
'--stdio-idle-timeout requires a non-negative integer (seconds). Got: (blank value)',
);
}
const n = Number(raw);
if (!Number.isInteger(n) || n < 0) {
throw new Error(
`--stdio-idle-timeout requires a non-negative integer (seconds). Got: ${JSON.stringify(raw)}`,
);
}
return n;
}