RomanNum3ral
/
gstack
mirror of https://github.com/garrytan/gstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

harden(gbrain-sources): route drift remove through #1734 guards + realpath drift check 

Absorbing #2031 un-blocked a destructive remove that bypassed the #1734
data-loss guards: ensureSourceRegistered's drift path issued
`gbrain sources remove` directly, without the detectAutopilot +
decideSourceRemove checks every other remove routes through via
safeSourcesRemove. gbrain >= 0.42's own prompt was accidentally blocking
that path; with --confirm-destructive passed it is live again.

- Drift remove now refuses LOUDLY (throws, actionable message) while an
  autopilot is active or when decideSourceRemove disallows; a silent
  changed=false would hide the drifted registration.
- decideSourceRemove's extraArgs (--keep-storage when supported) propagate
  to the remove call, matching safeSourcesRemove.
- Drift is realpath-normalized before being declared: a symlink alias of the
  same directory (macOS /tmp -> /private/tmp) is a match, not drift — the
  probable cause of #1985's reporter hitting the remove on an unmoved repo.
- Drift fires a loud stderr line (old -> new path); perpetual drift in logs
  is the trigger for promoting #1985's reindex-in-place design.

Tests: autopilot-active refusal (no remove in call log), fail-closed refusal
on unreadable sources list, --keep-storage propagation, symlink-alias
no-drift; existing drift tests pin the guard probes so a live autopilot on
the dev machine can't flip them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-07-09 19:02:11 -07:00
parent 0ad9695867
commit c20e4625fa
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
2 changed files with 172 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
lib/gbrain-sources.ts
View File

@ -10,8 +10,15 @@
*/
import { execFileSync, spawnSync } from "child_process";
import { realpathSync } from "fs";
import { withErrorContext } from "./gstack-memory-helpers";
import { execGbrainJson, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec";
import {
detectAutopilot,
decideSourceRemove,
type AutopilotProbe,
type DecideRemoveOpts,
} from "./gbrain-guards";
export interface SourceState {
/** "absent" — id not registered. "match" — id at expected path. "drift" — id at different path. */
@ -70,6 +77,33 @@ export interface EnsureOptions {
* mutations of process.env.PATH unless env is passed explicitly).
*/
env?: NodeJS.ProcessEnv;
/**
* #1734 test hooks for the drift-remove guards. Production callers leave
* these unset (real autopilot detection + real remove decision). Tests pin
* them so a live autopilot on the dev machine can't flip test outcomes.
*/
autopilotProbe?: AutopilotProbe;
removeDecision?: DecideRemoveOpts;
}
/**
* Path equality with realpath normalization (macOS /tmp -> /private/tmp,
* symlinked worktrees). A registered path that resolves to the same real
* directory is NOT drift — declaring it drift triggers a destructive
* remove+add and a full re-index for a no-op (#1985 reporter hit the remove
* on an unmoved repo).
*/
function samePath(registered: string | undefined, requested: string): boolean {
if (!registered) return false;
if (registered === requested) return true;
const real = (p: string): string => {
try {
return realpathSync(p);
} catch {
return p;
}
};
return real(registered) === real(requested);
}
/**
@ -142,9 +176,10 @@ export async function ensureSourceRegistered(
return withErrorContext(`ensureSourceRegistered:${id}`, () => {
const probed = probeSource(id, env);
// Disambiguate match-but-different-path
// Disambiguate match-but-different-path (realpath-normalized: a symlink
// alias of the same directory is a match, not drift).
let state: SourceState = probed;
if (probed.status === "match" && probed.registered_path !== path) {
if (probed.status === "match" && !samePath(probed.registered_path, path)) {
state = { status: "drift", registered_path: probed.registered_path };
}
@ -165,12 +200,40 @@ export async function ensureSourceRegistered(
// 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", "--confirm-destructive"], {
encoding: "utf-8",
timeout: 30_000,
env,
shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows
});
// Loud drift observability: if this line shows up on every sync for some
// environment, drift is perpetual there and the reindex-in-place design
// from #1985 should be promoted (drop+rebuild re-embeds the full index).
console.error(
`[gbrain-sources] drift: ${id} registered at ${state.registered_path} -> re-registering at ${path}`,
);
// #1734: this remove deletes the source's pages/chunks/embeddings, so it
// runs only behind the same data-loss guards as the orchestrator's
// safeSourcesRemove(). A refusal is FATAL here (not best-effort): without
// the remove the add cannot proceed, and returning changed=false would
// silently hide the drifted registration.
const ap = detectAutopilot(env ?? process.env, options.autopilotProbe ?? {});
if (ap.active) {
throw new Error(
`refusing drift re-register of ${id}: autopilot active (${ap.signal}). ` +
`Stop autopilot, then re-run /sync-gbrain.`,
);
}
const decision = decideSourceRemove(id, env ?? process.env, options.removeDecision ?? {});
if (!decision.allow) {
throw new Error(`refusing drift re-register of ${id}: ${decision.reason}`);
}
const rm = spawnSync(
"gbrain",
["sources", "remove", id, "--yes", "--confirm-destructive", ...decision.extraArgs],
{
encoding: "utf-8",
timeout: 30_000,
env,
shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows
},
);
if (rm.status !== 0) {
throw new Error(`gbrain sources remove ${id} failed: ${rm.stderr || rm.stdout || `exit ${rm.status}`}`);
}

102
test/gbrain-sources.test.ts
View File

@ -8,7 +8,7 @@
*/
import { describe, it, expect } from "bun:test";
import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync } from "fs";
import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync, symlinkSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";
@ -168,6 +168,13 @@ describe("ensureSourceRegistered", () => {
fake.cleanup();
});
// Drift-path tests pin the #1734 guard inputs (inactive autopilot, allowed
// remove) so a REAL autopilot running on the dev machine can't flip them.
const guardsPinnedInactive = {
autopilotProbe: { lockPaths: [], processRunning: () => false },
removeDecision: { keepStorage: false },
} as const;
it("recreates source when path differs (gbrain has no `sources update`), returns changed=true", async () => {
const fake = makeFakeGbrain({
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
@ -175,6 +182,7 @@ describe("ensureSourceRegistered", () => {
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
federated: true,
env: fake.env,
...guardsPinnedInactive,
});
expect(result.changed).toBe(true);
expect(result.state.status).toBe("match");
@ -201,6 +209,7 @@ describe("ensureSourceRegistered", () => {
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
federated: true,
env: fake.env,
...guardsPinnedInactive,
});
expect(result.changed).toBe(true);
expect(result.state.status).toBe("match");
@ -214,6 +223,97 @@ describe("ensureSourceRegistered", () => {
fake.cleanup();
});
// #1734 tripwire: the drift remove deletes pages/chunks/embeddings, so it
// must refuse while a gbrain autopilot is active — and refuse LOUDLY (throw),
// not silently return changed=false. Before the guard routing, this path
// issued the remove unconditionally.
it("REFUSES the drift remove while autopilot is active (throws, no remove issued)", async () => {
const fake = makeFakeGbrain({
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
});
await expect(
ensureSourceRegistered("gstack-code-foo", "/new/path", {
env: fake.env,
autopilotProbe: { lockPaths: [], processRunning: () => true },
removeDecision: { keepStorage: false },
}),
).rejects.toThrow(/autopilot active/);
const log = readFileSync(fake.logPath, "utf-8");
expect(log).not.toContain("sources remove");
expect(log).not.toContain("sources add");
fake.cleanup();
});
it("REFUSES the drift remove when decideSourceRemove disallows (fail closed, throws)", async () => {
const fake = makeFakeGbrain({
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
});
await expect(
ensureSourceRegistered("gstack-code-foo", "/new/path", {
env: fake.env,
autopilotProbe: { lockPaths: [], processRunning: () => false },
// A sources-list read failure makes decideSourceRemove fail closed.
removeDecision: {
keepStorage: false,
fetchRows: () => {
throw new Error("sources list unavailable");
},
},
}),
).rejects.toThrow(/fail closed/);
const log = readFileSync(fake.logPath, "utf-8");
expect(log).not.toContain("sources remove");
fake.cleanup();
});
it("propagates decideSourceRemove extraArgs (--keep-storage) to the drift remove", async () => {
const fake = makeFakeGbrain({
sources: [{ id: "gstack-code-foo", local_path: "/old/path" }],
});
const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", {
env: fake.env,
autopilotProbe: { lockPaths: [], processRunning: () => false },
removeDecision: { keepStorage: true },
});
expect(result.changed).toBe(true);
const log = readFileSync(fake.logPath, "utf-8");
expect(log).toContain(
"sources remove gstack-code-foo --yes --confirm-destructive --keep-storage",
);
fake.cleanup();
});
// Realpath normalization: a registered path that is a symlink alias of the
// requested path is a MATCH, not drift. Declaring it drift triggers a
// destructive remove + full re-index for a no-op (#1985 reporter hit the
// remove on an unmoved repo; macOS /tmp -> /private/tmp is the usual cause).
it("does NOT declare drift when registered path is a symlink alias of the requested path", async () => {
const base = mkdtempSync(join(tmpdir(), "gbrain-sources-realpath-"));
const realDir = join(base, "real-repo");
const linkDir = join(base, "link-repo");
mkdirSync(realDir, { recursive: true });
symlinkSync(realDir, linkDir);
const fake = makeFakeGbrain({
sources: [{ id: "gstack-code-foo", local_path: realDir }],
});
const result = await ensureSourceRegistered("gstack-code-foo", linkDir, {
env: fake.env,
...guardsPinnedInactive,
});
expect(result.changed).toBe(false);
expect(result.state.status).toBe("match");
const log = readFileSync(fake.logPath, "utf-8");
expect(log).not.toContain("sources remove");
expect(log).not.toContain("sources add");
fake.cleanup();
rmSync(base, { recursive: true, force: true });
});
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" }],