From 55ea62e031b0c79ebf83cd38a7c13220ea2aec6b Mon Sep 17 00:00:00 2001 From: Jayesh Betala Date: Wed, 17 Jun 2026 12:17:29 +0530 Subject: [PATCH] fix(gbrain): pass --confirm-destructive on drift re-register (#1985) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureSourceRegistered() handles match-but-different-path by removing the old source then re-adding it at the new path. The remove was issued as `gbrain sources remove --yes`, but gbrain >= 0.42 gates `sources remove` behind `--confirm-destructive` (`--yes` alone no longer suppresses the data-loss prompt). The remove therefore fails with "To proceed, pass --confirm-destructive", which ensureSourceRegistered surfaces as "source registration failed" — aborting the entire /sync-gbrain code stage for any already-registered source whose path has drifted. The memory and brain-sync stages still pass, so the code index silently stops refreshing. The orchestrator's own safeSourcesRemove() already passes --confirm-destructive; this brings the lib helper in line with that convention. Keeps --yes for older gbrain. Tests: extend the fake gbrain shim in gbrain-sources.test.ts to simulate the gbrain >= 0.42 guard (remove without --confirm-destructive exits 1), update the drift re-register assertion, and add a regression test that proves the drift path no longer throws. Both fail on main with the exact "To proceed, pass --confirm-destructive" error and pass with the fix. Fixes #1985 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/gbrain-sources.ts | 11 +++++++-- test/gbrain-sources.test.ts | 49 ++++++++++++++++++++++++++++++++++--- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/gbrain-sources.ts b/lib/gbrain-sources.ts index cdaa25eaa..9c5b2be82 100644 --- a/lib/gbrain-sources.ts +++ b/lib/gbrain-sources.ts @@ -124,7 +124,7 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState { * Behavior: * - status=absent → `gbrain sources add --path [--federated]`, returns changed=true. * - status=match + same path → no-op, returns changed=false. - * - status=match + different path → `sources remove` + `sources add`, returns changed=true. + * - status=match + different path → `sources remove --confirm-destructive` + `sources add`, returns changed=true. * (Skip when reregister_on_drift=false; returns changed=false.) * * Caller is responsible for catching errors. The function uses withErrorContext for @@ -157,8 +157,15 @@ export async function ensureSourceRegistered( } // For drift, remove first. + // + // #1985: gbrain >= 0.42 gates `sources remove` behind --confirm-destructive + // (`--yes` alone no longer suppresses the data-loss prompt). Without it the + // remove fails with "To proceed, pass --confirm-destructive", which surfaces + // as "source registration failed" and aborts the whole /sync-gbrain code + // stage for any source that has drifted to a new path. This matches the + // flag the orchestrator's own safeSourcesRemove() already passes. if (state.status === "drift") { - const rm = spawnSync("gbrain", ["sources", "remove", id, "--yes"], { + const rm = spawnSync("gbrain", ["sources", "remove", id, "--yes", "--confirm-destructive"], { encoding: "utf-8", timeout: 30_000, env, diff --git a/test/gbrain-sources.test.ts b/test/gbrain-sources.test.ts index 9486eb06c..63cbb65a7 100644 --- a/test/gbrain-sources.test.ts +++ b/test/gbrain-sources.test.ts @@ -32,8 +32,11 @@ interface FakeGbrainSetup { * Build a temp dir with a fake `gbrain` shell script on PATH. The fake honors: * gbrain sources list --json → cat $STATE_PATH * gbrain sources add --path

[--federated] → append to state, log - * gbrain sources remove --yes → drop from state, log - * gbrain --version → echo "gbrain 0.25.1" + * gbrain sources remove --confirm-destructive → drop from state, log + * (#1985: remove WITHOUT + * --confirm-destructive + * fails like gbrain >= 0.42) + * gbrain --version → echo "gbrain 0.42.40.0" * Anything else exits 1. */ function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path: string; federated?: boolean; page_count?: number }> }): FakeGbrainSetup { @@ -49,7 +52,7 @@ function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path: echo "$@" >> "${logPath}" case "$1 $2" in "--version ") - echo "gbrain 0.25.1" + echo "gbrain 0.42.40.0" exit 0 ;; "sources list") @@ -75,6 +78,16 @@ case "$1 $2" in ;; "sources remove") ID="$3" + # #1985: gbrain >= 0.42 gates remove behind --confirm-destructive; --yes + # alone no longer suppresses the data-loss prompt. Refuse without it so the + # drift re-register path is exercised against real gbrain 0.42 behavior. + case " $* " in + *" --confirm-destructive "*) : ;; + *) + echo "This will permanently delete pages. To proceed, pass --confirm-destructive" >&2 + exit 1 + ;; + esac NEW=$(jq --arg id "$ID" '.sources = (.sources | map(select(.id != $id)))' "${statePath}") echo "$NEW" > "${statePath}" exit 0 @@ -168,11 +181,39 @@ describe("ensureSourceRegistered", () => { expect(result.state.registered_path).toBe("/new/path"); const log = readFileSync(fake.logPath, "utf-8"); - expect(log).toContain("sources remove gstack-code-foo --yes"); + // #1985: the remove must carry --confirm-destructive (gbrain >= 0.42 gate). + expect(log).toContain("sources remove gstack-code-foo --yes --confirm-destructive"); expect(log).toContain("sources add gstack-code-foo --path /new/path --federated"); fake.cleanup(); }); + // #1985: regression. gbrain >= 0.42 refuses `sources remove` without + // --confirm-destructive. On the drift path that surfaces as "source + // registration failed" and aborts the /sync-gbrain code stage for every + // already-registered source whose path drifted. Before the fix the remove + // was issued with `--yes` only, so the guard-simulating fake rejects it and + // ensureSourceRegistered throws. The fix passes --confirm-destructive, so + // the re-register succeeds. + it("re-registers across the gbrain >= 0.42 destructive-remove guard (does not throw)", async () => { + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], + }); + const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", { + federated: true, + env: fake.env, + }); + expect(result.changed).toBe(true); + expect(result.state.status).toBe("match"); + expect(result.state.registered_path).toBe("/new/path"); + + // The old source was actually removed (the guarded remove succeeded), then + // the new path was added — not left behind as a stale duplicate. + const finalState = JSON.parse(readFileSync(fake.statePath, "utf-8")); + expect(finalState.sources).toHaveLength(1); + expect(finalState.sources[0].local_path).toBe("/new/path"); + fake.cleanup(); + }); + it("when reregister_on_drift=false and source is at different path, returns changed=false", async () => { const fake = makeFakeGbrain({ sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],