v0.35.3.0 fix wave: extract_facts items + git --no-recurse-submodules placement by garrytan · Pull Request #1053 · garrytan/gbrain

garrytan · 2026-05-16T00:24:48Z

Summary

Fix wave: 19 stale community PRs land as one bisect-friendly PR with the architectural fix none of them surfaced.

Two real bugs in shipping code on master since v0.28+:

extract_facts.entity_hints and X-API resolver candidates array fields missing items — JSON Schema strict-mode validators (Gemini Pro structured outputs, OpenAI strict tool defs) reject type: 'array' without items. ~12 community PRs converged on entity_hints; only @DmitryBMsk's fix: add missing items field to two array schemas (Gemini Pro strict JSON Schema compat) #910 caught candidates.
--no-recurse-submodules spliced before the subcommand verb — real git exits 129 with unknown option. Every remote-source clone/pull broken since v0.28. The fake-git test harness exits 0 regardless of argv shape, so CI never caught it. ~7 community PRs flagged this.

Codex outside-voice review on top of the 19 community PRs surfaced the structural finding none of them saw: three duplicate ParamDef → JSON Schema mappers existed across the MCP surface (stdio MCP, HTTP MCP tools/list, subagent brain-tool registry). The live HTTP MCP path at serve-http.ts:837 and the subagent registry at brain-allowlist.ts:84 would BOTH have stayed broken after a buildToolDefs-only patch.

This PR centralizes via one shared paramDefToSchema(p: ParamDef) helper consumed by all three call sites — closes the bug class at the architecture level. Plus structural CI guards so it can't drift again silently.

Test Coverage

CODE PATHS                                          COVERAGE
[+] src/mcp/tool-defs.ts paramDefToSchema (NEW)      ★★★ recursive items propagation, description, enum, default
[+] src/commands/serve-http.ts tools/list mapper     ★★★ consumes shared helper
[+] src/core/minions/tools/brain-allowlist.ts        ★★★ consumes shared helper
[+] src/core/operations.ts entity_hints              ★★★ structural guard + explicit fixture
[+] src/core/resolvers/.../handle-to-tweet candidates ★★★ explicit shape + required + additionalProperties
[+] src/core/git-remote.ts argv shape                ★★★ position-anchored: indexOf(flag) > indexOf(verb)

Tests: 11 new (+12 expect calls) across 3 test files
- test/mcp-tool-defs.test.ts: recursive walker + explicit entity_hints fixture + nested-array round-trip
- test/git-remote.test.ts: split snapshot + subcommand-position assertions on clone + pull
- test/resolvers.test.ts: explicit-import walker (NOT empty getDefaultRegistry) + candidates shape pin

COVERAGE: 100% of changed surfaces tested with regression guards

Pre-Landing Review

Run via /plan-eng-review + /codex review plan in this session. CLEAR after absorbing codex findings.

