mirror of https://github.com/garrytan/gstack.git
fix(gbrain): pass --confirm-destructive on drift re-register (#1985)
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 <id> --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) <noreply@anthropic.com>
This commit is contained in:
parent
c7ae63201a
commit
55ea62e031
|
|
@ -124,7 +124,7 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState {
|
|||
* Behavior:
|
||||
* - status=absent → `gbrain sources add <id> --path <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,
|
||||
|
|
|
|||
|
|
@ -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 <id> --path <p> [--federated] → append to state, log
|
||||
* gbrain sources remove <id> --yes → drop from state, log
|
||||
* gbrain --version → echo "gbrain 0.25.1"
|
||||
* gbrain sources remove <id> --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" }],
|
||||
|
|
|
|||
Loading…
Reference in New Issue