mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix: /sync-gbrain respects an existing valid .gbrain-source pin (#2417)
/sync-gbrain always derived a new worktree-scoped source ID, even when the repository already carried a valid .gbrain-source pin created through the native GBrain source workflow — silently bypassing the selected source boundary, registering a duplicate federated source, and routing later dream/cycle checks to the wrong source. Now a local pin is reused when it passes the fail-closed identity checks: the ID is syntactically valid, the source is registered, and the registered path realpath-resolves to the current checkout (so a stale or copied dotfile can't redirect a sync into another repo's source). A confirmed pin is treated as user-managed — synced and attached without add/remove, legacy migration, or federation changes. Dry-run stays spawn-free (reads only the local marker for previews). Missing, invalid, stale, or unreadable pins fall back to the existing generated source ID. Absorbs PR #2417 by @exGeni (applied via git am -3; 42 tests pass in test/gstack-gbrain-sync.test.ts including the new pin-respecting coverage: spawn-free dry-run, symlink-equivalent registered paths, non-dry-run sync/attach with no add/remove, dream routing, unreadable markers, config-backed env use). Co-authored-by: Evgenii Lopatin <e75533@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Evgenii Lopatin
Claude Fable 5
parent
acc354fcfa
commit
bfa579d4ea
+73
-23
@@ -29,7 +29,7 @@
|
||||
* than building a gstack-side daemon.
|
||||
*/
|
||||
|
||||
import { existsSync, statSync, mkdirSync, writeFileSync, readFileSync, unlinkSync, renameSync } from "fs";
|
||||
import { existsSync, statSync, mkdirSync, writeFileSync, readFileSync, unlinkSync, renameSync, realpathSync } from "fs";
|
||||
import { join, dirname } from "path";
|
||||
import { execSync, spawnSync } from "child_process";
|
||||
import { homedir, hostname } from "os";
|
||||
@@ -368,6 +368,42 @@ function deriveCodeSourceId(repoPath: string): string {
|
||||
return constrainSourceId("gstack-code", `${base}-${hostPathHash}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reuse an explicit repo pin when it names a registered source for this exact
|
||||
* checkout. The path check prevents a stale or copied dotfile from redirecting
|
||||
* a code sync into another repo's source.
|
||||
*/
|
||||
function readPinnedSourceId(repoPath: string): string | null {
|
||||
const pinPath = join(repoPath, ".gbrain-source");
|
||||
if (!existsSync(pinPath)) return null;
|
||||
|
||||
try {
|
||||
const sourceId = readFileSync(pinPath, "utf-8").trim();
|
||||
return /^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$/.test(sourceId) ? sourceId : null;
|
||||
} catch {
|
||||
// A pin is advisory. A permission race or a directory at this path must
|
||||
// not turn a sync preview into an unexpected crash.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function existingPinnedSourceId(repoPath: string, env?: NodeJS.ProcessEnv): string | null {
|
||||
const sourceId = readPinnedSourceId(repoPath);
|
||||
if (!sourceId) return null;
|
||||
|
||||
const registeredPath = sourceLocalPath(sourceId, env);
|
||||
if (!registeredPath) return null;
|
||||
try {
|
||||
return realpathSync(registeredPath) === realpathSync(repoPath) ? sourceId : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function resolveCodeSourceId(repoPath: string, env?: NodeJS.ProcessEnv): string {
|
||||
return existingPinnedSourceId(repoPath, env) ?? deriveCodeSourceId(repoPath);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-pathhash source id, kept for orphan detection only.
|
||||
*
|
||||
@@ -820,7 +856,13 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
||||
return { name: "code", ran: false, ok: true, duration_ms: 0, summary: "skipped (not in git repo)" };
|
||||
}
|
||||
|
||||
const sourceId = deriveCodeSourceId(root);
|
||||
// A preview must not spawn gbrain. Trust a syntactically-valid local pin
|
||||
// there; a real run confirms its registered path before using it.
|
||||
const gbrainEnv = args.mode === "dry-run" ? undefined : buildGbrainEnv({ announce: !args.quiet });
|
||||
const pinnedSourceId = args.mode === "dry-run"
|
||||
? readPinnedSourceId(root)
|
||||
: existingPinnedSourceId(root, gbrainEnv);
|
||||
const sourceId = pinnedSourceId ?? deriveCodeSourceId(root);
|
||||
|
||||
// Per-repo trust tier — checked BEFORE the dry-run branch so previews report
|
||||
// the refusal honestly instead of claiming they would sync.
|
||||
@@ -861,7 +903,9 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
||||
ran: false,
|
||||
ok: true,
|
||||
duration_ms: 0,
|
||||
summary: `would: gbrain sources add ${sourceId} --path ${root} --federated; gbrain sync --strategy code --source ${sourceId}; gbrain sources attach ${sourceId}`,
|
||||
summary: pinnedSourceId
|
||||
? `would: gbrain sync --strategy code --source ${sourceId}; gbrain sources attach ${sourceId}`
|
||||
: `would: gbrain sources add ${sourceId} --path ${root} --federated; gbrain sync --strategy code --source ${sourceId}; gbrain sources attach ${sourceId}`,
|
||||
detail: { source_id: sourceId, source_path: root, status: "skipped" },
|
||||
};
|
||||
}
|
||||
@@ -889,10 +933,9 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
||||
// gbrainEnv seeds DATABASE_URL from gbrain's config so this stage works
|
||||
// inside Next.js / Prisma / Rails projects with their own .env.local
|
||||
// (codex review #7 — bug fix is wider than #1508 as filed).
|
||||
const gbrainEnv = buildGbrainEnv({ announce: !args.quiet });
|
||||
const legacyId = deriveLegacyCodeSourceId(root);
|
||||
let legacyRemoved = false;
|
||||
if (legacyId !== sourceId) {
|
||||
if (!pinnedSourceId && legacyId !== sourceId) {
|
||||
// #1734: route through the data-loss guards (autopilot + source-safety).
|
||||
const rm = safeSourcesRemove(legacyId, gbrainEnv);
|
||||
if (rm.skipped && !args.quiet) {
|
||||
@@ -908,7 +951,9 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
||||
// pages); fall back to register-new → sync-OK → remove-old. Path-drift
|
||||
// (user moved the repo, etc.) skips migration with a warning.
|
||||
const pathOnlyHashLegacyId = derivePathOnlyHashLegacyId(root);
|
||||
const migration = planHostnameFoldMigration(root, sourceId, pathOnlyHashLegacyId, gbrainEnv);
|
||||
const migration = pinnedSourceId
|
||||
? { kind: "none", reason: "no-legacy-source" } as const
|
||||
: planHostnameFoldMigration(root, sourceId, pathOnlyHashLegacyId, gbrainEnv);
|
||||
if (migration.kind === "skipped-path-drift" && !args.quiet) {
|
||||
console.error(
|
||||
`[sync:code] hostname-fold migration skipped: legacy source ${migration.oldId} `
|
||||
@@ -919,21 +964,24 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
||||
console.error(`[sync:code] hostname-fold migration: renamed ${migration.oldId} → ${migration.newId} (pages preserved)`);
|
||||
}
|
||||
|
||||
// Step 1: Ensure source registered (idempotent). Single source of truth in lib —
|
||||
// no synchronous duplicate here (per /codex review #12).
|
||||
// Step 1: Ensure generated sources are registered. A confirmed explicit pin
|
||||
// belongs to the user: its realpath was checked above, so never remove/add it
|
||||
// merely because the registered spelling differs (e.g. a symlinked checkout).
|
||||
let registered = false;
|
||||
try {
|
||||
const result = await ensureSourceRegistered(sourceId, root, { federated: true, env: gbrainEnv });
|
||||
registered = result.changed;
|
||||
} catch (err) {
|
||||
return {
|
||||
name: "code",
|
||||
ran: true,
|
||||
ok: false,
|
||||
duration_ms: Date.now() - t0,
|
||||
summary: `source registration failed: ${(err as Error).message}`,
|
||||
detail: { source_id: sourceId, source_path: root, status: "failed" },
|
||||
};
|
||||
if (!pinnedSourceId) {
|
||||
try {
|
||||
const result = await ensureSourceRegistered(sourceId, root, { federated: true, env: gbrainEnv });
|
||||
registered = result.changed;
|
||||
} catch (err) {
|
||||
return {
|
||||
name: "code",
|
||||
ran: true,
|
||||
ok: false,
|
||||
duration_ms: Date.now() - t0,
|
||||
summary: `source registration failed: ${(err as Error).message}`,
|
||||
detail: { source_id: sourceId, source_path: root, status: "failed" },
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Always run the page-creating file walk first, then (for --full)
|
||||
@@ -1336,7 +1384,7 @@ export async function runDream(args: CliArgs): Promise<StageResult> {
|
||||
|
||||
if (args.mode === "dry-run") {
|
||||
const root = repoRoot();
|
||||
const sourceId = root ? deriveCodeSourceId(root) : null;
|
||||
const sourceId = root ? readPinnedSourceId(root) ?? deriveCodeSourceId(root) : null;
|
||||
return {
|
||||
name: "dream",
|
||||
ran: false,
|
||||
@@ -1348,6 +1396,7 @@ export async function runDream(args: CliArgs): Promise<StageResult> {
|
||||
};
|
||||
}
|
||||
|
||||
const gbrainEnv = buildGbrainEnv({ announce: !args.quiet });
|
||||
const localStatus = localEngineStatus({ noCache: false });
|
||||
if (localStatus === "timeout") {
|
||||
warnProbeTimeout("dream"); // #1964: slow-but-healthy — proceed
|
||||
@@ -1383,7 +1432,7 @@ export async function runDream(args: CliArgs): Promise<StageResult> {
|
||||
// code-callers/code-callees for this worktree. Falls back to plain `dream`
|
||||
// only when we can't derive the source id (not in a git repo).
|
||||
const root = repoRoot();
|
||||
const sourceId = root ? deriveCodeSourceId(root) : null;
|
||||
const sourceId = root ? resolveCodeSourceId(root, gbrainEnv) : null;
|
||||
const dreamArgs = sourceId ? ["dream", "--source", sourceId] : ["dream"];
|
||||
|
||||
// spawnGbrain seeds DATABASE_URL from gbrain's config via buildGbrainEnv.
|
||||
@@ -1675,7 +1724,8 @@ async function main(): Promise<void> {
|
||||
let cycle: CycleStatus | null = null;
|
||||
if (!args.dream && args.mode === "full" && !args.noDream && !args.noCode) {
|
||||
const root = repoRoot();
|
||||
cycle = root ? cycleCompleted(deriveCodeSourceId(root), process.env) : "unknown";
|
||||
const gbrainEnv = buildGbrainEnv({ announce: !args.quiet });
|
||||
cycle = root ? cycleCompleted(resolveCodeSourceId(root, gbrainEnv), gbrainEnv) : "unknown";
|
||||
}
|
||||
if (shouldRunDream(args, cycle)) {
|
||||
dreamStage = await runDream(args);
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
|
||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync, chmodSync } from "fs";
|
||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync, chmodSync, symlinkSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { spawnSync } from "child_process";
|
||||
@@ -62,6 +62,14 @@ describe("gstack-gbrain-sync CLI", () => {
|
||||
expect(source).toContain("localEngineStatus");
|
||||
});
|
||||
|
||||
it("uses GBrain's config environment when resolving dream sources", () => {
|
||||
const source = readFileSync(SCRIPT, "utf-8");
|
||||
|
||||
expect(source).not.toContain("resolveCodeSourceId(root, process.env)");
|
||||
expect(source).toContain("resolveCodeSourceId(root, gbrainEnv)");
|
||||
expect(source).toContain("cycleCompleted(resolveCodeSourceId(root, gbrainEnv), gbrainEnv)");
|
||||
});
|
||||
|
||||
it("--dry-run with --code-only reports the code import preview only", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
@@ -122,6 +130,139 @@ describe("gstack-gbrain-sync CLI", () => {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("uses a local .gbrain-source in dry-run without spawning gbrain", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
const bindir = mkdtempSync(join(tmpdir(), "gstack-pinned-source-bin-"));
|
||||
const repo = mkdtempSync(join(tmpdir(), "gstack-pinned-source-repo-"));
|
||||
const commandLog = join(home, "gbrain-commands.log");
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
spawnSync("git", ["init", "--quiet", "-b", "main"], { cwd: repo });
|
||||
writeFileSync(join(repo, ".gbrain-source"), "client-acme-app\n");
|
||||
writeFileSync(join(bindir, "gbrain"), `#!/bin/sh
|
||||
printf '%s\\n' "$*" >> "$GSTACK_TEST_GBRAIN_LOG"
|
||||
exit 99
|
||||
`);
|
||||
chmodSync(join(bindir, "gbrain"), 0o755);
|
||||
|
||||
const r = spawnSync("bun", [SCRIPT, "--dry-run", "--code-only", "--quiet"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 60000,
|
||||
cwd: repo,
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: home,
|
||||
GSTACK_HOME: gstackHome,
|
||||
GSTACK_TEST_GBRAIN_LOG: commandLog,
|
||||
PATH: `${bindir}:${process.env.PATH || ""}`,
|
||||
},
|
||||
});
|
||||
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain("gbrain sync --strategy code --source client-acme-app");
|
||||
expect(r.stdout).not.toContain("gbrain sources add");
|
||||
expect(r.stdout).not.toContain("--federated");
|
||||
expect(existsSync(commandLog)).toBe(false);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
rmSync(bindir, { recursive: true, force: true });
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("keeps a symlink-equivalent pinned source registered as-is", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
const repo = mkdtempSync(join(tmpdir(), "gstack-pinned-source-repo-"));
|
||||
const linkDir = mkdtempSync(join(tmpdir(), "gstack-pinned-source-link-"));
|
||||
const link = join(linkDir, "repo");
|
||||
const bindir = mkdtempSync(join(tmpdir(), "gstack-pinned-source-bin-"));
|
||||
const commandLog = join(home, "gbrain-commands.log");
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
mkdirSync(join(home, ".gbrain"), { recursive: true });
|
||||
writeFileSync(join(home, ".gbrain", "config.json"), JSON.stringify({ engine: "pglite", database_url: "pglite:///test" }));
|
||||
spawnSync("git", ["init", "--quiet", "-b", "main"], { cwd: repo });
|
||||
writeFileSync(join(repo, ".gbrain-source"), "client-acme-app\n");
|
||||
symlinkSync(repo, link, "dir");
|
||||
writeFileSync(join(bindir, "gbrain"), `#!/bin/sh
|
||||
printf '%s\\n' "$*" >> "$GSTACK_TEST_GBRAIN_LOG"
|
||||
case "$*" in
|
||||
--version) echo 'gbrain 0.42.0.0' ;;
|
||||
"sources list --json") echo '{"sources":[{"id":"client-acme-app","local_path":"${link}","page_count":1}]}' ;;
|
||||
"sync --strategy code --source client-acme-app"|"sources attach client-acme-app") ;;
|
||||
*) echo "unexpected gbrain command: $*" >&2; exit 1 ;;
|
||||
esac
|
||||
`);
|
||||
chmodSync(join(bindir, "gbrain"), 0o755);
|
||||
|
||||
const r = spawnSync("bun", [SCRIPT, "--code-only", "--quiet"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 60000,
|
||||
cwd: link,
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: home,
|
||||
GSTACK_HOME: gstackHome,
|
||||
GSTACK_TEST_GBRAIN_LOG: commandLog,
|
||||
PATH: `${bindir}:${process.env.PATH || ""}`,
|
||||
},
|
||||
});
|
||||
|
||||
const commands = readFileSync(commandLog, "utf-8");
|
||||
expect(r.status).toBe(0);
|
||||
expect(commands).toContain("sync --strategy code --source client-acme-app");
|
||||
expect(commands).toContain("sources attach client-acme-app");
|
||||
expect(commands).not.toMatch(/^sources (add|remove) /m);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
rmSync(linkDir, { recursive: true, force: true });
|
||||
rmSync(bindir, { recursive: true, force: true });
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("uses a local pin for a dry-run dream without spawning gbrain", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
const bindir = mkdtempSync(join(tmpdir(), "gstack-pinned-dream-bin-"));
|
||||
const repo = mkdtempSync(join(tmpdir(), "gstack-pinned-dream-repo-"));
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
spawnSync("git", ["init", "--quiet", "-b", "main"], { cwd: repo });
|
||||
writeFileSync(join(repo, ".gbrain-source"), "client-acme-app\n");
|
||||
writeFileSync(join(bindir, "gbrain"), "#!/bin/sh\nexit 99\n");
|
||||
chmodSync(join(bindir, "gbrain"), 0o755);
|
||||
|
||||
const r = spawnSync("bun", [SCRIPT, "--dry-run", "--dream", "--no-code", "--no-memory", "--no-brain-sync", "--quiet"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 60000,
|
||||
cwd: repo,
|
||||
env: { ...process.env, HOME: home, GSTACK_HOME: gstackHome, PATH: `${bindir}:${process.env.PATH || ""}` },
|
||||
});
|
||||
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain("gbrain dream --source client-acme-app");
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
rmSync(bindir, { recursive: true, force: true });
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("falls back to a derived source when .gbrain-source cannot be read", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
const repo = mkdtempSync(join(tmpdir(), "gstack-unreadable-pin-repo-"));
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
spawnSync("git", ["init", "--quiet", "-b", "main"], { cwd: repo });
|
||||
mkdirSync(join(repo, ".gbrain-source"));
|
||||
|
||||
const r = spawnSync("bun", [SCRIPT, "--dry-run", "--code-only", "--quiet"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 60000,
|
||||
cwd: repo,
|
||||
env: { ...process.env, HOME: home, GSTACK_HOME: gstackHome },
|
||||
});
|
||||
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toMatch(/gbrain sources add gstack-code-/);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("derived source ids are gbrain-valid (≤32 chars, alnum + interior hyphens, no dots) for any remote", () => {
|
||||
// gbrain enforces source ids to be 1-32 lowercase alnum chars with optional interior
|
||||
// hyphens. Pre-fix, the slug came from canonicalizeRemote() with only `/` and
|
||||
|
||||
Reference in New Issue
Block a user