diff --git a/src/commands/code.ts b/src/commands/code.ts index 976f1db..5be3d45 100644 --- a/src/commands/code.ts +++ b/src/commands/code.ts @@ -157,9 +157,22 @@ export async function cmdCode(ctx: AppContext, task: string, opts: CodeOpts): Pr repoSpec = parseRepoSpec(opts.repo); const co = ensureLocalClone(repoSpec); repoRoot = co.dir; - process.stderr.write( - `⎇ repo ${repoSpec.full} ${co.cloned ? "(cloned)" : "(reusing local clone)"}\n ${co.dir}\n`, - ); + // Say what actually happened to the mirror. "reusing local clone" was + // equally true of a mirror last fetched a week ago, which is exactly the + // case a user needs told rather than hidden behind a reassuring word. + const tip = co.freshness.remoteTip ? ` @ ${co.freshness.remoteTip.slice(0, 7)}` : ""; + const how = co.cloned + ? "(cloned)" + : co.freshness.state === "fresh" + ? "(fetched)" + : `(NOT REFRESHED — ${co.freshness.reason ?? "reason unknown"})`; + process.stderr.write(`⎇ repo ${repoSpec.full} ${how}${tip}\n ${co.dir}\n`); + if (co.freshness.state !== "fresh") { + process.stderr.write( + " ! this worktree will branch off whatever the mirror already had;\n" + + " its base is not known to match the remote.\n", + ); + } } catch (err) { process.stderr.write(`✗ ${err instanceof Error ? err.message : String(err)}\n`); return 1; diff --git a/src/core/repo.ts b/src/core/repo.ts index bef0428..4376117 100644 --- a/src/core/repo.ts +++ b/src/core/repo.ts @@ -13,6 +13,7 @@ import { spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; +import { defaultRunner, type Runner } from "./worktree.js"; export interface RepoSpec { owner: string; @@ -60,6 +61,8 @@ export function cloneArgs(spec: RepoSpec, dir: string, useGh: boolean): { cmd: s } export interface RepoCheckout { + /** How current the mirror is. Never assumed — always measured or reported unknown. */ + freshness: MirrorFreshness; /** Local git dir for the repo (the mirror). */ dir: string; /** True when this call performed the clone (vs reusing an existing mirror). */ @@ -71,9 +74,15 @@ export interface RepoCheckout { * its dir. Reuses an existing mirror. Throws with an actionable message on a * clone failure (private repo + no auth is the common case). */ -export function ensureLocalClone(spec: RepoSpec): RepoCheckout { +export function ensureLocalClone(spec: RepoSpec, run: Runner = defaultRunner()): RepoCheckout { const dir = localMirrorDir(spec); - if (existsSync(join(dir, ".git"))) return { dir, cloned: false }; + // A mirror that already exists is validated and fetched before anything + // branches off it. Reusing it on the strength of its path alone is how a + // task silently starts from a days-old tip. + if (existsSync(join(dir, ".git"))) { + const { freshness } = refreshMirror(spec, dir, run, { exists: true }); + return { dir, cloned: false, freshness }; + } const useGh = ghAvailable(); const { cmd, args } = cloneArgs(spec, dir, useGh); const r = spawnSync(cmd, args, { encoding: "utf8" }); @@ -84,7 +93,110 @@ export function ensureLocalClone(spec: RepoSpec): RepoCheckout { : "gh CLI not found — install it or set up git credentials for github.com"; throw new Error(`could not clone ${spec.full}: ${why}\n ${hint}`); } - return { dir, cloned: true }; + // A clone just came from the remote, so its tip is the remote tip by + // construction. Read it back rather than asserting it. + const tip = run("git", ["-C", dir, "rev-parse", "HEAD"]); + return { + dir, + cloned: true, + freshness: { + state: "fresh", + remoteTip: tip.status === 0 ? tip.stdout.trim() || null : null, + checkedAt: new Date().toISOString(), + }, + }; +} + + +/** How current a local mirror is, relative to its GitHub remote. */ +export type MirrorFreshnessState = "fresh" | "stale" | "unknown"; + +export interface MirrorFreshness { + state: MirrorFreshnessState; + /** Commit the remote default branch resolved to, when the fetch succeeded. */ + remoteTip: string | null; + checkedAt: string; + /** Why the state is not "fresh". Present whenever it is not. */ + reason?: string; +} + +export interface MirrorResult { + dir: string; + freshness: MirrorFreshness; +} + +/** + * Validate and refresh an existing mirror before anything branches off it. + * + * Three properties this function exists to guarantee: + * + * 1. The directory really is the repo that was asked for. The mirror path is + * derived from the slug alone, so any directory sitting at that path would + * otherwise be accepted as "octocat/hello-world" on the strength of its name. + * 2. The mirror is fetched, so a task worktree does not branch off a tip that + * was current days ago. + * 3. When step 2 cannot happen — offline, auth expired, remote gone — the + * result says so. It never degrades to "fresh" as a convenience. + * + * Read-only with respect to the user's working tree: it fetches into the object + * store and reads refs. It never checks out, resets, merges, pulls or cleans. + * + * Auth is the user's own git/gh configuration, inherited from the environment. + * No Aether credential is passed, and none is available to this function. + */ +export function refreshMirror( + spec: RepoSpec, + dir: string, + run: Runner, + options: { exists: boolean; now?: string }, +): MirrorResult { + const checkedAt = options.now ?? new Date().toISOString(); + if (!options.exists) { + return { dir, freshness: { state: "unknown", remoteTip: null, checkedAt, reason: "no local mirror yet" } }; + } + + const remote = run("git", ["-C", dir, "remote", "get-url", "origin"]); + if (remote.status !== 0) { + return { + dir, + freshness: { + state: "unknown", + remoteTip: null, + checkedAt, + reason: remote.stderr.trim() || "could not read the mirror's origin remote", + }, + }; + } + // parseRepoSpec already normalizes https/ssh/.git/trailing-slash forms, so + // comparing through it avoids a second, subtly different URL parser. + let actual: string; + try { + actual = parseRepoSpec(remote.stdout.trim()).full; + } catch { + throw new Error( + `local mirror at ${dir} does not point at ${spec.full} — its origin is "${remote.stdout.trim()}"`, + ); + } + if (actual !== spec.full) { + throw new Error(`local mirror at ${dir} does not point at ${spec.full} — its origin is ${actual}`); + } + + const fetched = run("git", ["-C", dir, "fetch", "--prune", "origin"]); + if (fetched.status !== 0) { + return { + dir, + freshness: { + state: "unknown", + remoteTip: null, + checkedAt, + reason: (fetched.stderr || fetched.stdout).trim() || "git fetch failed", + }, + }; + } + + const tip = run("git", ["-C", dir, "rev-parse", "FETCH_HEAD"]); + const remoteTip = tip.status === 0 ? tip.stdout.trim() || null : null; + return { dir, freshness: { state: "fresh", remoteTip, checkedAt } }; } /** One-line "open a PR" footer for a finished repo run. Pure. */ diff --git a/test/repo.test.ts b/test/repo.test.ts index 5cb2702..52be4e9 100644 --- a/test/repo.test.ts +++ b/test/repo.test.ts @@ -1,6 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { parseRepoSpec, cloneArgs, prCreateHint } from "../src/core/repo.js"; +import { parseRepoSpec, cloneArgs, prCreateHint, refreshMirror } from "../src/core/repo.js"; +import type { Runner, RunResult } from "../src/core/worktree.js"; test("parseRepoSpec accepts owner/name", () => { const s = parseRepoSpec("octocat/hello-world"); @@ -44,3 +45,97 @@ test("prCreateHint targets the repo + branch", () => { const hint = prCreateHint(parseRepoSpec("octocat/hello-world"), "aether/fix-1"); assert.match(hint, /gh pr create -R octocat\/hello-world --head aether\/fix-1 --fill/); }); + +// ── mirror freshness (SC-A2) ──────────────────────────────────────────────── +// An existing --repo mirror must never be reused as-is. Before a task worktree +// branches off it, the mirror's remote is validated and the mirror is fetched +// through the user's own git/gh auth. When that cannot happen, the result says +// stale or unknown — never fresh. + +function recordingRunner(table: Record): { run: Runner; calls: string[][] } { + const calls: string[][] = []; + const run: Runner = (cmd, args) => { + calls.push([cmd, ...args]); + const key = [cmd, ...args].join(" "); + for (const [pattern, result] of Object.entries(table)) { + if (key.startsWith(pattern)) return result; + } + return { status: 0, stdout: "", stderr: "" }; + }; + return { run, calls }; +} + +const OK = (stdout = ""): RunResult => ({ status: 0, stdout, stderr: "" }); + +test("an existing mirror is fetched, not silently reused", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const { run, calls } = recordingRunner({ + "git -C": OK("https://github.com/octocat/hello-world.git\n"), + }); + const result = refreshMirror(spec, "/mirror", run, { exists: true }); + assert.equal(result.freshness.state, "fresh"); + const fetched = calls.some((call) => call.includes("fetch")); + assert.equal(fetched, true, "an existing mirror must be fetched before use"); +}); + +test("a mirror pointing at a different repo is rejected, never used", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const { run } = recordingRunner({ + "git -C": OK("https://github.com/somebody-else/other-repo.git\n"), + }); + assert.throws( + () => refreshMirror(spec, "/mirror", run, { exists: true }), + /does not point at octocat\/hello-world/, + ); +}); + +test("a failed fetch reports unknown, never fresh", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const run: Runner = (_cmd, args) => { + if (args.includes("fetch")) return { status: 1, stdout: "", stderr: "Could not resolve host: github.com" }; + return OK("https://github.com/octocat/hello-world.git\n"); + }; + const result = refreshMirror(spec, "/mirror", run, { exists: true }); + assert.notEqual(result.freshness.state, "fresh"); + assert.equal(result.freshness.state, "unknown"); + assert.match(result.freshness.reason ?? "", /Could not resolve host/); +}); + +test("refreshing a mirror never checks out, resets or cleans the user's tree", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const { run, calls } = recordingRunner({ + "git -C": OK("https://github.com/octocat/hello-world.git\n"), + }); + refreshMirror(spec, "/mirror", run, { exists: true }); + for (const mutation of ["checkout", "reset", "clean", "merge", "pull", "rebase"]) { + assert.equal( + calls.some((call) => call.includes(mutation)), + false, + `refresh must not run git ${mutation} on the user's mirror`, + ); + } +}); + +test("no Aether credential is ever handed to git or gh", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const { run, calls } = recordingRunner({ + "git -C": OK("https://github.com/octocat/hello-world.git\n"), + }); + refreshMirror(spec, "/mirror", run, { exists: true }); + const flat = calls.flat().join(" "); + for (const leak of ["aek_", "Authorization", "http.extraheader", "GIT_ASKPASS", "x-access-token"]) { + assert.equal(flat.includes(leak), false, `credential material reached the git argv: ${leak}`); + } +}); + +test("a fresh mirror reports the exact base commit a worktree would branch from", () => { + const spec = parseRepoSpec("octocat/hello-world"); + const tip = "a".repeat(40); + const run: Runner = (_cmd, args) => { + if (args.includes("rev-parse")) return OK(tip + "\n"); + return OK("https://github.com/octocat/hello-world.git\n"); + }; + const result = refreshMirror(spec, "/mirror", run, { exists: true }); + assert.equal(result.freshness.state, "fresh"); + assert.equal(result.freshness.remoteTip, tip); +});