5 design decisions resolved (D1 full-spec candidates, D2 dual-direction resolver walker, D3 nested items propagation, D4 centralize via shared helper, D5 required + additionalProperties)
Codex caught 10 findings on the plan, 8 absorbed into the architecture (including the live HTTP MCP duplicate mapper the plan claimed didn't exist) and 2 corrective edits.
0 unresolved decisions, 0 critical gaps remaining.

Eval Results

No prompt-related files changed — evals skipped.

Plan Completion

All 11 implementation tasks DONE. Plan file: ~/.claude/plans/system-instruction-you-are-working-zany-reddy.md.

Task	Status	Files
T1 entity_hints items	DONE	src/core/operations.ts
T2 candidates full spec	DONE	src/core/resolvers/builtin/x-api/handle-to-tweet.ts
T3 shared paramDefToSchema	DONE	src/mcp/tool-defs.ts
T4 serve-http consumes helper	DONE	src/commands/serve-http.ts
T5 brain-allowlist consumes helper	DONE	src/core/minions/tools/brain-allowlist.ts
T6 split GIT_SSRF_FLAGS	DONE	src/core/git-remote.ts
T7 reposition flag	DONE	src/core/git-remote.ts
T8 structural guard + fixtures	DONE	test/mcp-tool-defs.test.ts
T9 mirror legacyInlineMap	DONE	test/mcp-tool-defs.test.ts
T10 git-remote position assertions	DONE	test/git-remote.test.ts
T11 resolver structural guard	DONE	test/resolvers.test.ts

Verification Results

bun run typecheck — green
bun run verify — green (typecheck + 8 shell pre-checks)
bun run test — 6565/0/0 (parallel + serial loop, 221s)
bun test test/mcp-tool-defs.test.ts test/git-remote.test.ts test/resolvers.test.ts test/agent-cli.test.ts test/oauth.test.ts test/http-transport.test.ts — 223/0/0
CI on initial push: Tier 1 + 4 test shards + gitleaks all pass; Tier 2 had ZE rerank cold-start timeout (now fixed in de73cbc8).
CI on rerank fix push: all green.

TODOS

No TODOS.md items closed by this wave. Existing v0.34.x/v0.35.x follow-ups preserved verbatim.

Docs sync

CLAUDE.md — updated annotations for src/mcp/tool-defs.ts (new exported recursive paramDefToSchema helper, three-consumer centralization), src/core/minions/tools/brain-allowlist.ts (paramsToInputSchema now consumes the shared helper), src/commands/serve-http.ts (HTTP MCP tools/list handler now consumes the shared helper, closes the items-dropped bug class). Added a new entry for src/core/git-remote.ts documenting the GIT_SSRF_FLAGS vs GIT_SSRF_SUBCOMMAND_FLAGS split and the position-anchored regression guard.
llms-full.txt — regenerated to match.
README.md, TODOS.md — no references to changed surfaces, no edits.
CHANGELOG.md — canonical v0.35.2.0 entry written by /ship.

Attribution

Bug locations cherry-picked from community PRs:

Wave 1 deepest variant: @DmitryBMsk (PR fix: add missing items field to two array schemas (Gemini Pro strict JSON Schema compat) #910) caught both entity_hints AND candidates.
Wave 2 cleanest naming: PR Fix git submodule flag placement #846 (GIT_SSRF_SUBCOMMAND_FLAGS semantic split).
Codex outside-voice review on top of all 19 PRs surfaced the live serve-http.ts:837 duplicate mapper, the third duplicate at brain-allowlist.ts:84, the hollow getDefaultRegistry() walk, the missing required array, and the verification-command typos — findings none of the 19 community PRs caught.

Superseded PRs being closed with thank-you notes after merge: #1028, #1023, #1020, #999, #985, #980, #979, #963, #904, #863, #862, #847, #846, #842, #832, #812 (16 total; #910 contribution incorporated, #800 doesn't exist).

Test plan

bun run typecheck green
bun run verify green (typecheck + 8 shell pre-checks)
bun run test green (6565/0/0 across parallel + serial)
Targeted tests for modified surfaces green (223/0/0)
CI green on initial push (Tier 1 + test shards + gitleaks)
Tier 2 LLM Skills green after rerank cold-start timeout fix
Manual real-git smoke after merge: bun -e "import('./src/core/git-remote.ts').then(m => m.cloneRepo('https://github.com/octocat/Hello-World.git', '/tmp/rg-smoke'))" — should succeed where pre-fix exited 129
After merge: close 16 duplicate community PRs with thank-you notes pointing at the merged commit per feedback_pr_wave_workflow.md

🤖 Generated with Claude Code

…chema 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) <noreply@anthropic.com>

…eet 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) <noreply@anthropic.com> Co-Authored-By: DmitryBMsk (PR #910)

Git CLI accepts two flag positions: git [global -c flags] <subcommand> [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) <noreply@anthropic.com>

… 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) <noreply@anthropic.com>

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) <noreply@anthropic.com>

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

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 <noreply@anthropic.com>

# Conflicts: # CHANGELOG.md # VERSION # package.json

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) <noreply@anthropic.com>

# Conflicts: # CHANGELOG.md # VERSION # package.json

garrytan and others added 6 commits May 15, 2026 17:23

chore: bump version and changelog (v0.35.2.0)

ff9ea81

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

garrytan changed the title ~~fix wave: extract_facts items + git --no-recurse-submodules placement~~ v0.35.2.0 fix wave: extract_facts items + git --no-recurse-submodules placement May 16, 2026

garrytan and others added 3 commits May 15, 2026 18:52

Merge remote-tracking branch 'origin/master' into garrytan/krakow-v2

41a6f81

# Conflicts: # CHANGELOG.md # VERSION # package.json

garrytan changed the title ~~v0.35.2.0 fix wave: extract_facts items + git --no-recurse-submodules placement~~ v0.35.3.0 fix wave: extract_facts items + git --no-recurse-submodules placement May 16, 2026

Merge remote-tracking branch 'origin/master' into garrytan/krakow-v2

1099dc0

# Conflicts: # CHANGELOG.md # VERSION # package.json

100yenadmin mentioned this pull request May 17, 2026

Merge upstream GBrain v0.35.1.1 while preserving Eva OpenClaw defaults electricsheephq/eva-brain#102

Merged

garrytan merged commit 2504abe into master May 17, 2026
7 checks passed

skalingclouds mentioned this pull request May 18, 2026

fix(sync): pass no-recurse-submodules to git pull #1023

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.35.3.0 fix wave: extract_facts items + git --no-recurse-submodules placement#1053

v0.35.3.0 fix wave: extract_facts items + git --no-recurse-submodules placement#1053
garrytan merged 10 commits into
masterfrom
garrytan/krakow-v2

garrytan commented May 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

garrytan commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Coverage

Pre-Landing Review

Eval Results

Plan Completion

Verification Results

TODOS

Docs sync

Attribution

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

garrytan commented May 16, 2026 •

edited

Loading