diff --git a/README.md b/README.md index 636e977b..fd8f2291 100644 --- a/README.md +++ b/README.md @@ -474,7 +474,10 @@ Work handed to another service manager, such as `systemd-run` or a container daemon, escapes the scope. If the supervisor itself is killed, its command keeps running and its scratch is kept; the next `quest worktree` command drops the run once its scope is empty and then releases the checkout. `status` lists each -run's scope, which `systemctl --user stop` ends. +run's scope, with the reason a run it cannot inspect is kept until an +inspection succeeds. `quest worktree stop example` kills what crashed +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 @@ -493,10 +496,22 @@ quest worktree restore example checkout's disk again; then reinstall or rebuild its scratch, for example with `just install`. -Quest never removes a checkout. Once one is finished, remove it with -`git worktree unlock` and `git worktree remove`, which refuses modified or -untracked files but deletes ignored ones; `release` then drops its record. A -failed creation stays charged until `release` drops it. Records live in +Once a checkout is finished, remove it and its record: + +```sh +quest worktree remove example +``` + +`remove` never forces. It refuses while supervised runs are live, and refuses +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 origin's GitHub repository (a fork's parent) 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 +removed with `git worktree remove` only drops its record, as with `release`. +A failed creation stays charged until `release` drops it. Records live in `$QUEST_WORKTREE_HOME`, or `$XDG_STATE_HOME/quest/worktrees` (default `~/.local/state/quest/worktrees`). The registry lock of a process that exited on this host is reclaimed; a live lock, or one held from another host, is never diff --git a/quest/m0/README.md b/quest/m0/README.md index 8387cfbe..de87782d 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -36,7 +36,6 @@ ownership-manifest design: ## Required - [Gate headroom](/quest/m0/gate-headroom.md) - gated commands start at once while the host has room -- [Worktree removal](/quest/m0/worktree.md) - remove finished checkouts without losing work - [Agent lifecycle integration](/quest/m0/worktree-agents.md) - reclaim completed and suspended checkouts through their harness - [Agent workflow helpers](/quest/m0/helpers/README.md) - CLI commands for CI waits, PR status, claims, quest moves, and serialized merges diff --git a/quest/m0/kix/runner.md b/quest/m0/kix/runner.md index 1039d501..d8108a5f 100644 --- a/quest/m0/kix/runner.md +++ b/quest/m0/kix/runner.md @@ -86,7 +86,6 @@ replacing the hosted Agent SDK runtime: decision inbox: each runner's online state, its accounts, and the preference-order control. Decided 2026-10-18 as a follow-up of the decision inbox (#144), folded here rather than its own quest. - -## Related - -- [Worktree removal](/quest/m0/worktree.md) - removes finished checkouts; reuse `quest worktree run` for supervision, scratch release, and disk admission +- Reuse `quest worktree` in the job adapter: `run` for supervision, scratch + release, and disk admission, `remove` once a job's checkout is finished, + and `stop` for a crashed supervisor's leftover processes. diff --git a/quest/m0/worktree-agents.md b/quest/m0/worktree-agents.md index 1144218a..8b3296fe 100644 --- a/quest/m0/worktree-agents.md +++ b/quest/m0/worktree-agents.md @@ -13,6 +13,10 @@ merge. for expensive phases. Remove the skill's physical-core agent-count and load average admission rules once command admission is integrated. Disk capacity gates checkout allocation, not the number of reasoning agents. +- Landed: `quest worktree run` supervises a job and releases its scratch, + `remove` deletes a finished checkout without losing work (a squash-merged + head counts only with exact PR-head evidence), and `stop` ends a crashed + supervisor's leftover processes. - A 2026-10-19 transcript audit counted 1,731 load-average polling commands in moq-dev/moq and moq-dev/moq.pro sessions since Oct 14, so the remaining agent-level load rules still cost tokens until this lands. @@ -39,5 +43,4 @@ merge. ## Related -- [Worktree removal](/quest/m0/worktree.md) - removes finished checkouts; `quest worktree run` already supervises jobs and releases scratch - [Own-PC runner](/quest/m0/kix/runner.md) - reuse lifecycle ownership in the hosted job adapter diff --git a/quest/m0/worktree.md b/quest/m0/worktree.md deleted file mode 100644 index bbe0857a..00000000 --- a/quest/m0/worktree.md +++ /dev/null @@ -1,49 +0,0 @@ -# [M] Remove finished quest worktree checkouts - -## Goal - -`quest worktree remove` deletes a finished checkout that `quest worktree` created -and drops its record, without losing work: it refuses while supervised runs are -live and refuses a checkout that still holds anything not already preserved. -`quest worktree stop` ends a crashed supervisor's leftover processes. - -## Plan - -- Landed in [kixelated/quest#127](https://github.com/kixelated/quest/pull/127): - disk capacity and reservations, `create`, `release` (delete declared scratch - in place), and `restore`. Landed with this quest's first slice: scratch - declared at `create`, and `run`, which supervises a command in a systemd user - scope per run, kills leftover descendants once the command exits, and prunes - scratch when the checkout's last run stops. Crashed supervisors are - reconciled by the next `quest worktree` command once their scope is empty, - and a dead process's registry lock is reclaimed. -- Decided (user, 2026-10-21): prune in place. Release never removes the - checkout or its Git state. Checkout-independent recovery is out of scope: - four review rounds of the checkpoint-and-remove design kept finding - data-loss edge cases. -- Decided 2026-10-21 (user): process ownership is a systemd user scope, on - Linux only; other hosts fail explicitly. Process groups and PID files cannot - prove detached descendants stopped after a supervisor crash. -- Remaining (maintainer, 2026-10-21, from - [kixelated/quest#127](https://github.com/kixelated/quest/pull/127)): - `quest worktree remove` for finished checkouts. Never force: refuse modified, - untracked, or undeclared ignored files, and commits no branch or remote ref - reaches. Treat a squash-merged branch as finished only with exact PR-head - evidence from the forge, behind an interface that keeps the core - forge-independent. -- Remaining (from this quest's first slice): `quest worktree stop ` ends a - crashed supervisor's leftover processes through its recorded scopes, without - calling `systemctl --user stop` directly. A stale inspection-failure reason - on a checkout is cleared by the next successful inspection, rather than - lingering until the next successful release. -- Out of scope: terminal Ctrl-C reaches the command twice (once from the - terminal, once forwarded by the supervisor). It is shared with `quest gate` - and planned separately in [Gate headroom](/quest/m0/gate-headroom.md). -- Measured 2026-10-21 on this repository: a fresh checkout is 2.9 MB, 394 MB - after installing dependencies and building, and 2.9 MB once `run` released - it. A suspended job keeps the same released footprint until it is restored. -- Native agent lifecycle adoption stays in the agent integration quest. - -## Related - -- [Agent lifecycle integration](/quest/m0/worktree-agents.md) - native agents and workflow adoption diff --git a/src/cli/github.ts b/src/cli/github.ts index 4cab269f..f8e39b70 100644 --- a/src/cli/github.ts +++ b/src/cli/github.ts @@ -223,6 +223,22 @@ export class GitHub { } } +/** + * 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. + */ +export async function mergedHead(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 } } } } } }", + { owner: repository.owner, name: repository.repo, oid: 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); +} + /** A login, a committer name without spaces, or `-` for a deleted account. */ export function actor(value: unknown): string { if (value === null || value === undefined) return "-"; diff --git a/src/cli/main.ts b/src/cli/main.ts index f07e6089..c908ae36 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -18,9 +18,9 @@ import { unblocked, } from "../core"; import { AgentInputError, HARNESSES, type Launch, processDependencies, runAgent, stopAgent, stopAll } from "./agent"; -import { held, questBranch, reach, target, upstreamHead } from "./claims"; +import { held, questBranch, reach, target, upstream, upstreamHead } from "./claims"; import { gate } from "./gate"; -import { gitHubDependencies, GitHubInputError, pullRequest } from "./github"; +import { gitHubDependencies, GitHubInputError, mergedHead, pullRequest, remoteUrl } from "./github"; import { landed } from "./landed"; import { type Report, moveQuest, removeQuest } from "./move"; import { init, uninstall } from "./setup"; @@ -378,7 +378,9 @@ export function main(argv: string[], out: Output): number | Promise { const worktree = program .command("worktree") - .description("Reserve disk for Git checkouts and release it by pruning their scratch in place"); + .description( + "Reserve disk for Git checkouts, release it by pruning their scratch in place, and remove finished ones", + ); const state = () => new Worktrees(); const record = (value: unknown) => print(JSON.stringify(value)); worktree @@ -413,7 +415,7 @@ export function main(argv: string[], out: Output): number | Promise { "When no other supervised run remains, the scratch declared at creation is pruned. Put the whole job, " + "such as an agent session or a script that installs, builds, and tests, in one run.\n\n" + "A supervisor that crashes leaves its run recorded; the next worktree command drops it once its scope " + - "is empty, and releases the checkout. Needs Linux 5.14+ with a systemd 254+ user manager and unified cgroup " + + "is empty, and releases the checkout; `quest worktree stop` ends what it left. Needs Linux 5.14+ with a systemd 254+ user manager and unified cgroup " + "v2, and fails on other hosts. Exits with the command's status; a release that fails is reported, leaves " + "the checkout reserved, and turns success into exit 1.", ) @@ -442,6 +444,45 @@ export function main(argv: string[], out: Output): number | Promise { "and drops the record of a removed checkout", ) .action((id: string) => record(state().release(id))); + const settled = (result: Promise) => { + pending = result.then( + (value) => { + record(value); + return 0; + }, + (error: unknown) => { + warn(error instanceof Error ? error.message : String(error)); + return 1; + }, + ); + }; + // Only a commit no local ref reaches asks the forge, so other removals need no token. + const merged = async (repository: string, commit: string) => { + try { + remoteUrl(repository, "origin"); + } catch { + 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); + }; + worktree + .command("remove ") + .description( + "Delete a finished checkout and drop its record, never forcing. 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 origin's GitHub repository merged a PR whose head is exactly " + + "that commit. Prunes the scratch first, so a refusal over ignored files leaves the checkout released.", + ) + .action((id: string) => settled(state().remove(id, merged))); + worktree + .command("stop ") + .description( + "Kill the processes crashed supervisors left in the checkout's scopes and wait until they are gone, then " + + "release the checkout once no run remains. Runs whose supervisor is alive are left to it and reported " + + "as a failure.", + ) + .action((id: string) => settled(state().stop(id))); worktree .command("restore ") .description("Reserve a released checkout's disk again before rebuilding its scratch") diff --git a/src/cli/supervise.ts b/src/cli/supervise.ts index 93875f5e..74116cec 100644 --- a/src/cli/supervise.ts +++ b/src/cli/supervise.ts @@ -50,6 +50,8 @@ export interface Scopes { check(): void; /** Whether any process may still run in the scope. */ live(unit: string): boolean; + /** Kill every process left in the scope and resolve once it is empty, including when it is already gone. */ + stop(unit: string): Promise; /** * Run a command in a new scope with inherited streams, then stop its remaining descendants. * Resolves with the command's exit status only once the scope is empty. Rejects with `Retained` @@ -143,19 +145,32 @@ export function kill(group: string): void { } } +/** The scope's cgroup directory, or undefined once systemd no longer knows the unit. */ +function scopeGroup(unit: string): string | undefined { + const group = execFileSync("systemctl", ["--user", "show", "--property=ControlGroup", "--value", unit], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + if (!group) return undefined; + if (!group.endsWith(`/${unit}`)) throw new Error(`unexpected cgroup for ${unit}: ${group}`); + return join("/sys/fs/cgroup", group); +} + export const systemdScopes: Scopes = { check() { if (process.platform !== "linux" || !killable()) throw new Error(UNSUPPORTED); }, live(unit) { - const group = execFileSync("systemctl", ["--user", "show", "--property=ControlGroup", "--value", unit], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }).trim(); - if (!group) return false; - if (!group.endsWith(`/${unit}`)) throw new Error(`unexpected cgroup for ${unit}: ${group}`); - return populated(join("/sys/fs/cgroup", group)); + const group = scopeGroup(unit); + return group !== undefined && populated(group); + }, + + async stop(unit) { + const group = scopeGroup(unit); + if (group === undefined) return; + kill(group); + await vacated(group); }, run(unit, cwd, command) { diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index 47214064..e5b8f06c 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -1,5 +1,5 @@ // 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. +// once the last supervised run in the checkout stops. Remove deletes a checkout that holds no unpreserved work. import { execFileSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { @@ -27,6 +27,8 @@ import { type Owner, Retained, type Scopes, alive, currentOwner, systemdScopes } export interface Run { unit: string; owner: Owner; + /** Why the run is kept although it may have stopped; cleared once its scope is inspected again. */ + failure?: string; } export interface WorktreeRecord { id: string; @@ -65,6 +67,9 @@ export function save(path: string, value: unknown): void { function code(error: unknown): string | undefined { return (error as NodeJS.ErrnoException).code; } +function message(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} /** The entry itself, without following a final symlink; undefined when it is missing. */ function entry(path: string): Stats | undefined { try { @@ -160,6 +165,38 @@ function lockReason(path: string): string | undefined { } } +function interrupted(record: WorktreeRecord): void { + if (record.phase === "creating") + throw new Error( + `creation was interrupted; inspect and remove the partial checkout with git worktree remove, then release again: ${record.path}`, + ); +} + +/** Git status entries, plus ignored paths (collapsed to their directories) when `ignored` is set. */ +function changes(root: string, ignored: boolean): string[] { + const args = ["status", "--porcelain=v1", "-z", "--untracked-files=normal", ...(ignored ? ["--ignored"] : [])]; + return git(root, ...args) + .split("\0") + .filter(Boolean); +} +function clean(root: string): void { + const found = changes(root, false); + if (found.length) throw new Error(`checkout has modified or untracked files: ${found.slice(0, 20).join(", ")}`); +} + +/** + * HEAD and the refs removal deletes with the checkout, such as `refs/bisect/*`, that no branch or + * 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()); +} + +/** Whether the forge merged a change, such as a squash-merged PR, whose head is exactly `commit`. */ +export type Merged = (repository: string, commit: string) => Promise; + /** Allocated blocks, including ignored build output; symlinks are never traversed and hard links count once. */ export function diskUsage(path: string, seen = new Set()): number { const stat = entry(path); @@ -291,12 +328,14 @@ export class Worktrees { return record; } /** Unknown counts as running, so scratch is never deleted under a process that may use it. */ - private running(record: WorktreeRecord, run: Run): boolean { + private running(run: Run): boolean { if (alive(run.owner)) return true; try { - return this.scopes.live(run.unit); + const live = this.scopes.live(run.unit); + delete run.failure; + return live; } catch (error) { - record.failure = `cannot inspect supervised run ${run.unit}: ${error instanceof Error ? error.message : error}`; + run.failure = `cannot inspect supervised run ${run.unit}: ${message(error)}`; return true; } } @@ -304,17 +343,23 @@ export class Worktrees { private reconcile(registry: Registry): void { for (const record of [...registry.records]) { if (!record.runs.length) continue; - record.runs = record.runs.filter((run) => this.running(record, run)); + record.runs = record.runs.filter((run) => this.running(run)); if (!record.runs.length) this.settle(registry, record); } } + private idle(record: WorktreeRecord): void { + if (record.runs.length) + throw new Error( + `supervised runs are live in this checkout: ${record.runs.map((run) => run.unit).join(", ")}`, + ); + } /** Releases a checkout whose last run stopped, keeping it charged and noting why on failure. */ private settle(registry: Registry, record: WorktreeRecord): void { try { this.prune(registry, record); delete record.failure; } catch (error) { - record.failure = `release failed; the checkout stays reserved: ${error instanceof Error ? error.message : error}`; + record.failure = `release failed; the checkout stays reserved: ${message(error)}`; } } /** @@ -432,10 +477,7 @@ export class Worktrees { registry.records.splice(registry.records.indexOf(record), 1); return; } - if (record.phase === "creating") - throw new Error( - `creation was interrupted; inspect and remove the partial checkout with git worktree remove, then release again: ${record.path}`, - ); + interrupted(record); const root = canonical(record.path); const paths = record.scratch.filter((path) => present(root, path)); // Reruns skip pruned paths. const admin = entry(join(root, ".git")); @@ -477,12 +519,88 @@ export class Worktrees { release(id: string): WorktreeRecord { return this.locked((registry) => { const record = this.find(registry, id); - if (record.runs.length) + this.idle(record); + this.prune(registry, record); + delete record.failure; + return record; + }); + } + /** + * Deletes a finished checkout and drops its record, never forcing. 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. + */ + async remove(id: string, merged: Merged = async () => false): Promise { + // The forge is asked outside the lock; the locked pass below inspects the checkout again. + const pending = this.locked((registry) => { + const record = this.find(registry, id); + this.idle(record); + if (!this.owned(record)) return []; + interrupted(record); + const root = canonical(record.path); + clean(root); + return unreached(root).map((commit) => ({ repository: record.repository, commit })); + }); + const landed = new Set(); + for (const { repository, commit } of pending) if (await merged(repository, commit)) landed.add(commit); + return this.locked((registry) => { + const record = this.find(registry, id); + this.idle(record); + const drop = () => registry.records.splice(registry.records.indexOf(record), 1); + if (!this.owned(record)) { + drop(); + return record; + } + interrupted(record); + const root = canonical(record.path); + clean(root); + const kept = unreached(root).filter((commit) => !landed.has(commit)); + if (kept.length) throw new Error( - `supervised runs are live in this checkout: ${record.runs.map((run) => run.unit).join(", ")}`, + `commits that no branch or remote-tracking ref reaches, and no merged change has as its head: ${kept.join(", ")}`, ); this.prune(registry, record); - delete record.failure; + const left = changes(root, true); + if (left.length) + throw new Error( + `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); + try { + git(record.repository, "worktree", "remove", record.path); + } catch (error) { + try { + git(record.repository, "worktree", "lock", "--reason", reason, record.path); + } catch (relock) { + throw new Error(`${message(error)}; locking the checkout again failed: ${message(relock)}`); + } + throw error; + } + drop(); + return record; + }); + } + /** + * Ends the processes a crashed supervisor left in this checkout's scopes; reconciliation then + * releases the checkout once no run remains. Runs whose supervisor is alive are left to it, and + * reported as a failure once the crashed ones are stopped. + */ + async stop(id: string): Promise { + const orphans = this.locked((registry) => + this.find(registry, id) + .runs.filter((run) => !alive(run.owner)) + .map((run) => run.unit), + ); + for (const unit of orphans) await this.scopes.stop(unit); + return this.lockedWait((registry) => { + const record = this.find(registry, id); + const supervised = record.runs.filter((run) => alive(run.owner)); + if (supervised.length) + throw new Error( + `runs whose supervisor is alive are left to it; signal it instead: ${supervised.map((run) => `${run.unit} (process ${run.owner.pid} on ${run.owner.host})`).join(", ")}`, + ); return record; }); } @@ -525,7 +643,9 @@ export class Worktrees { const current = registry.records.find((item) => item.id === record.id); if (!current) return undefined; // Reconciliation drops a retained run once this process exits and its scope is empty. - if ("error" in result && result.error instanceof Retained) current.failure = result.error.message; + const retained = current.runs.find((item) => item.unit === run.unit); + if (retained && "error" in result && result.error instanceof Retained) + retained.failure = result.error.message; else current.runs = current.runs.filter((item) => item.unit !== run.unit); // A command that never started, such as on a host without a user manager, keeps its scratch. if (!current.runs.length && "code" in result) this.settle(registry, current); diff --git a/test/worktree.run.test.ts b/test/worktree.run.test.ts index 5e8953fc..c3989938 100644 --- a/test/worktree.run.test.ts +++ b/test/worktree.run.test.ts @@ -118,10 +118,11 @@ describe.runIf(process.platform === "linux")("supervised runs under systemd", { expect(orphan).toBeDefined(); expect(() => worktrees.release(record.id)).toThrow("supervised runs are live"); expect(existsSync(join(record.path, "cache"))).toBe(true); - // Stopping a scope waits until it is empty. - const stop = spawnSync("systemctl", ["--user", "stop", orphan.unit], { encoding: "utf8" }); - expect(stop.status, stop.stderr).toBe(0); - expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); + // Stopping kills what the scope holds and waits until it is empty, which releases the checkout. + expect(await worktrees.stop(record.id)).toMatchObject({ phase: "released", runs: [] }); + expect( + spawnSync("systemctl", ["--user", "is-active", orphan.unit], { encoding: "utf8" }).stdout.trim(), + ).not.toBe("active"); expect(existsSync(join(record.path, "cache"))).toBe(false); }); }); diff --git a/test/worktree.test.ts b/test/worktree.test.ts index bedb486e..3f006896 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -15,7 +15,8 @@ import { import { hostname, tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { Retained, type Scopes, currentOwner, kill, killable } from "../src/cli/supervise"; +import { type Owner, Retained, type Scopes, currentOwner, kill, killable } from "../src/cli/supervise"; +import { GitHub, mergedHead } from "../src/cli/github"; import { main } from "../src/cli/main"; import { Worktrees, diskUsage } from "../src/cli/worktree"; import { run } from "./fixture"; @@ -36,17 +37,28 @@ function repository(path: string): string { return path; } -/** Scopes whose runs finish when the test says so, and stay live while listed. */ +/** Scopes that are never live and whose runs exit at once, except where `overrides` says otherwise. */ +function stubScopes(overrides: Partial): Scopes { + return { check() {}, live: () => false, run: () => Promise.resolve(0), stop: async () => {}, ...overrides }; +} +/** Scopes whose runs finish when the test says so, and stay live while listed until stopped. */ function fakeScopes() { const live = new Set(); const finish = new Map void>(); - const scopes: Scopes = { - check() {}, + const scopes = stubScopes({ live: (unit) => live.has(unit), run: (unit) => new Promise((resolve) => finish.set(unit, resolve)), - }; + stop: async (unit) => void live.delete(unit), + }); return { live, finish, scopes }; } +/** Record a run whose supervisor crashed, as `quest worktree run` leaves it. */ +function orphan(home: string, unit: string, owner: Owner = deadOwner()) { + const file = join(home, "registry.json"); + const registry = JSON.parse(readFileSync(file, "utf8")); + registry.records[0].runs.push({ unit, owner }); + writeFileSync(file, JSON.stringify(registry)); +} function fixture(scopes?: Scopes) { const root = mkdtempSync(join(tmpdir(), "quest-worktree-test-")); roots.push(root); @@ -189,6 +201,103 @@ describe("worktrees", { timeout: 20_000 }, () => { expect(() => worktrees.restore(record.id)).toThrow("unknown worktree"); expect(worktrees.create(repo, "work", 10_000_000).phase).toBe("active"); }); + it("removes a finished checkout once a branch or remote-tracking ref reaches its commits", async () => { + const { repo, worktrees } = fixture(); + const forge = vi.fn(async () => false); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + mkdirSync(join(record.path, "cache")); + writeFileSync(join(record.path, "cache", "artifact"), "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}`, + ); + expect(forge.mock.calls).toEqual([[record.repository, head]]); + git(record.path, "update-ref", "refs/remotes/origin/work", "HEAD"); + forge.mockClear(); + expect((await worktrees.remove(record.id, forge)).id).toBe(record.id); + expect(forge).not.toHaveBeenCalled(); + expect(existsSync(record.path)).toBe(false); + expect(git(repo, "worktree", "list", "--porcelain")).not.toContain(".worktrees/work"); + expect(worktrees.status().records).toEqual([]); + await expect(worktrees.remove(record.id)).rejects.toThrow("unknown worktree"); + // A branch made in the checkout outlives it, so its commits are preserved. + const branched = worktrees.create(repo, "branched", 10_000_000); + git(branched.path, "switch", "-c", "topic"); + git(branched.path, "commit", "--allow-empty", "-m", "on a branch"); + const tip = git(branched.path, "rev-parse", "HEAD"); + await worktrees.remove(branched.id); + expect(git(repo, "rev-parse", "topic")).toBe(tip); + // A checkout already removed by hand only drops its record. + const gone = worktrees.create(repo, "gone", 10_000_000); + git(repo, "worktree", "unlock", gone.path); + git(repo, "worktree", "remove", gone.path); + 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 () => { + const { repo, worktrees } = fixture(); + const record = worktrees.create(repo, "work", 10_000_000); + git(record.path, "commit", "--allow-empty", "-m", "first"); + const first = git(record.path, "rev-parse", "HEAD"); + git(record.path, "commit", "--allow-empty", "-m", "second"); + const head = git(record.path, "rev-parse", "HEAD"); + // A merged change whose head is an ancestor leaves the later commit unpreserved. + await expect(worktrees.remove(record.id, async (_, commit) => commit === first)).rejects.toThrow(head); + expect(existsSync(record.path)).toBe(true); + await worktrees.remove( + record.id, + async (repository, commit) => repository === record.repository && commit === head, + ); + expect(existsSync(record.path)).toBe(false); + }); + it("refuses to remove a checkout holding work that is not preserved", async () => { + const { live, scopes } = fakeScopes(); + const { root, repo, home, worktrees } = fixture(scopes); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + const kept = () => { + expect(existsSync(join(record.path, "file"))).toBe(true); + expect(worktrees.status().records).toHaveLength(1); + }; + writeFileSync(join(record.path, "file"), "modified\n"); + await expect(worktrees.remove(record.id)).rejects.toThrow("modified or untracked files: M file"); + git(record.path, "checkout", "--", "file"); + writeFileSync(join(record.path, "new"), "untracked"); + await expect(worktrees.remove(record.id)).rejects.toThrow("modified or untracked files: ?? new"); + rmSync(join(record.path, "new")); + kept(); + // A ref that only this checkout holds, such as a bisect mark, is deleted with it. + git(record.path, "commit", "--allow-empty", "-m", "marked"); + const marked = git(record.path, "rev-parse", "HEAD"); + git(record.path, "update-ref", "refs/bisect/bad", "HEAD"); + git(record.path, "checkout", "--detach", "HEAD~1"); + await expect(worktrees.remove(record.id)).rejects.toThrow(marked); + git(record.path, "update-ref", "-d", "refs/bisect/bad"); + 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: !! .env", + ); + 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")); + // Git refuses a checkout with submodules; the checkout keeps its lock and record. + const sub = repository(join(root, "sub")); + git(record.path, "-c", "protocol.file.allow=always", "submodule", "add", "--quiet", sub, "sub"); + git(record.path, "commit", "-m", "submodule"); + git(record.path, "update-ref", "refs/remotes/origin/work", "HEAD"); + await expect(worktrees.remove(record.id)).rejects.toThrow("submodules"); + kept(); + expect(worktrees.release(record.id).phase).toBe("released"); + }); it("serializes reservations across independent repositories and preserves failed allocations", () => { const { root, repo, home, worktrees } = fixture(); const second = repository(join(root, "second")); @@ -275,7 +384,7 @@ describe("worktrees", { timeout: 20_000 }, () => { expect(() => worktrees.release(replaced.id)).toThrow("ownership/lock changed"); expect(existsSync(join(other.path, "cache"))).toBe(true); }); - it("wires capacity, reservation, and scratch through the command line", () => { + it("wires capacity, reservation, scratch, stop, and removal through the command line", async () => { const { repo, home } = fixture(); vi.stubEnv("QUEST_WORKTREE_HOME", home); expect(run("worktree", "create", "cli", "--root", repo).code).toBe(2); @@ -289,6 +398,19 @@ describe("worktrees", { timeout: 20_000 }, () => { const status = run("worktree", "status"); expect(status.code).toBe(0); expect(JSON.parse(status.stdout).records[0].phase).toBe("released"); + // The checkout's commit is on main, so removing it asks no forge, and the fixture has no origin. + let stdout = ""; + const out = { stdout: (text: string) => (stdout += text), stderr: () => {} }; + expect(await main(["worktree", "stop", "cli"], out)).toBe(0); + expect(await main(["worktree", "remove", "cli"], out)).toBe(0); + expect( + stdout + .trim() + .split("\n") + .map((line) => JSON.parse(line).path), + ).toEqual([path, path]); + expect(existsSync(path)).toBe(false); + expect(await main(["worktree", "remove", "cli"], out)).toBe(1); }); }); @@ -319,10 +441,7 @@ describe("supervised runs", { timeout: 20_000 }, () => { const { repo, home, worktrees } = fixture(scopes); const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); mkdirSync(join(record.path, "cache")); - const file = join(home, "registry.json"); - const registry = JSON.parse(readFileSync(file, "utf8")); - registry.records[0].runs = [{ unit: "orphan.scope", owner: deadOwner() }]; - writeFileSync(file, JSON.stringify(registry)); + orphan(home, "orphan.scope"); live.add("orphan.scope"); expect(worktrees.status().records[0].runs).toHaveLength(1); expect(() => worktrees.release(record.id)).toThrow("supervised runs are live"); @@ -331,26 +450,53 @@ describe("supervised runs", { timeout: 20_000 }, () => { expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); expect(existsSync(join(record.path, "cache"))).toBe(false); }); - it("treats a run it cannot inspect as live", () => { + it("treats a run it cannot inspect as live until an inspection succeeds", () => { const { repo, home } = fixture(); - const worktrees = new Worktrees(home, { - check() {}, - live: () => { - throw new Error("no user manager"); - }, - run: () => Promise.resolve(0), - }); + let inspect = (): boolean => { + throw new Error("no user manager"); + }; + const worktrees = new Worktrees(home, stubScopes({ live: () => inspect() })); const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); mkdirSync(join(record.path, "cache")); - const file = join(home, "registry.json"); - const registry = JSON.parse(readFileSync(file, "utf8")); - registry.records[0].runs = [{ unit: "orphan.scope", owner: deadOwner() }]; - writeFileSync(file, JSON.stringify(registry)); + orphan(home, "orphan.scope"); expect(worktrees.status().records[0]).toMatchObject({ phase: "active", - failure: expect.stringContaining("no user manager"), + runs: [{ unit: "orphan.scope", failure: expect.stringContaining("no user manager") }], }); expect(existsSync(join(record.path, "cache"))).toBe(true); + // The next inspection that succeeds clears the stale reason, though the run is still live. + inspect = () => true; + const [run] = worktrees.status().records[0].runs; + expect(run.unit).toBe("orphan.scope"); + expect(run.failure).toBeUndefined(); + inspect = () => false; + expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); + }); + it("stops a crashed supervisor's processes and leaves a live supervisor's run alone", async () => { + const { live, scopes } = fakeScopes(); + const stop = vi.spyOn(scopes, "stop"); + const { repo, home, worktrees } = fixture(scopes); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + mkdirSync(join(record.path, "cache")); + orphan(home, "crashed.scope"); + orphan(home, "supervised.scope", currentOwner()); + live.add("crashed.scope"); + await expect(worktrees.stop(record.id)).rejects.toThrow( + `left to it; signal it instead: supervised.scope (process ${process.pid}`, + ); + expect(stop.mock.calls).toEqual([["crashed.scope"]]); + expect(worktrees.status().records[0].runs.map((run) => run.unit)).toEqual(["supervised.scope"]); + expect(existsSync(join(record.path, "cache"))).toBe(true); + // With only a crashed supervisor's run left, stopping it releases the checkout. + const file = join(home, "registry.json"); + const registry = JSON.parse(readFileSync(file, "utf8")); + registry.records[0].runs = []; + writeFileSync(file, JSON.stringify(registry)); + orphan(home, "crashed-again.scope"); + live.add("crashed-again.scope"); + expect(worktrees.status().records[0].runs).toHaveLength(1); + expect(await worktrees.stop(record.id)).toMatchObject({ phase: "released", runs: [] }); + expect(existsSync(join(record.path, "cache"))).toBe(false); }); it("reports a failed release as a retained reservation", async () => { const { finish, scopes } = fakeScopes(); @@ -368,11 +514,7 @@ describe("supervised runs", { timeout: 20_000 }, () => { }); it("keeps scratch when the command never started", async () => { const { repo, home } = fixture(); - const worktrees = new Worktrees(home, { - check() {}, - live: () => false, - run: () => Promise.reject(new Error("no user manager")), - }); + const worktrees = new Worktrees(home, stubScopes({ run: () => Promise.reject(new Error("no user manager")) })); const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); mkdirSync(join(record.path, "cache")); await expect(worktrees.run(record.id, ["build"])).rejects.toThrow("no user manager"); @@ -381,20 +523,18 @@ describe("supervised runs", { timeout: 20_000 }, () => { }); it("keeps a run recorded when its processes cannot be confirmed stopped", async () => { const { repo, home } = fixture(); - const worktrees = new Worktrees(home, { - check() {}, - live: () => false, - run: () => Promise.reject(new Retained("cannot confirm that the scope stopped")), - }); + const worktrees = new Worktrees( + home, + stubScopes({ run: () => Promise.reject(new Retained("cannot confirm that the scope stopped")) }), + ); const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); mkdirSync(join(record.path, "cache")); await expect(worktrees.run(record.id, ["build"])).rejects.toThrow("cannot confirm"); // This process owns the run until it exits; then reconciliation asks the scope. expect(worktrees.status().records[0]).toMatchObject({ phase: "active", - failure: "cannot confirm that the scope stopped", + runs: [{ failure: "cannot confirm that the scope stopped" }], }); - expect(worktrees.status().records[0].runs).toHaveLength(1); expect(() => worktrees.release(record.id)).toThrow("supervised runs are live"); expect(existsSync(join(record.path, "cache"))).toBe(true); }); @@ -415,6 +555,36 @@ 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) { + const sent: unknown[] = []; + const fetch = (async (_url: unknown, init?: RequestInit) => { + sent.push(JSON.parse(String(init?.body)).variables); + return Response.json({ data: { repository: { object: found } } }); + }) as typeof globalThis.fetch; + return { sent, client: new GitHub({ fetch, now: Date.now, sleep: async () => {}, token: "test", warn() {} }) }; + } + 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 () => { + const { sent, client } = github(pulls({ merged: true, headRefOid: "abc" })); + expect(await mergedHead(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( + false, + ); + expect(await mergedHead(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); + }); +}); + describe("cgroup kill", () => { it("requires cgroup.kill in this process's unified cgroup", () => { const root = mkdtempSync(join(tmpdir(), "quest-cgroup-test-"));