Skip to content

v0.31.1 feat: thin-client mode actually works (Issue #734)#772

Open
garrytan wants to merge 11 commits intomasterfrom
garrytan/columbus-v1
Open

v0.31.1 feat: thin-client mode actually works (Issue #734)#772
garrytan wants to merge 11 commits intomasterfrom
garrytan/columbus-v1

Conversation

@garrytan
Copy link
Copy Markdown
Owner

@garrytan garrytan commented May 9, 2026

Summary

Closes #734. Thin-client mode (gbrain init --mcp-only) was a half-built bridge — 9 obvious local-only commands refused, the other ~25 silently fell through to an empty local PGLite and returned "No results." against a populated remote brain. Hermes/Neuromancer hit this in production against a 102k-page wintermute brain.

v0.31.1 routes every read + write + admin op through MCP on thin-client installs, before opening any local engine. Every refused command carries a pinpoint hint naming the closest path. New get_brain_identity op + 60s-cached identity banner kills the "am I empty?" confusion. New oauth_client_scopes_probe doctor check catches v0.29.2/v0.30.0 thin-clients without admin scope before they fail mid-command.

Routing

  • THIN_CLIENT_REFUSE_HINTS table in src/cli.ts for every refused command
  • runThinClientRouted hooks INTO existing op-dispatch path (CDX-1 — no parallel module)
  • salience / anomalies / graph-query / think get per-command thin-client routing branches
  • ENG-2 renderer parity via JSON.parse(JSON.stringify(result)) on local-engine path

New ops

  • get_brain_identity — read scope, banner-only, returns counters from existing engine.getStats() (60s client-cache bounds frequency)

Banner (cherry-pick B)

  • [thin-client → wintermute:3131 · brain: 102k pages, 265k chunks · v0.31.1] on stderr before each routed command
  • 60s TTL in-memory cache keyed by mcp_url
  • --quiet, GBRAIN_NO_BANNER=1, non-TTY default suppressed (override with GBRAIN_BANNER=1)

Doctor scope-probe (CDX-5)

  • New oauth_client_scopes_probe check probes read tier (get_brain_identity) + admin tier (get_health)
  • Surfaces missing-admin-scope BEFORE commands fail mid-flight, with exact remediation
  • GBRAIN_DOCTOR_SKIP_SCOPE_PROBE=1 env-flag for fixtures
  • opts.skipScopeProbe parameter on collectRemoteDoctorReport(config, opts) for clean test isolation

Error handling (CDX-4)

  • Hardened callRemoteTool: all transport errors normalized to RemoteMcpError via toRemoteMcpError funnel
  • New RemoteMcpErrorReason stable union; RemoteMcpErrorDetail.kind ('timeout'|'aborted'|'unreachable'); RemoteMcpErrorDetail.code for tool_error
  • extractToolErrorCode parses JSON envelopes first, falls back to substring detection
  • --timeout=Ns global flag (ENG-4): 30s default, 180s for think; SIGINT aborts via AbortController
  • Exhaustive TS never switch on RemoteMcpError.reason at the dispatcher

Refused with hints (CDX-2 audit)

  • sync, embed, extract, migrate, apply-migrations, repair-jsonb, integrity, serve, dream, orphans, transcripts, storage, takes, sources — each refused at top level with pinpoint hint naming closest remote path or MCP tool

Test Coverage

95 tests across 7 files cover the v0.31.1 surface; all pass.

File Cases Purpose
test/get-brain-identity.test.ts 9 New op contract (scope, params, shape)
test/oauth-scope-probe.test.ts 8 buildScopeCheck pure-function status semantics
test/cli-options.test.ts 28 --timeout=Ns flag (8 new cases)
test/cli-dispatch-thin-client.test.ts 14 Refresh refuse-hint format assertions
test/mcp-client.test.ts 13 Existing tests — all pass with hardening pass
test/doctor-remote.test.ts 15 Refactored to opts.skipScopeProbe (lint compliance)
test/doctor-report-remote.test.ts 8 Existing — pass unchanged

Tests: 4452 → 4454 (+2 new test files; +25 new test cases across modified files).

The most ambitious parity test (test/thin-client-parity.test.ts — byte-equal stdout for 12+ ops via in-process MCP server) is deferred to TODOS.md as a v0.31.x backfill — needs MCP server harness infrastructure that doesn't exist today. ENG-2's JSON-shape normalization + per-command test coverage is the interim guard.

Pre-Landing Review

CEO Review (1 run, CLEAR PLAN): 4 proposals surfaced, 2 accepted (refuse-better hints, identity banner), 2 deferred (timing telemetry → v0.31.x, job-routing → v0.31.2).

Codex Review (1 run, issues_found → resolved): 9 findings; 6 decisions (CDX-1..6) all accepted. Codex's #1+#8 forced an architecture rewrite (drop the parallel src/core/thin-client/ module; route inline in cli.ts). Codex's #2+#7 forced honest framing (no false "Liskov substitutability" claim — server intentionally treats remote callers differently for think.--save/--take and put_page auto-link, both trust-boundary policy). Codex's #5 caught a false premise in the original error-model design.

Eng Review (1 run, CLEAR PLAN): 5 issues, 0 critical gaps after resolution (ENG-1 _meta.routed_via for JSON envelopes, ENG-2 renderer parity normalization, ENG-3 oauth_client_scopes doctor [superseded by CDX-5], ENG-4 timeout/abort policy, ENG-5 get_brain_identity banner [refined by CDX-3]).

Plan Completion

All 13 implementation tasks complete (10 substantive, 1 superseded, 2 cross-cuts). Plan + decision history pinned at ~/.claude/plans/how-to-make-mcp-iterative-liskov.md.

TODOS

7 follow-up entries added to TODOS.md for v0.31.x:

  • v0.31.x: routed-call timing telemetry (GBRAIN_TIMING=1)
  • v0.31.2: job-submission routing for dream/embed/extract
  • Per-subcommand thin-client routing for takes/sources
  • Privacy decision: get_recent_transcripts localOnly lift
  • Trust-boundary policy review for remote-caller gates
  • v0.32.0: flip gbrain auth register-client default to read,write,admin
  • v0.31.x: cross-process OAuth token cache
  • v0.31.x: parity test (test/thin-client-parity.test.ts)

Documentation

CLAUDE.md gains a "Thin-client routing (v0.31.1, Issue #734)" section under "Key files" annotating every changed/new file with its v0.31.1 contract. CHANGELOG.md gets the full release-summary entry per CLAUDE.md voice contract (numbers-that-matter table with before/after, "what this means" closer, "To take advantage of v0.31.1" block with exact remediation commands, itemized changes, contributor section). llms.txt + llms-full.txt regenerated.

Test plan

  • bun run verify clean
  • bun run test 4452+ pass (1 pre-existing per-machine timeout flake on skillpack-check — 20s command, 5s test timeout, identical to master)
  • bun run test:e2e 449 pass / 8 fail across 5 files; all 5 failing files are pre-existing master failures (my branch didn't modify any of those test files — git log master..HEAD empty for them)
  • v0.31.1-specific test surface: 95/95 passing
  • Master merged into branch (resolved VERSION/package.json/CHANGELOG.md conflicts; 0.31.1 wins over 0.30.2)

🤖 Generated with Claude Code


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

garrytan and others added 11 commits May 8, 2026 16:19
Lightweight read-scope op that returns {version, engine, page_count,
chunk_count, last_sync_iso} for the thin-client identity banner.
Reuses engine.getStats() — banner's 60s TTL cache (next commit) bounds
frequency to ≤1/60s per CLI process. Banner-only op, no cliHints.

Pinned by 9 tests in test/get-brain-identity.test.ts.

Part of v0.31.1 fix for #734 (thin-client mode silently routing
~25 CLI commands to empty local PGLite). See plan at
~/.claude/plans/how-to-make-mcp-iterative-liskov.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
CDX-4 (Codex outside-voice finding): the previous callRemoteTool let
plain Error escape — undici network errors, AbortError, JSON parse
failures all bubbled untyped. Plan called for an exhaustive switch on
RemoteMcpError.reason at the dispatcher; that contract was unsound.

Hardening:
- New CallRemoteToolOptions {timeoutMs?, signal?} (4th arg, optional).
- buildAbortController composes external signal with timeout into a
  single signal threaded through the SDK transport's requestInit.
- toRemoteMcpError funnel converts ANY thrown value to RemoteMcpError
  before re-raising; the outermost try/catch guarantees the contract.
- RemoteMcpErrorReason exported as a stable union type.
- RemoteMcpErrorDetail.kind ('timeout'|'aborted'|'unreachable') sub-tags
  network errors so the dispatcher can render the right hint.
- RemoteMcpErrorDetail.code carries server-supplied error codes on
  tool_error (e.g. 'missing_scope') for pinpoint refusal hints.
- extractToolErrorCode parses JSON envelopes first, falls back to
  substring detection for legacy server messages.

All 13 existing mcp-client tests still pass. Typecheck clean.

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
New global flag --timeout that accepts ms / s / m / ms-suffix forms
("30s", "2m", "500ms", "500"). Default null = per-command default
(30s for most ops, 180s for `think` per ENG-4). Plumbs through to
callRemoteTool's AbortController via cliOpts.timeoutMs.

Rejection cases (timeoutMs stays null, flag falls through):
- --timeout=0 (must be positive)
- --timeout=garbage (no parse)

Pinned by 8 new tests in test/cli-options.test.ts (total 28 pass).

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The keystone fix for Issue #734. Inserts the routing seam INSIDE the
existing op-dispatch path in cli.ts:78-138 (per Codex finding CDX-1) —
no parallel `src/core/thin-client/` module. Routing is a ~80-line
conditional that runs BEFORE connectEngine() so thin-client installs
never open the empty local PGLite.

Architecture (CDX-1, CDX-4, ENG-2, ENG-4):
- Existing arg parser, image-to-base64 transform, stdin handler, and
  required-param check run UNCHANGED before the routing branch. Zero
  duplicated parsers.
- New runThinClientRouted(op, params, cfg, cliOpts) calls callRemoteTool
  with {timeoutMs, signal}; default 180s for `think`, 30s otherwise;
  --timeout flag overrides.
- SIGINT abort threaded into AbortController → exit 130.
- Exhaustive TS `never` switch on RemoteMcpError.reason produces canned,
  actionable user messages per failure mode (ENG-4 contract).
- ENG-2 renderer parity: local-engine path runs JSON.parse(JSON.stringify())
  on the result before formatResult, killing the Date/bigint/Buffer drift
  class without per-command renderer audit.
- THIN_CLIENT_REFUSE_HINTS table replaces the generic refusal message
  with pinpoint hints (CDX-5 / cherry-pick A). Adds dream/transcripts/storage
  to the refused set with their own hints.
- localOnly ops on thin-client refuse via refuseThinClient (with hint).

Pinned by 14 cli-dispatch-thin-client tests (all pass). Typecheck clean.

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Prints "[thin-client → wintermute.fly.dev:3131 · brain: 102k pages,
265k chunks · v0.31.1]" to stderr before each routed command. Kills the
"am I empty?" confusion that drove the original Hermes/Neuromancer
report against wintermute (102k pages → empty CLI search results).

Cache: 60s TTL, in-memory Map keyed by mcp_url so switching hosts via
`gbrain init` invalidates cleanly. Cross-process file cache deferred.

Suppression: --quiet, GBRAIN_NO_BANNER=1, non-TTY default suppresses
unless GBRAIN_BANNER=1 explicitly opts in (clean pipes for shell flows).

Failure mode: banner fetch errors swallowed; underlying command runs
normally. Banner is observability, never load-bearing. The hardened
callRemoteTool will surface the same error class on the actual call
if the host is genuinely unreachable.

Inline in cli.ts per CDX-1 (no parallel module). _clearIdentityCacheForTest
exported as test escape hatch.

Backed by the new `get_brain_identity` MCP op (read-scope, banner-only).

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…anomalies/graph-query/think)

