Skip to content

fix(git-remote): move --no-recurse-submodules from global to per-command args#985

Closed
kyledeanjackson wants to merge 2 commits into
garrytan:masterfrom
kyledeanjackson:lattice-v0.33.2+bh-carries
Closed

fix(git-remote): move --no-recurse-submodules from global to per-command args#985
kyledeanjackson wants to merge 2 commits into
garrytan:masterfrom
kyledeanjackson:lattice-v0.33.2+bh-carries

Conversation

@kyledeanjackson

Copy link
Copy Markdown

Problem

GIT_SSRF_FLAGS in src/core/git-remote.ts places --no-recurse-submodules in the global-options slot (before the subcommand). On git ≥2.43 this fails:

$ git -C /path -c http.followRedirects=false -c protocol.file.allow=never -c protocol.ext.allow=never --no-recurse-submodules pull --ff-only
unknown option: --no-recurse-submodules

--no-recurse-submodules is a per-command option for clone / pull / fetch only, not a top-level git flag. Older git versions were more lenient about flag position; modern git (2.43+, default on Ubuntu 24.04 and current macOS) enforces strict placement.

Repro

Reproduces on git 2.43.0 / Ubuntu 24.04 LTS on every gbrain sync operation. The error surfaces as:

[gbrain phase] sync.git_pull error 5ms (git pull failed in /apps/kyle/lattice/bh-brain: Command failed: git -C /apps/kyl)
Warning: git pull failed: git pull failed in /apps/kyle/lattice/bh-brain: Command failed: git -C /apps/kyle/lattice/bh-brain -

Found while upgrading BH-DevServer (Ubuntu 24.04, git 2.43.0) to v0.33.2 — every autopilot cycle's sync phase reported failed git pull on all federated sources. Cycle completed (embed --stale still ran) but staleness detection from upstream commits was bypassed.

Fix

  • Drop --no-recurse-submodules from GIT_SSRF_FLAGS
  • Append it explicitly to clone args (after 'clone')
  • Append it explicitly to pull args (after '--ff-only')

Net result: same submodule-recursion safety guarantee, with correct flag placement that works on git 2.43+.

Testing

Verified locally: sync.git_pull startsync.git_pull done <N>ms (success) across 3 federated sources on dev-server post-fix. Pre-fix all three failed in <10ms with the unknown-option error.

kyledeanjackson and others added 2 commits May 14, 2026 14:48
…and args

Bug: GIT_SSRF_FLAGS placed --no-recurse-submodules in the global-options
slot (before the subcommand). On git ≥2.43 this fails with:

  unknown option: --no-recurse-submodules

git accepts --no-recurse-submodules only as a per-command option for
clone / pull / fetch — not as a top-level git option. The previous
placement happened to work on older git versions that were more lenient
about flag position, but breaks on Ubuntu 24.04's git 2.43.0 (and on
modern macOS git).

Fix:
- Drop --no-recurse-submodules from GIT_SSRF_FLAGS
- Append it explicitly to the clone args after 'clone'
- Append it explicitly to the pull args after '--ff-only'

Net result: same submodule-recursion guarantee, with correct flag
placement that works on git 2.43+.

Discovered while upgrading BH-DevServer to gbrain v0.33.2 — first cycle
of lattice-autopilot post-restart failed sync.git_pull on all three
sources (bh-brain, bh-vault, bh-studio-general) with the unknown-option
error. Pull command exec'd directly via execFileSync confirmed the
placement was the cause.

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

Bug context: the upstream DIR_PATTERN is a hard-coded alternation of
canonical dir names (people, companies, meetings, ...). Sites with custom
slug layouts — numbered dirs like '03-ventures'/'06-people'/'07-decisions',
or shorthand wikilink aliases like 'venture/'/'person/'/'decision/' —
have ZERO of their entries match the whitelist, so 'gbrain extract links
--source db' produces 0 links across N pages and the brain reports brain
score 46/100 with link_density_score 0 / no_orphans_score 0 (every page
orphaned).

This is the symptom BHSOFM-48 captured in our deployment (0 links / 599
pages pre-upgrade, 0 links / 735 pages post-upgrade to v0.33.2 — the
v0.32.8 'multi-source bug class extermination' addressed cross-source
resolution, NOT this regex).

Fix (config-driven, default behavior unchanged):

- Introduce DEFAULT_ENTITY_DIRS as the canonical-dirs export
- Add buildDirPattern(extraDirs) + buildEntityRegexes(extraDirs) factories
  that build the regex bundle from default ∪ extras
- Add getAutoLinkExtraDirs(engine) helper that reads auto_link.extra_dirs
  from engine config — supports JSON array string or comma-separated
- Refactor extractEntityRefs + extractPageLinks to accept optional extras
- Wire callers (extract.ts extractLinksFromDB, operations.ts put_page
  autolink path) to fetch from config + pass through

Backward compat:
- Module-level constants ENTITY_REF_RE / WIKILINK_RE / QUALIFIED_WIKILINK_RE
  retained, built from default dirs only — same behavior as before for any
  direct importer
- extractEntityRefs / extractPageLinks signatures backward-compat (extras
  is optional); callers that don't pass it get default behavior
- Config key not set = default behavior (no regression for non-BH users)

For BH:
  gbrain config set auto_link.extra_dirs '["03-ventures","06-people","07-decisions","08-writing","00-inbox","02-being-human","04-projects","05-research","venture","decision","person","research","writing"]'

Then 'gbrain extract links --source db' should produce >0 links and
link_density_score should improve.

Upstream PR forthcoming to garrytan/gbrain — solves the same class of
problem for any user with custom dir layouts.

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

Copy link
Copy Markdown
Author

Adding a second commit to this branch — ce0b12d feat(link-extraction): make entity-dir whitelist extensible via auto_link.extra_dirs config.

Same scope, different file. Full description in the commit message. Happy to split into two separate PRs if you'd prefer — the changes are independent (different files, different bug class). Let me know.

garrytan added a commit that referenced this pull request May 17, 2026
… placement (#1053)

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

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

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

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

* chore: bump version and changelog (v0.35.2.0)

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

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

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@garrytan

Copy link
Copy Markdown
Owner

Thanks @kyledeanjackson — already shipped in master as of v0.35.3.0. src/core/git-remote.ts:25-56 splits the flags into two distinct constants: GIT_SSRF_FLAGS (global -c config, spread BEFORE the verb) and GIT_SSRF_SUBCOMMAND_FLAGS (--no-recurse-submodules, spread AFTER the verb). test/git-remote.test.ts has a position-anchored regression guard (argv.indexOf('--no-recurse-submodules') > argv.indexOf(verb)).

If your install is still hitting exit 129 on gbrain sync or remote-source clone/pull after upgrade, please reopen with the output of gbrain doctor --json and the exact failing command.

Closing as already-shipped. Multiple independent community fixes for this bug — appreciate you chasing it.

@garrytan garrytan closed this May 18, 2026
@garrytan garrytan mentioned this pull request May 26, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants