From 2504abe47fcf1b8c4832027dc82fdc250a9eacfa Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 17 May 2026 08:00:29 -0700 Subject: [PATCH] v0.35.3.0 fix wave: extract_facts items + git --no-recurse-submodules placement (#1053) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(mcp): centralize ParamDef→JSON Schema via shared paramDefToSchema Three duplicate inline mappers existed across the MCP surface: - src/mcp/tool-defs.ts (stdio MCP buildToolDefs) - src/commands/serve-http.ts:837 (live HTTP MCP tools/list) - src/core/minions/tools/brain-allowlist.ts:84 (subagent tool registry) Each had subtly different items propagation. The HTTP MCP variant dropped items entirely, leaving extract_facts.entity_hints broken for OAuth- authenticated remote agents even after a buildToolDefs-only patch. The subagent variant propagated one level of items but used the same shallow shape so nested arrays would silently drop. Extract a single recursive paramDefToSchema helper exported from src/mcp/tool-defs.ts and have all three mappers consume it. Closes the bug class at the architecture level instead of patching one site at a time. The helper copies type, description, enum, default, and recursively rebuilds items so array-of-arrays preserves inner shape. Key ordering (type, description, enum, default, items) matches the pre-v0.34 inline mappers so JSON.stringify output stays byte-stable for every existing operation that does not use nested arrays. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(schema): add items to extract_facts.entity_hints and handle-to-tweet candidates Two array fields shipped without the items property required by JSON Schema. Strict-mode validators (Gemini Pro structured outputs, OpenAI strict tool definitions) reject the entire schema when any type:'array' lacks items. Downstream agents on those providers couldn't use extract_facts or the x_handle_to_tweet resolver. extract_facts.entity_hints — declared items: { type: 'string' } matching the handler at src/core/operations.ts:2733 which already coerces the runtime value to string[]. handle_to_tweet outputSchema.candidates — full XTweetCandidate spec including required + additionalProperties: false. The XTweetCandidate TypeScript interface declares all five fields as required; without required in the JSON Schema, a validator would accept {} as a valid candidate. additionalProperties: false closes the OpenAI strict-mode contract. 19 community PRs (#1028 #999 #980 #979 #910 #904 #847 #832 #863 #862 #812 for entity_hints; #910 caught candidates) converged on these locations. This wave cherry-picks the deepest variant (#910 surfaced both bugs) and centralizes via the paramDefToSchema helper from the preceding commit so the live HTTP MCP tools/list path is also fixed. Co-Authored-By: Claude Opus 4.7 (1M context) Co-Authored-By: DmitryBMsk (PR #910) * fix(git-remote): move --no-recurse-submodules after the subcommand verb Git CLI accepts two flag positions: git [global -c flags] [subcommand flags] [args] Global -c config flags belong before the verb. Subcommand-specific flags (like --no-recurse-submodules) belong after. Pre-v0.34 GIT_SSRF_FLAGS spliced both kinds before the verb, so cloneRepo invoked: git -c http.followRedirects=false ... --no-recurse-submodules clone URL DIR Real git rejects this with exit 129 ("unknown option: --no-recurse-submodules") because --no-recurse-submodules is a clone subcommand flag, not a global config flag. Every remote-source clone broke in production from v0.28 onward. The fake-git harness in test/git-remote.test.ts exits 0 regardless of argv shape, which is why CI never caught it. Split GIT_SSRF_FLAGS (3 -c config flags, spread BEFORE the verb) from GIT_SSRF_SUBCOMMAND_FLAGS (--no-recurse-submodules, spread AFTER the verb). cloneRepo and pullRepo both spread the new constant after their respective verbs. The constant names signal the position rule so future additions land in the right place. 7 community PRs converged on this location (#1023 #1020 #985 #963 #846 #842 — #800 doesn't exist). This wave cherry-picks the semantic- constant approach from #846's GIT_SSRF_SUBCOMMAND_FLAGS name (the clearest signal of the position rule). Co-Authored-By: Claude Opus 4.7 (1M context) * test(mcp+git+resolvers): structural array-items + subcommand-position guards Three new tests / test groups close the bug classes the wave fixes: test/mcp-tool-defs.test.ts — recursive structural guard walks every operation's inputSchema and fails with a property path if any type:'array' lacks items.type. Explicit fixture assertions for extract_facts.entity_hints.items.type and a synthetic nested-array ParamDef pinning items.items.type recursion. Without the explicit fixtures the legacyInlineMap byte-equality test is mirror-theater — mirroring both sides of the equality preserves the blind spot. test/git-remote.test.ts — split snapshot test into GIT_SSRF_FLAGS (3 global -c entries) and GIT_SSRF_SUBCOMMAND_FLAGS (--no-recurse-submodules). cloneRepo + pullRepo argv tests now assert the subcommand flag appears AFTER the verb index. Pre-v0.34 the pinned argv slice prefix included --no-recurse-submodules, which baked the bug into the test suite (codex catch). test/resolvers.test.ts — recursive walk over both inputSchema AND outputSchema for builtin resolvers (xHandleToTweetResolver, urlReachableResolver). Explicit imports rather than getDefaultRegistry(), which starts empty until commands/resolvers.ts runs — codex catch on a hollow-walk failure mode. Dedicated case pins candidates items shape including required + additionalProperties. Reference legacyInlineMap in mcp-tool-defs.test.ts mirrors the new recursive paramDefToSchema helper. No current op uses nested arrays so the byte-equality test stays green for every existing operation. Co-Authored-By: Claude Opus 4.7 (1M context) * test(e2e): raise rerank timeouts for ZE live cold-start The first rerank call of a CI run hits ZeroEntropy's cold-start latency (observed ~5-6s on Tier 2 LLM Skills runners; subsequent calls < 500ms). Two timeouts fired simultaneously at ~5s: 1. bun:test's default 5000ms per-test timeout caused (fail). 2. gateway.rerank's DEFAULT_RERANK_TIMEOUT_MS = 5000 fired right after, reported as "Unhandled error between tests". The next rerank test (top_n=2) ran in 409ms because the API was already warm. Cold-start is the only issue. Pass explicit timeoutMs to each rerank() call and a longer per-test timeout (30s) on both ZE rerank tests. Production DEFAULT_RERANK_TIMEOUT_MS stays at 5s for the search hot path — these E2E tests bypass it locally without changing the default that protects user latency. Unrelated to the fix-wave in this PR (mcp-tool-defs + git-remote + resolver guards). Lands here to keep Tier 2 LLM Skills green. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: bump version and changelog (v0.35.2.0) Co-Authored-By: Claude Opus 4.7 (1M context) * docs: sync for v0.35.2.0 Update CLAUDE.md Key files annotations for the v0.35.2.0 fix wave: - src/mcp/tool-defs.ts: document new exported recursive paramDefToSchema helper and the three-consumer centralization (stdio MCP, HTTP MCP tools/list, subagent registry). - src/core/minions/tools/brain-allowlist.ts: paramsToInputSchema now consumes the shared helper. - src/commands/serve-http.ts: tools/list handler now consumes the shared helper (closes the HTTP MCP items-dropped bug class). - src/core/git-remote.ts: new entry. Documents the GIT_SSRF_FLAGS (global config, pre-verb) vs GIT_SSRF_SUBCOMMAND_FLAGS (subcommand-scoped, post-verb) split, the 7-month silent regression, and the position-anchored regression guard in test/git-remote.test.ts. Regenerated llms-full.txt to match. Co-Authored-By: Claude Opus 4.7 * chore: rebump version to v0.35.3.0 Queue moved while this PR was open — v0.35.2.0 was claimed by master's v0.35.1.0 sibling work. Advancing one slot. No code changes; only: - VERSION + package.json: 0.35.2.0 → 0.35.3.0 - CHANGELOG.md: rewritten header + inline references - CLAUDE.md: rewritten 4 key-file annotations - llms-full.txt + llms.txt: regenerated to mirror CLAUDE.md Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 44 ++++++ CLAUDE.md | 7 +- VERSION | 2 +- llms-full.txt | 7 +- package.json | 2 +- src/commands/serve-http.ts | 8 +- src/core/git-remote.ts | 30 +++- src/core/minions/tools/brain-allowlist.ts | 8 +- src/core/operations.ts | 2 +- .../builtin/x-api/handle-to-tweet.ts | 16 +- src/mcp/tool-defs.ts | 36 ++++- test/e2e/zeroentropy-live.test.ts | 14 +- test/git-remote.test.ts | 33 ++++- test/mcp-tool-defs.test.ts | 137 ++++++++++++++++-- test/resolvers.test.ts | 95 ++++++++++++ 15 files changed, 395 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f5e7626d..7d8bd94a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ All notable changes to GBrain will be documented in this file. +## [0.35.3.0] - 2026-05-15 + +**Fix wave: 19 stale community PRs land as one bisect-friendly PR with the architectural fix none of them surfaced.** + +Two real bugs, both in shipping code on master since v0.28+, both fixed at the architecture level instead of one site at a time. The first: `extract_facts.entity_hints` and the X-API resolver's `candidates` output schema both declared `type: 'array'` without `items`, so strict-mode validators (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the entire tool schema. Twelve community PRs converged on the entity_hints fix; only one (#910 from @DmitryBMsk) caught the candidates side. The second: every remote-source `git clone` and `git pull` invocation has been broken for ~7 months because `--no-recurse-submodules` was spliced BEFORE the subcommand verb in `GIT_SSRF_FLAGS`. Real git rejects this with exit 129 ("unknown option"); the fake-git test harness exits 0 regardless of argv shape, so CI never caught it. Seven community PRs flagged this. Codex outside-voice review on top of all 19 PRs surfaced the structural finding none of them saw: three duplicate `ParamDef→JSON Schema` mappers existed across the MCP surface, not one. The live HTTP MCP `tools/list` path (`serve-http.ts:837`) and the subagent brain-tool registry (`brain-allowlist.ts:84`) would both have stayed broken after a `buildToolDefs`-only patch. + +### What you can now do + +**Use `extract_facts` from strict-mode agents again.** Gemini Pro structured outputs and OpenAI strict tool definitions both reject `type: 'array'` without `items`. The `entity_hints` param now declares `items: { type: 'string' }` and the resolver `candidates` output declares the full `XTweetCandidate` interface with `required: [tweet_id, text, created_at, score, url]` and `additionalProperties: false`. The schema matches the TypeScript interface byte-for-byte — single source of truth. + +**Clone and pull remote sources over real git again.** `cloneRepo` and `pullRepo` now spread `--no-recurse-submodules` AFTER the verb where it belongs. Real git stops rejecting the call with exit 129. Constant naming signals the position rule so future flag additions land in the right place: `GIT_SSRF_FLAGS` is global config (spread before the verb), `GIT_SSRF_SUBCOMMAND_FLAGS` is subcommand-scoped (spread after). + +**Get one canonical ParamDef→schema mapper.** New `paramDefToSchema(p: ParamDef)` helper exported from `src/mcp/tool-defs.ts`. Three consumers now share one source of truth: `buildToolDefs` (stdio MCP), `serve-http.ts:837` (HTTP MCP `tools/list`), and `brain-allowlist.ts:84` (subagent tool registry). Recursive on `items` so nested array-of-arrays preserves inner shape on the wire — closes the same bug class one layer deeper. + +**Trust structural guards in CI.** Three new tests fail loudly with property paths if any future array drift reintroduces the bug class. `test/mcp-tool-defs.test.ts` walks every operation's `inputSchema` recursively. `test/git-remote.test.ts` asserts `--no-recurse-submodules` indexOf > verb indexOf — position-anchored, not just inclusion. `test/resolvers.test.ts` explicitly imports `xHandleToTweetResolver` + `urlReachableResolver` and walks both `inputSchema` AND `outputSchema` (the previously planned `getDefaultRegistry()` walk would have silently passed against zero resolvers — codex catch). + +### Itemized changes + +- `src/mcp/tool-defs.ts` exports new recursive `paramDefToSchema(p: ParamDef)` helper. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable for every operation that doesn't use nested arrays. +- `src/commands/serve-http.ts:837-849` swaps the inline ParamDef destructure for `paramDefToSchema(v)`. Closes the HTTP MCP `tools/list` bug. OAuth-authenticated remote agents (Claude Desktop, ChatGPT, Perplexity over HTTP MCP) now see `items` on every array param. +- `src/core/minions/tools/brain-allowlist.ts:84-97` swaps the inline destructure inside `paramsToInputSchema()` for `paramDefToSchema(v)`. Preserves the `required:` aggregation at `:95` (that's at the tool-def level, not the param level — codex explicitly out-of-scope). +- `src/core/operations.ts:2699` adds `items: { type: 'string' }` to `entity_hints`. The handler at `:2733` already coerced with `Array.isArray(...)` so runtime shape is unchanged; only the schema declaration was broken. +- `src/core/resolvers/builtin/x-api/handle-to-tweet.ts:102` replaces `candidates: { type: 'array' }` with full-spec items matching the `XTweetCandidate` interface 10 lines above, including `required: [all 5 fields]` and `additionalProperties: false` (D5 decision: without `required`, schema permits `{}`). +- `src/core/git-remote.ts:29-44` splits `GIT_SSRF_FLAGS` (3 `-c` config flags, spread BEFORE the verb) from new exported `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` (spread AFTER). `cloneRepo:156` and `pullRepo:182` now spread the new constant in subcommand position. +- `test/mcp-tool-defs.test.ts` adds: (a) explicit fixture for `extract_facts.entity_hints.items.type === 'string'`, (b) synthetic nested-array ParamDef pinning `items.items.type` recursion, (c) `findArrayWithoutItems` walker that fails the suite with a property path on any `type: 'array'` lacking `items.type`. `legacyInlineMap` reference mirrors the new recursive helper. +- `test/git-remote.test.ts` snapshot split: `GIT_SSRF_FLAGS` pins to 3 elements (no submodules), new `GIT_SSRF_SUBCOMMAND_FLAGS` snapshot pins to 1. `cloneRepo` + `pullRepo` argv tests assert `indexOf(--no-recurse-submodules) > indexOf(verb)` — position-anchored regression guard. Pre-v0.35.3 the existing test at `:233` baked the bug in via `argv.slice(0, GIT_SSRF_FLAGS.length)`. +- `test/resolvers.test.ts` (existing file) gets a new `describe` block. Explicitly imports `xHandleToTweetResolver` + `urlReachableResolver` and walks both `inputSchema` AND `outputSchema` recursively. Negative coverage guard asserts `builtins.length >= 2` so a future autoformatter dropping the array can't silently turn the walk into a no-op. +- `test/e2e/zeroentropy-live.test.ts` raises rerank test timeouts to handle ZeroEntropy's cold-start latency (observed ~5-6s on Tier 2 runners; subsequent calls < 500ms). Passes explicit `timeoutMs: 25000` to each rerank() call and a 30s bun:test per-test timeout. Production `DEFAULT_RERANK_TIMEOUT_MS = 5000` in `gateway.ts` stays put for the search hot path. +- Contributed by: @DmitryBMsk (PR #910 — deepest variant of the entity_hints + candidates double-fix); cleanest naming from PR #846 (`GIT_SSRF_SUBCOMMAND_FLAGS`). 17 superseded PRs being closed with thank-you notes after this merge: #1028, #1023, #1020, #999, #985, #980, #979, #963, #904, #863, #862, #847, #846, #842, #832, #812. + +## To take advantage of v0.35.3.0 + +`gbrain upgrade` is all you need. There's no schema migration, no config change, no manual action. + +1. Run `gbrain upgrade` — the new tool schemas reach your MCP clients on next restart. +2. Restart your MCP clients (Claude Code, Claude Desktop, ChatGPT, Cursor, etc.) so they re-fetch `tools/list`. +3. Verify `extract_facts` works from strict-mode agents: + ```bash + gbrain --tools-json | jq '.tools[] | select(.name == "extract_facts") | .inputSchema.properties.entity_hints' + ``` + Should show `{"type":"array","items":{"type":"string"},...}` — pre-fix, `items` was missing. +4. If you use remote-source git clones (`gbrain config get sources` shows any with a remote URL), they'll start working again on the next `gbrain sync`. Pre-fix, every clone of a remote-source repository was silently failing with git exit 129. + +If `gbrain doctor` flags anything after upgrade, please file an issue with the doctor output. This was a 7-month silent bug — the doctor's structural guards should catch the next one before it ships. ## [0.35.1.1] - 2026-05-16 **Fix wave: `gbrain eval longmemeval` actually runs against the public _s split.** diff --git a/CLAUDE.md b/CLAUDE.md index 2be8b7ed6..8e1e02c12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,7 @@ strict behavior when unset. - `src/core/storage.ts` — Pluggable storage interface (S3, Supabase Storage, local) - `src/core/storage-config.ts` (v0.22.11) — Storage tiering: `loadStorageConfig` reads `gbrain.yml`, normalizes deprecated keys (`git_tracked` / `supabase_only`) to canonical (`db_tracked` / `db_only`) with once-per-process deprecation warning, and runs `normalizeAndValidateStorageConfig` (auto-fixes missing trailing `/`, throws `StorageConfigError` on tier overlap). Path-segment matcher: `media/x/` does NOT match `media/xerox/foo`. Replaces gray-matter (broken on delimiter-less YAML) with a dedicated parser for the `gbrain.yml` shape. - `src/core/disk-walk.ts` (v0.22.11) — `walkBrainRepo(repoPath)` returns `Map` from one recursive `readdirSync`. Skips dot-dirs, `node_modules`, non-`.md` files. Used by `gbrain storage status` to replace per-page `existsSync + statSync` (~400K syscalls on 200K-page brains → tens). +- `src/core/git-remote.ts` (v0.35.3.0) — SSRF-hardened git invocations for remote-source `cloneRepo` and `pullRepo`. Exports two distinct flag constants because `git`'s argv grammar treats them differently: `GIT_SSRF_FLAGS` (3 `-c` config flags — `protocol.allow=user`, `protocol.file.allow=never`, `http.allowRedirects=false`) is **global config**, spread BEFORE the subcommand verb. New `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` is **subcommand-scoped**, spread AFTER the verb. Pre-v0.35.3 a single combined `GIT_SSRF_FLAGS` array spread `--no-recurse-submodules` before the verb where real git rejects it with exit 129 ("unknown option"); the fake-git test harness exited 0 regardless of argv shape, so CI missed it for ~7 months and every remote-source clone/pull was silently broken. `cloneRepo` argv: `git clone --depth=1 [--branch X] -- `. `pullRepo` argv: `git -C pull --ff-only`. Pinned by `test/git-remote.test.ts` position-anchored regression guard (`argv.indexOf('--no-recurse-submodules') > argv.indexOf(verb)`). - `src/commands/storage.ts` (v0.22.11) — `gbrain storage status [--repo P] [--json]`. Split into pure data (`getStorageStatus`) + JSON formatter + human formatter (ASCII-only per D10) matching the `orphans.ts` pattern. `PageCountsByTier` and `DiskUsageByTier` are distinct nominal types so swaps fail at compile time. - `gbrain.yml` (brain repo root, v0.22.11) — Optional storage tiering config. Top-level `storage:` section with `db_tracked:` and `db_only:` array-valued keys. `gbrain sync` auto-manages `.gitignore` for `db_only` paths on successful sync (skips on dry-run, blocked-by-failures, submodule context, or `GBRAIN_NO_GITIGNORE=1`). `gbrain export --restore-only [--repo P] [--type T] [--slug-prefix S]` repopulates missing `db_only` files from the database. - `src/core/supabase-admin.ts` — Supabase admin API (project discovery, pgvector check) @@ -149,8 +150,8 @@ strict behavior when unset. - `src/core/minions/wait-for-completion.ts` (v0.15) — poll-until-terminal helper for CLI callers. `TimeoutError` does NOT cancel the job; `AbortSignal` exits without throwing. Default `pollMs`: 1000 on Postgres, 250 on PGLite inline. - `src/core/minions/transcript.ts` (v0.15) — renders `subagent_messages` + `subagent_tool_executions` to markdown. Tool rows splice under their owning assistant `tool_use` by `tool_use_id`. UTF-8-safe truncation; unknown block types fall through to fenced JSON. - `src/core/minions/plugin-loader.ts` (v0.15) — `GBRAIN_PLUGIN_PATH` discovery. Absolute paths only, left-wins collision, `gbrain.plugin.json` with `plugin_version: "gbrain-plugin-v1"`, plugins ship DEFS only (no new tools), `allowed_tools:` validated at load time against the derived registry. -- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. -- `src/mcp/tool-defs.ts` (v0.15) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. +- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29, v0.35.3.0) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. **v0.35.3.0:** `paramsToInputSchema()` now consumes `paramDefToSchema` from `src/mcp/tool-defs.ts` instead of its own inline destructure. Required-aggregation at the tool-def level stays here (out of scope for the shared helper, which is per-param). Closes the third drift site in the ParamDef→JSON Schema bug class. +- `src/mcp/tool-defs.ts` (v0.15, v0.35.3.0) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. **v0.35.3.0:** exports the new recursive `paramDefToSchema(p: ParamDef)` helper — single source of truth for ParamDef→JSON Schema mapping. Three consumers now share one mapper: `buildToolDefs` (stdio MCP), `src/commands/serve-http.ts:837` (HTTP MCP `tools/list`), and `src/core/minions/tools/brain-allowlist.ts:84` (subagent tool registry). Pre-v0.35.3, three inline destructures had drifted across the surface — the live HTTP MCP path dropped `items` on every array param after a v0.32 review caught only the stdio side. Recursive on `items` so nested array-of-arrays preserves inner shape on the wire. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable. `test/mcp-tool-defs.test.ts` adds a `findArrayWithoutItems` walker that fails the suite with a property path on any future `type: 'array'` lacking `items.type`. - `src/core/minions/attachments.ts` — Attachment validation (path traversal, null byte, oversize, base64, duplicate detection) - `src/commands/agent.ts` (v0.16) — `gbrain agent run [flags]` CLI. Submits `subagent` (or N children + 1 aggregator) under `{allowProtectedSubmit: true}`. Single-entry `--fanout-manifest` short-circuits. Children get `on_child_fail: 'continue'` + `max_stalled: 3`. `--follow` is the default on TTY; streams logs + polls `waitForCompletion` in parallel. Ctrl-C detaches, does not cancel. - `src/commands/agent-logs.ts` (v0.16) — `gbrain agent logs [--follow] [--since]`. Merges JSONL heartbeat audit + `subagent_messages` into a chronological timeline. `parseSince` accepts ISO-8601 or relative (`5m`, `1h`, `2d`). Transcript tail renders only for terminal jobs. @@ -160,7 +161,7 @@ strict behavior when unset. - `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path. **v0.34.1.0 (#870):** stdin `'end'` / `'close'` shutdown hooks are skipped when `process.env.MCP_STDIO === '1'`. Gateway-piped stdio MCP wrappers (OpenClaw's `bundle-mcp`, similar) pipe the JSON-RPC handshake then close their stdin half; pre-fix this killed the server before the first tool call landed. Signal handlers (SIGTERM / SIGINT / SIGHUP) and the parent-process watchdog still cover legitimate disconnects. `src/commands/serve.ts` exposes `ServeOptions.mcpStdio?: boolean` as a test seam so the runtime guard is exercisable without process.env mutation. Pinned by `test/serve-stdio-lifecycle.test.ts`. - `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `validateParams(op, params)`. Single source of truth for `(ctx, params)` handler arg order and the 5-field `OperationContext` shape (engine + config + logger + dryRun + remote). Defaults to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `summarizeMcpParams(opName, params)` — privacy-preserving redactor for `mcp_request_log` and the admin SSE feed. Returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`. Intersects submitted top-level keys against the operation's declared `params` allow-list (declared keys preserved as a sorted array for debug visibility; unknown keys counted but never named, closing the attacker-controlled-key-name leak). Byte counts bucketed up to nearest 1KB so an attacker can't binary-search secret-content sizes via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`. - `src/mcp/rate-limit.ts` (v0.22.7) — Bounded-LRU token-bucket limiter. `buildDefaultLimiters()` returns the two-bucket pipeline: pre-auth IP (30/60s, fires BEFORE the DB lookup so brute-force load against `access_tokens` is actually capped) + post-auth token-id (60/60s). Tracks `lastTouchedMs` separately from `lastRefillMs` so an exhausted key can't be reset by hammering past the TTL. LRU cap bounds memory under attacker-controlled key growth. -- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. +- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. **v0.35.3.0:** the inline ParamDef→schema mapper at `:837-849` (HTTP MCP `tools/list` handler) is replaced with `paramDefToSchema(v)` from `src/mcp/tool-defs.ts`. Pre-fix this site silently dropped `items` on every array param so strict-mode OAuth clients (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the whole tool list. Single mapper now serves stdio MCP, HTTP MCP `tools/list`, and the subagent registry. - `src/core/sql-query.ts` (v0.31.3) — Engine-aware tagged-template SQL adapter for OAuth/admin/auth infrastructure. `sqlQueryForEngine(engine)` returns a `SqlQuery` (`(strings, ...values) => Promise`) that walks the template, builds `$N` positional SQL, asserts every value is a `SqlValue` (string | number | bigint | boolean | Date | null), and routes through `engine.executeRaw(sql, params)` so Postgres goes via postgres.js's `unsafe(sql, params)` path and PGLite via its embedded `db.query(sql, params)`. Deliberately narrower than postgres.js's `sql` tag: no nested fragments, no `sql.json()`, no `sql.unsafe()`, no `sql.begin()`, no array binding. The narrow surface is the feature — codex finding #7 from the v0.31 plan review argued the adapter should stay scalar-only or it drifts into a partial postgres.js clone. JSONB writes go through the separate `executeRawJsonb(engine, sql, scalarParams, jsonbParams)` helper that composes positional `$N::jsonb` casts and passes JS objects through; the v0.12.0 double-encode bug class doesn't apply because positional binding through `unsafe()` reaches the wire protocol with the correct type oid (verified by `test/sql-query.test.ts` on PGLite and `test/e2e/auth-permissions.test.ts:67` on Postgres). `scripts/check-jsonb-pattern.sh` doesn't fire because `executeRawJsonb(...)` is a method call, not the banned literal-template-tag interpolation pattern. Consumed by `src/commands/auth.ts`, `src/commands/serve-http.ts`, `src/core/oauth-provider.ts`, `src/commands/files.ts`, and `src/mcp/http-transport.ts` so all five sites work against PGLite and Postgres uniformly. Closes the bug where `gbrain auth` + `gbrain serve --http` were silently Postgres-only because they routed every SQL through the postgres.js singleton (community PR #681). - `src/commands/serve.ts` (v0.31.3) — `gbrain serve` stdio MCP entrypoint with idempotent shutdown across every parent-disconnect signal. Stdio EOF, SIGTERM, SIGINT, SIGHUP, and parent-process death (every reparent case — PID 1, launchd subreaper, systemd, tmux, or a parent shell with `PR_SET_CHILD_SUBREAPER`) all funnel into one `cleanup(reason)` path that releases the engine and the PGLite write-lock dir within 5 seconds. Pre-v0.31.3 the stdio MCP server held the lock indefinitely after Claude Desktop / Cursor / launchd-managed gateways disconnected, forcing a 5-minute stale-lock wait on the next start. Watchdog reparent check is `getParentPid() !== initialParentPid` (capturing the initial ppid once at install time and firing on any change); the previous `=== 1` check missed the subreaper case under launchd / systemd. Bun's `process.ppid` cache is stale across reparenting (see [oven-sh/bun#30305](https://github.com/oven-sh/bun/issues/30305)) so `getParentPid()` runs `spawnSync('ps', ['-o', 'ppid=', '-p', PID])` per tick to read the live kernel PPID. Startup probe verifies `ps` is on PATH; if not (stripped containers, busybox without procps), the watchdog skips installing AND emits a loud `[gbrain serve] watchdog disabled: ps unavailable, parent-death detection unavailable — child will rely on stdin EOF / signals only` stderr line so operators see the degraded mode at boot. Pinned by `test/serve-stdio-lifecycle.test.ts` (22 cases). Closes #413, #446. Credit @Aragorn2046 (origin features in #591) and @seungsu-kr (rebased submitter, Bun ppid workaround). - `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`. `sweepExpiredTokens()` runs on startup wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper. **v0.34.1.0 (#909):** `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1) — public PKCE clients (Claude Code, Cursor, every other PKCE-first MCP client) store `client_secret_hash = NULL` and the response payload omits `client_secret` entirely. Confidential clients (default `client_secret_post` and explicit `client_secret_basic`) keep their one-time-reveal shape. `getClient` correctly normalizes a NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts the public client at `/token`. **v0.34.1.0 (#861 + #876):** `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) and surfaces both on the returned `AuthInfo`. Pre-v60 / pre-v61 brains degrade gracefully via `isUndefinedColumnError` fallback so the upgrade chain is non-blocking on legacy DBs. diff --git a/VERSION b/VERSION index f4b4f188c..1de48e265 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.35.1.1 \ No newline at end of file +0.35.3.0 \ No newline at end of file diff --git a/llms-full.txt b/llms-full.txt index 7c0641d5e..0d7198fe6 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -168,6 +168,7 @@ strict behavior when unset. - `src/core/storage.ts` — Pluggable storage interface (S3, Supabase Storage, local) - `src/core/storage-config.ts` (v0.22.11) — Storage tiering: `loadStorageConfig` reads `gbrain.yml`, normalizes deprecated keys (`git_tracked` / `supabase_only`) to canonical (`db_tracked` / `db_only`) with once-per-process deprecation warning, and runs `normalizeAndValidateStorageConfig` (auto-fixes missing trailing `/`, throws `StorageConfigError` on tier overlap). Path-segment matcher: `media/x/` does NOT match `media/xerox/foo`. Replaces gray-matter (broken on delimiter-less YAML) with a dedicated parser for the `gbrain.yml` shape. - `src/core/disk-walk.ts` (v0.22.11) — `walkBrainRepo(repoPath)` returns `Map` from one recursive `readdirSync`. Skips dot-dirs, `node_modules`, non-`.md` files. Used by `gbrain storage status` to replace per-page `existsSync + statSync` (~400K syscalls on 200K-page brains → tens). +- `src/core/git-remote.ts` (v0.35.3.0) — SSRF-hardened git invocations for remote-source `cloneRepo` and `pullRepo`. Exports two distinct flag constants because `git`'s argv grammar treats them differently: `GIT_SSRF_FLAGS` (3 `-c` config flags — `protocol.allow=user`, `protocol.file.allow=never`, `http.allowRedirects=false`) is **global config**, spread BEFORE the subcommand verb. New `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` is **subcommand-scoped**, spread AFTER the verb. Pre-v0.35.3 a single combined `GIT_SSRF_FLAGS` array spread `--no-recurse-submodules` before the verb where real git rejects it with exit 129 ("unknown option"); the fake-git test harness exited 0 regardless of argv shape, so CI missed it for ~7 months and every remote-source clone/pull was silently broken. `cloneRepo` argv: `git clone --depth=1 [--branch X] -- `. `pullRepo` argv: `git -C pull --ff-only`. Pinned by `test/git-remote.test.ts` position-anchored regression guard (`argv.indexOf('--no-recurse-submodules') > argv.indexOf(verb)`). - `src/commands/storage.ts` (v0.22.11) — `gbrain storage status [--repo P] [--json]`. Split into pure data (`getStorageStatus`) + JSON formatter + human formatter (ASCII-only per D10) matching the `orphans.ts` pattern. `PageCountsByTier` and `DiskUsageByTier` are distinct nominal types so swaps fail at compile time. - `gbrain.yml` (brain repo root, v0.22.11) — Optional storage tiering config. Top-level `storage:` section with `db_tracked:` and `db_only:` array-valued keys. `gbrain sync` auto-manages `.gitignore` for `db_only` paths on successful sync (skips on dry-run, blocked-by-failures, submodule context, or `GBRAIN_NO_GITIGNORE=1`). `gbrain export --restore-only [--repo P] [--type T] [--slug-prefix S]` repopulates missing `db_only` files from the database. - `src/core/supabase-admin.ts` — Supabase admin API (project discovery, pgvector check) @@ -257,8 +258,8 @@ strict behavior when unset. - `src/core/minions/wait-for-completion.ts` (v0.15) — poll-until-terminal helper for CLI callers. `TimeoutError` does NOT cancel the job; `AbortSignal` exits without throwing. Default `pollMs`: 1000 on Postgres, 250 on PGLite inline. - `src/core/minions/transcript.ts` (v0.15) — renders `subagent_messages` + `subagent_tool_executions` to markdown. Tool rows splice under their owning assistant `tool_use` by `tool_use_id`. UTF-8-safe truncation; unknown block types fall through to fenced JSON. - `src/core/minions/plugin-loader.ts` (v0.15) — `GBRAIN_PLUGIN_PATH` discovery. Absolute paths only, left-wins collision, `gbrain.plugin.json` with `plugin_version: "gbrain-plugin-v1"`, plugins ship DEFS only (no new tools), `allowed_tools:` validated at load time against the derived registry. -- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. -- `src/mcp/tool-defs.ts` (v0.15) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. +- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29, v0.35.3.0) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. **v0.35.3.0:** `paramsToInputSchema()` now consumes `paramDefToSchema` from `src/mcp/tool-defs.ts` instead of its own inline destructure. Required-aggregation at the tool-def level stays here (out of scope for the shared helper, which is per-param). Closes the third drift site in the ParamDef→JSON Schema bug class. +- `src/mcp/tool-defs.ts` (v0.15, v0.35.3.0) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. **v0.35.3.0:** exports the new recursive `paramDefToSchema(p: ParamDef)` helper — single source of truth for ParamDef→JSON Schema mapping. Three consumers now share one mapper: `buildToolDefs` (stdio MCP), `src/commands/serve-http.ts:837` (HTTP MCP `tools/list`), and `src/core/minions/tools/brain-allowlist.ts:84` (subagent tool registry). Pre-v0.35.3, three inline destructures had drifted across the surface — the live HTTP MCP path dropped `items` on every array param after a v0.32 review caught only the stdio side. Recursive on `items` so nested array-of-arrays preserves inner shape on the wire. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable. `test/mcp-tool-defs.test.ts` adds a `findArrayWithoutItems` walker that fails the suite with a property path on any future `type: 'array'` lacking `items.type`. - `src/core/minions/attachments.ts` — Attachment validation (path traversal, null byte, oversize, base64, duplicate detection) - `src/commands/agent.ts` (v0.16) — `gbrain agent run [flags]` CLI. Submits `subagent` (or N children + 1 aggregator) under `{allowProtectedSubmit: true}`. Single-entry `--fanout-manifest` short-circuits. Children get `on_child_fail: 'continue'` + `max_stalled: 3`. `--follow` is the default on TTY; streams logs + polls `waitForCompletion` in parallel. Ctrl-C detaches, does not cancel. - `src/commands/agent-logs.ts` (v0.16) — `gbrain agent logs [--follow] [--since]`. Merges JSONL heartbeat audit + `subagent_messages` into a chronological timeline. `parseSince` accepts ISO-8601 or relative (`5m`, `1h`, `2d`). Transcript tail renders only for terminal jobs. @@ -268,7 +269,7 @@ strict behavior when unset. - `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path. **v0.34.1.0 (#870):** stdin `'end'` / `'close'` shutdown hooks are skipped when `process.env.MCP_STDIO === '1'`. Gateway-piped stdio MCP wrappers (OpenClaw's `bundle-mcp`, similar) pipe the JSON-RPC handshake then close their stdin half; pre-fix this killed the server before the first tool call landed. Signal handlers (SIGTERM / SIGINT / SIGHUP) and the parent-process watchdog still cover legitimate disconnects. `src/commands/serve.ts` exposes `ServeOptions.mcpStdio?: boolean` as a test seam so the runtime guard is exercisable without process.env mutation. Pinned by `test/serve-stdio-lifecycle.test.ts`. - `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `validateParams(op, params)`. Single source of truth for `(ctx, params)` handler arg order and the 5-field `OperationContext` shape (engine + config + logger + dryRun + remote). Defaults to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `summarizeMcpParams(opName, params)` — privacy-preserving redactor for `mcp_request_log` and the admin SSE feed. Returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`. Intersects submitted top-level keys against the operation's declared `params` allow-list (declared keys preserved as a sorted array for debug visibility; unknown keys counted but never named, closing the attacker-controlled-key-name leak). Byte counts bucketed up to nearest 1KB so an attacker can't binary-search secret-content sizes via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`. - `src/mcp/rate-limit.ts` (v0.22.7) — Bounded-LRU token-bucket limiter. `buildDefaultLimiters()` returns the two-bucket pipeline: pre-auth IP (30/60s, fires BEFORE the DB lookup so brute-force load against `access_tokens` is actually capped) + post-auth token-id (60/60s). Tracks `lastTouchedMs` separately from `lastRefillMs` so an exhausted key can't be reset by hammering past the TTL. LRU cap bounds memory under attacker-controlled key growth. -- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. +- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. **v0.35.3.0:** the inline ParamDef→schema mapper at `:837-849` (HTTP MCP `tools/list` handler) is replaced with `paramDefToSchema(v)` from `src/mcp/tool-defs.ts`. Pre-fix this site silently dropped `items` on every array param so strict-mode OAuth clients (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the whole tool list. Single mapper now serves stdio MCP, HTTP MCP `tools/list`, and the subagent registry. - `src/core/sql-query.ts` (v0.31.3) — Engine-aware tagged-template SQL adapter for OAuth/admin/auth infrastructure. `sqlQueryForEngine(engine)` returns a `SqlQuery` (`(strings, ...values) => Promise`) that walks the template, builds `$N` positional SQL, asserts every value is a `SqlValue` (string | number | bigint | boolean | Date | null), and routes through `engine.executeRaw(sql, params)` so Postgres goes via postgres.js's `unsafe(sql, params)` path and PGLite via its embedded `db.query(sql, params)`. Deliberately narrower than postgres.js's `sql` tag: no nested fragments, no `sql.json()`, no `sql.unsafe()`, no `sql.begin()`, no array binding. The narrow surface is the feature — codex finding #7 from the v0.31 plan review argued the adapter should stay scalar-only or it drifts into a partial postgres.js clone. JSONB writes go through the separate `executeRawJsonb(engine, sql, scalarParams, jsonbParams)` helper that composes positional `$N::jsonb` casts and passes JS objects through; the v0.12.0 double-encode bug class doesn't apply because positional binding through `unsafe()` reaches the wire protocol with the correct type oid (verified by `test/sql-query.test.ts` on PGLite and `test/e2e/auth-permissions.test.ts:67` on Postgres). `scripts/check-jsonb-pattern.sh` doesn't fire because `executeRawJsonb(...)` is a method call, not the banned literal-template-tag interpolation pattern. Consumed by `src/commands/auth.ts`, `src/commands/serve-http.ts`, `src/core/oauth-provider.ts`, `src/commands/files.ts`, and `src/mcp/http-transport.ts` so all five sites work against PGLite and Postgres uniformly. Closes the bug where `gbrain auth` + `gbrain serve --http` were silently Postgres-only because they routed every SQL through the postgres.js singleton (community PR #681). - `src/commands/serve.ts` (v0.31.3) — `gbrain serve` stdio MCP entrypoint with idempotent shutdown across every parent-disconnect signal. Stdio EOF, SIGTERM, SIGINT, SIGHUP, and parent-process death (every reparent case — PID 1, launchd subreaper, systemd, tmux, or a parent shell with `PR_SET_CHILD_SUBREAPER`) all funnel into one `cleanup(reason)` path that releases the engine and the PGLite write-lock dir within 5 seconds. Pre-v0.31.3 the stdio MCP server held the lock indefinitely after Claude Desktop / Cursor / launchd-managed gateways disconnected, forcing a 5-minute stale-lock wait on the next start. Watchdog reparent check is `getParentPid() !== initialParentPid` (capturing the initial ppid once at install time and firing on any change); the previous `=== 1` check missed the subreaper case under launchd / systemd. Bun's `process.ppid` cache is stale across reparenting (see [oven-sh/bun#30305](https://github.com/oven-sh/bun/issues/30305)) so `getParentPid()` runs `spawnSync('ps', ['-o', 'ppid=', '-p', PID])` per tick to read the live kernel PPID. Startup probe verifies `ps` is on PATH; if not (stripped containers, busybox without procps), the watchdog skips installing AND emits a loud `[gbrain serve] watchdog disabled: ps unavailable, parent-death detection unavailable — child will rely on stdin EOF / signals only` stderr line so operators see the degraded mode at boot. Pinned by `test/serve-stdio-lifecycle.test.ts` (22 cases). Closes #413, #446. Credit @Aragorn2046 (origin features in #591) and @seungsu-kr (rebased submitter, Bun ppid workaround). - `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`. `sweepExpiredTokens()` runs on startup wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper. **v0.34.1.0 (#909):** `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1) — public PKCE clients (Claude Code, Cursor, every other PKCE-first MCP client) store `client_secret_hash = NULL` and the response payload omits `client_secret` entirely. Confidential clients (default `client_secret_post` and explicit `client_secret_basic`) keep their one-time-reveal shape. `getClient` correctly normalizes a NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts the public client at `/token`. **v0.34.1.0 (#861 + #876):** `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) and surfaces both on the returned `AuthInfo`. Pre-v60 / pre-v61 brains degrade gracefully via `isUndefinedColumnError` fallback so the upgrade chain is non-blocking on legacy DBs. diff --git a/package.json b/package.json index 49d0b9839..470821e09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.35.1.1", + "version": "0.35.3.0", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 752324b3c..db00d3540 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -28,6 +28,7 @@ import { GBrainOAuthProvider } from '../core/oauth-provider.ts'; import type { SqlQuery } from '../core/oauth-provider.ts'; import { hasScope, ALLOWED_SCOPES_LIST } from '../core/scope.ts'; import { summarizeMcpParams, dispatchToolCall } from '../mcp/dispatch.ts'; +import { paramDefToSchema } from '../mcp/tool-defs.ts'; import { getBrainHotMemoryMeta } from '../core/facts/meta-hook.ts'; import { loadConfig } from '../core/config.ts'; import { buildError, serializeError } from '../core/errors.ts'; @@ -840,12 +841,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type, - description: v.description, - ...(v.enum ? { enum: v.enum } : {}), - ...(v.default !== undefined ? { default: v.default } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params).filter(([, v]) => v.required).map(([k]) => k), }, diff --git a/src/core/git-remote.ts b/src/core/git-remote.ts index 348488fd0..dbbc339d1 100644 --- a/src/core/git-remote.ts +++ b/src/core/git-remote.ts @@ -20,16 +20,38 @@ import { join } from 'path'; import { isInternalUrl } from './url-safety.ts'; /** - * SSRF-defensive flag set. Used by both cloneRepo and pullRepo. + * Git CLI accepts two flag positions: + * git [global -c flags] [subcommand flags] [args] + * + * Global flags (the `-c key=value` config overrides) MUST come before the + * subcommand. Subcommand-specific flags (like `--no-recurse-submodules`) + * MUST come after the subcommand. Mixing the two positions makes git fail + * with `unknown option` (exit 129). Pre-v0.34 the single GIT_SSRF_FLAGS + * constant spliced both positions before the verb; real git rejected the + * subcommand flag but the test harness used a fake-git script that didn't + * validate, so every remote-source clone/pull broke silently in production. + * + * Split into two constants so the call-site spread is unambiguous and the + * type/name signal the position rule. + */ + +/** + * Global git config flags. Spread BEFORE the subcommand verb. * - http.followRedirects=false: closes DNS rebinding via redirect chains * - protocol.file.allow=never: no local-file URLs (defense in depth) * - protocol.ext.allow=never: no external helpers (`git-remote-foo`) - * - --no-recurse-submodules: .gitmodules cannot become a second fetch surface */ export const GIT_SSRF_FLAGS = [ '-c', 'http.followRedirects=false', '-c', 'protocol.file.allow=never', '-c', 'protocol.ext.allow=never', +] as const; + +/** + * Subcommand-level flags. Spread AFTER the subcommand verb (clone/pull). + * - --no-recurse-submodules: .gitmodules cannot become a second fetch surface + */ +export const GIT_SSRF_SUBCOMMAND_FLAGS = [ '--no-recurse-submodules', ] as const; @@ -153,7 +175,7 @@ export function cloneRepo(url: string, destDir: string, opts: CloneOpts = {}): v } } - const args: string[] = [...GIT_SSRF_FLAGS, 'clone']; + const args: string[] = [...GIT_SSRF_FLAGS, 'clone', ...GIT_SSRF_SUBCOMMAND_FLAGS]; if (opts.depth !== 0) { args.push(`--depth=${opts.depth ?? 1}`); } @@ -179,7 +201,7 @@ export function cloneRepo(url: string, destDir: string, opts: CloneOpts = {}): v /** Pull a repo with --ff-only and the same SSRF-defensive flags as cloneRepo. */ export function pullRepo(repoPath: string, opts: { timeoutMs?: number } = {}): void { - const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'pull', '--ff-only']; + const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'pull', ...GIT_SSRF_SUBCOMMAND_FLAGS, '--ff-only']; try { execFileSync('git', args, { stdio: ['ignore', 'pipe', 'pipe'], diff --git a/src/core/minions/tools/brain-allowlist.ts b/src/core/minions/tools/brain-allowlist.ts index 8fbe32d23..cedd4f46e 100644 --- a/src/core/minions/tools/brain-allowlist.ts +++ b/src/core/minions/tools/brain-allowlist.ts @@ -26,6 +26,7 @@ import type { BrainEngine } from '../../engine.ts'; import type { GBrainConfig } from '../../config.ts'; import { operations } from '../../operations.ts'; import type { Operation, OperationContext } from '../../operations.ts'; +import { paramDefToSchema } from '../../../mcp/tool-defs.ts'; import type { ToolCtx, ToolDef } from '../types.ts'; /** @@ -85,12 +86,7 @@ function paramsToInputSchema(op: Operation): Record { return { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params).filter(([, v]) => v.required).map(([k]) => k), }; diff --git a/src/core/operations.ts b/src/core/operations.ts index 2d763ef9b..49ed17700 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -2696,7 +2696,7 @@ const extract_facts: Operation = { params: { turn_text: { type: 'string', required: true, description: 'The user message or page body to extract facts from. Sanitized via INJECTION_PATTERNS before the LLM call.' }, session_id: { type: 'string', description: 'Opaque session id (e.g. topic-id from MCP _meta.session_id, or CLI --session). Stored on each fact for the recall --session filter. Not an auth surface.' }, - entity_hints: { type: 'array', description: 'Existing canonical entity slugs the agent has already resolved. Helps the extractor pick the right slug.' }, + entity_hints: { type: 'array', items: { type: 'string' }, description: 'Existing canonical entity slugs the agent has already resolved. Helps the extractor pick the right slug.' }, is_dream_generated: { type: 'boolean', description: 'When true, extraction is skipped (anti-loop). Caller flips this on for pages with dream_generated:true frontmatter.' }, visibility: { type: 'string', description: 'Default visibility for extracted facts. private (default) | world.' }, }, diff --git a/src/core/resolvers/builtin/x-api/handle-to-tweet.ts b/src/core/resolvers/builtin/x-api/handle-to-tweet.ts index 9c0cea8ec..ffaefbbfa 100644 --- a/src/core/resolvers/builtin/x-api/handle-to-tweet.ts +++ b/src/core/resolvers/builtin/x-api/handle-to-tweet.ts @@ -99,7 +99,21 @@ export const xHandleToTweetResolver: Resolver { + return { + type: p.type === 'array' ? 'array' : p.type, + ...(p.description ? { description: p.description } : {}), + ...(p.enum ? { enum: p.enum } : {}), + ...(p.default !== undefined ? { default: p.default } : {}), + ...(p.items ? { items: paramDefToSchema(p.items) } : {}), + }; +} + export function buildToolDefs(ops: Operation[]): McpToolDef[] { return ops.map(op => ({ name: op.name, @@ -17,12 +44,7 @@ export function buildToolDefs(ops: Operation[]): McpToolDef[] { inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params) .filter(([, v]) => v.required) diff --git a/test/e2e/zeroentropy-live.test.ts b/test/e2e/zeroentropy-live.test.ts index 852801143..772c7b1c1 100644 --- a/test/e2e/zeroentropy-live.test.ts +++ b/test/e2e/zeroentropy-live.test.ts @@ -99,6 +99,14 @@ describe('ZE live — embed round-trip', () => { }); }); +// ZE rerank API has multi-second cold-start latency on the first request +// of a CI run (observed ~5-6s on Tier 2 runners; subsequent calls < 500ms). +// The production DEFAULT_RERANK_TIMEOUT_MS in gateway.ts stays at 5s for the +// search hot path; these E2E tests pass an explicit input.timeoutMs and a +// longer bun:test per-test timeout so cold-start doesn't flake CI. +const ZE_RERANK_TIMEOUT_MS = 25_000; +const ZE_TEST_TIMEOUT_MS = 30_000; + describe('ZE live — rerank round-trip', () => { test('rerank({query, documents}) returns sorted RerankResult[]', async () => { if (skipAll) { @@ -112,6 +120,7 @@ describe('ZE live — rerank round-trip', () => { 'My cat likes to eat tuna fish.', 'Chlorophyll absorbs red and blue light during photosynthesis.', ], + timeoutMs: ZE_RERANK_TIMEOUT_MS, }); expect(out.length).toBe(3); for (const r of out) { @@ -128,7 +137,7 @@ describe('ZE live — rerank round-trip', () => { // the cat doc must NOT be at the top. const topIndex = out[0]!.index; expect(topIndex).not.toBe(1); // index 1 is the cat doc - }); + }, ZE_TEST_TIMEOUT_MS); test('rerank with top_n=2 returns at most 2 results', async () => { if (skipAll) { @@ -139,9 +148,10 @@ describe('ZE live — rerank round-trip', () => { query: 'photosynthesis', documents: ['photosynthesis a', 'cats b', 'photosynthesis c'], topN: 2, + timeoutMs: ZE_RERANK_TIMEOUT_MS, }); expect(out.length).toBeLessThanOrEqual(2); - }); + }, ZE_TEST_TIMEOUT_MS); }); describe('ZE live — flexible dims', () => { diff --git a/test/git-remote.test.ts b/test/git-remote.test.ts index a8c744d6d..6d4a60d05 100644 --- a/test/git-remote.test.ts +++ b/test/git-remote.test.ts @@ -4,6 +4,7 @@ import { join } from 'path'; import { tmpdir } from 'os'; import { GIT_SSRF_FLAGS, + GIT_SSRF_SUBCOMMAND_FLAGS, parseRemoteUrl, RemoteUrlError, cloneRepo, @@ -81,11 +82,22 @@ const fakePath = (): string => `${FAKE_GIT_DIR}:${process.env.PATH ?? ''}`; // --------------------------------------------------------------------------- describe('GIT_SSRF_FLAGS', () => { - test('exact shape — codex SSRF lockdown', () => { + test('exact shape — global -c config flags only (spread BEFORE the verb)', () => { expect([...GIT_SSRF_FLAGS]).toEqual([ '-c', 'http.followRedirects=false', '-c', 'protocol.file.allow=never', '-c', 'protocol.ext.allow=never', + ]); + }); +}); + +describe('GIT_SSRF_SUBCOMMAND_FLAGS', () => { + test('exact shape — subcommand-level flags only (spread AFTER the verb)', () => { + // v0.34 fix wave: --no-recurse-submodules is a clone/pull subcommand + // flag, not a global flag. Real git exits 129 with "unknown option" + // when it appears before the verb. The pre-v0.34 single-constant + // spread baked the bug in. + expect([...GIT_SSRF_SUBCOMMAND_FLAGS]).toEqual([ '--no-recurse-submodules', ]); }); @@ -229,12 +241,22 @@ describe('cloneRepo', () => { const calls = readArgvLog(); expect(calls.length).toBe(1); const argv = calls[0]; - // Pin the SSRF flags before the 'clone' verb (codex Q2 invariant). + // Global -c config flags must appear BEFORE the 'clone' verb. expect(argv.slice(0, GIT_SSRF_FLAGS.length)).toEqual([...GIT_SSRF_FLAGS]); expect(argv).toContain('clone'); expect(argv).toContain('--depth=1'); expect(argv).toContain('https://example.com/repo'); expect(argv[argv.length - 1]).toBe(dest); + // v0.34 fix wave: subcommand flags MUST appear after the verb. Real + // git rejects `git --no-recurse-submodules clone ...` with exit 129. + // The fake-git harness returned 0 for any argv shape, so this + // position-anchored assertion is the structural regression test. + const cloneIdx = argv.indexOf('clone'); + expect(cloneIdx).toBeGreaterThan(-1); + for (const subFlag of GIT_SSRF_SUBCOMMAND_FLAGS) { + const flagIdx = argv.indexOf(subFlag); + expect(flagIdx).toBeGreaterThan(cloneIdx); + } }); test('depth=0 means no --depth flag (full clone)', async () => { @@ -309,6 +331,13 @@ describe('pullRepo', () => { expect(argv.slice(2, 2 + GIT_SSRF_FLAGS.length)).toEqual([...GIT_SSRF_FLAGS]); expect(argv).toContain('pull'); expect(argv).toContain('--ff-only'); + // v0.34 fix wave: subcommand flag position assertion. + const pullIdx = argv.indexOf('pull'); + expect(pullIdx).toBeGreaterThan(-1); + for (const subFlag of GIT_SSRF_SUBCOMMAND_FLAGS) { + const flagIdx = argv.indexOf(subFlag); + expect(flagIdx).toBeGreaterThan(pullIdx); + } rmSync(repo, { recursive: true, force: true }); }); diff --git a/test/mcp-tool-defs.test.ts b/test/mcp-tool-defs.test.ts index 5309ce39d..7ee4dd015 100644 --- a/test/mcp-tool-defs.test.ts +++ b/test/mcp-tool-defs.test.ts @@ -10,10 +10,39 @@ import { describe, test, expect } from 'bun:test'; import { operations } from '../src/core/operations.ts'; -import { buildToolDefs } from '../src/mcp/tool-defs.ts'; +import { buildToolDefs, paramDefToSchema } from '../src/mcp/tool-defs.ts'; +import type { ParamDef } from '../src/core/operations.ts'; -// Pre-extraction inline shape — lifted verbatim from the original -// src/mcp/server.ts block so any future drift fails this test loudly. +// Reference shape — mirrors the canonical `paramDefToSchema` helper from +// src/mcp/tool-defs.ts. Drift between the helper and this reference fails +// the byte-equality test loudly. +// +// v0.34 update: paramDefToSchema is recursive on `items` so nested +// array-of-arrays preserves the inner shape on the MCP wire. The reference +// below mirrors that recursion. The previous shallow `{ items: { type: +// v.items.type } }` (legacy buildToolDefs) silently dropped nested items +// — explicit fixture assertions below catch the drift class. +// +// `default` is included to match paramDefToSchema; no current op uses +// `default:` at the ParamDef level so the round-trip is unchanged for +// every existing operation, but new ops that add a default get it on the +// wire automatically. +type ParamDefLike = { + type: 'string' | 'number' | 'boolean' | 'object' | 'array'; + description?: string; + enum?: string[]; + default?: unknown; + items?: ParamDefLike; +}; +function referenceParamDefToSchema(p: ParamDefLike): Record { + return { + type: p.type === 'array' ? 'array' : p.type, + ...(p.description ? { description: p.description } : {}), + ...(p.enum ? { enum: p.enum } : {}), + ...(p.default !== undefined ? { default: p.default } : {}), + ...(p.items ? { items: referenceParamDefToSchema(p.items) } : {}), + }; +} function legacyInlineMap(ops: typeof operations) { return ops.map(op => ({ name: op.name, @@ -21,12 +50,7 @@ function legacyInlineMap(ops: typeof operations) { inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, referenceParamDefToSchema(v)]), ), required: Object.entries(op.params) .filter(([, v]) => v.required) @@ -65,3 +89,98 @@ describe('buildToolDefs', () => { } }); }); + +// --------------------------------------------------------------------------- +// Structural array-items guard (v0.34 fix wave). +// +// JSON Schema strict-mode validators (Gemini Pro strict, OpenAI structured +// outputs) reject `type: 'array'` without `items`. Pre-v0.34 this happened +// in production: `extract_facts.entity_hints` and `handle_to_tweet`'s +// `candidates` both shipped as bare arrays. +// +// This recursive guard walks every tool def's inputSchema and fails the +// suite with a property path if any array lacks `items.type`. Drift-proof +// against future ops adding bare arrays. +// --------------------------------------------------------------------------- + +interface SchemaNode { + type?: unknown; + properties?: Record; + items?: SchemaNode; + [k: string]: unknown; +} + +function findArrayWithoutItems(node: SchemaNode, path: string[]): string[] { + const violations: string[] = []; + if (node && typeof node === 'object') { + if (node.type === 'array') { + if (!node.items || typeof node.items !== 'object') { + violations.push(`${path.join('.') || ''} (array missing items)`); + } else if (!('type' in node.items)) { + violations.push(`${path.join('.') || ''}.items (items missing type)`); + } else { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + if (node.properties && typeof node.properties === 'object') { + for (const [k, child] of Object.entries(node.properties)) { + violations.push(...findArrayWithoutItems(child as SchemaNode, [...path, k])); + } + } + if (node.items && typeof node.items === 'object' && node.type !== 'array') { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + return violations; +} + +describe('paramDefToSchema structural guard', () => { + test('every operation inputSchema array has items.type set (no bare arrays)', () => { + const allViolations: string[] = []; + for (const def of buildToolDefs(operations)) { + const v = findArrayWithoutItems(def.inputSchema as SchemaNode, [def.name]); + allViolations.push(...v); + } + expect(allViolations).toEqual([]); + }); + + test('extract_facts.entity_hints declares items.type as string', () => { + const def = buildToolDefs(operations).find(d => d.name === 'extract_facts'); + expect(def).toBeDefined(); + const eh = (def!.inputSchema.properties as Record).entity_hints; + expect(eh.type).toBe('array'); + expect(eh.items).toBeDefined(); + expect((eh.items as SchemaNode).type).toBe('string'); + }); + + test('paramDefToSchema recursively propagates nested items.items.type', () => { + // Synthetic ParamDef: array-of-arrays-of-strings. No current op uses + // this shape, so this test pins the contract for future ops and proves + // the helper recurses (closes the v0.32 nested-drop bug class). + const nested: ParamDef = { + type: 'array', + items: { + type: 'array', + items: { type: 'string' }, + }, + }; + const schema = paramDefToSchema(nested) as SchemaNode; + expect(schema.type).toBe('array'); + expect((schema.items as SchemaNode).type).toBe('array'); + expect(((schema.items as SchemaNode).items as SchemaNode).type).toBe('string'); + }); + + test('paramDefToSchema preserves description on nested items', () => { + const p: ParamDef = { + type: 'array', + description: 'outer', + items: { + type: 'string', + description: 'inner', + }, + }; + const schema = paramDefToSchema(p) as SchemaNode; + expect(schema.description).toBe('outer'); + expect((schema.items as SchemaNode).description).toBe('inner'); + }); +}); diff --git a/test/resolvers.test.ts b/test/resolvers.test.ts index 4ca2d98f1..de9482c3c 100644 --- a/test/resolvers.test.ts +++ b/test/resolvers.test.ts @@ -620,3 +620,98 @@ describe('x_handle_to_tweet resolver', () => { expect(query).toContain('words'); }); }); + +// --------------------------------------------------------------------------- +// Structural array-items guard for resolver inputSchema + outputSchema. +// +// v0.34 fix wave: handle_to_tweet's `candidates` field shipped as bare +// `type: 'array'` without `items` (caught by community PR #910). Same +// bug class as extract_facts.entity_hints, different surface. Resolvers +// don't ride through buildToolDefs so the MCP-side structural guard +// doesn't see them. +// +// This guard walks the builtin resolvers explicitly (NOT via +// getDefaultRegistry(), which starts empty until commands/resolvers.ts +// registers — codex catch). Both inputSchema AND outputSchema are +// checked because resolvers are bidirectional. +// --------------------------------------------------------------------------- + +interface SchemaNode { + type?: unknown; + properties?: Record; + items?: SchemaNode; + [k: string]: unknown; +} + +function findArrayWithoutItems(node: SchemaNode | undefined, path: string[]): string[] { + const violations: string[] = []; + if (!node || typeof node !== 'object') return violations; + if (node.type === 'array') { + if (!node.items || typeof node.items !== 'object') { + violations.push(`${path.join('.') || ''} (array missing items)`); + } else if (!('type' in node.items)) { + violations.push(`${path.join('.') || ''}.items (items missing type)`); + } else { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + if (node.properties && typeof node.properties === 'object') { + for (const [k, child] of Object.entries(node.properties)) { + violations.push(...findArrayWithoutItems(child as SchemaNode, [...path, k])); + } + } + return violations; +} + +describe('builtin resolver schemas — structural array-items guard', () => { + // Explicit-import list. getDefaultRegistry() returns an empty registry + // until commands/resolvers.ts registers, so walking it would silently + // pass with zero resolvers visited (codex finding). + const builtins = [ + { name: 'url_reachable', resolver: urlReachableResolver }, + { name: 'x_handle_to_tweet', resolver: xHandleToTweetResolver }, + ]; + + for (const { name, resolver } of builtins) { + test(`${name}: inputSchema has no bare arrays`, () => { + const violations = findArrayWithoutItems( + resolver.inputSchema as SchemaNode | undefined, + [name, 'inputSchema'], + ); + expect(violations).toEqual([]); + }); + + test(`${name}: outputSchema has no bare arrays`, () => { + const violations = findArrayWithoutItems( + resolver.outputSchema as SchemaNode | undefined, + [name, 'outputSchema'], + ); + expect(violations).toEqual([]); + }); + } + + test('x_handle_to_tweet candidates declares full item shape with required + additionalProperties:false', () => { + const out = xHandleToTweetResolver.outputSchema as SchemaNode; + const candidates = (out.properties as Record).candidates; + expect(candidates.type).toBe('array'); + expect(candidates.items).toBeDefined(); + const items = candidates.items as SchemaNode; + expect(items.type).toBe('object'); + expect(items.properties).toBeDefined(); + const props = items.properties as Record; + expect(Object.keys(props).sort()).toEqual( + ['created_at', 'score', 'text', 'tweet_id', 'url'], + ); + expect((items as { required?: unknown }).required).toEqual( + ['tweet_id', 'text', 'created_at', 'score', 'url'], + ); + expect((items as { additionalProperties?: unknown }).additionalProperties).toBe(false); + }); + + test('coverage: at least 2 builtins walked (catches future regression where registry import drops)', () => { + // Negative guard against codex's "underspecified registry walk" finding: + // if this list ever empties (e.g. an autoformatter drops the array), + // the rest of the suite would silently pass. + expect(builtins.length).toBeGreaterThanOrEqual(2); + }); +});