From 0b9c55036afec99ef75ddb8939cc0de42d5f21f6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 00:54:12 -0800 Subject: [PATCH 1/3] chore(quest): claim quest/m0/worktree-ignored-scratch.md Quest-Claim: a4dc2fb0b35a7163 From ff73e1d56f2cd4fbfebf5dcee5baf2253bc6c968 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 02:00:58 -0800 Subject: [PATCH 2/3] fix(worktree): remove ignored build output and land-merged heads without --force Co-Authored-By: Claude Opus 5.5 --- README.md | 12 ++-- quest/m0/worktree-ignored-scratch.md | 40 ----------- src/cli/github.ts | 22 ++++-- src/cli/main.ts | 10 +-- src/cli/worktree.ts | 44 +++++------- test/worktree.test.ts | 104 +++++++++++++++++++-------- 6 files changed, 119 insertions(+), 113 deletions(-) delete mode 100644 quest/m0/worktree-ignored-scratch.md diff --git a/README.md b/README.md index 951ea142..e7053b71 100644 --- a/README.md +++ b/README.md @@ -1616,7 +1616,7 @@ supervisors left in their scopes, waits until they are empty, and so releases the checkout; it leaves runs whose supervisor is alive to that supervisor. Release deletes only declared scratch, and refuses a path that Git does not -ignore, that holds tracked files or another Git worktree, or that is reached +ignore, that holds tracked files or another repository or worktree, or that is reached through a symbolic link; links at or inside scratch are removed, never followed. Missing paths are skipped, so release can be rerun. The checkout, its Git state, and untracked files stay in place, and `status` charges what remains. A failed @@ -1643,16 +1643,18 @@ modified or untracked files and any commit, at `HEAD` or under the checkout's own refs such as `refs/bisect/`, that no branch or remote-tracking ref reaches. A squash-merged change leaves its commits unreached, so they count as preserved only when the GitHub upstream that holds origin's claims has a merged -PR whose head is exactly that commit; other commits need no forge or token. -It then prunes the scratch and refuses ignored files that remain, leaving the -checkout released, before `git worktree remove` deletes it. A checkout already +PR whose head is that commit or descends from it, as when `quest land` merged +the base into the head; other commits need no forge or token. Every ignored +path is build output that removal deletes, declared as scratch or not, except +one holding a nested repository or worktree, which it refuses. Only then does +`git worktree remove` delete the checkout. A checkout already removed with `git worktree remove` only drops its record, as with `release`. New checkouts initialize their submodules recursively before returning. Restoring one preserves its existing submodule pins and local work. Creation verifies any new module tips not reached by remote-tracking branches (such as SHA-fetched pins or cloned tags) against origin and records them under `refs/remotes/quest-pins/`, outside origin's branch-prune namespace. Removal also checks -submodules' files outside declared scratch and every module store, including deinitialized ones; +submodules' files and every module store, including deinitialized ones; commits at their HEAD or refs must be reachable from a remote-tracking ref, since module stores and their local branches are deleted with the checkout, and a module store must have no linked worktrees of its own. diff --git a/quest/m0/worktree-ignored-scratch.md b/quest/m0/worktree-ignored-scratch.md deleted file mode 100644 index 6b99a8b6..00000000 --- a/quest/m0/worktree-ignored-scratch.md +++ /dev/null @@ -1,40 +0,0 @@ -# [M] Finished checkouts remove without --force - -## Goal - -`quest worktree remove` deletes a finished agent checkout without `--force` -whenever everything left in it is git-ignored build output or commits that -merged changes already hold. It still refuses anything that could be work: -modified or untracked files, commits nothing reaches or merged, nested -repositories, and submodule worktrees. - -## Plan - -Planned 2026-11-10 (maintainer, /quest-plan in moq-dev/moq.pro). Milestone: -✅ m0 (rejected: m1, drop). - -A /quest-spawn run in moq-dev/moq.pro left 40 finished agent checkouts, and -`quest worktree remove` refused 38 of them, so the maintainer had to run a -`--force` pass. Two different causes: - -- **Ignored build output (30 checkouts).** The scratch is fixed at creation by - `derivedDefaults`, from the main checkout's top-level ignored entries, which - there are only `.direnv` and `.scratch`. Agents' `node_modules`, `target`, - `.terraform`, `app/.svelte-kit`, gate stamps and `.tmp/` are all ignored but - not scratch, so they block removal. - Decision: ✅ at remove time, treat every git-ignored path as scratch, except - nested repositories and linked worktrees, which keep their own checks - (rejected: a per-repository scratch list, which every repository must keep - current; deriving from the checkout's own top-level entries at release, - which still misses deeper paths). The nested-repository rule is what - saved real work in that run: an uncommitted regression test sat in a - submodule worktree under `.tmp/`. -- **Merged heads refused (7 checkouts).** Each refused commit was exactly the - head of a PR that had merged through `quest land`, such as - [moq-dev/moq.pro#3771](https://github.com/moq-dev/moq.pro/pull/3771) at - `6bcb980f0`, yet `remove` reported "no merged change has as its head". Find - why `merged` missed them (a landed squash, the forge query, or the claims - lookup) and fix it, with a test on a checkout whose only unreached commit is - a merged PR's head. - -Public API: none. Wire: none. diff --git a/src/cli/github.ts b/src/cli/github.ts index 81224465..cfe4b89f 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -555,10 +555,10 @@ export async function requirements(github: GitHub, repository: Repository, branc } /** - * Whether a merged PR in `repository` had exactly `commit` as its head, as when it was squash-merged; - * false for a commit GitHub does not have. + * Whether a merged PR in `repository` had `commit` as its head or below it, as when `quest land` merged + * the base into the head before squashing; false for a commit GitHub does not have. */ -export async function mergedHead(github: GitHub, repository: Repository, commit: string): Promise { +export async function mergedHolds(github: GitHub, repository: Repository, commit: string): Promise { const data = await github.graphql( "query($owner: String!, $name: String!, $oid: GitObjectID!) { repository(owner: $owner, name: $name) { " + "object(oid: $oid) { ... on Commit { associatedPullRequests(first: 100) { nodes { merged headRefOid } } } } } }", @@ -567,7 +567,21 @@ export async function mergedHead(github: GitHub, repository: Repository, commit: const found = object(data.repository).object; const pulls = found === null || found === undefined ? undefined : object(found).associatedPullRequests; if (pulls === null || pulls === undefined) return false; // Unknown, or not a commit. - return array(object(pulls).nodes).some((pull) => pull.merged === true && pull.headRefOid === commit); + for (const pull of array(object(pulls).nodes)) { + if (pull.merged !== true) continue; + const head = string(pull.headRefOid); + if (head === commit) return true; + // Association alone is no proof the merged head still reaches the commit, such as after a force push. + // Comparing from the head leaves no commits or files to send when it does. + const response = await github.request( + `/repos/${encodeURIComponent(repository.owner)}/${encodeURIComponent(repository.repo)}/compare/${head}...${commit}`, + { + allow: [404], + }, + ); + if (response.ok && Number(object(await response.json()).ahead_by) === 0) return true; + } + return false; } /** A login, a committer name without spaces, or `-` for a deleted account. */ diff --git a/src/cli/main.ts b/src/cli/main.ts index 95cd490d..068a3087 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -28,7 +28,7 @@ import { gate, QUICK_MS } from "./gate"; import { gitHubDependencies, GitHubInputError, - mergedHead, + mergedHolds, pullRequest, readRemote, remoteRepository, @@ -905,15 +905,15 @@ export function main(argv: string[], out: Output): number | Promise { const origin = readRemote(repository, "origin"); if (origin === null) return false; // No forge to ask. const up = await upstream(repository, origin, { github: () => gitHubDependencies(warn), warn }); - return up.github !== null && mergedHead(up.github.client, up.github.repository, commit); + return up.github !== null && mergedHolds(up.github.client, up.github.repository, commit); }; worktree .command("remove ") .description( "Delete a finished checkout and drop its record after recursive safety checks. Refuses while supervised runs are live, " + - "and refuses modified, untracked, or ignored files beyond the declared scratch, and commits no branch " + - "or remote-tracking ref reaches, unless the GitHub upstream that holds origin's claims merged a PR whose head is exactly " + - "that commit. Submodules must have no changed or ignored files and no commits their remote refs do not reach, including in deinitialized module stores, and module stores must have no linked worktrees of their own. Prunes the scratch first, so a refusal over ignored files leaves the checkout released.", + "and refuses modified or untracked files, an ignored path holding a nested repository or worktree, and commits no branch " + + "or remote-tracking ref reaches, unless the GitHub upstream that holds origin's claims merged a PR whose head is " + + "that commit or descends from it. Every other ignored path, declared as scratch or not, is deleted with the checkout. Submodules must have no changed files and no commits their remote refs do not reach, including in deinitialized module stores, and module stores must have no linked worktrees of their own.", ) .action((id: string) => settled(state().remove(id, merged))); worktree diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index c8e5c532..c3d1c4c1 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -1,5 +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. +// once the last supervised run in the checkout stops. Remove deletes a checkout that holds no unpreserved work, +// counting every ignored path as scratch. import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; import { @@ -153,17 +154,6 @@ function exclude(repository: string): void { writeFileSync(file, `${old}${old && !old.endsWith("\n") ? "\n" : ""}/.worktrees/\n`); } -/** The first linked worktree of any repository, marked by a `.git` file, at or under `path`; symlinks are never followed. */ -function linkedWorktree(path: string): string | undefined { - if (!entry(path)?.isDirectory()) return undefined; - if (entry(join(path, ".git"))?.isFile()) return path; - for (const child of readdirSync(path)) { - const found = child === ".git" ? undefined : linkedWorktree(join(path, child)); - if (found) return found; - } - return undefined; -} - /** * Git's lock reason for the checkout actually at `path`, read through its own `.git` file and * administrative directory, so a root replaced by another checkout reports that checkout's lock. @@ -307,7 +297,7 @@ function unreached(root: string): string[] { 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`. */ +/** Whether the forge merged a change, such as a squash-merged PR, whose head is or descends from `commit`. */ export type Merged = (repository: string, commit: string) => Promise; /** @@ -731,7 +721,7 @@ export class Worktrees { throw new Error(`scratch includes tracked files: ${tracked.slice(0, 20).join(", ")}`); if (!ignored(owner, [local]).has(local)) throw new Error(`scratch must be ignored by Git: ${path}`); } - // Another checkout, such as one created inside this one, is work rather than scratch. + // Another repository or checkout, such as one created inside this one, is work rather than scratch. const registered = checkouts(record.repository) .map((checkout) => checkout.path) .filter((path) => path !== root); @@ -741,8 +731,8 @@ export class Worktrees { const target = canonical(join(root, path)); const nested = registered.find((checkout) => checkout === target || checkout.startsWith(target + sep)) ?? - linkedWorktree(join(root, path)); - if (nested) throw new Error(`scratch contains a Git worktree: ${path} holds ${nested}`); + repositoryWithin(join(root, path)); + if (nested) throw new Error(`scratch contains a Git repository or worktree: ${path} holds ${nested}`); } // Removal unlinks symlinks rather than following them; force skips paths an overlapping parent removed. for (const path of paths) rmSync(join(root, path), { recursive: true, force: true }); @@ -764,9 +754,9 @@ export class Worktrees { } /** * Deletes a finished checkout and drops its record after checking files and commits recursively. Refuses while supervised runs - * are live, and refuses a checkout holding modified, untracked, or ignored files beyond its - * declared scratch, or a commit no branch or remote-tracking ref reaches. Such a commit counts as - * preserved only when `merged` reports a merged change whose head is exactly that commit. + * are live, and refuses a checkout holding modified or untracked files, an ignored path holding a + * repository or worktree, or a commit no branch or remote-tracking ref reaches. Such a commit counts as + * preserved only when `merged` reports a merged change that holds it. Every other ignored path is scratch. */ async remove(id: string, merged: Merged = async () => false): Promise { // The forge is asked outside the lock; the locked pass below inspects the checkout again. @@ -795,16 +785,14 @@ export class Worktrees { const kept = unreached(root).filter((commit) => !landed.has(commit)); if (kept.length) throw new Error( - `commits that no branch or remote-tracking ref reaches, and no merged change has as its head: ${kept.join(", ")}`, - ); - this.prune(registry, record); - const left = [root, ...modules].flatMap((repository) => - changes(repository, true).map((change) => `${repository}: ${change}`), - ); - if (left.length) - throw new Error( - `ignored files outside the declared scratch remain, so the checkout was only released: ${left.slice(0, 20).join(", ")}`, + `commits that no branch or remote-tracking ref reaches, and no merged change holds: ${kept.join(", ")}`, ); + // Ignored paths are build output, except a nested repository or worktree, whose commits and files are work. + for (const repository of [root, ...modules]) + for (const change of changes(repository, true)) { + const nested = repositoryWithin(join(repository, change.slice(3))); + if (nested) throw new Error(`ignored path holds a Git repository or worktree: ${nested}`); + } const reason = `quest worktree ${record.id}`; git(record.repository, ["worktree", "unlock", record.path]); try { diff --git a/test/worktree.test.ts b/test/worktree.test.ts index fac9a704..e95d0b5a 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -18,7 +18,7 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, describe, expect, it, vi } from "vitest"; import { type Owner, Retained, type Scopes, currentOwner, kill, killable } from "../src/cli/supervise"; -import { GitHub, mergedHead } from "../src/cli/github"; +import { GitHub, mergedHolds } from "../src/cli/github"; import { REFERENCE_LINE } from "../src/core"; import { main } from "../src/cli/main"; import { Worktrees, canonical, derivedDefaults, diskUsage } from "../src/cli/worktree"; @@ -231,11 +231,13 @@ describe("worktrees", { timeout: 20_000 }, () => { const record = await worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); mkdirSync(join(record.path, "cache")); writeFileSync(join(record.path, "cache", "artifact"), "disposable"); + // Ignored build output nobody declared is scratch too. + writeFileSync(join(record.path, ".env"), "disposable"); writeFileSync(join(record.path, "file"), "pushed\n"); git(record.path, "commit", "-am", "pushed"); const head = git(record.path, "rev-parse", "HEAD"); await expect(worktrees.remove(record.id, forge)).rejects.toThrow( - `remote-tracking ref reaches, and no merged change has as its head: ${head}`, + `remote-tracking ref reaches, and no merged change holds: ${head}`, ); expect(forge.mock.calls).toEqual([[record.repository, head]]); git(record.path, "update-ref", "refs/remotes/origin/work", "HEAD"); @@ -260,7 +262,7 @@ describe("worktrees", { timeout: 20_000 }, () => { await worktrees.remove(gone.id); expect(worktrees.status().records).toEqual([]); }); - it("removes a squash-merged checkout only when a merged change's head is exactly its commit", async () => { + it("removes a squash-merged checkout only when a merged change holds its commit", async () => { const { repo, worktrees } = fixture(); const record = await worktrees.create(repo, "work", 10_000_000); git(record.path, "commit", "--allow-empty", "-m", "first"); @@ -299,20 +301,25 @@ describe("worktrees", { timeout: 20_000 }, () => { await expect(worktrees.remove(record.id)).rejects.toThrow(marked); git(record.path, "update-ref", "-d", "refs/bisect/bad"); kept(); + // A repository or another checkout under an ignored path is work, even nested deep in scratch. + const clone = join(record.path, "cache", "deep", "clone"); + mkdirSync(clone, { recursive: true }); + git(clone, "init", "-q"); + await expect(worktrees.remove(record.id)).rejects.toThrow( + `ignored path holds a Git repository or worktree: ${clone}`, + ); + expect(existsSync(join(clone, ".git"))).toBe(true); + rmSync(join(record.path, "cache"), { recursive: true }); + const agent = await worktrees.create(record.path, "agent", 10_000_000); + await expect(worktrees.remove(record.id)).rejects.toThrow( + `ignored path holds a Git repository or worktree: ${agent.path}`, + ); + await worktrees.remove(agent.id); + kept(); orphan(home, "orphan.scope"); live.add("orphan.scope"); await expect(worktrees.remove(record.id)).rejects.toThrow("supervised runs are live"); live.delete("orphan.scope"); - // Ignored files nobody declared as scratch stay; the declared scratch is pruned first. - mkdirSync(join(record.path, "cache")); - writeFileSync(join(record.path, ".env"), "secret"); - await expect(worktrees.remove(record.id)).rejects.toThrow( - "declared scratch remain, so the checkout was only released:", - ); - expect(existsSync(join(record.path, "cache"))).toBe(false); - expect(readFileSync(join(record.path, ".env"), "utf8")).toBe("secret"); - expect(worktrees.status().records[0].phase).toBe("released"); - rmSync(join(record.path, ".env")); // A clean submodule whose commits its remote has does not prevent removal. const sub = repository(join(root, "sub")); git(record.path, "-c", "protocol.file.allow=always", "submodule", "add", "--quiet", sub, "sub"); @@ -515,10 +522,15 @@ describe("worktrees", { timeout: 20_000 }, () => { writeFileSync(join(nested, "file"), "local work"); await expect(worktrees.remove(record.id)).rejects.toThrow("modified or untracked"); git(nested, "checkout", "--", "file"); - writeFileSync(join(nested, ".env"), "ignored work"); - await expect(worktrees.remove(record.id)).rejects.toThrow("ignored files"); - expect(readFileSync(join(nested, ".env"), "utf8")).toBe("ignored work"); - rmSync(join(nested, ".env")); + const clone = join(nested, "cache", "clone"); + mkdirSync(clone, { recursive: true }); + git(clone, "init", "-q"); + await expect(worktrees.remove(record.id)).rejects.toThrow( + `ignored path holds a Git repository or worktree: ${clone}`, + ); + rmSync(join(nested, "cache"), { recursive: true }); + // Ignored build output in a submodule is scratch. + writeFileSync(join(nested, ".env"), "build output"); git( nested, "-c", @@ -625,21 +637,31 @@ describe("worktrees", { timeout: 20_000 }, () => { expect(() => worktrees.release(record.id)).toThrow("ownership/lock changed"); expect(() => worktrees.restore(record.id)).toThrow("ownership/lock changed"); }); - it("refuses scratch that contains another Git worktree", async () => { + it("refuses scratch that contains another Git repository or worktree", async () => { const { root, repo, worktrees } = fixture(); const outer = await worktrees.create(repo, "outer", 10_000_000, "HEAD", [".worktrees"]); // Created from inside the outer checkout, so it lands in the outer checkout's ignored .worktrees/. const inner = await worktrees.create(outer.path, "inner", 10_000_000); expect(inner.path).toBe(join(outer.path, ".worktrees", "inner")); - expect(() => worktrees.release(outer.id)).toThrow("scratch contains a Git worktree"); + expect(() => worktrees.release(outer.id)).toThrow("scratch contains a Git repository or worktree"); expect(existsSync(join(inner.path, "file"))).toBe(true); // A linked worktree of an unrelated repository, such as an agent's checkout under an ignored directory. const other = await worktrees.create(repo, "other", 10_000_000, "HEAD", ["cache"]); const foreign = repository(join(root, "foreign")); git(foreign, "worktree", "add", "--detach", join(other.path, "cache", "agent")); - expect(() => worktrees.release(other.id)).toThrow("scratch contains a Git worktree: cache"); + expect(() => worktrees.release(other.id)).toThrow("scratch contains a Git repository or worktree: cache"); expect(existsSync(join(other.path, "cache", "agent", "file"))).toBe(true); - expect(worktrees.status().records.map((record) => record.phase)).toEqual(["active", "active", "active"]); + // A temporary clone is a repository whose commits and files are work too. + const cloned = await worktrees.create(repo, "cloned", 10_000_000, "HEAD", ["cache"]); + repository(join(cloned.path, "cache", "clone")); + expect(() => worktrees.release(cloned.id)).toThrow("scratch contains a Git repository or worktree: cache"); + expect(existsSync(join(cloned.path, "cache", "clone", "file"))).toBe(true); + expect(worktrees.status().records.map((record) => record.phase)).toEqual([ + "active", + "active", + "active", + "active", + ]); }); it.skipIf(process.platform === "win32")("refuses a root replaced by a link to another checkout", async () => { const { repo, worktrees } = fixture(); @@ -853,10 +875,21 @@ describe("supervised runs", { timeout: 20_000 }, () => { }); describe("merged PR heads on GitHub", () => { - /** GitHub answering the commit query with `found`, recording the variables it was sent. */ - function github(found: unknown) { + /** + * GitHub answering the commit query with `found` and each compare from a head with `ahead`, or 404 + * for a head it lacks, recording the variables and compares it was sent. + */ + function github(found: unknown, ahead: Record = {}) { const sent: unknown[] = []; - const fetch = (async (_url: unknown, init?: RequestInit) => { + const fetch = (async (url: unknown, init?: RequestInit) => { + const compare = /\/compare\/(.+)$/.exec(String(url))?.[1]; + if (compare) { + sent.push(compare); + const head = compare.split("...")[0]; + return head in ahead + ? Response.json({ ahead_by: ahead[head] }) + : Response.json({ message: "Not Found" }, { status: 404 }); + } sent.push(JSON.parse(String(init?.body)).variables); return Response.json({ data: { repository: { object: found } } }); }) as typeof globalThis.fetch; @@ -865,20 +898,29 @@ describe("merged PR heads on GitHub", () => { const repository = { owner: "example", repo: "project" }; const pulls = (...nodes: unknown[]) => ({ associatedPullRequests: { nodes } }); - it("accepts only a merged PR whose head is exactly the commit", async () => { + it("accepts a merged PR whose head is the commit or descends from it", async () => { const { sent, client } = github(pulls({ merged: true, headRefOid: "abc" })); - expect(await mergedHead(client, repository, "abc")).toBe(true); + expect(await mergedHolds(client, repository, "abc")).toBe(true); expect(sent).toEqual([{ owner: "example", name: "project", oid: "abc" }]); - // A PR that contains the commit below a later head, or was closed unmerged, is no evidence. - expect(await mergedHead(github(pulls({ merged: true, headRefOid: "later" })).client, repository, "abc")).toBe( + // `quest land` merged the base into the head before squashing it. + const landed = github(pulls({ merged: false, headRefOid: "open" }, { merged: true, headRefOid: "landed" }), { + open: 0, + landed: 0, + }); + expect(await mergedHolds(landed.client, repository, "abc")).toBe(true); + expect(landed.sent.slice(1)).toEqual(["landed...abc"]); + // The merged head no longer reaches the commit, as after a force push, or GitHub cannot compare them. + const forced = github(pulls({ merged: true, headRefOid: "later" }), { later: 1 }); + expect(await mergedHolds(forced.client, repository, "abc")).toBe(false); + expect(await mergedHolds(github(pulls({ merged: true, headRefOid: "later" })).client, repository, "abc")).toBe( false, ); - expect(await mergedHead(github(pulls({ merged: false, headRefOid: "abc" })).client, repository, "abc")).toBe( + expect(await mergedHolds(github(pulls({ merged: false, headRefOid: "abc" })).client, repository, "abc")).toBe( false, ); // GitHub does not have the commit, or the object is not a commit. - expect(await mergedHead(github(null).client, repository, "abc")).toBe(false); - expect(await mergedHead(github({}).client, repository, "abc")).toBe(false); + expect(await mergedHolds(github(null).client, repository, "abc")).toBe(false); + expect(await mergedHolds(github({}).client, repository, "abc")).toBe(false); }); }); From dfcdf5e8140cde8fa93b31d40b5356ab443cf9a2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 11 Nov 2026 02:47:57 -0800 Subject: [PATCH 3/3] fix(worktree): treat bare repositories under scratch as work Co-Authored-By: Claude Opus 5.5 --- src/cli/worktree.ts | 8 +++++++- test/worktree.test.ts | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index c3d1c4c1..3bd7b445 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -319,11 +319,17 @@ export function diskUsage(path: string, seen = new Set(), skip = new Set return bytes; } -/** The first repository or linked worktree, marked by a `.git` entry, at or under `path`; symlinks are never followed. */ +/** + * The first repository or linked worktree, marked by a `.git` entry, or bare repository, holding `HEAD`, + * `objects/`, and `refs/`, at or under `path`; symlinks are never followed. + */ function repositoryWithin(path: string): string | undefined { if (!entry(path)?.isDirectory()) return undefined; const children = readdirSync(path, { withFileTypes: true }); + const has = (name: string, directory: boolean) => + children.some((child) => child.name === name && (directory ? child.isDirectory() : child.isFile())); if (children.some((child) => child.name === ".git")) return path; + if (has("HEAD", false) && has("objects", true) && has("refs", true)) return path; for (const child of children) { const found = child.isDirectory() ? repositoryWithin(join(path, child.name)) : undefined; if (found) return found; diff --git a/test/worktree.test.ts b/test/worktree.test.ts index e95d0b5a..a811cc05 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -310,6 +310,13 @@ describe("worktrees", { timeout: 20_000 }, () => { ); expect(existsSync(join(clone, ".git"))).toBe(true); rmSync(join(record.path, "cache"), { recursive: true }); + const mirror = join(record.path, "cache", "mirror.git"); + mkdirSync(mirror, { recursive: true }); + git(mirror, "init", "-q", "--bare"); + await expect(worktrees.remove(record.id)).rejects.toThrow( + `ignored path holds a Git repository or worktree: ${mirror}`, + ); + rmSync(join(record.path, "cache"), { recursive: true }); const agent = await worktrees.create(record.path, "agent", 10_000_000); await expect(worktrees.remove(record.id)).rejects.toThrow( `ignored path holds a Git repository or worktree: ${agent.path}`,