Skip to content

fix: sourceLocalPath handles wrapped {sources:[...]} shape from gbrain v0.18.0+#1610

Open
pennyultima wants to merge 1 commit into
garrytan:mainfrom
pennyultima:fix/sources-list-shape-mismatch
Open

fix: sourceLocalPath handles wrapped {sources:[...]} shape from gbrain v0.18.0+#1610
pennyultima wants to merge 1 commit into
garrytan:mainfrom
pennyultima:fix/sources-list-shape-mismatch

Conversation

@pennyultima
Copy link
Copy Markdown

Closes #1609

What

gbrain sources list --json has returned {sources: [...]} since gbrain v0.18.0 (commit 90c5d93, "multi-source brains," 2026-04-22). sourceLocalPath() in bin/gstack-gbrain-sync.ts:291 typed the response as a raw Array<...> and called .find() directly, crashing /sync-gbrain --full:

gstack-gbrain-sync fatal: list.find is not a function.
  (In 'list.find((s) => s.id === sourceId)', 'list.find' is undefined)

The bug only surfaces on full reindex (runCodeImport is the only caller of sourceLocalPath), so the cheap incremental path users hit via the hourly LaunchAgent didn't trigger it. That's why it's been latent.

How

Defensive against both shapes — works on any gbrain version, wrapped (v0.18.0+) or raw array (older):

export function sourceLocalPath(sourceId: string, env?: NodeJS.ProcessEnv): string | null {
  const raw = execGbrainJson<
    | Array<{ id: string; local_path?: string }>
    | { sources?: Array<{ id: string; local_path?: string }> }
  >(["sources", "list", "--json"], { baseEnv: env });
  if (!raw) return null;
  const list = Array.isArray(raw) ? raw : raw.sources ?? [];
  const found = list.find((s) => s.id === sourceId);
  return found?.local_path ?? null;
}

Why CI didn't catch it

The existing 3 unit tests for sourceLocalPath all use the raw-array shape in their fixtures:

stdout: JSON.stringify([{ id: "...", local_path: "..." }])

The function passes them because the mock matches the function's (wrong) type assumption. The real gbrain CLI returns the wrapped shape, and the function crashes there.

This PR updates the fixtures to use the live wrapped shape and keeps the raw-array tests for back-compat. Net: 5 tests cover both shapes.

bun test test/gstack-gbrain-sync.test.ts -t "sourceLocalPath"
 5 pass
 0 fail

Pre-fix code fails 2 of these (the new wrapped-shape tests). Post-fix passes all 5.

Audit of other call sites

I grepped for every consumer of gbrain sources list --json across bin/ and lib/:

Call site Status
bin/gstack-gbrain-sync.ts:292 (this PR) ❌ → ✅
lib/gbrain-sources.ts:54 (probeSource) ✅ already unwraps .sources
lib/gbrain-sources.ts:165 (sourcePageCount) ✅ already unwraps .sources
bin/gstack-gbrain-source-wireup:215,298 (shell) ✅ uses jq '.sources[]'
lib/gbrain-local-status.ts:224 ✅ only checks exit code, doesn't parse

So this is the only broken call site. The lib code does it right; the orchestrator should ideally use lib/gbrain-sources.ts directly (a future refactor — not in this PR to keep the diff minimal).

Suggested follow-up: contract test

Worth adding test/contract/gbrain-cli.test.ts that runs against a real gbrain binary in CI (probably gated on a GSTACK_CONTRACT_TEST=1 env var so unit tests stay fast). That would catch any future JSON shape drift between gstack and gbrain at PR time instead of in users' terminals. Happy to write this in a follow-up if the maintainer wants it.

Test plan

  • bun test test/gstack-gbrain-sync.test.ts -t "sourceLocalPath" — 5/5 pass
  • Verified on the reporter's machine (gstack v1.40.0.0 + gbrain v0.33.1.0): patched orchestrator runs successfully where the unpatched version crashed.

🤖 Generated with Claude Code

…n v0.18.0+

`gbrain sources list --json` has returned `{sources: [...]}` since v0.18.0
(commit 90c5d93, "multi-source brains"). `sourceLocalPath()` typed the
response as a raw `Array<...>` and called `.find()` directly, crashing
`/sync-gbrain --full` with: `list.find is not a function`.

The bug only surfaces on full reindex (`runCodeImport` is the only caller)
so the cheap incremental path didn't hit it.

Patch is defensive against both shapes so the helper works on any gbrain
version: wrapped (v0.18.0+) or raw array (older).

Test fixture audit: the existing 3 tests for sourceLocalPath all used the
raw-array shape, which is why CI didn't catch the bug. Added matching
tests for the live wrapped shape and kept the raw-array tests for
back-compat coverage. 5 tests pass; pre-fix code would have failed the
two new wrapped-shape tests.

Closes garrytan#1609

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

Thanks for pinning this. This appears to be the same sourceLocalPath wrapped {sources:[...]} crash that is already carried by canonical maintainer wave #1594, which closes #1567 and says it supersedes #1564/#1571. The wave touches the same function in bin/gstack-gbrain-sync.ts and adds regression coverage for the wrapped shape, while also bundling the adjacent gbrain API-drift fixes.

I think #1594 is the canonical landing path now. If anything from this PR is still missing after that wave, the useful follow-up would be a targeted regression against current main rather than a parallel sourceLocalPath PR.

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.

gstack-gbrain-sync: sources list --json shape mismatch crashes /sync-gbrain --full on gbrain ≥ 0.18.0

2 participants