refactor(gbrain-sources): centralize sources-list shape handling in parseSourcesList (#1576)

#1576's crash in sourceLocalPath was already fixed in v1.42.0.0 (dual-shape
handling). But the readers disagreed: sourceLocalPath accepted both the wrapped
{sources:[...]} object (v0.20+) and a bare array, while probeSource and
sourcePageCount accepted only the wrapped shape. Extract one parseSourcesList()
normalizer and route all three through it, so the shape assumption lives in a
single place. This is also the base the #1734 remote_url audit builds on.

parseSourcesList returns [] for null/garbage rather than throwing; callers treat
'no rows' as absent. New test/gbrain-sources-parse.test.ts pins both shapes plus
the garbage paths and confirms config.remote_url survives for the audit.

#1576 is closeable as already-fixed in v1.42.0.0.

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

bin/gstack-gbrain-sync.ts

@@ -37,7 +37,7 @@ import { createHash } from "crypto";
 
 import "../lib/conductor-env-shim";
 import { detectEngineTier, withErrorContext, canonicalizeRemote } from "../lib/gstack-memory-helpers";
-import { ensureSourceRegistered, sourcePageCount } from "../lib/gbrain-sources";
+import { ensureSourceRegistered, sourcePageCount, parseSourcesList } from "../lib/gbrain-sources";
 import { localEngineStatus, type LocalEngineStatus } from "../lib/gbrain-local-status";
 import { buildGbrainEnv, spawnGbrain, execGbrainJson } from "../lib/gbrain-exec";
 
@@ -407,10 +407,7 @@ export function sourceLocalPath(sourceId: string, env?: NodeJS.ProcessEnv): stri
     { baseEnv: env },
   );
   if (!raw) return null;
-  const list: Array<{ id?: string; local_path?: string }> = Array.isArray(raw)
-    ? (raw as Array<{ id?: string; local_path?: string }>)
-    : ((raw as { sources?: Array<{ id?: string; local_path?: string }> }).sources ?? []);
-  const found = list.find((s) => s.id === sourceId);
+  const found = parseSourcesList(raw).find((s) => s.id === sourceId);
   return found?.local_path ?? null;
 }
 

lib/gbrain-sources.ts

@@ -26,6 +26,37 @@ export interface EnsureResult {
   state: SourceState;
 }
 
+/**
+ * One row of `gbrain sources list --json`. `config.remote_url` distinguishes
+ * URL-managed sources (gbrain owns the clone, may auto-reclone) from
+ * path-managed ones (user owns the working tree) — load-bearing for the #1734
+ * destructive-op guards.
+ */
+export interface GbrainSourceRow {
+  id?: string;
+  local_path?: string;
+  page_count?: number;
+  config?: { remote_url?: string | null } | null;
+}
+
+/**
+ * Normalize `gbrain sources list --json` output to an array of source rows.
+ *
+ * gbrain has shipped two shapes: a wrapped `{ sources: [...] }` object (v0.20+)
+ * and, in older/other variants, a bare top-level array. #1576 was a crash when a
+ * reader assumed one shape; the parse is centralized here so every reader
+ * (probeSource, sourcePageCount, sourceLocalPath, the #1734 remote_url audit)
+ * agrees on the shape in ONE place. Returns [] for null/garbage rather than
+ * throwing — callers treat "no rows" as absent.
+ */
+export function parseSourcesList(raw: unknown): GbrainSourceRow[] {
+  if (Array.isArray(raw)) return raw as GbrainSourceRow[];
+  if (raw && typeof raw === "object" && Array.isArray((raw as { sources?: unknown }).sources)) {
+    return (raw as { sources: GbrainSourceRow[] }).sources;
+  }
+  return [];
+}
+
 export interface EnsureOptions {
   /** Pass --federated to `gbrain sources add`. Default false. */
   federated?: boolean;
@@ -69,14 +100,14 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState {
     throw err;
   }
 
-  let parsed: { sources?: Array<{ id?: string; local_path?: string }> };
+  let parsed: unknown;
   try {
     parsed = JSON.parse(stdout);
   } catch (err) {
     throw new Error(`gbrain sources list returned non-JSON output: ${(err as Error).message}`);
   }
 
-  const sources = parsed.sources || [];
+  const sources = parseSourcesList(parsed);
   const match = sources.find((s) => s.id === id);
   if (!match) return { status: "absent" };
   return {
@@ -173,8 +204,7 @@ export function sourcePageCount(id: string, env?: NodeJS.ProcessEnv): number | n
   }
 
   try {
-    const parsed = JSON.parse(stdout) as { sources?: Array<{ id?: string; page_count?: number }> };
-    const match = (parsed.sources || []).find((s) => s.id === id);
+    const match = parseSourcesList(JSON.parse(stdout)).find((s) => s.id === id);
     if (!match) return null;
     if (typeof match.page_count !== "number") return null;
     return match.page_count;

test/gbrain-sources-parse.test.ts

@@ -0,0 +1,49 @@
+import { describe, test, expect } from "bun:test";
+import { parseSourcesList } from "../lib/gbrain-sources";
+
+// #1576 hardening: `gbrain sources list --json` has shipped two shapes — a
+// wrapped `{ sources: [...] }` object (v0.20+) and a bare top-level array.
+// parseSourcesList is the single place that normalizes both, so every reader
+// (probeSource, sourcePageCount, sourceLocalPath, the #1734 remote_url audit)
+// agrees on the shape. These tests pin both shapes plus the garbage paths.
+describe("parseSourcesList", () => {
+  const rows = [
+    { id: "a", local_path: "/x", page_count: 3 },
+    { id: "b", local_path: "/y", config: { remote_url: "https://example.com/r.git" } },
+  ];
+
+  test("wrapped { sources: [...] } shape", () => {
+    expect(parseSourcesList({ sources: rows })).toEqual(rows);
+  });
+
+  test("bare top-level array shape", () => {
+    expect(parseSourcesList(rows)).toEqual(rows);
+  });
+
+  test("both shapes yield identical rows (shape-independent)", () => {
+    expect(parseSourcesList({ sources: rows })).toEqual(parseSourcesList(rows));
+  });
+
+  test("null / undefined → empty array (no throw)", () => {
+    expect(parseSourcesList(null)).toEqual([]);
+    expect(parseSourcesList(undefined)).toEqual([]);
+  });
+
+  test("object without sources key → empty array", () => {
+    expect(parseSourcesList({ pages: [] })).toEqual([]);
+  });
+
+  test("sources key present but not an array → empty array", () => {
+    expect(parseSourcesList({ sources: "oops" })).toEqual([]);
+  });
+
+  test("scalar garbage → empty array", () => {
+    expect(parseSourcesList("nope")).toEqual([]);
+    expect(parseSourcesList(42)).toEqual([]);
+  });
+
+  test("preserves config.remote_url for the #1734 audit", () => {
+    const parsed = parseSourcesList({ sources: rows });
+    expect(parsed.find((r) => r.id === "b")?.config?.remote_url).toBe("https://example.com/r.git");
+  });
+});