gstack icon indicating copy to clipboard operation
gstack copied to clipboard

fix(sync): accept wrapped {sources:[...]} shape from gbrain >= 0.35.x

Open tonyjzhou opened this issue 2 months ago • 1 comments

Summary

bin/gstack-gbrain-sync.ts's sourceLocalPath helper assumed gbrain sources list --json returns a bare [...] array. gbrain 0.35.x returns {"sources": [...]} instead, so /sync-gbrain crashes on the first call inside runCodeImport's hostname-fold migration probe with TypeError: list.find is not a function.

The fix accepts both shapes (legacy bare array + current wrapped object). The 2-line production change is paired with 3 new tests that pin the wrapped shape so the regression class is closed.

Repro

Fresh /sync-gbrain on v1.40.0.0 with gbrain 0.35.3.0 (local-stdio MCP):

$ bun run ~/.claude/skills/gstack/bin/gstack-gbrain-sync.ts
[gbrain-sync] mode=incremental engine=pglite
gstack-gbrain-sync fatal: list.find is not a function. (In 'list.find((s) => s.id === sourceId)', 'list.find' is undefined)

Crash site: bin/gstack-gbrain-sync.ts:297 inside sourceLocalPath.

Root cause

sourceLocalPath reads gbrain sources list --json via execGbrainJson<Array<...>> and immediately calls .find() on the result. The <T> cast in execGbrainJson is type-asserted, not validated — JSON.parse(stdout) as T lies whenever the CLI's actual shape diverges from the declared one.

Current gbrain (0.35.x) returns:

{
  "sources": [
    { "id": "default", "page_count": 672, ... },
    { "id": "gstack-code-roster-4f3a08f7", "page_count": 51, ... }
  ]
}

— not the bare array the orchestrator expects. {...}.find is undefined → TypeError → orchestrator abort before any of code / memory / brain-sync stages can run.

Why the existing tests didn't catch it

test/gstack-gbrain-sync.test.ts's 3 existing sourceLocalPath tests all stub stdout: JSON.stringify([...]) — they pin the OLD bare-array shape. The test fixtures and the production CLI drifted: nothing in CI compares the orchestrator's expectations against actual gbrain output.

Fix

bin/gstack-gbrain-sync.ts — unwrap .sources when the payload is an object; keep bare-array support for older gbrain installs:

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

Tests added

test/gstack-gbrain-sync.test.ts — 3 new tests in the existing describe("sourceLocalPath", …) block:

  1. accepts the wrapped {sources:[...]} shape returned by gbrain >= 0.35.x — happy path with the new shape.
  2. returns null when wrapped payload has no matching source id — wrapped + missing.
  3. returns null when wrapped payload omits the sources array — wrapped + malformed.

Verification cycle (TDD):

  • Revert the production change → run new tests → all 3 fail with the exact production stack trace (list.find is not a function).
  • Re-apply the production change → run full file → 38 pass, 0 fail (was 35 pre-change).
  • Full bun run test confirms no other test regressed.

Suggested CHANGELOG entry

For whichever patch release this lands in:

Fixed

  • bin/gstack-gbrain-sync.ts: sourceLocalPath accepts both the bare-array shape (legacy gbrain) and the wrapped {sources:[...]} shape (gbrain >= 0.35.x). The previous code crashed /sync-gbrain with TypeError: list.find is not a function on the first migration probe. Three regression tests pin both shapes plus the wrapped-malformed case.

Out of scope (separate concerns surfaced while investigating)

  • execGbrainJson<T> lies at the FFI boundary — the generic JSON.parse(stdout) as T cast lets any CLI shape divergence slip through silently. A shape validator (zod / runtime check) at the wrapper level would prevent the next instance of this class. Larger change; happy to open a separate issue if useful.
  • test/gbrain-lib-verify.test.ts:198 (rejects invalid var names) is failing on main independently of this PR. Unrelated to TS code paths; pre-existing on 026751ea. Mentioning here so the failing-tests CI signal isn't misattributed.

Test plan

  • [x] bun test test/gstack-gbrain-sync.test.ts — 38/38 pass
  • [x] bun run test — no new failures attributable to this change
  • [x] TDD cycle: tests fail on reverted production code with the exact production stack trace; pass after fix
  • [x] Manual /sync-gbrain run on Roster repo with gbrain 0.35.3.0 local-stdio — code (52 pages) + memory (100 imported) + brain-sync stages all complete

🤖 Generated with Claude Code

tonyjzhou avatar May 17 '26 16:05 tonyjzhou

Hitting the same bug — gstack 1.40.0.0 + gbrain 0.35.7.0, local-stdio MCP, on macOS in a Conductor workspace. /sync-gbrain crashes in sourceLocalPath before any stage runs. Your fix resolves it for me (applied locally + verified gstack-gbrain-sync (full) completes cleanly through code → memory → brain-sync; 311 pages indexed, gbrain search returns hits).

One bit of supporting evidence that the bug isn't gbrain-version-specific the way the title suggests: when I traced it, lib/gbrain-sources.ts:79, 176-177 was already handling both shapes for probeSource + sourcePageCount. Only sourceLocalPath in bin/gstack-gbrain-sync.ts was missed — so this is a consistency fix as much as a 0.35.x compat fix.

Independently arrived at the same patch + same test trio at cfeddersen/gstack@fix/source-local-path-wrapped-shape; happy to close mine in favor of yours.

Worth noting #1567, #1576 (the issues) and #1571 (parallel PR) are all the same bug.

cfeddersen avatar May 18 '26 11:05 cfeddersen

Thanks @tonyjzhou — sourceLocalPath wrapped-object fix shipped in v1.42.0.0 Daegu wave (#1594) via @jakehann11's #1571. Your PR is cited as a supersedes ref in the wave commit body.

garrytan avatar May 26 '26 03:05 garrytan