v0.37.7.0 fix wave: federated brains + autopilot safety + OAuth confidential clients#1253
Open
garrytan wants to merge 15 commits into
Open
v0.37.7.0 fix wave: federated brains + autopilot safety + OAuth confidential clients#1253garrytan wants to merge 15 commits into
garrytan wants to merge 15 commits into
Conversation
`createEngine()` from src/core/engine-factory.ts only constructs the engine; callers MUST call connect() before any executeRaw. The reindex-frontmatter CLI was constructing the engine and going straight to countAffected, which crashed on PGLite with "PGLite not connected. Call connect() first." even on --dry-run. Fix follows the existing-command pattern (src/commands/auth.ts, src/commands/backfill.ts, src/commands/integrity.ts all do the same): pass toEngineConfig(cfg) into both createEngine() AND engine.connect(), then engine.initSchema() (idempotent on a current schema, ~1ms cost). Pre-fix verification: codex outside-voice CF5 flagged the related "can't import connectEngine from cli.ts" misdirection in the original fix plan. This implementation uses the canonical sibling pattern instead. Regression test pinned at test/reindex-frontmatter-connect.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
v0.37.5.0 claimed by #1229 (warsaw-v4); v0.37.6.0 by #1246 (OpenRouter recipe). v0.37.7.0 is the next free slot for this fix wave. CHANGELOG entry stubbed in user-facing voice per CLAUDE.md "CHANGELOG voice + release-summary format" — ELI10 lead-first, real fix details below. The "## To take advantage of v0.37.7.0" block follows the v0.13+ self-repair pattern from CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bug: when the worker resumed a subagent job whose persisted last message was an assistant turn with text-only content (no tool_use blocks), the replay reconciler at subagent.ts:241-247 had no branch for that case. The main loop then called messages.create against a conversation ending in assistant role, which Sonnet 4.6+ rejects with HTTP 400 "This model does not support assistant message prefill." 3 retries later → dead-letter, despite all the job's work having committed in earlier turns. @zscgeek's bug report pinned this exactly: dream-cycle Otter corpus runs hit ~7% dead-letter rate, every dead job's last subagent_messages row was a text-only synthesis summary listing slugs that already existed in `pages`. Their proposed fix mirrors this implementation. Fix: add an else branch to the assistant-tail check that mirrors the live-loop terminal logic at subagent.ts:440-447 — reconstruct finalText from the persisted text blocks, return stop_reason='end_turn' immediately. No LLM call, no schema change. Two new regression cases: - text-only terminal on resume returns immediately with zero messages.create calls - tool-use replay path unchanged (existing behavior preserved) Codex outside-voice (CF13) initially flagged this fix as mis-targeted, claiming subagent.ts already handled the case. /investigate run revealed the live-loop terminal at :440-447 was covered but the REPLAY-path terminal at :241-247 was missing — both branches need symmetric handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The autopilot lockfile was hardcoded at `~/.gbrain/autopilot.lock`
(via `process.env.HOME`), bypassing GBRAIN_HOME. Two brains pointed
at different GBRAIN_HOME directories still wrote to the same global
lockfile; one would silently take over the other on each restart.
Fix: route through `gbrainPath('autopilot.lock')` from
src/core/config.ts (imported aliased as gbrainHomePath since the
local `gbrainPath` var in installAutopilot references the CLI
binary path). The mkdirSync(`~/.gbrain`) call also routes through
the helper so the directory is created in the right place too.
Co-authored with @rafaelreis-r — same fix shape as PR #1227,
re-implemented against current master per the wave's
"re-implement, credit, close" workflow.
Tests cover: one GBRAIN_HOME → one canonical lock; two
GBRAIN_HOME values → two distinct locks; default fall-through
still works.
Co-Authored-By: rafaelreis-r <noreply@github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The graph-query CLI silently dropped edges to pages in other sources
on federated brains. Users had no signal those edges existed unless
they read the source code.
Fix:
- New --include-foreign flag (off by default, preserves the existing
scoping contract; on = explicit cross-source traversal).
- After every traversal, count edges from rootSlug whose target page
lives in a different source. When count > 0 AND user didn't opt in,
emit a stderr footer:
`(N edge(s) to foreign-source pages hidden; pass --include-foreign
to include them)`
- The "no edges found" path also runs the count + footer so users
discover foreign edges even when scoped traversal returned nothing.
- Thin-client path skips the count (engine query not available);
future T1 work threads source resolution through MCP for that path.
- Single quotation correctness in count SQL: page_links table is
`links` (not `page_links`); JOIN both endpoints to pages and compare
source_id, NULL-safe via `IS NOT NULL` guards on both sides.
- Fail-open on missing source_id column for pre-v0.18 brains: return 0
(no foreign edges to report) instead of throwing.
4 new test cases: footer fires on scoped query with foreign edge,
--include-foreign suppresses footer, zero-foreign no-footer case,
pluralization regression guard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Federated-brain users running destructive ops (extract, import,
purge) need a way to verify which source they're targeting BEFORE
the op runs. Pre-fix, the only way was to grep config files or run
the op with --dry-run and inspect output.
New command:
gbrain sources current # human output
gbrain sources current --json # machine-readable
gbrain sources current --source X # show what an explicit --source
# X would resolve to (validates
# X exists in the sources table)
Output names BOTH the resolved source id AND which tier of the 6-tier
resolution chain won (flag / env / dotfile / local_path /
brain_default / seed_default), plus a `detail` line naming the
winning signal (e.g. "GBRAIN_SOURCE=dept-x" or ".gbrain-source" or
"/work/gstack/src").
Implementation:
- New `resolveSourceWithTier()` in source-resolver.ts as an additive
variant of `resolveSourceId()`. Walks the same 6 steps in the same
order; just returns `{ source_id, tier, detail? }` instead of bare
string. Existing `resolveSourceId()` unchanged — all callers
continue working.
- New `SOURCE_TIER_NAMES` const + `SourceTier` type export so the
CLI, doctor (Tier 5 follow-up), and future MCP consumers share one
vocabulary instead of inlining strings.
- Help text updated; `current` subcommand registered in dispatcher.
11 new tests pin the 6-tier ladder + priority semantics. Existing
19 source-resolver tests still pass (regression preserved).
Per codex CF3 (the existing src/core/source-resolver.ts was missed
in the original plan). Re-uses the existing helper instead of
inventing a duplicate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Federated brain users running `gbrain extract` had no way to scope
extraction to one source. The DB path walks all sources together via
listAllPageRefs(), which is correct for cross-source resolution but
sometimes the user wants to extract per-source explicitly (e.g.
re-running extract on a specific source after a manual import).
The pre-existing `--source` flag is the data-source axis (fs|db) and
can't be repurposed. New flag `--source-id <id>` joins it on the
brain-source-id axis:
gbrain extract all --source db --source-id alpha
-> walks only alpha-source pages; extracts links + timeline
from those, into the alpha source
Important: the resolver maps (allSlugs + slugToSources) stay built
from the FULL listAllPageRefs result, not the scoped subset. This
ensures qualified cross-source wikilinks like `[[other-src:slug]]`
still resolve correctly even when the extract walk is scoped — the
filter is on which pages we extract FROM, not what we can resolve TO.
Threaded through both `extractLinksFromDB` and `extractTimelineFromDB`
with backward-compat: callers passing no opts get the old behavior.
4 new test cases pin: walks-all-without-flag baseline,
alpha-only-when-scoped-to-alpha, beta-only-when-scoped-to-beta,
empty-set-on-unknown-source.
Note: #1204's wider "silent 0 links" report on federated brains has
additional facets beyond this flag (resolver path edge cases on
overlapping slugs). The scoped-walk fix gives users an explicit
workaround AND closes the per-source extraction gap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three items deferred from v0.37.7.0: 1. #1173 .sql indexing — verify-first gate found tree-sitter-sql.wasm missing from src/assets/wasm/grammars/. Dedicated wave needed: vendor the wasm, add .sql to walker filter, address slug-shape collision with #1172. 2. #1204 deeper investigation — wave added --source-id flag as workaround. Underlying silent-zero-links bug on unscoped federated extracts needs its own /investigate pass against a cross-source-duplicate-slug fixture. 3. Tier 5N doctor sweep for dead-lettered subagent jobs matching the #1151 fingerprint. Deferred to v0.37.8+ behind the islamabad doctor.ts conflict resolution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sync walker descended into git submodules and indexed their markdown content as if it belonged to the parent brain. Users with submodules in their brain repo saw foreign content in their pages table. Fix: pruneDir gains an optional `parentDir` arg. When set, the helper stats `<parentDir>/<name>/.git` and skips the directory if `.git` exists as a FILE (gitfile pointer — the canonical submodule shape). Directories containing `.git` as a DIRECTORY (a real nested repo, not a submodule) are descended into; the inner `.git` dir itself is then dot-prefix-excluded. Callers updated to pass parentDir: - src/commands/extract.ts walkMarkdownFiles - src/core/cycle/transcript-discovery.ts walker Back-compat preserved: existing pruneDir(name) callers without parentDir get the pre-v0.37.7.0 behavior unchanged. Companion `.gitignore`-respect feature from PR #1159 (@jetsetterfl) NOT in this wave — it would require adding the `ignore` npm package as a dep, which the plan's "no new deps in this PR" gate excludes. Filed as follow-up TODO for a dedicated wave. 5 new test cases pin the submodule shape + back-compat + nested-repo ambiguity. Existing extract-fs / extract-db tests unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The convention skill didn't have a tier-by-tier reference for how gbrain resolves the active source. Users running federated brains had to read the source code to know which signal wins. Added: - Canonical 6-tier table (flag → env → dotfile → local_path → brain_default → seed_default) matching src/core/source-resolver.ts. - Pointer to `gbrain sources current` (new in v0.37.7.0) as the verification command. - The CLI-layer trust boundary note: operations.ts handlers don't read env/dotfile (preserves v0.34.1.0 source-isolation work for MCP callers). - Per-command flag map: --source, --source-id (extract), and --include-foreign (graph-query). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`gbrain import --source dept-x ./pages` silently fell back to the default source because the CLI parser never consumed --source. PR #707's design intent excluded the flag explicitly; users had no signal their pages were going to the wrong place. #1167 + #1222 filed the regression. Fix: parse `--source-id <id>` (matching v0.37.7.0 extract.ts T2's naming convention — --source-id stays out of conflict with future axes that may want --source). When set, the flag value wins over any programmatic opts.sourceId; back-compat preserved for callers that pass sourceId via opts only. Also threaded into the positional-dir arg parser's flagValues set so `--source-id <value> <dir>` doesn't treat <value> as the dir. Note on related surfaces: - `gbrain query "X" --source_id dept-x` already routed correctly via the operations.ts query op (added in v0.34) — no fix needed. - `gbrain extract --source-id <id>` shipped in T2. - `gbrain sync --source <id>` already worked (pre-existing). - `gbrain sources current` (shipped in T4) is the verification tool — run it before destructive ops to confirm routing. Closes the silent-fallback for the import path. Co-authored with @tyad67-netizen (#1168), @hnshah (#1124, #1120), whose patches informed the shape; re-implemented against current master per the wave's "re-implement, credit, close" workflow. 3 new test cases pin: default-without-flag, --source-id-routes-correctly, flag-value-not-treated-as-dirArg. Co-Authored-By: tyad67-netizen <noreply@github.com> Co-Authored-By: hnshah <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-fix: when database_url was unset/malformed, the DB-health-check reconnect loop logged `config.database_url undefined` forever because the catch swallowed every error type uniformly. launchd's KeepAlive=true respawned immediately on any exit, so even when the process did exit, it came right back into the same bad state. @colin477 reported the daemon-thrash pattern. Two-part fix: 1. In-process error classifier — `classifyReconnectError(err)`: - `unrecoverable` (database_url missing/empty/malformed, auth failure, no-brain-configured): exit immediately with a clear stderr line. Pattern-matched against postgres / config-loader error shapes. Tests pin the matcher against the #1162 fingerprint exactly. - `recoverable` (network blip, pool saturated, connection refused on a port coming up, Supabase 503): retry. Up to GBRAIN_AUTOPILOT_MAX_RECONNECT_FAILS (default 30 = ~5min) before finally giving up with `max_reconnect_fails_exceeded`. - Counter resets on every successful health probe or reconnect. 2. launchd plist gains `ThrottleInterval=60`. Combined with the in-process exit, launchd waits 60s before relaunching instead of immediate respawn. Pure-function `generateLaunchdPlist()` exported for tests. 16 new test cases: - 11 classifier cases (database_url shapes, malformed URL, auth, role-does-not-exist with quoted name, network blip, pool saturated, 503, non-Error inputs, case-insensitivity) - 5 plist generator cases (ThrottleInterval=60, KeepAlive preserved, wrapper path, XML escaping, StandardErrorPath). Pre-existing autopilot-lock-path tests unchanged — both fixes land cleanly side-by-side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
v0.34.1.0 (#909) fixed PUBLIC PKCE clients (client_secret=undefined) by normalizing NULL → undefined in getClient. Confidential clients regressed: the MCP SDK's clientAuth middleware does plaintext `client.client_secret !== presented_secret` compare, but gbrain stores SHA-256 hashes, so the SDK's compare always failed for authorization_code and refresh_token grants on confidential clients. Result: /token returned `invalid_client` for every confidential exchange. Fix shape per locked-decision-5: custom /token middleware BEFORE the SDK's authRouter, similar to the pre-existing client_credentials handler. The middleware: 1. Detects confidential auth via `client_secret` in body (client_secret_post) OR `Authorization: Basic` header (client_secret_basic per RFC 6749 §2.3.1). 2. Falls through to the SDK when neither is present (public PKCE path stays canonical, preserves v0.34.1.0 behavior). 3. Calls new `verifyConfidentialClientSecret(clientId, presented)` on the provider which does SHA-256 hash compare ourselves (same shape as exchangeClientCredentials' existing hash check). 4. On verification success, calls existing `exchangeAuthorizationCode` / `exchangeRefreshToken` directly with the validated client. 5. RFC 6749 §5.2 error semantics: 401 invalid_client for auth failures, 400 invalid_grant for code/token problems. Per CLAUDE.md "GBRAIN:RLS_EXEMPT" annotation contract: this surface sits in front of the SDK's clientAuth and doesn't depend on the SDK's plaintext compare working — the SDK's middleware never fires for confidential paths the new middleware claims. 7 new test cases pin: correct-secret-returns-client, wrong-secret opaque rejection, non-existent client, public-client refuses the confidential path, case-sensitivity, soft-deleted revocation, verify-then-exchange-refresh round-trip with second-use rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(T12/T13/T14) Three v0.37.7.0 doctor checks landing in one atomic commit (single file, shared merge-conflict surface with garrytan/islamabad-v3 per locked decision 1): 1. source_routing_health (T12 / #1167): Sample non-default sources for pages; warn when a registered source has zero pages (silent-collapse-to-default fingerprint). D5 lock: total-sample cap of 200 pages across all sources, with per-source cap = min(50, ceil(200/N)) so a 20-source CEO brain pays 200 selects, not 1000. Fix hint paste-ready to `gbrain sources current --json` for verification. 2. oauth_confidential_client_health (T13 / #1166): Probe every oauth_clients row. Confidential clients (auth_method != 'none') must have a non-NULL client_secret_hash; if any row claims confidential auth but stores NULL hash, that's the pre-v0.37.7.0 regression. Public clients (auth_method='none') correctly keep NULL hash per v0.34.1.0 #909. Fix hint: `gbrain auth revoke-client + register-client` OR `gbrain upgrade`. Pre-OAuth schemas (missing oauth_clients table) skip gracefully. 3. autopilot_lock_scope (T14 / #1226): Detect stale ~/.gbrain/autopilot.lock outside the current GBRAIN_HOME. Codex CF11: dangerous to paste-ready `rm` without verifying the owning PID isn't a live process. Hint reads the PID file and gives the user a `ps -p <pid>` check before any delete — matches sshd-style stale-lock recovery hints. 9 new test cases pin the canonical paths. Pre-existing 80+ doctor checks unchanged. Expected to conflict with garrytan/islamabad-v3 at merge time. The 3 new check functions live in their own block far from the islamabad skill_brain_first check; the conflict surface should be limited to the `checks.push(...)` call site near the end of runDoctor's DB-checks phase (~10 lines). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on lint) The new source-resolver-with-tier.test.ts from T4 mutated process.env.GBRAIN_SOURCE directly in two cases, which violates scripts/check-test-isolation.sh R1 (env mutations leak across parallel-loaded test files in the same shard process). Fix: wrap both mutation sites in withEnv() from test/helpers/with-env.ts, which saves+restores via try/finally per the canonical pattern in CLAUDE.md. Pure refactor — all 11 cases still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
14 atomic per-Tier commits closing user pain across federated brains, autopilot blast-radius, OAuth confidential clients, and a handful of silent-CLI-failures.
Highlights (by user pain class):
Federated brains:
gbrain import --source-id <id>actually routes to the named source (bug: import --source and search --source silently ignore source routing #1167, gbrain put/import always use 'default' source — no way to specify source_id from CLI or per-project config #1222). Pre-fix the flag was silently ignored.gbrain extract --source-id <id>scopes extraction to one source (gbrain extract is not source-aware — silently inserts 0 links on federated brains; doctor recommends a command that cannot work #1204). Cross-source resolution preserved.gbrain graph-queryshows a count of hidden foreign-source edges with a paste-ready--include-foreignhint (graph-query silently drops edges to foreign-source pages under scoped queries #1153). No more silent drops.gbrain sources currentechoes the resolved source + which tier of the 6-tier chain won (gbrain put/import always use 'default' source — no way to specify source_id from CLI or per-project config #1222). Verify before destructive ops.Autopilot blast-radius:
GBRAIN_HOMEso two brains don't thrash (autopilot lock file ignores GBRAIN_HOME — second brain silently respawns forever #1226, absorbed PR fix(autopilot): scope lock file to GBRAIN_HOME #1227 @rafaelreis-r).database_url undefinedetc.); launchd plist gainsThrottleInterval=60so the OS backs off instead of respawning forever (autopilot: reconnect loop after 5 consecutive failures logs "config.database_url undefined" indefinitely, never exits #1162 @colin477).OAuth confidential clients:
client_secret_basicandclient_secret_postexchange returnedinvalid_clientbecause the MCP SDK does plaintext compare and gbrain stores SHA-256 hashes (oauth: clientsStore.getClient() returns client_secret_hash where SDK expects plaintext client_secret; breaks /token and /revoke for all authorization_code clients #1166 @dcarolan1). Custom /token middleware verifies hashes ourselves before delegating to existing exchange methods.Silent CLI failures:
gbrain reindex-frontmatterno longer crashes before doing anything (gbrain reindex-frontmattercreates an unconnected PGLite engine and fails before dry-run #1225 @oudouusa).Doctor (Tier 5, K-N): 3 new checks for source-routing health, OAuth-confidential health, and stale autopilot lockfile (with PID-safe rm hint).
Docs: brain-routing convention skill documents the 6-tier source resolution chain explicitly.
Test plan
bun run verifyclean (typecheck + 4 grep guards)bun run test: 8185 pass / 5 pre-existing failures (verified on master @ 1580c6d — same 5 fail)Codex outside-voice findings (all absorbed)
The /plan-eng-review codex pass caught 3 verified factual errors in the original plan:
src/core/source-resolver.tsalready exists (v0.18.0, 6-tier resolver) — plan was inventing a duplicate. Fix: reuse + addresolveSourceWithTier()additively (T4).src/commands/query.tsdoes not exist — actual file issearch.ts. Plan-text drift corrected.connectEngineis not exported from cli.ts — circular dep direction. Fix: reindex-frontmatter follows the existing-command pattern instead (T7).Additional codex findings folded in:
ctx.sourceIdper v0.34.1.0.ps -p <pid>verification before deletion.Out of scope (filed as follow-ups in TODOS.md)
--source-idworkaround. Underlying silent-zero-links bug on unscoped federated extracts needs its own /investigate pass.subagent_terminal_dead_lettersdoctor check — T9 fix landed; doctor sweep deferred to v0.37.8 once islamabad-v3 doctor.ts conflict resolves.Community PRs absorbed (closing in follow-up comments)
ignorenpm package, plan's "no new deps" gate excludes; PR stays open as the canonical landing path for a follow-up wave.)Each PR will be closed with a credit comment naming this PR + Co-Authored-By trailers in the relevant commits.
🤖 Generated with Claude Code