From d25b053535a78f7b142f9277d8e2baeecec3664b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 22:58:39 -0800 Subject: [PATCH 1/3] chore(quest): claim quest/m1/git-env.md Quest-Claim: 27ba81576c70c2b5 From 4d09aaac8f3d0533e024c4a302366d8917e32442 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 23:05:55 -0800 Subject: [PATCH 2/3] fix: every Git call ignores an inherited repository location Move discoveryEnv() to src/cli/git.ts with one shared git() helper, fold the per-file helpers into it, and drop GIT_DIR and the other repository-local variables from every Git call on an explicit path and from the command `quest worktree run` and `quest agent` start, keeping -c config. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- src/cli/agent.ts | 17 ++----- src/cli/claims.ts | 26 ++-------- src/cli/git.ts | 48 ++++++++++++++++++ src/cli/github.ts | 11 +--- src/cli/land-apply.ts | 2 + src/cli/land-fix.ts | 2 + src/cli/landed.ts | 7 ++- src/cli/main.ts | 3 +- src/cli/move.ts | 39 ++++---------- src/cli/review.ts | 32 ++++++------ src/cli/setup.ts | 14 ++--- src/cli/start.ts | 32 +++++------- src/cli/tree.ts | 8 ++- src/cli/worktree-hook.ts | 29 ++++------- src/cli/worktree.ts | 104 ++++++++++++++------------------------ test/snapshot.test.ts | 30 ++++++++++- test/worktree.run.test.ts | 25 +++++++-- 18 files changed, 225 insertions(+), 208 deletions(-) create mode 100644 src/cli/git.ts diff --git a/README.md b/README.md index b5a59f7c..784fd193 100644 --- a/README.md +++ b/README.md @@ -1318,7 +1318,9 @@ Creation prints JSON with the ID and the detached, locked checkout path under `.worktrees/`, and records the `--scratch` paths. Names are unique per repository; where several repositories use one, the other commands take the ID. `run` restores a released checkout, runs the command there, and once it exits kills any descendants it -left behind. When no other run remains in the checkout, it releases it: the +left behind. The command gets Quest's environment without Git's +repository-location variables, such as `GIT_DIR` and `GIT_INDEX_FILE`, which a +Git hook would otherwise point at the repository it ran in; `-c` config is kept. When no other run remains in the checkout, it releases it: the declared scratch is deleted and the checkout keeps only its source. Put a whole job, such as an agent session or a script that installs, builds, and tests, in one run, since the next run starts without scratch. diff --git a/src/cli/agent.ts b/src/cli/agent.ts index 732dd0ea..b80fd3e2 100644 --- a/src/cli/agent.ts +++ b/src/cli/agent.ts @@ -6,13 +6,7 @@ // variables, and reads the agent's session, reply, and cost. Its own arguments // name the harness, worktree, key, model, effort, and permission mode, so a // permission classifier reading the call sees the whole launch. -import { - type ChildProcess, - type ChildProcessWithoutNullStreams, - execFileSync, - spawn, - spawnSync, -} from "node:child_process"; +import { type ChildProcess, type ChildProcessWithoutNullStreams, spawn, spawnSync } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; import { chmodSync, @@ -31,6 +25,7 @@ import { constants, tmpdir } from "node:os"; import { join } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; +import { discoveryEnv, git } from "./git"; import { alive, programCommand, programEnvironment, requireProgram, signalProcess } from "./program"; import { systemdScopes } from "./supervise"; import { Worktrees, save } from "./worktree"; @@ -291,10 +286,7 @@ const digest = (value: string) => createHash("sha256").update(value).digest("hex export function spawnDirectory(base: string, worktree: string): string { let common: string; try { - common = execFileSync("git", ["-C", worktree, "rev-parse", "--path-format=absolute", "--git-common-dir"], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }).trim(); + common = git(worktree, ["rev-parse", "--path-format=absolute", "--git-common-dir"]); } catch { throw new AgentInputError(`${worktree} is not inside a Git repository`); } @@ -677,7 +669,8 @@ export async function runAgent( }), ); }; - const env = programEnvironment(launchEnvironment(deps.env, harness.keyVariable, key)); + // The agent works in its own checkout, so a location inherited from a Git hook would name the wrong one. + const env = discoveryEnv(programEnvironment(launchEnvironment(deps.env, harness.keyVariable, key))); if (command === undefined) { supervised = checkouts!.run(checkout!, [program, ...args], { env, spawned: connect }); supervised.catch(reject); diff --git a/src/cli/claims.ts b/src/cli/claims.ts index 514dd0be..7ad64de8 100644 --- a/src/cli/claims.ts +++ b/src/cli/claims.ts @@ -2,9 +2,8 @@ // PR's head, or an open PR from a branch of that name in any repository. // Commands that must not disturb held work look here first, and `quest start` // records a claim the same way, judging readiness by the upstream tree. -import { execFileSync } from "node:child_process"; - import { handedBack, landed } from "../core"; +import { git } from "./git"; import { type GitHubDependencies, type Repository, @@ -99,7 +98,7 @@ export async function upstream(root: string, remote: Remote, deps: ClaimDependen ? self.repository : (await repositoryInfo(client, pushed)).repository; // The README where this checkout's tree sits, which may be below the top level. - const prefix = gitOutput(root, ["rev-parse", "--show-prefix"]); + const prefix = git(root, ["rev-parse", "--show-prefix"]); let up = self; while (up.parent !== null && (await has(client, up.parent, `${prefix}quest/README.md`))) up = await repositoryInfo(client, up.parent); @@ -154,7 +153,7 @@ export function remoteBranches(root: string, remote: string): Map { const refs = new Map(); - for (const line of gitOutput(root, ["ls-remote", remote, ...patterns]).split("\n")) { + for (const line of git(root, ["ls-remote", remote, ...patterns]).split("\n")) { if (line === "") continue; const [sha, ref] = line.split("\t"); refs.set(ref, sha); @@ -341,23 +340,8 @@ export function upstreamRefs(root: string, remote: string): UpstreamRefs { /** Make `sha` from `remote` available locally. */ export function ensure(root: string, remote: string, sha: string): void { try { - gitOutput(root, ["cat-file", "-e", `${sha}^{commit}`]); + git(root, ["cat-file", "-e", `${sha}^{commit}`]); } catch { - gitOutput(root, ["fetch", "--quiet", "--no-tags", "--no-write-fetch-head", remote, sha]); - } -} - -/** Run git in `root` and return its trimmed stdout; an unset config key is empty rather than an error. */ -export function gitOutput(root: string, args: string[]): string { - try { - return execFileSync("git", ["-C", root, ...args], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, - }).trim(); - } catch (error) { - if (args[0] === "config" && (error as { status?: number }).status === 1) return ""; - const stderr = String((error as { stderr?: unknown }).stderr ?? "").trim(); - throw new Error(`git ${args[0]} failed${stderr ? `: ${stderr}` : ""}`); + git(root, ["fetch", "--quiet", "--no-tags", "--no-write-fetch-head", remote, sha]); } } diff --git a/src/cli/git.ts b/src/cli/git.ts new file mode 100644 index 00000000..1d9fe536 --- /dev/null +++ b/src/cli/git.ts @@ -0,0 +1,48 @@ +// Git run on an explicit path. Quest may itself run from a Git hook or `git submodule foreach`, whose +// repository-local variables would otherwise point every such call at the repository that started it. +import { execFileSync, spawnSync } from "node:child_process"; + +let located: string[] | undefined; +/** + * `env` without the repository-local variables a Git hook or `git submodule foreach` exports, such as + * GIT_DIR, GIT_WORK_TREE and GIT_INDEX_FILE, which override discovery from a path. Git drops the same + * ones when it enters another repository, and keeps `-c` config. + */ +export function discoveryEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + located ??= execFileSync("git", ["rev-parse", "--local-env-vars"], { encoding: "utf8" }) + .split("\n") + .filter((name) => name && name !== "GIT_CONFIG_PARAMETERS" && name !== "GIT_CONFIG_COUNT"); + const result = { ...env }; + for (const name of located) delete result[name]; + return result; +} + +/** A Git command that exited unsuccessfully, with what it printed. */ +export class GitError extends Error { + constructor( + command: string, + readonly status: number | null, + readonly stdout: string, + readonly stderr: string, + ) { + super(`git ${command} failed${stderr.trim() ? `: ${stderr.trim()}` : ""}`); + } +} + +/** + * Git run in the repository at `root`, without prompting, returning its output without trailing newlines. + * A `config` lookup that matches nothing is empty rather than an error. + */ +export function git(root: string, args: string[], input?: string): string { + const result = spawnSync("git", ["-C", root, ...args], { + encoding: "utf8", + env: { ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0" }, + input, + maxBuffer: 1 << 30, + stdio: [input === undefined ? "ignore" : "pipe", "pipe", "pipe"], + }); + if (result.error) throw result.error; + if (result.status === 0) return result.stdout.replace(/\n+$/, ""); + if (result.status === 1 && args[0] === "config") return ""; + throw new GitError(args[0], result.status, result.stdout, result.stderr); +} diff --git a/src/cli/github.ts b/src/cli/github.ts index b76787b2..d2889432 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -1,6 +1,7 @@ // The GitHub client shared by the commands that talk to GitHub, outside the repository-independent core. import { execFileSync } from "node:child_process"; import { setTimeout as sleep } from "node:timers/promises"; +import { git } from "./git"; const API = "https://api.github.com"; const MAX_SLEEP = 2_147_483_647; @@ -149,16 +150,6 @@ function rewritten(root: string, name: string): { fetch: string; push: string } return { fetch: urls.fetch, push: urls.push ?? urls.fetch }; } -/** Run git in `root` for its output; a lookup that matches nothing, which exits 1, is empty. */ -function git(root: string, args: string[]): string { - try { - return execFileSync("git", ["-C", root, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); - } catch (error) { - if (args[0] === "config" && (error as { status?: number }).status === 1) return ""; - throw error; - } -} - /** Read `name` like `readRemote`, failing when there is no such remote. */ export function requireRemote(root: string, name: string): Remote { const remote = readRemote(root, name); diff --git a/src/cli/land-apply.ts b/src/cli/land-apply.ts index a1b746b6..4cc260f3 100644 --- a/src/cli/land-apply.ts +++ b/src/cli/land-apply.ts @@ -12,6 +12,7 @@ import { requireRemote, string, } from "./github"; +import { discoveryEnv } from "./git"; import { contains, git, gitEnvironment, tryGit } from "./land"; import { Refusal } from "./start"; @@ -180,6 +181,7 @@ function userGit(root: string, args: string[], input?: string): string { cwd: root, input, encoding: "utf8", + env: discoveryEnv(), }); if (result.error) throw result.error; if (result.status !== 0) throw new Error(`git ${args[0]} failed: ${result.stderr.trim()}`); diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index c16bfb2f..d5335ad4 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -4,6 +4,7 @@ import { spawn, spawnSync } from "node:child_process"; import { constants, closeSync, existsSync, fstatSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { basename, join, resolve } from "node:path"; import prompt from "../../assets/land-fix.md"; +import { discoveryEnv } from "./git"; import { programEnvironment, requireProgram } from "./program"; export class FixerAgentError extends Error { @@ -229,6 +230,7 @@ export async function fix( const head = spawnSync("git", ["rev-parse", "--verify", "--quiet", `refs/heads/${ref}^{commit}`], { cwd: root, encoding: "utf8", + env: discoveryEnv(), }).stdout.trim(); if (!/^[a-f0-9]{40,64}$/.test(head)) throw new Error(`fix needs a ${ref} branch`); return `- #${number}: branch ${ref} at ${head}`; diff --git a/src/cli/landed.ts b/src/cli/landed.ts index d122ae17..d2035456 100644 --- a/src/cli/landed.ts +++ b/src/cli/landed.ts @@ -6,6 +6,7 @@ import { spawnSync } from "node:child_process"; import { type Finding, branchQuest } from "../core"; +import { discoveryEnv } from "./git"; /** * A finding when `branch` (default: the current branch) is a quest's branch @@ -52,7 +53,11 @@ function currentBranch(root: string): string { } function git(root: string, args: string[]): { status: number | null; stdout: string; stderr: string } { - const result = spawnSync("git", ["-C", root, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + const result = spawnSync("git", ["-C", root, ...args], { + encoding: "utf8", + env: discoveryEnv(), + stdio: ["ignore", "pipe", "pipe"], + }); if (result.error) throw new Error(`running git: ${result.error.message}`); return result; } diff --git a/src/cli/main.ts b/src/cli/main.ts index 499d20a9..d2edafc0 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -756,7 +756,8 @@ export function main(argv: string[], out: Output): number | Promise { .command("run") .summary("Run a command in a checkout, then release it once every process stops") .description( - "Run a command in the checkout, restoring it first when released. The command and its descendants " + + "Run a command in the checkout, restoring it first when released, without the repository-location " + + "variables, such as GIT_DIR, that a Git hook exports. The command and its descendants " + "run in a new, delegated systemd user scope; once the command exits, descendants still in the scope " + "are killed. A `quest gate` inside it owns its command through a child cgroup of the scope. " + "When no other supervised run remains, the scratch declared at creation is pruned. Put the whole job, " + diff --git a/src/cli/move.ts b/src/cli/move.ts index ae02b0f2..52b74402 100644 --- a/src/cli/move.ts +++ b/src/cli/move.ts @@ -1,12 +1,12 @@ // `quest mv` and `quest rm`: git moves or deletes the files and stages the // change, and the core edits the links in the documents that remain. // `quest check --fix` makes `rm`'s link edit for quests the history deleted. -import { execFileSync } from "node:child_process"; import { mkdirSync, statSync, writeFileSync } from "node:fs"; import { join, posix, relative, resolve, sep } from "node:path"; import { check, formatFinding, isQuest, move, normalize, permanent, questBranch, relocate, remove } from "../core"; import { type ClaimDependencies, claims } from "./claims"; +import { GitError, git } from "./git"; import { checkout, exists, sources, under } from "./tree"; /** What a command changed, and what it left for the caller to look at. */ @@ -61,7 +61,7 @@ export async function moveQuest( const edited = move(docs, from, to); mkdirSync(join(root, posix.dirname(to)), { recursive: true }); - git(root, "mv", "--", from, to); + git(root, ["mv", "--", from, to]); write(root, edited.texts); const warnings = edited.missed.map( (link) => @@ -101,7 +101,7 @@ export function removeQuest(root: string, arg: string, recursive: boolean): Repo const gone = new Set(doomed.map((doc) => doc.path)); const edited = remove(docs, (path) => gone.has(path)); - git(root, "rm", "-q", ...(whole ? ["-r"] : []), "--", whole ? epic! : target); + git(root, ["rm", "-q", ...(whole ? ["-r"] : []), "--", whole ? epic! : target]); write(root, edited); return { changes: [...doomed.map((doc) => `D ${doc.path}`), ...modified(edited)], @@ -122,7 +122,7 @@ export function removeQuest(root: string, arg: string, recursive: boolean): Repo */ export function fixLinks(root: string): string[] { workTree(root, "quest check --fix reads the deleted quests from git history"); - const unmerged = git(root, "ls-files", "--unmerged", "-z", "--", "quest") + const unmerged = git(root, ["ls-files", "--unmerged", "-z", "--", "quest"]) .split("\0") .filter((line) => line !== "") .map((line) => line.slice(line.indexOf("\t") + 1)); @@ -133,8 +133,7 @@ export function fixLinks(root: string): string[] { if (heads.length === 0) return []; const deleted = new Set( // `--relative` names paths from `root`, as the tree's links do, when the tree is nested in its repository. - git( - root, + git(root, [ "log", "--relative", "--diff-filter=D", @@ -145,7 +144,7 @@ export function fixLinks(root: string): string[] { ...heads, "--", "quest", - ) + ]) .split("\0") .filter(isQuest), ); @@ -157,7 +156,7 @@ export function fixLinks(root: string): string[] { /** Whether `ref` names a commit, which an unborn `HEAD` or a missing `MERGE_HEAD` does not. */ function verified(root: string, ref: string): boolean { try { - git(root, "rev-parse", "--quiet", "--verify", `${ref}^{commit}`); + git(root, ["rev-parse", "--quiet", "--verify", `${ref}^{commit}`]); return true; } catch { return false; @@ -181,26 +180,17 @@ function treePath(root: string, arg: string): string { /** Fail outside a git work tree, saying `why` the command needs one. */ function workTree(root: string, why = "quest mv and rm stage their change with git") { try { - git(root, "rev-parse", "--is-inside-work-tree"); + git(root, ["rev-parse", "--is-inside-work-tree"]); } catch { throw new Error(`${root} is not in a git work tree; ${why}`); } } -function git(root: string, ...args: string[]): string { - try { - return execFileSync("git", ["-C", root, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); - } catch (error) { - const stderr = (error as { stderr?: string }).stderr?.trim(); - throw new Error(stderr || (error as Error).message); - } -} - /** Write and stage each edited document. */ function write(root: string, texts: Map) { if (texts.size === 0) return; for (const [path, text] of texts) writeFileSync(join(root, path), text); - git(root, "add", "--", ...texts.keys()); + git(root, ["add", "--", ...texts.keys()]); } function modified(texts: Map): string[] { @@ -217,17 +207,10 @@ function mentions(root: string, path: string): string[] { const pattern = `${path.replace(/[.[\]()*+?{}|^$\\]/g, "\\$&")}\\.*([^A-Za-z0-9_.-]|$)`; let out: string; try { - out = execFileSync( - "git", - ["-C", root, "grep", "--untracked", "--exclude-standard", "-n", "-I", "-E", "-e", pattern], - { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }, - ); + out = git(root, ["grep", "--untracked", "--exclude-standard", "-n", "-I", "-E", "-e", pattern]); } catch (error) { // git grep exits 1 when nothing matches. - if ((error as { status?: number }).status === 1) return []; + if (error instanceof GitError && error.status === 1) return []; throw error; } return out diff --git a/src/cli/review.ts b/src/cli/review.ts index af4eaff2..dea2c68e 100644 --- a/src/cli/review.ts +++ b/src/cli/review.ts @@ -11,7 +11,7 @@ import { mkdirSync, readFileSync } from "node:fs"; import { dirname, join, posix } from "node:path"; import { type HarnessName, LINE_LIMIT, boundedLines, strippedEnvironment } from "./agent"; -import { gitOutput } from "./claims"; +import { git } from "./git"; import { programEnvironment, requireProgram } from "./program"; import { save } from "./worktree"; @@ -654,7 +654,7 @@ export function baseInstructions(root: string, base: string, changed: string[]): for (let i = 1; i <= parts.length; i++) dirs.add(parts.slice(0, i).join("/")); } const candidates = [...dirs].sort().flatMap((dir) => INSTRUCTION_FILES.map((name) => posix.join(dir, name))); - const listed = gitOutput(root, ["ls-tree", "-z", base, "--", ...candidates]); + const listed = git(root, ["ls-tree", "-z", base, "--", ...candidates]); const modes = new Map(); for (const entry of listed.split("\0").filter((e) => e !== "")) { const [meta, path] = entry.split("\t"); @@ -678,10 +678,10 @@ export function baseInstructions(root: string, base: string, changed: string[]): return given; } -const show = (root: string, base: string, path: string) => gitOutput(root, ["show", `${base}:${path}`]); +const show = (root: string, base: string, path: string) => git(root, ["show", `${base}:${path}`]); function exists(root: string, base: string, path: string): boolean { - return gitOutput(root, ["ls-tree", "--name-only", base, "--", path]) !== ""; + return git(root, ["ls-tree", "--name-only", base, "--", path]) !== ""; } /** The system or developer instructions every turn carries. */ @@ -795,7 +795,7 @@ export function laterPrompt(round: LaterRound): string { /** `/` split at the longest remote name it starts with. */ function parseBase(root: string, base: string): { remote: string; branch: string } { - const remotes = gitOutput(root, ["remote"]).split("\n").filter(Boolean); + const remotes = git(root, ["remote"]).split("\n").filter(Boolean); const remote = remotes.filter((name) => base.startsWith(`${name}/`)).sort((a, b) => b.length - a.length)[0]; if (remote === undefined || base.length === remote.length + 1) { throw new ReviewInputError(`--base must be / with a configured remote: ${base}`); @@ -804,7 +804,7 @@ function parseBase(root: string, base: string): { remote: string; branch: string } function defaultBranch(root: string, remote: string): string { - const line = gitOutput(root, ["ls-remote", "--symref", remote, "HEAD"]).split("\n")[0] ?? ""; + const line = git(root, ["ls-remote", "--symref", remote, "HEAD"]).split("\n")[0] ?? ""; const match = /^ref: refs\/heads\/(\S+)\tHEAD$/.exec(line); if (!match) throw new Error(`could not read ${remote}'s default branch; pass --base`); return match[1]; @@ -834,13 +834,13 @@ function load(path: string): State | undefined { function sessionBranch(root: string, name: string | undefined): string { if (name !== undefined) { try { - return gitOutput(root, ["check-ref-format", "--branch", name]); + return git(root, ["check-ref-format", "--branch", name]); } catch { throw new ReviewInputError(`--branch is not a valid branch name: ${name}`); } } try { - return gitOutput(root, ["symbolic-ref", "--short", "HEAD"]); + return git(root, ["symbolic-ref", "--short", "HEAD"]); } catch { throw new ReviewInputError( "HEAD is detached; pass --branch to name the branch whose review session this continues", @@ -853,18 +853,18 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies): // Native Windows installs the reviewer CLIs as batch shims, which `run` cannot start without a shell. if (process.platform === "win32") throw new ReviewInputError("quest review does not support native Windows yet; use WSL"); - const root = gitOutput(options.root, ["rev-parse", "--show-toplevel"]); + const root = git(options.root, ["rev-parse", "--show-toplevel"]); const harness = options.harness ?? other(runningHarness(deps.env)); const reviewer = REVIEWERS[harness]; const branch = sessionBranch(root, options.branch); - if (gitOutput(root, ["status", "--porcelain"]) !== "") { + if (git(root, ["status", "--porcelain"]) !== "") { throw new ReviewInputError( "the checkout has uncommitted changes; commit them, since only the committed branch is reviewed", ); } - const statePath = sessionFile(gitOutput(root, ["rev-parse", "--path-format=absolute", "--git-common-dir"]), branch); + const statePath = sessionFile(git(root, ["rev-parse", "--path-format=absolute", "--git-common-dir"]), branch); const dir = dirname(statePath); mkdirSync(dir, { recursive: true }); const stored = load(statePath); @@ -885,17 +885,17 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies): const baseRef = options.base ?? `origin/${defaultBranch(root, "origin")}`; const { remote, branch: baseBranch } = parseBase(root, baseRef); const tracking = `refs/remotes/${remote}/${baseBranch}`; - gitOutput(root, ["fetch", "--quiet", "--no-tags", remote, `+refs/heads/${baseBranch}:${tracking}`]); - const baseSha = gitOutput(root, ["rev-parse", "--verify", `${tracking}^{commit}`]); - const head = gitOutput(root, ["rev-parse", "HEAD"]); + git(root, ["fetch", "--quiet", "--no-tags", remote, `+refs/heads/${baseBranch}:${tracking}`]); + const baseSha = git(root, ["rev-parse", "--verify", `${tracking}^{commit}`]); + const head = git(root, ["rev-parse", "HEAD"]); let mergeBase: string; try { - mergeBase = gitOutput(root, ["merge-base", baseSha, head]); + mergeBase = git(root, ["merge-base", baseSha, head]); } catch { throw new ReviewInputError(`HEAD shares no history with ${baseRef}`); } const change: Change = { base: baseRef, mergeBase, head }; - const changed = gitOutput(root, ["diff", "--name-only", "-z", `${mergeBase}..${head}`]) + const changed = git(root, ["diff", "--name-only", "-z", `${mergeBase}..${head}`]) .split("\0") .filter(Boolean); diff --git a/src/cli/setup.ts b/src/cli/setup.ts index 843e2217..c1838feb 100644 --- a/src/cli/setup.ts +++ b/src/cli/setup.ts @@ -34,7 +34,7 @@ import { planInit, renderSettings, } from "../core"; -import { gitOutput } from "./claims"; +import { git } from "./git"; import { DRIVER, OLD_DRIVERS } from "./resolve"; import { SKILLS } from "./skills"; @@ -100,10 +100,10 @@ export function heal(root: string): void { */ function register(root: string): void { try { - const current = gitOutput(root, ["config", "--get", "merge.quest.driver"]); + const current = git(root, ["config", "--get", "merge.quest.driver"]); if (current !== "" && !OLD_DRIVERS.includes(current)) return; - if (gitOutput(root, ["rev-parse", "--is-inside-work-tree"]) !== "true") return; - gitOutput(root, ["config", "--local", "merge.quest.driver", DRIVER]); + if (git(root, ["rev-parse", "--is-inside-work-tree"]) !== "true") return; + git(root, ["config", "--local", "merge.quest.driver", DRIVER]); } catch { // Not a repository, or its config is not writable. } @@ -112,9 +112,9 @@ function register(root: string): void { /** Remove the merge driver from the clone's config, inside a git work tree. */ function unregister(root: string): void { try { - if (gitOutput(root, ["rev-parse", "--is-inside-work-tree"]) !== "true") return; - if (gitOutput(root, ["config", "--local", "--get", "merge.quest.driver"]) === "") return; - gitOutput(root, ["config", "--local", "--remove-section", "merge.quest"]); + if (git(root, ["rev-parse", "--is-inside-work-tree"]) !== "true") return; + if (git(root, ["config", "--local", "--get", "merge.quest.driver"]) === "") return; + git(root, ["config", "--local", "--remove-section", "merge.quest"]); } catch { // Not a repository, or its config is not writable. } diff --git a/src/cli/start.ts b/src/cli/start.ts index 179e6c4c..e0da1f6b 100644 --- a/src/cli/start.ts +++ b/src/cli/start.ts @@ -1,7 +1,6 @@ // `quest start`: claim a ready quest by pushing an empty claim commit to its // branch and opening a draft PR from it right away, and release that claim // while nothing but the claim commit is on the branch. -import { execFileSync } from "node:child_process"; import { randomBytes } from "node:crypto"; import { questBranch } from "../core"; @@ -11,7 +10,6 @@ import { type Target, type Upstream, ensure, - gitOutput, held, mergedPull, openPullRequests, @@ -21,6 +19,7 @@ import { upstream, upstreamRefs, } from "./claims"; +import { GitError, git } from "./git"; import { object, requireRemote, string } from "./github"; /** Someone else holds the quest, or the claim cannot be released; distinct from a failure. */ @@ -58,7 +57,7 @@ export async function startQuest( if (claim) throw new Refusal(`${path} is held by ${describe(claim.branch, claim.pull, Date.now())}`); // A nonce keeps two claimers on the same base, identity, and second from building the same commit. const nonce = randomBytes(8).toString("hex"); - const sha = gitOutput(root, [ + const sha = git(root, [ "commit-tree", `${base}^{tree}`, "-p", @@ -109,7 +108,7 @@ async function obstructed( const patterns = parts.slice(1).map((_, i) => `refs/heads/${parts.slice(0, i + 1).join("/")}`); let listing: string; try { - listing = gitOutput(root, ["ls-remote", up.push, ...patterns, `refs/heads/${branch}/*`]); + listing = git(root, ["ls-remote", up.push, ...patterns, `refs/heads/${branch}/*`]); } catch { return null; } @@ -147,7 +146,7 @@ function checkout(root: string, branch: string, sha: string, warn: (message: str const ref = `refs/heads/${branch}`; let current = ""; try { - current = gitOutput(root, ["symbolic-ref", "-q", "HEAD"]); + current = git(root, ["symbolic-ref", "-q", "HEAD"]); } catch { // Detached HEAD. } @@ -162,7 +161,7 @@ function checkout(root: string, branch: string, sha: string, warn: (message: str `switch to it, merge the claim commit ${sha} into it, and resume that work before pushing`, ); } else if (!succeeds(root, ["switch", "--quiet", "-c", branch, sha])) { - gitOutput(root, ["update-ref", ref, sha, ""]); + git(root, ["update-ref", ref, sha, ""]); warn(`created ${branch} but could not switch to it; run git switch ${branch}`); } } @@ -243,11 +242,11 @@ export async function releaseQuest( if (!tip) throw new Error(`${remote} has no branch ${branch} to release`); ensure(root, up.push, tip); const { base } = upstreamRefs(root, up.git); - const parents = gitOutput(root, ["rev-list", "--parents", "-n", "1", tip]).split(" ").slice(1); + const parents = git(root, ["rev-list", "--parents", "-n", "1", tip]).split(" ").slice(1); // Only the claim itself: one empty commit straight on the default branch, not one following pushed work. const claim = parents.length === 1 && - gitOutput(root, ["rev-parse", `${tip}^{tree}`]) === gitOutput(root, ["rev-parse", `${parents[0]}^{tree}`]) && + git(root, ["rev-parse", `${tip}^{tree}`]) === git(root, ["rev-parse", `${parents[0]}^{tree}`]) && succeeds(root, ["merge-base", "--is-ancestor", parents[0], base]); if (!claim) { throw new Refusal( @@ -312,18 +311,15 @@ function deleteBranch(root: string, remote: string, branch: string, sha: string) */ function push(root: string, remote: string, args: string[], rejected: string): void { try { - execFileSync("git", ["-C", root, "push", "--porcelain", remote, ...args], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, - }); + git(root, ["push", "--porcelain", remote, ...args]); } catch (error) { - const { stdout, stderr } = error as { stdout?: string; stderr?: string }; + if (!(error instanceof GitError)) throw error; + const { stdout, stderr } = error; // --porcelain reports each ref on stdout: `!` and `[rejected]` for a ref that is not as expected. - if (/^!\t.*\t\[rejected\]/m.test(stdout ?? "")) throw new Refusal(rejected); + if (/^!\t.*\t\[rejected\]/m.test(stdout)) throw new Refusal(rejected); // stderr may hold only the remote's chatter, so add each refused ref's reason from stdout. - const refused = (stdout ?? "").split("\n").filter((line) => line.startsWith("!")); - const detail = [(stderr ?? "").trim(), ...refused].filter((line) => line !== "").join("\n"); + const refused = stdout.split("\n").filter((line) => line.startsWith("!")); + const detail = [stderr.trim(), ...refused].filter((line) => line !== "").join("\n"); throw new Error(`git push to ${remote} failed: ${detail}`); } } @@ -331,7 +327,7 @@ function push(root: string, remote: string, args: string[], rejected: string): v /** Whether a git command exits zero. */ function succeeds(root: string, args: string[]): boolean { try { - gitOutput(root, args); + git(root, args); return true; } catch { return false; diff --git a/src/cli/tree.ts b/src/cli/tree.ts index 6d1f1dd8..06c402d4 100644 --- a/src/cli/tree.ts +++ b/src/cli/tree.ts @@ -6,6 +6,7 @@ import { existsSync, readFileSync, readdirSync } from "node:fs"; import { join } from "node:path"; import { type Doc, type Source, comparePaths, isQuest, parse } from "../core"; +import { discoveryEnv } from "./git"; /** * `/` as a user would name it in an error: `./quest`, not the @@ -318,7 +319,12 @@ function gitmodules(repo: Repo, source: string[]): Map { function git(repo: Repo, args: string[], input = ""): Buffer { const where = "dir" in repo ? ["-C", repo.dir] : [`--git-dir=${repo.gitdir}`]; try { - return execFileSync("git", [...where, ...args], { input, maxBuffer: 1 << 30, stdio: "pipe" }); + return execFileSync("git", [...where, ...args], { + env: discoveryEnv(), + input, + maxBuffer: 1 << 30, + stdio: "pipe", + }); } catch (error) { const stderr = String((error as { stderr?: unknown }).stderr ?? "").trim(); throw new Error(`git ${args[0]} failed${stderr ? `: ${stderr}` : ""}`); diff --git a/src/cli/worktree-hook.ts b/src/cli/worktree-hook.ts index 81f4f95f..5bb70020 100644 --- a/src/cli/worktree-hook.ts +++ b/src/cli/worktree-hook.ts @@ -1,9 +1,9 @@ // Claude Code's worktree lifecycle hooks, backed by `quest worktree`. `quest init` installs one // static entry per event, each calling `quest worktree hook`, so the logic upgrades with the binary. -import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import { join } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; +import { git } from "./git"; import { Busy, type Merged, @@ -11,7 +11,6 @@ import { Worktrees, canonical, derivedDefaults, - discoveryEnv, mainCheckout, } from "./worktree"; @@ -61,18 +60,10 @@ async function waiting(action: () => T | Promise): Promise { } } -function git(path: string, ...args: string[]): string { - return execFileSync("git", ["-C", path, ...args], { - encoding: "utf8", - env: discoveryEnv(), - stdio: ["ignore", "pipe", "pipe"], - }).trim(); -} - /** The top of the checkout holding `cwd`, or undefined outside any. */ function toplevel(cwd: string): string | undefined { try { - return canonical(git(cwd, "rev-parse", "--show-toplevel")); + return canonical(git(cwd, ["rev-parse", "--show-toplevel"])); } catch { return undefined; } @@ -84,9 +75,9 @@ function toplevel(cwd: string): string | undefined { */ function base(cwd: string): string { try { - return git(cwd, "rev-parse", "--verify", "--quiet", "refs/remotes/origin/HEAD^{commit}"); + return git(cwd, ["rev-parse", "--verify", "--quiet", "refs/remotes/origin/HEAD^{commit}"]); } catch { - return git(cwd, "rev-parse", "HEAD"); + return git(cwd, ["rev-parse", "HEAD"]); } } @@ -102,7 +93,7 @@ function pullRequest(cwd: string, name: string): string | undefined { const number = /^pr-([1-9][0-9]*)$/.exec(name)?.[1]; if (number === undefined) return undefined; try { - git(cwd, "remote", "get-url", "origin"); + git(cwd, ["remote", "get-url", "origin"]); } catch { return undefined; } @@ -111,7 +102,7 @@ function pullRequest(cwd: string, name: string): string | undefined { [`refs/merge-requests/${number}/head`, `refs/remotes/origin/mr/${number}`], ]); const heads = new Map(); - for (const line of git(cwd, "ls-remote", "origin", ...tracking.keys()).split("\n")) { + for (const line of git(cwd, ["ls-remote", "origin", ...tracking.keys()]).split("\n")) { const [sha, ref] = line.split("\t"); if (sha && ref && tracking.has(ref)) heads.set(ref, sha); } @@ -123,8 +114,8 @@ function pullRequest(cwd: string, name: string): string | undefined { const [ref] = heads.keys(); if (ref === undefined) return undefined; const local = tracking.get(ref)!; - git(cwd, "fetch", "--quiet", "--no-tags", "--no-write-fetch-head", "origin", `+${ref}:${local}`); - return git(cwd, "rev-parse", "--verify", "--quiet", `${local}^{commit}`); + git(cwd, ["fetch", "--quiet", "--no-tags", "--no-write-fetch-head", "origin", `+${ref}:${local}`]); + return git(cwd, ["rev-parse", "--verify", "--quiet", `${local}^{commit}`]); } /** Claude's name for a workflow agent's worktree, `-`, which carries no agent ID. */ @@ -142,7 +133,7 @@ async function subagentCheckout(worktrees: Worktrees, input: HookInput): Promise while (path && !record) { const checkout = path; record = await waiting(() => worktrees.registered(checkout)); - path = git(checkout, "rev-parse", "--show-superproject-working-tree") || undefined; + path = git(checkout, ["rev-parse", "--show-superproject-working-tree"]) || undefined; } const agent = field(input, "agent_id"); if (record?.name === `agent-${agent}`) return record; @@ -178,7 +169,7 @@ export async function claudeHook( const main = mainCheckout(project); let enclosing = cwd; while (mainCheckout(enclosing) !== main) { - const parent = git(enclosing, "rev-parse", "--show-superproject-working-tree"); + const parent = git(enclosing, ["rev-parse", "--show-superproject-working-tree"]); if (!parent) break; enclosing = parent; } diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index 38c52fc6..a28b7301 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -1,6 +1,6 @@ // Git checkouts under a shared disk budget. Release prunes declared scratch in place, by hand or // once the last supervised run in the checkout stops. Remove deletes a checkout that holds no unpreserved work. -import { execFile, execFileSync } from "node:child_process"; +import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; import { type Stats, @@ -22,6 +22,7 @@ import { homedir } from "node:os"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { promisify } from "node:util"; import { setTimeout as sleep } from "node:timers/promises"; +import { GitError, discoveryEnv, git } from "./git"; import { type Owner, Retained, type RunOptions, type Scopes, alive, currentOwner, systemdScopes } from "./supervise"; /** A supervised command, live while its owner runs or its scope holds a process. */ @@ -55,30 +56,8 @@ interface Registry { /** Another live process holds the registry lock. */ export class Busy extends Error {} -let located: string[] | undefined; -/** - * The environment for Git run on an explicit `-C` path. The repository-local variables a Git hook or - * `git submodule foreach` exports, such as GIT_DIR and GIT_WORK_TREE, override discovery from that - * path, so they are dropped as Git drops them when it runs in another repository, keeping `-c` config. - */ -export function discoveryEnv(): NodeJS.ProcessEnv { - located ??= execFileSync("git", ["rev-parse", "--local-env-vars"], { encoding: "utf8" }) - .split("\n") - .filter((name) => name && name !== "GIT_CONFIG_PARAMETERS" && name !== "GIT_CONFIG_COUNT"); - const env = { ...process.env }; - for (const name of located) delete env[name]; - return env; -} -function git(root: string, ...args: string[]): string { - return execFileSync("git", ["-C", root, ...args], { - encoding: "utf8", - env: discoveryEnv(), - maxBuffer: 64 * 1024 * 1024, - stdio: ["ignore", "pipe", "pipe"], - }); -} /** Git that may wait on the network, run without blocking concurrent hooks and commands. */ -function gitAsync(root: string, ...args: string[]): Promise { +function gitAsync(root: string, args: string[]): Promise { return promisify(execFile)("git", ["-C", root, ...args], { env: discoveryEnv(), maxBuffer: 64 * 1024 * 1024 }); } /** Write `value` as JSON, user-only, replacing `path` atomically. */ @@ -117,7 +96,7 @@ export function canonical(path: string): string { } /** Git's registered checkouts, with canonical paths and lock reasons. */ function checkouts(repository: string): { path: string; locked?: string }[] { - return git(repository, "worktree", "list", "--porcelain", "-z") + return git(repository, ["worktree", "list", "--porcelain", "-z"]) .split("\0\0") .flatMap((entry) => { const lines = entry.split("\0"); @@ -151,15 +130,9 @@ function present(root: string, path: string): boolean { /** The paths Git ignores; check-ignore exits 1 when it matches none. */ function ignored(root: string, paths: string[]): Set { try { - const output = execFileSync("git", ["-C", root, "check-ignore", "-z", "--stdin"], { - encoding: "utf8", - env: discoveryEnv(), - input: `${paths.join("\0")}\0`, - stdio: ["pipe", "pipe", "pipe"], - }); - return new Set(output.split("\0")); + return new Set(git(root, ["check-ignore", "-z", "--stdin"], `${paths.join("\0")}\0`).split("\0")); } catch (error) { - if ((error as { status?: number }).status === 1) return new Set(); + if (error instanceof GitError && error.status === 1) return new Set(); throw error; } } @@ -226,13 +199,11 @@ function changes(root: string, ignored: boolean): string[] { "--ignore-submodules=none", ...(ignored ? ["--ignored"] : []), ]; - return git(root, ...args) - .split("\0") - .filter(Boolean); + return git(root, args).split("\0").filter(Boolean); } /** Initialized submodule checkouts, including nested ones. NULs preserve spaces and newlines. */ function submodules(root: string): string[] { - return git(root, "submodule", "foreach", "--quiet", "--recursive", 'printf "%s\\0" "$toplevel/$sm_path"') + return git(root, ["submodule", "foreach", "--quiet", "--recursive", 'printf "%s\\0" "$toplevel/$sm_path"']) .split("\0") .filter(Boolean) .map(canonical); @@ -257,7 +228,7 @@ function clean(root: string): { modules: string[]; stores: Set } { const found = changes(root, false); if (found.length) throw new Error(`checkout has modified or untracked files: ${found.slice(0, 20).join(", ")}`); for (const repository of [root, ...modules]) { - for (const record of git(repository, "ls-files", "--stage", "-z").split("\0")) { + for (const record of git(repository, ["ls-files", "--stage", "-z"]).split("\0")) { if (!record.startsWith("160000 ")) continue; const path = join(repository, record.slice(record.indexOf("\t") + 1)); if (entry(path)?.isDirectory() && !existsSync(join(path, ".git")) && readdirSync(path).length) @@ -265,8 +236,8 @@ function clean(root: string): { modules: string[]; stores: Set } { } } const stores = new Set([ - ...moduleStores(git(root, "rev-parse", "--path-format=absolute", "--git-path", "modules").trim()), - ...modules.map((path) => git(path, "rev-parse", "--absolute-git-dir").trim()), + ...moduleStores(git(root, ["rev-parse", "--path-format=absolute", "--git-path", "modules"])), + ...modules.map((path) => git(path, ["rev-parse", "--absolute-git-dir"])), ]); for (const store of stores) { // Removal deletes the store, and with it the Git directories of the store's own linked worktrees. @@ -276,8 +247,7 @@ function clean(root: string): { modules: string[]; stores: Set } { `submodule ${store} has linked worktrees that removal would break; remove or prune them first: ${linked.join(", ")}`, ); // Local branches and tags in a module store disappear with the worktree. Only remote refs preserve them. - const commit = git( - root, + const commit = git(root, [ "--git-dir", store, "--work-tree", @@ -288,7 +258,7 @@ function clean(root: string): { modules: string[]; stores: Set } { "--all", "--not", "--remotes", - ).trim(); + ]); if (commit) throw new Error(`submodule ${store} has commits no remote-tracking ref reaches: ${commit}`); } return { modules, stores }; @@ -299,9 +269,9 @@ function clean(root: string): { modules: string[]; stores: Set } { * remote-tracking ref reaches. */ function unreached(root: string): string[] { - const own = git(root, "for-each-ref", "--format=%(objectname)", "refs/worktree", "refs/bisect", "refs/rewritten"); - const tips = new Set([git(root, "rev-parse", "HEAD").trim(), ...own.split("\n").filter(Boolean)]); - return [...tips].filter((tip) => git(root, "rev-list", "-n1", tip, "--not", "--branches", "--remotes").trim()); + const own = git(root, ["for-each-ref", "--format=%(objectname)", "refs/worktree", "refs/bisect", "refs/rewritten"]); + const tips = new Set([git(root, ["rev-parse", "HEAD"]), ...own.split("\n").filter(Boolean)]); + return [...tips].filter((tip) => git(root, ["rev-list", "-n1", tip, "--not", "--branches", "--remotes"])); } /** Whether the forge merged a change, such as a squash-merged PR, whose head is exactly `commit`. */ @@ -340,14 +310,14 @@ function repositoryWithin(path: string): string | undefined { /** The repository's main checkout, which every linked worktree of it shares, from any path inside one. */ export function mainCheckout(path: string): string { - const [main] = git(path, "worktree", "list", "--porcelain", "-z").split("\0\0"); + const [main] = git(path, ["worktree", "list", "--porcelain", "-z"]).split("\0\0"); const lines = main?.split("\0") ?? []; const found = lines.find((line) => line.startsWith("worktree "))?.slice(9); if (!found || lines.includes("bare")) throw new Error(`repository has no main checkout: ${path}`); // Git lists an absorbed submodule's store as its main worktree; core.worktree // still names the actual checkout relative to that store. - if (canonical(found) === canonical(git(path, "rev-parse", "--path-format=absolute", "--git-common-dir").trim())) - return canonical(git(path, "--git-dir", found, "rev-parse", "--show-toplevel").trim()); + if (canonical(found) === canonical(git(path, ["rev-parse", "--path-format=absolute", "--git-common-dir"]))) + return canonical(git(path, ["--git-dir", found, "rev-parse", "--show-toplevel"])); return canonical(found); } @@ -366,7 +336,7 @@ export function derivedDefaults(main: string): { reserve: number; scratch: strin const scratch = names.filter((name) => matched.has(name) && !repositoryWithin(join(main, name))).sort(); const skip = new Set([join(main, ".git"), join(main, ".worktrees"), join(main, ".claude", "worktrees")]); // A new checkout clones its own module stores, about the size of the main checkout's. - const modules = git(main, "rev-parse", "--path-format=absolute", "--git-path", "modules").trim(); + const modules = git(main, ["rev-parse", "--path-format=absolute", "--git-path", "modules"]); return { reserve: Math.max(1, diskUsage(main, new Set(), skip) + diskUsage(modules)), scratch }; } @@ -594,7 +564,7 @@ export class Worktrees { if (!Number.isSafeInteger(reserve) || reserve <= 0) throw new Error("reserve must be a positive safe integer in bytes, including expected build growth"); const declared = scratch.map(childPath); - const repository = canonical(git(root, "rev-parse", "--show-toplevel").trim()); + const repository = canonical(git(root, ["rev-parse", "--show-toplevel"])); if (registry.records.some((item) => item.name === name && item.repository === repository)) throw new Error( `worktree name already exists in this repository; use restore or another name: ${name}`, @@ -617,8 +587,7 @@ export class Worktrees { registry.records.push(record); save(join(this.home, "registry.json"), registry); // Reservation survives a crash in git. try { - git( - repository, + git(repository, [ "worktree", "add", "--detach", @@ -627,7 +596,7 @@ export class Worktrees { `quest worktree ${record.id}`, path, ref, - ); + ]); return record; } catch (error) { record.failure = String(error); @@ -637,14 +606,14 @@ export class Worktrees { }); try { // Cloning can need the network; the reservation stays charged without holding the shared lock. - await gitAsync(record.path, "submodule", "update", "--init", "--recursive"); + await gitAsync(record.path, ["submodule", "update", "--init", "--recursive"]); for (const module of submodules(record.path)) { - const untracked = (ref: string) => git(module, "rev-list", "-n1", ref, "--not", "--remotes").trim(); + const untracked = (ref: string) => git(module, ["rev-list", "-n1", ref, "--not", "--remotes"]); const fetch = (refs: string[]) => - gitAsync(module, "fetch", "--no-tags", "--no-write-fetch-head", "origin", ...refs); + gitAsync(module, ["fetch", "--no-tags", "--no-write-fetch-head", "origin", ...refs]); // Named tag refs work even when the server refuses unadvertised SHA wants. // Keep preservation refs outside origin's branch-prune namespace. - const tags = git(module, "for-each-ref", "--format=%(refname)", "refs/tags") + const tags = git(module, ["for-each-ref", "--format=%(refname)", "refs/tags"]) .split("\n") .filter(Boolean) .filter(untracked); @@ -654,8 +623,8 @@ export class Worktrees { ); const tips = [ ...new Set([ - git(module, "rev-parse", "HEAD").trim(), - ...git(module, "for-each-ref", "--format=%(objectname)").split("\n").filter(Boolean), + git(module, ["rev-parse", "HEAD"]), + ...git(module, ["for-each-ref", "--format=%(objectname)"]).split("\n").filter(Boolean), ]), ].filter(untracked); // A pin update may have fetched only a SHA, without an advertised branch or tag. @@ -720,7 +689,7 @@ export class Worktrees { const owner = modules.find((module) => absolute.startsWith(module + sep)) ?? root; const local = relative(owner, absolute).split(sep).join("/"); // icase covers filesystem aliases of tracked paths on case-insensitive hosts. - const tracked = git(owner, "ls-files", "-z", "--", `:(literal,icase)${local}`) + const tracked = git(owner, ["ls-files", "-z", "--", `:(literal,icase)${local}`]) .split("\0") .filter(Boolean); if (tracked.length) @@ -802,20 +771,19 @@ export class Worktrees { `ignored files outside the declared scratch remain, so the checkout was only released: ${left.slice(0, 20).join(", ")}`, ); const reason = `quest worktree ${record.id}`; - git(record.repository, "worktree", "unlock", record.path); + git(record.repository, ["worktree", "unlock", record.path]); try { // Git requires --force for initialized submodules. The recursive checks above // cover their files and every module store, including deinitialized modules. - git( - record.repository, + git(record.repository, [ "worktree", "remove", ...(stores.size || modules.length ? ["--force"] : []), record.path, - ); + ]); } catch (error) { try { - git(record.repository, "worktree", "lock", "--reason", reason, record.path); + git(record.repository, ["worktree", "lock", "--reason", reason, record.path]); } catch (relock) { throw new Error(`${message(error)}; locking the checkout again failed: ${message(relock)}`); } @@ -889,7 +857,9 @@ export class Worktrees { record.runs.push(run); // Recorded before the scope exists, so a crash is reconciled. return { id: record.id, path: record.path }; }); - const result = await this.scopes.run(run.unit, record.path, command, options).then( + // The command runs in this checkout, where a location inherited from a Git hook names the wrong repository. + const env = discoveryEnv(options?.env); + const result = await this.scopes.run(run.unit, record.path, command, { ...options, env }).then( (code) => ({ code }), (error: unknown) => ({ error }), ); diff --git a/test/snapshot.test.ts b/test/snapshot.test.ts index 448b218f..f6c78d6c 100644 --- a/test/snapshot.test.ts +++ b/test/snapshot.test.ts @@ -2,7 +2,7 @@ import { execFileSync } from "node:child_process"; import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { basename, dirname, join } from "node:path"; -import { describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { checkout, snapshot } from "../src/cli/tree"; import { check, formatFinding } from "../src/core"; @@ -137,6 +137,10 @@ describe("committed submodule links", { timeout: 20_000 }, () => { }); describe("checkout submodule links", { timeout: 20_000 }, () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + test("reads a checked-out submodule from disk", () => { const { tree, module } = fixture(); writeFileSync(join(module, "dirty.md"), "# Local only\n"); @@ -180,6 +184,30 @@ describe("checkout submodule links", { timeout: 20_000 }, () => { expect(seen).toEqual([]); }); + test("ignores the repository location a Git hook exports", () => { + const { tree, module } = fixture(); + const nested = initRepository(); + writeFileSync(join(nested, "README.md"), "# Nested\n"); + commit(nested); + git(module, "-c", "protocol.file.allow=always", "submodule", "add", "-q", nested, "nested"); + commit(module); + git(module, "submodule", "deinit", "-q", "-f", "nested"); + tree.append( + "quest/m0/epic/one.md", + "\n[Nested](../../../dependency/nested/README.md) and [missing](../../../dependency/nested/missing.md).\n", + ); + commit(tree.path); + // A pre-commit hook in the superproject runs `quest check` with these set. + const store = join(tree.path, ".git"); + vi.stubEnv("GIT_DIR", store); + vi.stubEnv("GIT_WORK_TREE", tree.path); + vi.stubEnv("GIT_INDEX_FILE", join(store, "index")); + const result = run("--root", tree.path, "check"); + expect(result.stderr).toBe( + "quest: quest/m0/epic/one.md:11: link does not resolve: ../../../dependency/nested/missing.md\n", + ); + }); + test("finds a deinitialized submodule's git directory by its name", () => { const { tree } = fixture(); git(tree.path, "submodule", "deinit", "-q", "-f", "dependency"); diff --git a/test/worktree.run.test.ts b/test/worktree.run.test.ts index 9dae8ab6..bf4d7706 100644 --- a/test/worktree.run.test.ts +++ b/test/worktree.run.test.ts @@ -30,8 +30,8 @@ async function fixture() { return { dir, home, worktrees, record }; } -/** The CLI supervising `script` in the checkout, with its streams piped here. */ -function supervise(home: string, script: string, ...args: string[]) { +/** The CLI supervising `script` in the checkout, with `env` added to its own and its streams piped here. */ +function supervise(home: string, script: string, args: string[] = [], env: NodeJS.ProcessEnv = {}) { const child = spawn( "bun", [ @@ -47,7 +47,7 @@ function supervise(home: string, script: string, ...args: string[]) { "sh", ...args, ], - { env: { ...process.env, QUEST_WORKTREE_HOME: home }, stdio: ["ignore", "pipe", "pipe"] }, + { env: { ...process.env, ...env, QUEST_WORKTREE_HOME: home }, stdio: ["ignore", "pipe", "pipe"] }, ); let stdout = ""; let stderr = ""; @@ -83,8 +83,7 @@ describe.runIf(process.platform === "linux")("supervised runs under systemd", { const run = supervise( home, `sleep 1000 & echo $! > ${JSON.stringify(pidFile)}; printf '%s|' "$@"; pwd; echo ready; exit 7`, - "a $b", - "--flag", + ["a $b", "--flag"], ); const result = await run.exited; expect(result.code, result.stderr).toBe(7); @@ -94,6 +93,22 @@ describe.runIf(process.platform === "linux")("supervised runs under systemd", { expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); }); + it("runs the command in the checkout's repository when started from a Git hook, keeping -c config", async () => { + const { dir, home, record } = await fixture(); + const store = join(dir, "repo", ".git"); + // What a hook in the main checkout exports, run by `git -c quest.probe=kept commit`. + const hook = { + GIT_DIR: store, + GIT_WORK_TREE: join(dir, "repo"), + GIT_INDEX_FILE: join(store, "index"), + GIT_CONFIG_PARAMETERS: "'quest.probe'='kept'", + }; + const run = supervise(home, "git rev-parse --show-toplevel; git config quest.probe", [], hook); + const result = await run.exited; + expect(result.code, result.stderr).toBe(0); + expect(result.stdout).toBe(`${record.path}\nkept\n`); + }); + it("forwards cancellation, stops live descendants, then releases", async () => { const { dir, home, worktrees, record } = await fixture(); const pidFile = join(dir, "descendant"); From 915b521315c4e760dcc4b06d746f91669262243e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 3 Nov 2026 23:18:58 -0800 Subject: [PATCH 3/3] quest: finish m1/git-env Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 1 - quest/m1/git-env.md | 60 --------------------------------------------- 2 files changed, 61 deletions(-) delete mode 100644 quest/m1/git-env.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 2a897ce1..95d2526c 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -24,7 +24,6 @@ earlier "Later work" milestone became m2. - [Open kix.quest](/quest/m1/open.md) - the Worker moves to kix.quest and any owner may enroll - [Release with worktree hooks](/quest/m1/release.md) - tag a release that has `quest worktree hook`, so SETUP.md's release routes work -- [Git location variables](/quest/m1/git-env.md) - every Git call on an explicit path ignores a hook's inherited `GIT_DIR` and index - [Kix for any repository](/quest/m1/kix/README.md) - onboarding, read-only CI checks, contributor sandboxing, and the maintainer guide - [Quest log theme](/quest/m1/theme/README.md) - an MMO-style identity, copy, board, landing page, and CLI output for the demo and launch - [Launch material](/quest/m1/launch.md) - README, quickstart, demo, comparison, and launch-post drafts diff --git a/quest/m1/git-env.md b/quest/m1/git-env.md deleted file mode 100644 index 2337ca27..00000000 --- a/quest/m1/git-env.md +++ /dev/null @@ -1,60 +0,0 @@ -# [S] Every Git call ignores inherited repository location - -## Goal - -Every Git subprocess the CLI runs on an explicit path finds that path's -repository, even when `quest` runs from a Git hook or -`git submodule foreach`. They share one helper that drops the -repository-local variables (`GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE` -and the rest of `git rev-parse --local-env-vars`) and keeps `-c` config, -as Git itself does when it enters another repository. The command that -`quest worktree run` starts in a reserved checkout gets the same treatment. - -## Plan - -Decided 2026-11-03 (coordinator, /quest-complete follow-ups, under the -user's 2026-11-02 instruction to decide follow-ups from evidence): plan it, -[S], m1. No moq-dev repository runs `quest` from a Git hook (none has -lefthook, pre-commit, husky or a hooks directory), so only adopters with -such hooks reach it. - -Decided 2026-11-03 (user, /quest-complete, on -[moq-dev/quest#455](https://github.com/moq-dev/quest/pull/455)): - -- Should `quest worktree run` drop inherited Git location variables from the - user's command? Today it hands the command Quest's own environment, so a - command started from a hook in one checkout acts on that checkout's - repository, not the reserved one it runs in. - - ✅ Drop them, keeping `-c` config (recommended): the command runs in a - different checkout, where an inherited location names the wrong - repository - - Pass the environment through unchanged, as now - -Evidence, gathered 2026-11-03: - -- [moq-dev/quest#445](https://github.com/moq-dev/quest/pull/445) hit it in - the worktree hook: with an inherited `GIT_DIR` naming a submodule, the - hook created the checkout inside the submodule. It fixed only - `src/cli/worktree.ts` and `worktree-hook.ts`, through `discoveryEnv()`. -- A probe on 2026-11-03: a `pre-commit` hook gets `GIT_INDEX_FILE` in every - checkout, and `GIT_DIR` too in a linked worktree. A merge driver gets - neither in a main checkout. -- The other helpers pass the environment through: - `gitOutput` (`src/cli/claims.ts`), `git` in `src/cli/move.ts`, - `src/cli/github.ts`, `src/cli/tree.ts` and `src/cli/landed.ts`, - `spawnDirectory` (`src/cli/agent.ts`) and the call in `src/cli/start.ts`. - `tree.ts` runs Git in a submodule to resolve links into it, so a - `quest check` from a hook reads the superproject there. - -Work: - -- Move `discoveryEnv()` from `src/cli/worktree.ts` to a shared module. - Have every `git -C ` helper above use it. Fold the near-duplicate - `git` helpers into it where their error handling allows, rather than - patching each one. -- Leave `src/runner/` as it is: the runner process isn't started by Git. -- Test `quest check` with links into a submodule, run under an inherited - `GIT_DIR` and `GIT_INDEX_FILE`. -- Have `quest worktree run` drop the same variables from the user's - command, keeping `-c` config (`src/cli/supervise.ts:339` hands it Quest's - own environment today), and test it from a hook-like environment.