These four CLI commands bypass the operation-layer dispatch and call
engine methods directly today, so the cli.ts routing seam doesn't catch
them. Each gets a thin per-command branch: when isThinClient(cfg),
callRemoteTool against the corresponding op; otherwise existing engine
path runs unchanged.

Mappings:
- gbrain salience    → get_recent_salience  (read scope, 30s timeout)
- gbrain anomalies   → find_anomalies       (read scope, 30s timeout)
- gbrain graph-query → traverse_graph       (read scope, 30s timeout)
- gbrain think       → think                (write scope, 180s timeout)

`think` is a special case: the server's think op intentionally disables
--save/--take for remote callers (operations.ts:1103-1135 trust-boundary
gate per CLAUDE.md subagent-isolation policy). Thin-client think prints a
loud warning when those flags are set so users know what they lose
instead of silent ignoring. Documented as v0.31.x policy review in plan.

Output format unchanged on both paths — the MCP op handler IS the engine
method, so the unpacked tool result has identical shape.

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
\`gbrain remote doctor\` gains a 5th check that probes the read + admin
scope tiers via two harmless read-only MCP calls (get_brain_identity
and get_health). Surfaces v0.29.2/v0.30.0 thin-client clients that
registered with read+write only and now hit \`gbrain stats\` /
\`gbrain history\` and fail mid-flight — instead of failing
mid-command, doctor names the exact remediation:

  On the host: gbrain auth register-client <name> --grant-types
    client_credentials --scopes read,write,admin

Status semantics (informational by default):
- read.missing_scope  → fail (broken setup)
- admin.missing_scope → warn + pinpoint hint (the load-bearing case)
- both succeed        → ok
- non-scope probe errors (parse/network/timeout) → ok with
  detail.inconclusive=true (doctor's overall status doesn't flap)

GBRAIN_DOCTOR_SKIP_SCOPE_PROBE=1 env-flag for test fixtures that mock
/mcp at JSON-RPC initialize level only (MCP SDK Client hangs on shape
mismatch and doesn't always honor AbortSignal — adversarial test
behavior we don't want to bake into doctor).

Pinned by 8 cases in test/oauth-scope-probe.test.ts (pure-function
buildScopeCheck) plus unchanged passing of all 23 doctor-remote tests.

CDX-5 from the codex outside-voice review. Keeps host-side
\`gbrain auth register-client\` default at \`read\` (no breaking change
for existing scrapers); puts the migration burden on the THIN-CLIENT
side where it belongs.

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ol hints (CDX-2)

Per the CDX-2 op-coverage audit: takes and sources are multi-subcommand
CLIs with mixed local/routable surface. Their READ subcommands
(takes_list, takes_search, sources_list, sources_status) have MCP
equivalents — those land in v0.31.x with per-subcommand splits.

For v0.31.1, refuse both at the top level with hints naming the MCP
tools so agents know exactly which tools to invoke directly. Honest
framing per CDX-2: "thin-client gbrain routes the read+write+admin op
surface; multi-subcommand CLIs land incrementally."

Per-subcommand routing recorded as v0.31.x TODO in the plan.

Storage is also refused (filesystem-bound; no remote equivalent).

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…CLAUDE.md

Cross-cut for v0.31.1 ship:
- VERSION: 0.30.0 → 0.31.1
- package.json: "version": "0.31.1" (bun install refreshed bun.lock)
- CHANGELOG.md: full release-summary entry per CLAUDE.md voice contract
  (numbers-that-matter table with before/after comparison, what-this-means
  closer, take-advantage block with exact remediation commands, itemized
  changes by surface, contributor section with plan/decision-history pointer)
- TODOS.md: 7 follow-up entries for v0.31.x (timing telemetry, job-routing,
  per-subcommand takes/sources split, transcripts privacy decision,
  trust-boundary policy review, register-client default flip, cross-process
  token cache, parity test backfill)
- CLAUDE.md: new "Thin-client routing" section under "Key files" annotating
  every changed/new file with its v0.31.1 contract — src/cli.ts routing
  seam, src/core/mcp-client.ts hardening, src/core/cli-options.ts --timeout,
  src/core/doctor-remote.ts scope-probe, get_brain_identity op, per-command
  routing in salience/anomalies/graph-query/think.

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ms.txt

Replaces the env-var GBRAIN_DOCTOR_SKIP_SCOPE_PROBE module-mutation in
test/doctor-remote.test.ts with an explicit opts arg threaded through
collectRemoteDoctorReport(config, opts). Satisfies the test-isolation
lint (rule R1: no process.env.X = ... in non-serial unit files).

Production callers still honor the env-flag for ops bypass; opts wins
when both are set.

Also regenerates llms.txt + llms-full.txt to match the v0.31.1 CLAUDE.md
additions (build:llms drift check passes).

Part of v0.31.1 fix for #734.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Merged v0.30.1 (operational hardening) and v0.30.2 (dream synthesize fat-transcript chunking) from master into the v0.31.1 thin-client branch.

Conflict resolutions:
- VERSION: 0.31.1 wins (newer than master's 0.30.2)
- package.json: 0.31.1 wins
- CHANGELOG.md: v0.31.1 entry sits above master's v0.30.2/v0.30.1 entries (newest first per Keep-a-Changelog)
- Other files: auto-merged cleanly

llms.txt + llms-full.txt regenerated post-merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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.

Thin-client mode (--mcp-only): CLI commands silently hit empty local PGLite instead of routing through MCP

1 participant