From 66746df6f2034fb8b7c111b3be7fa590e68cdd53 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 14:06:14 -0700 Subject: [PATCH 1/4] chore(quest): claim quest/m0/worktree.md Quest-Claim: 4465a18959f4b93c From ab772aaef3034ae7a7ef0fe22ba9a84983ee4f0c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 14:26:41 -0700 Subject: [PATCH 2/4] feat(worktree): supervise runs in systemd scopes and release automatically `quest worktree run -- ` runs a job in a checkout inside a systemd user scope named once per run. A handshake proves the shell is in that scope's v2 cgroup before the supervisor lets the command start, so a dead supervisor can never start it. When the command exits, descendants left in the scope are killed, and once the scope is empty and no other run remains, the scratch declared at `create` is pruned. Runs are recorded before their scope exists. Every registry operation reconciles runs whose owner (PID plus Linux start time) exited and whose scope is empty, so a crashed supervisor's checkout is released only after its orphans stop. A run that cannot be inspected or confirmed stopped stays recorded and the checkout stays reserved, with the reason noted. `release` refuses while runs are live. Non-Linux hosts fail explicitly. The registry lock now names its owner in a file inside the lock directory and is taken by an atomic rename, so the lock of a process that exited on this host is reclaimed by unlinking that exact owner file, never a later owner's lock. Also: scratch is declared at `create` instead of `release`, a scratch symlink to another worktree is unlinked rather than refused, and CI starts a systemd user manager for the real-scope fixtures (orphan cleanup, cancellation, crash reconciliation). The quest now covers only `quest worktree remove`; the runner and agent quests no longer wait on it. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/check.yml | 6 + README.md | 54 +++++-- quest/m0/README.md | 2 +- quest/m0/kix/runner.md | 4 +- quest/m0/worktree-agents.md | 2 +- quest/m0/worktree.md | 67 ++++---- src/cli/main.ts | 48 +++++- src/cli/supervise.ts | 245 +++++++++++++++++++++++++++++ src/cli/worktree.ts | 302 ++++++++++++++++++++++++++++-------- test/worktree.run.test.ts | 127 +++++++++++++++ test/worktree.test.ts | 267 +++++++++++++++++++++++++------ 11 files changed, 949 insertions(+), 175 deletions(-) create mode 100644 src/cli/supervise.ts create mode 100644 test/worktree.run.test.ts diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index d8eeabf2..cd7e83bc 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -29,6 +29,12 @@ jobs: determinate: false - name: Install dependencies run: nix develop --command just install + # `quest worktree run` owns processes through a systemd user scope, which the tests exercise. + - name: Start a systemd user manager + run: | + sudo loginctl enable-linger "$USER" + sudo systemctl start "user@$(id -u).service" + echo "XDG_RUNTIME_DIR=/run/user/$(id -u)" >> "$GITHUB_ENV" - name: Check run: nix develop --command just check - name: Test diff --git a/README.md b/README.md index 6be86111..a7c091b5 100644 --- a/README.md +++ b/README.md @@ -403,9 +403,7 @@ guarantee or timeout: a command waits as long as the host stays busy. The experimental `quest worktree` commands share one disk budget across this user's repositories and release a checkout's disk by deleting its declared scratch in place. They only operate on Git worktrees they create; use the -owning application for Codex, Claude, or other managed checkouts. Process -supervision remains unfinished: stop agents and commands in a checkout before -releasing it. +owning application for Codex, Claude, or other managed checkouts. Set a shared capacity and an explicit reservation in bytes, covering the checkout and its expected build output. Measure representative jobs before @@ -415,30 +413,52 @@ writers, and Git objects stored in the shared repository are not charged. ```sh quest worktree capacity 20000000000 -quest worktree create example --reserve 3000000000 --ref HEAD +quest worktree create example --reserve 3000000000 --scratch node_modules dist +quest worktree run example -- sh -c 'just install && just test' quest worktree status -quest worktree release example --scratch node_modules dist -quest worktree restore example ``` Creation prints JSON with the ID and the detached, locked checkout path under -`.worktrees/`. Release deletes only its `--scratch` paths, and refuses one that -Git does not ignore, that holds tracked files or another Git worktree, or that -is reached through a symbolic link; links 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. Restore -reserves the checkout's disk again; then reinstall or rebuild its scratch, for -example with `just install`. +`.worktrees/`, and records the `--scratch` paths. `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 +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. + +`run` owns processes through a systemd user scope per run, so it needs Linux +with a systemd 254+ user manager and unified cgroup v2; it fails on other hosts. +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. + +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 +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 +automatic release is recorded on the checkout, which stays reserved. To manage a +checkout by hand, stop its unsupervised work first: + +```sh +quest worktree release example +quest worktree restore example +``` + +`release` refuses while supervised runs are live. `restore` reserves the +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 `$QUEST_WORKTREE_HOME`, or `$XDG_STATE_HOME/quest/worktrees` (default -`~/.local/state/quest/worktrees`). No live lock is stolen: after a crash, -confirm the registry lock's owner has exited before removing it. This initial -interface refuses unknown or changed checkout ownership. +`~/.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 +stolen. This initial interface refuses unknown or changed checkout ownership. ## Work with your agents diff --git a/quest/m0/README.md b/quest/m0/README.md index 589735b7..20bee186 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -36,7 +36,7 @@ ownership-manifest design: ## Required - [Gate contention measurements](/quest/m0/local-gate.md) - evidence that gated checks stay responsive under shared load -- [Worktree process ownership](/quest/m0/worktree.md) - supervise checkout commands and release checkouts automatically +- [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 448d5ac7..1039d501 100644 --- a/quest/m0/kix/runner.md +++ b/quest/m0/kix/runner.md @@ -87,6 +87,6 @@ replacing the hosted Agent SDK runtime: preference-order control. Decided 2026-10-18 as a follow-up of the decision inbox (#144), folded here rather than its own quest. -## Required +## Related -- [Worktree process ownership](/quest/m0/worktree.md) - reuse checkout supervision, scratch release, and disk admission +- [Worktree removal](/quest/m0/worktree.md) - removes finished checkouts; reuse `quest worktree run` for supervision, scratch release, and disk admission diff --git a/quest/m0/worktree-agents.md b/quest/m0/worktree-agents.md index 804ff316..510bcf5c 100644 --- a/quest/m0/worktree-agents.md +++ b/quest/m0/worktree-agents.md @@ -39,9 +39,9 @@ merge. ## Required -- [Worktree process ownership](/quest/m0/worktree.md) - supervised commands and automatic scratch release - [Expensive command gate](/quest/m0/local-gate.md) - resource control before removing agent-count throttles ## 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 index dd0c07a2..f065ec83 100644 --- a/quest/m0/worktree.md +++ b/quest/m0/worktree.md @@ -1,49 +1,38 @@ -# [M] Own worktree processes and release checkouts automatically +# [S] Remove finished quest worktree checkouts ## Goal -`quest worktree` supervises the commands that run in a checkout it created, -then releases that checkout automatically once they stop by pruning its -declared scratch in place. A crash or cancellation is reconciled without losing -work, stealing a live lock, or deleting scratch a running process still uses. +`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. ## Plan - Landed in [kixelated/quest#127](https://github.com/kixelated/quest/pull/127): - manual disk capacity and reservations, `create`, `release` (delete declared - scratch in place), and `restore` (reserve again; the caller rebuilds the - scratch) of Git-owned checkouts. The README section "Prunable local - checkouts" documents it as experimental and tells users to stop all work - before releasing. -- Decided (user, 2026-10-21): prune in place. Release deletes only declared - scratch that Git ignores, that holds no tracked files or other Git worktree, - and that is not reached through a symlink; it 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, while the - disk cost is scratch (2026-10-15 on this repository: 1.6 MB fresh source, - 376 MB built). -- Decided 2026-10-21 (user): this quest brings its own process-ownership - backend; `quest gate` admits on CPU load and memory only and owns no - descendants. The Linux systemd scope backend (scopes, slot handshake, - `cgroup.kill` inode pinning, prerequisite probes, opt-in systemd tests) was - dropped from the gate and is restorable from commit 5684307 in - [kixelated/quest#128](https://github.com/kixelated/quest/pull/128). Process - groups and PID files cannot prove detached descendants stopped after a - supervisor crash, so hosts without a supported backend fail explicitly - instead of guessing. -- Own foreground child processes and their descendants until they stop; only - then prune scratch and release the reservation. Record ownership so a crash - can be reconciled, including a stale registry lock whose owner has exited. - Report failed cleanup as a retained allocation, not success. -- Release automatically at lifecycle boundaries: before a blocking handoff and - at completion, once meaningful edits are committed. An asynchronous question - with independent work still running is not a release boundary. Automatic - release needs the scratch declarations without a caller; recording them at - `create` is the expected route. -- Add cancellation and crash fixtures: interrupt a supervised command with live - descendants, kill the supervisor, and reconcile. Measure retained disk after - complete and suspended jobs. + 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. +- 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 diff --git a/src/cli/main.ts b/src/cli/main.ts index 614768b7..f320e73a 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -371,14 +371,52 @@ export function main(argv: string[], out: Output): number | Promise { .description("Create an exclusively owned detached Git worktree under .worktrees/") .requiredOption("--reserve ", "Checkout and expected build growth in bytes", positiveInteger("bytes")) .option("--ref ", "Initial Git revision", "HEAD") - .action((name: string, options: { reserve: number; ref: string }) => - record(state().create(root(), name, options.reserve, options.ref)), + .option( + "--scratch ", + "Ignored paths release deletes, such as dependencies and build output; they must hold no tracked files", + [], + ) + .action((name: string, options: { reserve: number; ref: string; scratch: string[] }) => + record(state().create(root(), name, options.reserve, options.ref, options.scratch)), ); + worktree + .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 in a new systemd user scope; once the command exits, descendants still in the scope are killed. " + + "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 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.", + ) + .argument("", "Checkout ID or name") + .argument("", "Executable to run; put -- before it when any argument starts with -") + .argument("[args...]", "Arguments forwarded verbatim") + .action((id: string, command: string, args: string[]) => { + pending = state() + .run(id, [command, ...args]) + .then( + ({ code, failure }) => { + if (!failure) return code; + warn(failure); + return code || 1; + }, + (error: unknown) => { + warn(error instanceof Error ? error.message : String(error)); + return 1; + }, + ); + }); worktree .command("release ") - .description("Delete declared scratch and keep the checkout; drop the record of a removed checkout") - .option("--scratch ", "Ignored paths without tracked files, such as dependencies and build output", []) - .action((id: string, options: { scratch: string[] }) => record(state().release(id, options.scratch))); + .description( + "Delete the scratch declared at creation and keep the checkout; refuses while supervised runs are live, " + + "and drops the record of a removed checkout", + ) + .action((id: string) => record(state().release(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 new file mode 100644 index 00000000..bbdd72a6 --- /dev/null +++ b/src/cli/supervise.ts @@ -0,0 +1,245 @@ +// Process ownership for supervised checkout commands. Each run gets its own systemd user scope, +// named once and never reused, so its cgroup holds every descendant that does not ask another +// service manager to start it. Other hosts fail explicitly rather than guess from process groups. +import { execFileSync, spawn } from "node:child_process"; +import { readFileSync, watch, writeFileSync } from "node:fs"; +import { constants, hostname } from "node:os"; +import { join } from "node:path"; +import type { Readable, Writable } from "node:stream"; + +/** A process, identified so that a reused PID never looks like its predecessor. */ +export interface Owner { + host: string; + pid: number; + /** Linux start time in clock ticks since boot. */ + start?: string; +} + +/** Field 22 of /proc//stat; the command name in parentheses may hold spaces. */ +function startTime(pid: number): string | undefined { + if (process.platform !== "linux") return undefined; + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + return stat.slice(stat.lastIndexOf(")") + 2).split(" ")[19]; +} + +export function currentOwner(): Owner { + return { host: hostname(), pid: process.pid, start: startTime(process.pid) }; +} + +/** False only when the owner provably exited; another host's process is assumed to live. */ +export function alive(owner: Owner): boolean { + if (owner.host !== hostname() || !Number.isSafeInteger(owner.pid) || owner.pid <= 0) return true; + if (owner.start !== undefined && process.platform === "linux") { + try { + return startTime(owner.pid) === owner.start; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; + } + } + try { + process.kill(owner.pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ESRCH"; + } +} + +export interface Scopes { + /** Throws on a platform this backend cannot own processes on. */ + check(): void; + /** Whether any process may still run in the scope. */ + live(unit: string): boolean; + /** + * 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` + * when that cannot be confirmed, and otherwise only when the command never started. + */ + run(unit: string, cwd: string, command: string[]): Promise; +} + +/** The command may have started and its scope may still hold processes, so its run stays recorded. */ +export class Retained extends Error {} + +export const UNSUPPORTED = + "quest worktree run needs Linux with a systemd user manager (systemd 254 or later) and unified cgroup v2"; + +/** + * The shell proves it runs in the scope's own v2 cgroup, then waits for the supervisor before it + * executes the command. A supervisor that died never acknowledges, so the command never starts. + */ +const handshake = + "IFS=: read -r hierarchy controllers path &3; ' + + "read -r admitted <&4 || exit 125; " + + 'exec 3>&- 4<&-; exec "$@"'; + +function populated(group: string): boolean { + try { + return /^populated 1$/m.test(readFileSync(join(group, "cgroup.events"), "utf8")); + } catch (error) { + // ENODEV: the cgroup was removed between open and read. + if (["ENOENT", "ENODEV"].includes((error as NodeJS.ErrnoException).code ?? "")) return false; + throw error; + } +} + +/** Resolves once the cgroup has no process left, including when it is already gone. */ +function vacated(group: string): Promise { + return new Promise((resolve, reject) => { + let watcher: ReturnType | undefined; + let finished = false; + const finish = (error?: unknown) => { + if (finished) return; + finished = true; + watcher?.close(); + if (error) reject(error); + else resolve(); + }; + const check = () => { + try { + if (!populated(group)) finish(); + } catch (error) { + finish(error); + } + }; + try { + // The kernel notifies cgroup.events when the last process leaves, not when the cgroup is removed. + watcher = watch(join(group, "cgroup.events"), check); + watcher.on("error", finish); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") return finish(error); + } + // Registration precedes the read, covering an exit during setup. + check(); + }); +} + +/** SIGKILL every process in the cgroup; one already removed has none. */ +function kill(group: string): void { + try { + writeFileSync(join(group, "cgroup.kill"), "1"); + } catch (error) { + if (!["ENOENT", "ENODEV"].includes((error as NodeJS.ErrnoException).code ?? "")) throw error; + } +} + +export const systemdScopes: Scopes = { + check() { + if (process.platform !== "linux") 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)); + }, + + run(unit, cwd, command) { + return new Promise((resolve, reject) => { + const child = spawn( + "systemd-run", + [ + "--user", + "--scope", + "--quiet", + "--collect", + `--unit=${unit}`, + "--expand-environment=no", + "--", + "/bin/sh", + "-c", + handshake, + "quest-worktree", + ...command, + ], + { cwd, stdio: ["inherit", "inherit", "inherit", "pipe", "pipe"] }, + ); + const report = child.stdio[3] as Readable; + const admission = (child.stdio as unknown as (Writable | null)[])[4]!; + let group: string | undefined; + let admitted = false; + let interrupted = 0; + let failure: Error | undefined; + // The first signal reaches the command so its own handling runs; a second stops the whole scope. + const interrupt = (name: NodeJS.Signals) => { + if (interrupted) { + if (group) kill(group); + return; + } + interrupted = 128 + constants.signals[name]; + if (admitted) child.kill(name); + else admission.destroy(); // The shell reads end-of-file and never starts the command. + }; + const handlers = { + SIGINT: () => interrupt("SIGINT"), + SIGTERM: () => interrupt("SIGTERM"), + SIGHUP: () => interrupt("SIGHUP"), + }; + for (const [name, handler] of Object.entries(handlers)) process.on(name, handler); + const cleanup = () => { + for (const [name, handler] of Object.entries(handlers)) process.off(name, handler); + }; + + admission.on("error", (error: NodeJS.ErrnoException) => { + if (error.code !== "EPIPE") failure = error; + }); + let text = ""; + report.setEncoding("utf8"); + report.on("data", (data: string) => { + text += data; + if (group !== undefined || !text.endsWith("\n")) return; + const path = text.slice(0, -1); + // A unique unit name can never name a later scope, so this path stays this run's. + if (!path.startsWith("/") || path.includes("\n") || !path.endsWith(`/${unit}`)) { + failure = new Error(`${UNSUPPORTED}: the command ran outside its scope (${path})`); + admission.destroy(); + return; + } + group = join("/sys/fs/cgroup", path); + if (interrupted) return; + admitted = true; + admission.end("admitted\n"); + }); + + child.once("error", (error) => { + cleanup(); + reject( + (error as NodeJS.ErrnoException).code === "ENOENT" + ? new Error(`${UNSUPPORTED}: systemd-run is missing`) + : error, + ); + }); + child.once("exit", async (code, signal) => { + try { + // Descendants that outlive the command are orphans of this run: stop them. + if (group) { + try { + kill(group); + await vacated(group); + } catch (error) { + throw new Retained( + `cannot confirm that ${unit} stopped: ${error instanceof Error ? error.message : error}`, + ); + } + } + if (failure) throw failure; + if (!admitted && !interrupted) + throw new Error( + `${UNSUPPORTED}: systemd-run exited with ${code ?? signal} before the command started`, + ); + resolve(interrupted || (code ?? 128 + constants.signals[signal!])); + } catch (error) { + reject(error); + } finally { + cleanup(); + } + }); + }); + }, +}; diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index 7b669f2d..4d476d36 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -1,4 +1,5 @@ -// Git checkouts under a shared disk budget. Release prunes declared scratch in place. +// 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. import { execFileSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { @@ -10,20 +11,32 @@ import { readdirSync, realpathSync, renameSync, + rmdirSync, rmSync, statSync, statfsSync, + unlinkSync, writeFileSync, } from "node:fs"; import { homedir } from "node:os"; import { dirname, isAbsolute, join, resolve, sep } from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; +import { type Owner, Retained, type Scopes, alive, currentOwner, systemdScopes } from "./supervise"; +/** A supervised command, live while its owner runs or its scope holds a process. */ +export interface Run { + unit: string; + owner: Owner; +} export interface WorktreeRecord { id: string; name: string; repository: string; path: string; reserve: number; + /** Ignored paths, declared at creation, that release deletes. */ + scratch: string[]; + runs: Run[]; phase: "creating" | "active" | "released"; failure?: string; } @@ -33,6 +46,9 @@ interface Registry { records: WorktreeRecord[]; } +/** Another live process holds the registry lock. */ +class Busy extends Error {} + function git(root: string, ...args: string[]): string { return execFileSync("git", ["-C", root, ...args], { encoding: "utf8", @@ -45,12 +61,15 @@ function save(path: string, value: unknown): void { writeFileSync(temporary, `${JSON.stringify(value, null, 2)}\n`, { mode: 0o600 }); renameSync(temporary, path); } +function code(error: unknown): string | undefined { + return (error as NodeJS.ErrnoException).code; +} /** The entry itself, without following a final symlink; undefined when it is missing. */ function entry(path: string): Stats | undefined { try { return lstatSync(path); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; + if (code(error) === "ENOENT") return undefined; throw error; } } @@ -59,7 +78,7 @@ function device(path: string): number { try { return statSync(path).dev; } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT" || dirname(path) === path) throw error; + if (code(error) !== "ENOENT" || dirname(path) === path) throw error; return device(dirname(path)); } } @@ -135,7 +154,7 @@ function lockReason(path: string): string | undefined { const admin = /^gitdir: (.+)$/m.exec(readFileSync(join(path, ".git"), "utf8"))?.[1]; return admin && readFileSync(resolve(path, admin.trim(), "locked"), "utf8").replace(/\r?\n$/, ""); } catch (error) { - if (["ENOENT", "ENOTDIR", "EISDIR"].includes((error as NodeJS.ErrnoException).code ?? "")) return undefined; + if (["ENOENT", "ENOTDIR", "EISDIR"].includes(code(error) ?? "")) return undefined; throw error; } } @@ -155,41 +174,114 @@ export function diskUsage(path: string, seen = new Set()): number { return bytes; } +/** + * The registry lock is a directory holding one file named for its owner. It is created whole by a + * rename, which fails while a non-empty lock exists. A dead owner's lock is reclaimed by unlinking + * that exact owner file, so a reclaimer can never remove a lock some later owner took. + */ +function acquire(lock: string): string { + const token = randomUUID(); + const staging = `${lock}.${token}`; + mkdirSync(staging, { mode: 0o700 }); + try { + writeFileSync(join(staging, `${token}.json`), JSON.stringify(currentOwner()), { mode: 0o600 }); + for (;;) { + try { + renameSync(staging, lock); + return token; + } catch (error) { + if (!existsSync(lock)) throw error; + } + let holders: string[]; + try { + holders = readdirSync(lock); + } catch (error) { + if (code(error) === "ENOENT") continue; + throw error; + } + const holder = holders[0]; + if (holder !== undefined) { + let owner: Owner; + try { + owner = JSON.parse(readFileSync(join(lock, holder), "utf8")); + } catch (error) { + if (code(error) === "ENOENT") continue; + throw error; + } + if (alive(owner)) + throw new Busy( + `worktree registry is locked by process ${owner.pid} on ${owner.host ?? "an unknown host"}: ${lock}`, + ); + try { + unlinkSync(join(lock, holder)); + } catch (error) { + if (code(error) !== "ENOENT") throw error; + } + } + // An empty lock is mid-release or reclaimed; a later owner's file makes this fail instead. + try { + rmdirSync(lock); + } catch (error) { + if (!["ENOENT", "ENOTEMPTY", "EEXIST"].includes(code(error) ?? "")) throw error; + } + } + } finally { + rmSync(staging, { recursive: true, force: true }); + } +} +function unlock(lock: string, token: string): void { + unlinkSync(join(lock, `${token}.json`)); + try { + rmdirSync(lock); + } catch (error) { + if (!["ENOENT", "ENOTEMPTY", "EEXIST"].includes(code(error) ?? "")) throw error; + } +} + /** All allocations share one registry across repositories for this user/host. */ export class Worktrees { readonly home: string; constructor( home = process.env.QUEST_WORKTREE_HOME ?? join(process.env.XDG_STATE_HOME ?? join(homedir(), ".local", "state"), "quest", "worktrees"), + private readonly scopes: Scopes = systemdScopes, ) { mkdirSync(home, { recursive: true, mode: 0o700 }); this.home = realpathSync(home); } + /** Every operation first reconciles runs whose supervisor and processes are gone. */ private locked(action: (registry: Registry) => T): T { const lock = join(this.home, "lock"); + const token = acquire(lock); try { - mkdirSync(lock, { mode: 0o700 }); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "EEXIST") - throw new Error( - `worktree registry is locked: ${lock}; never remove a live lock; after a crash inspect owner.json and reconcile it explicitly`, - ); - throw error; - } - try { - save(join(lock, "owner.json"), { pid: process.pid, started: new Date().toISOString() }); const file = join(this.home, "registry.json"); const registry: Registry = existsSync(file) ? JSON.parse(readFileSync(file, "utf8")) : { version: 1, capacity: 0, records: [] }; if (registry.version !== 1) throw new Error("unsupported worktree registry version"); + for (const record of registry.records) { + record.scratch ??= []; + record.runs ??= []; + } try { + this.reconcile(registry); return action(registry); } finally { save(file, registry); } } finally { - rmSync(lock, { recursive: true }); + unlock(lock, token); + } + } + /** Waits out another live holder, whose critical sections are short; a dead one is reclaimed. */ + private async lockedWait(action: (registry: Registry) => T): Promise { + for (;;) { + try { + return this.locked(action); + } catch (error) { + if (!(error instanceof Busy)) throw error; + } + await sleep(100); } } private find(registry: Registry, id: string): WorktreeRecord { @@ -197,6 +289,33 @@ export class Worktrees { if (!record) throw new Error(`unknown worktree: ${id}`); 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 { + if (alive(run.owner)) return true; + try { + return this.scopes.live(run.unit); + } catch (error) { + record.failure = `cannot inspect supervised run ${run.unit}: ${error instanceof Error ? error.message : error}`; + return true; + } + } + /** Drops finished runs a crashed supervisor left behind, and releases a checkout none remain in. */ + 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)); + if (!record.runs.length) this.settle(registry, record); + } + } + /** 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}`; + } + } /** * Bytes on disk, plus the growth active reservations still expect. Released checkouts keep only * what remains. `local` is the expected growth on the filesystem holding `parent`. @@ -242,7 +361,8 @@ export class Worktrees { return { capacity: registry.capacity, allocated: used + growth, records: registry.records }; }); } - create(root: string, name: string, reserve: number, ref = "HEAD"): WorktreeRecord { + /** `scratch` names the ignored paths release deletes, such as dependencies and build output. */ + create(root: string, name: string, reserve: number, ref = "HEAD", scratch: string[] = []): WorktreeRecord { return this.locked((registry) => { if (!/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/.test(name)) throw new Error("name must contain only letters, digits, dots, underscores, or hyphens"); @@ -250,12 +370,22 @@ export class Worktrees { throw new Error("reserve must be a positive safe integer in bytes, including expected build growth"); if (registry.records.some((item) => item.name === name)) throw new Error(`worktree name already exists; use restore or another name: ${name}`); + const declared = scratch.map(childPath); const repository = canonical(git(root, "rev-parse", "--show-toplevel").trim()); const path = join(repository, ".worktrees", name); mkdirSync(dirname(path), { recursive: true }); if (existsSync(path)) throw new Error(`refusing to adopt an existing checkout: ${path}`); this.admit(registry, reserve, dirname(path)); - const record: WorktreeRecord = { id: randomUUID(), name, repository, path, reserve, phase: "creating" }; + const record: WorktreeRecord = { + id: randomUUID(), + name, + repository, + path, + reserve, + scratch: declared, + runs: [], + phase: "creating", + }; registry.records.push(record); save(join(this.home, "registry.json"), registry); // Reservation survives a crash in git. try { @@ -296,52 +426,62 @@ export class Worktrees { * The checkout, its Git state, and untracked source stay in place. A checkout that Git and the * disk both lack, such as one removed with `git worktree remove`, drops its record instead. */ - release(id: string, scratch: string[]): WorktreeRecord { + private prune(registry: Registry, record: WorktreeRecord): void { + if (!this.owned(record)) { + 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}`, + ); + const root = canonical(record.path); + const paths = record.scratch.filter((path) => present(root, path)); // Reruns skip pruned paths. + const admin = entry(join(root, ".git")); + for (const path of paths) { + // Case folding and other platform aliases can still name the checkout's .git entry. + const stat = entry(join(root, path)); + if (admin && stat?.dev === admin.dev && stat.ino === admin.ino) + throw new Error(`scratch path is the checkout's .git entry: ${path}`); + } + if (paths.length) { + // icase, because a case-insensitive filesystem resolves `CACHE` to a tracked `cache`. + const pathspecs = paths.map((path) => `:(literal,icase)${path}`); + const tracked = git(root, "ls-files", "-z", "--", ...pathspecs) + .split("\0") + .filter(Boolean); + if (tracked.length) throw new Error(`scratch includes tracked files: ${tracked.slice(0, 20).join(", ")}`); + const matched = ignored(root, paths); + const unignored = paths.filter((path) => !matched.has(path)); + if (unignored.length) throw new Error(`scratch must be ignored by Git: ${unignored.join(", ")}`); + // Another 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); + for (const path of paths) { + // A symlink is unlinked rather than followed, whatever it points to. + if (entry(join(root, path))?.isSymbolicLink()) continue; + 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}`); + } + // 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 }); + } + record.phase = "released"; + } + /** Prunes the scratch declared at creation; refuses while a supervised run may still use it. */ + release(id: string): WorktreeRecord { return this.locked((registry) => { const record = this.find(registry, id); - if (!this.owned(record)) { - registry.records.splice(registry.records.indexOf(record), 1); - return record; - } - if (record.phase === "creating") + if (record.runs.length) throw new Error( - `creation was interrupted; inspect and remove the partial checkout with git worktree remove, then release again: ${record.path}`, + `supervised runs are live in this checkout: ${record.runs.map((run) => run.unit).join(", ")}`, ); - const root = canonical(record.path); - const paths = scratch.map(childPath).filter((path) => present(root, path)); // Reruns skip pruned paths. - const admin = entry(join(root, ".git")); - for (const path of paths) { - // Case folding and other platform aliases can still name the checkout's .git entry. - const stat = entry(join(root, path)); - if (admin && stat?.dev === admin.dev && stat.ino === admin.ino) - throw new Error(`scratch path is the checkout's .git entry: ${path}`); - } - if (paths.length) { - // icase, because a case-insensitive filesystem resolves `CACHE` to a tracked `cache`. - const pathspecs = paths.map((path) => `:(literal,icase)${path}`); - const tracked = git(root, "ls-files", "-z", "--", ...pathspecs) - .split("\0") - .filter(Boolean); - if (tracked.length) - throw new Error(`scratch includes tracked files: ${tracked.slice(0, 20).join(", ")}`); - const matched = ignored(root, paths); - const unignored = paths.filter((path) => !matched.has(path)); - if (unignored.length) throw new Error(`scratch must be ignored by Git: ${unignored.join(", ")}`); - // Another 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); - for (const path of paths) { - 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}`); - } - // 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 }); - } - record.phase = "released"; + this.prune(registry, record); + delete record.failure; return record; }); } @@ -350,11 +490,47 @@ export class Worktrees { return this.locked((registry) => { const record = this.find(registry, id); if (!this.owned(record)) throw new Error(`checkout is missing; release drops its record: ${record.path}`); - if (record.phase === "active") return record; - if (record.phase !== "released") throw new Error(`cannot restore a ${record.phase} checkout`); - this.admit(registry, Math.max(0, record.reserve - diskUsage(record.path)), dirname(record.path)); - record.phase = "active"; + this.activate(registry, record); return record; }); } + private activate(registry: Registry, record: WorktreeRecord): void { + if (record.phase === "active") return; + if (record.phase !== "released") throw new Error(`cannot use a ${record.phase} checkout`); + this.admit(registry, Math.max(0, record.reserve - diskUsage(record.path)), dirname(record.path)); + record.phase = "active"; + } + /** + * Runs a command in the checkout, restoring it first when released. Once the command and every + * descendant have stopped, and no other run remains, its declared scratch is pruned. Resolves + * with the command's status; a failed release is recorded and leaves the checkout reserved. + */ + async run(id: string, command: string[]): Promise<{ code: number; failure?: string }> { + this.scopes.check(); + const run: Run = { unit: `quest-worktree-${randomUUID()}.scope`, owner: currentOwner() }; + const record = this.locked((registry) => { + const record = this.find(registry, id); + if (!this.owned(record)) throw new Error(`checkout is missing; release drops its record: ${record.path}`); + this.activate(registry, record); + 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).then( + (code) => ({ code }), + (error: unknown) => ({ error }), + ); + // Waits out a busy lock rather than failing: the run's record keeps its scratch until this lands. + const failure = await this.lockedWait((registry) => { + 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; + 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); + return current.failure; + }); + if ("error" in result) throw result.error; + return { code: result.code, failure }; + } } diff --git a/test/worktree.run.test.ts b/test/worktree.run.test.ts new file mode 100644 index 00000000..a76c81ca --- /dev/null +++ b/test/worktree.run.test.ts @@ -0,0 +1,127 @@ +// Supervised runs against the host's real systemd user manager. On Linux these need systemd 254+ +// and unified cgroup v2; a host without them fails here rather than skipping. +import { execFileSync, spawn, spawnSync } from "node:child_process"; +import { once } from "node:events"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { describe, expect, it } from "vitest"; +import { Worktrees } from "../src/cli/worktree"; +import { tempDir } from "./fixture"; + +const root = resolve(import.meta.dirname, ".."); + +function fixture() { + const dir = tempDir(); + const repo = join(dir, "repo"); + mkdirSync(repo); + const git = (...args: string[]) => execFileSync("git", ["-C", repo, ...args], { stdio: "ignore" }); + git("init", "-b", "main"); + git("config", "user.name", "Fixture"); + git("config", "user.email", "fixture@localhost"); + writeFileSync(join(repo, ".gitignore"), "cache/\n.worktrees/\n"); + git("add", "."); + git("commit", "-m", "initial"); + const home = join(dir, "state"); + const worktrees = new Worktrees(home); + worktrees.configure(100_000_000); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + mkdirSync(join(record.path, "cache")); + 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[]) { + const child = spawn( + "bun", + [ + `--config=${join(root, "bunfig.toml")}`, + join(root, "src/cli/bin.ts"), + "worktree", + "run", + "work", + "--", + "/bin/sh", + "-c", + script, + "sh", + ...args, + ], + { env: { ...process.env, QUEST_WORKTREE_HOME: home }, stdio: ["ignore", "pipe", "pipe"] }, + ); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + const ready = new Promise((resolve, reject) => { + child.stdout.on("data", (data: string) => { + stdout += data; + if (stdout.includes("ready\n")) resolve(); + }); + child.once("exit", () => reject(new Error(`exited before ready: ${stdout} ${stderr}`))); + }); + ready.catch(() => {}); // Awaited only by cases that wait for readiness. + child.stderr.on("data", (data: string) => (stderr += data)); + const exited = once(child, "exit").then(([code, signal]) => ({ code, signal, stdout, stderr })); + return { child, ready, exited }; +} + +/** Gone, or a zombie its new parent has not reaped yet: either way it runs no code. */ +function stopped(pid: number): boolean { + try { + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + return stat.slice(stat.lastIndexOf(")") + 2).startsWith("Z"); + } catch { + return true; + } +} + +describe.runIf(process.platform === "linux")("supervised runs under systemd", { timeout: 30_000 }, () => { + it("forwards arguments and status, stops orphaned descendants, then releases", async () => { + const { dir, home, worktrees, record } = fixture(); + const pidFile = join(dir, "orphan"); + const run = supervise( + home, + `sleep 1000 & echo $! > ${JSON.stringify(pidFile)}; printf '%s|' "$@"; pwd; echo ready; exit 7`, + "a $b", + "--flag", + ); + const result = await run.exited; + expect(result.code, result.stderr).toBe(7); + expect(result.stdout).toBe(`a $b|--flag|${record.path}\nready\n`); + expect(stopped(Number(readFileSync(pidFile, "utf8")))).toBe(true); + expect(existsSync(join(record.path, "cache"))).toBe(false); + expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); + }); + + it("forwards cancellation, stops live descendants, then releases", async () => { + const { dir, home, worktrees, record } = fixture(); + const pidFile = join(dir, "descendant"); + const run = supervise(home, `sleep 1000 & echo $! > ${JSON.stringify(pidFile)}; echo ready; wait`); + await run.ready; + expect(worktrees.status().records[0].runs).toHaveLength(1); + run.child.kill("SIGTERM"); + const result = await run.exited; + expect(result.code, result.stderr).toBe(143); + expect(stopped(Number(readFileSync(pidFile, "utf8")))).toBe(true); + expect(existsSync(join(record.path, "cache"))).toBe(false); + expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); + }); + + it("keeps a crashed supervisor's scratch until its scope is empty", async () => { + const { home, worktrees, record } = fixture(); + const run = supervise(home, "echo ready; exec sleep 1000"); + await run.ready; + run.child.kill("SIGKILL"); + await run.exited; + // The command outlives its supervisor, so reconciliation keeps the run and its scratch. + const [orphan] = worktrees.status().records[0].runs; + 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: [] }); + expect(existsSync(join(record.path, "cache"))).toBe(false); + }); +}); diff --git a/test/worktree.test.ts b/test/worktree.test.ts index 77c7727e..e2a4663c 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -1,4 +1,4 @@ -import { execFileSync, spawn } from "node:child_process"; +import { execFileSync, spawn, spawnSync } from "node:child_process"; import { appendFileSync, existsSync, @@ -12,9 +12,11 @@ import { truncateSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; +import { hostname, tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { Retained, type Scopes, currentOwner } from "../src/cli/supervise"; +import { main } from "../src/cli/main"; import { Worktrees, diskUsage } from "../src/cli/worktree"; import { run } from "./fixture"; @@ -33,15 +35,31 @@ function repository(path: string): string { git(path, "commit", "-m", "initial"); return path; } -function fixture() { + +/** Scopes whose runs finish when the test says so, and stay live while listed. */ +function fakeScopes() { + const live = new Set(); + const finish = new Map void>(); + const scopes: Scopes = { + check() {}, + live: (unit) => live.has(unit), + run: (unit) => new Promise((resolve) => finish.set(unit, resolve)), + }; + return { live, finish, scopes }; +} +function fixture(scopes?: Scopes) { const root = mkdtempSync(join(tmpdir(), "quest-worktree-test-")); roots.push(root); const repo = repository(join(root, "repo")); const home = join(root, "state"); - const worktrees = new Worktrees(home); + const worktrees = new Worktrees(home, scopes); worktrees.configure(100_000_000); return { root, repo, home, worktrees }; } +/** A process that has exited, identified so it can never match a live one. */ +function deadOwner() { + return { host: hostname(), pid: spawnSync(process.execPath, ["-e", ""]).pid!, start: "0" }; +} afterEach(() => { vi.unstubAllEnvs(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); @@ -52,7 +70,8 @@ afterEach(() => { describe("worktrees", { timeout: 20_000 }, () => { it("prunes declared scratch in place and keeps source, Git state, and untracked files", () => { const { repo, worktrees } = fixture(); - const record = worktrees.create(repo, "work", 10_000_000); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache/", "cache/nested", "missing"]); + expect(record.scratch).toEqual(["cache", "cache/nested", "missing"]); writeFileSync(join(record.path, "file"), "committed\n"); git(record.path, "commit", "-am", "local only"); const head = git(record.path, "rev-parse", "HEAD"); @@ -64,7 +83,7 @@ describe("worktrees", { timeout: 20_000 }, () => { mkdirSync(join(record.path, "cache", "nested"), { recursive: true }); writeFileSync(join(record.path, "cache", "nested", "artifact"), "disposable"); const status = git(record.path, "status", "--porcelain=v1"); - expect(worktrees.release(record.id, ["cache/", "cache/nested", "missing"]).phase).toBe("released"); + expect(worktrees.release(record.id).phase).toBe("released"); expect(existsSync(join(record.path, "cache"))).toBe(false); expect(git(record.path, "status", "--porcelain=v1")).toBe(status); expect(git(record.path, "rev-parse", "HEAD")).toBe(head); @@ -76,44 +95,60 @@ describe("worktrees", { timeout: 20_000 }, () => { }); it("refuses scratch that is tracked, not ignored, or outside the checkout", () => { const { repo, worktrees } = fixture(); - const record = worktrees.create(repo, "work", 10_000_000); - writeFileSync(join(record.path, "new"), "source"); - mkdirSync(join(record.path, "cache")); - writeFileSync(join(record.path, "cache", "forced"), "tracked despite the ignore rule"); - git(record.path, "add", "-f", "cache/forced"); - expect(() => worktrees.release(record.id, ["file"])).toThrow("tracked files: file"); - expect(() => worktrees.release(record.id, ["cache"])).toThrow("tracked files: cache/forced"); + for (const path of ["../outside", ".git", ".GIT", "/absolute", "."]) + expect(() => worktrees.create(repo, "invalid", 10_000_000, "HEAD", [path])).toThrow("relative path"); + let count = 0; + const declare = (scratch: string) => { + const record = worktrees.create(repo, `case-${count++}`, 1_000_000, "HEAD", [scratch]); + writeFileSync(join(record.path, "new"), "source"); + mkdirSync(join(record.path, "cache")); + writeFileSync(join(record.path, "cache", "forced"), "tracked despite the ignore rule"); + git(record.path, "add", "-f", "cache/forced"); + return record; + }; + expect(() => worktrees.release(declare("file").id)).toThrow("tracked files: file"); + expect(() => worktrees.release(declare("cache").id)).toThrow("tracked files: cache/forced"); // Another spelling, which a case-insensitive filesystem resolves to the tracked directory. - mkdirSync(join(record.path, "CACHE"), { recursive: true }); + const aliased = declare("CACHE"); + mkdirSync(join(aliased.path, "CACHE"), { recursive: true }); appendFileSync(join(repo, ".git", "info", "exclude"), "/CACHE/\n"); - expect(() => worktrees.release(record.id, ["CACHE"])).toThrow("tracked files: cache/forced"); - expect(() => worktrees.release(record.id, ["new"])).toThrow("ignored by Git: new"); - expect(() => worktrees.release(record.id, ["../outside"])).toThrow("relative path"); - expect(() => worktrees.release(record.id, [".git"])).toThrow("relative path"); - expect(() => worktrees.release(record.id, [".GIT"])).toThrow("relative path"); + expect(() => worktrees.release(aliased.id)).toThrow("tracked files: cache/forced"); + const unignored = declare("new"); + expect(() => worktrees.release(unignored.id)).toThrow("ignored by Git: new"); // Any other alias of the .git entry, here a hard link that an ignore rule matches. - linkSync(join(record.path, ".git"), join(record.path, ".env")); - expect(() => worktrees.release(record.id, [".env"])).toThrow(".git entry: .env"); - expect(readFileSync(join(record.path, "new"), "utf8")).toBe("source"); - expect(readFileSync(join(record.path, "cache", "forced"), "utf8")).toBe("tracked despite the ignore rule"); - expect(worktrees.status().records[0].phase).toBe("active"); + const linked = declare(".env"); + linkSync(join(linked.path, ".git"), join(linked.path, ".env")); + expect(() => worktrees.release(linked.id)).toThrow(".git entry: .env"); + expect(readFileSync(join(unignored.path, "new"), "utf8")).toBe("source"); + expect(readFileSync(join(unignored.path, "cache", "forced"), "utf8")).toBe("tracked despite the ignore rule"); + expect(worktrees.status().records.every((record) => record.phase === "active")).toBe(true); }); it.skipIf(process.platform === "win32")("never follows symlinks out of the checkout", () => { const { root, repo, worktrees } = fixture(); const outside = join(root, "outside"); mkdirSync(join(outside, "cache"), { recursive: true }); writeFileSync(join(outside, "cache", "keep"), "outside"); - const record = worktrees.create(repo, "work", 10_000_000); - symlinkSync(outside, join(record.path, "linked")); - expect(() => worktrees.release(record.id, ["linked/cache"])).toThrow("reached through a symlink"); + const through = worktrees.create(repo, "through", 10_000_000, "HEAD", ["linked/cache"]); + symlinkSync(outside, join(through.path, "linked")); + expect(() => worktrees.release(through.id)).toThrow("reached through a symlink"); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache", ".env"]); mkdirSync(join(record.path, "cache")); symlinkSync(outside, join(record.path, "cache", "link")); symlinkSync(outside, join(record.path, ".env")); - worktrees.release(record.id, ["cache", ".env"]); + worktrees.release(record.id); expect(existsSync(join(record.path, "cache"))).toBe(false); expect(existsSync(join(record.path, ".env"))).toBe(false); expect(readFileSync(join(outside, "cache", "keep"), "utf8")).toBe("outside"); }); + it.skipIf(process.platform === "win32")("unlinks a scratch symlink to another worktree", () => { + const { repo, worktrees } = fixture(); + const other = worktrees.create(repo, "other", 10_000_000); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", [".env"]); + symlinkSync(other.path, join(record.path, ".env")); + expect(worktrees.release(record.id).phase).toBe("released"); + expect(existsSync(join(record.path, ".env"))).toBe(false); + expect(existsSync(join(other.path, "file"))).toBe(true); + }); it.skipIf(process.platform === "win32")("charges allocated blocks of dangling symlinks and sparse files", () => { const { root } = fixture(); const dangling = join(root, "dangling"); @@ -135,7 +170,7 @@ describe("worktrees", { timeout: 20_000 }, () => { worktrees.configure(reserve); const record = worktrees.create(repo, "work", reserve); expect(worktrees.status().allocated).toBe(reserve); - worktrees.release(record.id, []); + worktrees.release(record.id); const retained = worktrees.status().allocated; expect(retained).toBeGreaterThan(0); expect(retained).toBeLessThan(reserve); @@ -145,11 +180,11 @@ describe("worktrees", { timeout: 20_000 }, () => { }); it("drops the record once its checkout is removed", () => { const { repo, worktrees } = fixture(); - const record = worktrees.create(repo, "work", 10_000_000); - expect(() => worktrees.release(record.id, ["missing"])).not.toThrow(); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["missing"]); + expect(() => worktrees.release(record.id)).not.toThrow(); git(repo, "worktree", "unlock", record.path); git(repo, "worktree", "remove", record.path); - worktrees.release(record.id, []); + worktrees.release(record.id); expect(worktrees.status().records).toEqual([]); expect(() => worktrees.restore(record.id)).toThrow("unknown worktree"); expect(worktrees.create(repo, "work", 10_000_000).phase).toBe("active"); @@ -161,14 +196,28 @@ describe("worktrees", { timeout: 20_000 }, () => { worktrees.create(repo, "first", 12_000_000); expect(() => new Worktrees(home).create(second, "second", 12_000_000)).toThrow("disk capacity exceeded"); expect(() => worktrees.configure(1_000_000)).toThrow("below existing"); - mkdirSync(join(home, "lock")); - expect(() => worktrees.status()).toThrow("never remove a live lock"); - rmSync(join(home, "lock"), { recursive: true }); expect(() => worktrees.create(second, "failed", 1_000_000, "nonexistent-ref")).toThrow(); expect(worktrees.status().records.find((record) => record.name === "failed")?.phase).toBe("creating"); - worktrees.release("failed", []); + worktrees.release("failed"); expect(worktrees.status().records.map((record) => record.name)).toEqual(["first"]); }); + it("never steals a live registry lock and reclaims one whose owner exited", () => { + const { home, worktrees } = fixture(); + const lock = join(home, "lock"); + mkdirSync(lock); + writeFileSync(join(lock, "live.json"), JSON.stringify(currentOwner())); + expect(() => worktrees.status()).toThrow(`locked by process ${process.pid}`); + // Another host's process cannot be checked, so it is assumed to be live. + writeFileSync(join(lock, "live.json"), JSON.stringify({ ...deadOwner(), host: "elsewhere" })); + expect(() => worktrees.status()).toThrow("on elsewhere"); + writeFileSync(join(lock, "live.json"), JSON.stringify(deadOwner())); + expect(worktrees.status().capacity).toBe(100_000_000); + expect(existsSync(lock)).toBe(false); + // An empty lock is one whose owner was releasing or reclaiming it. + mkdirSync(lock); + expect(worktrees.status().capacity).toBe(100_000_000); + expect(existsSync(lock)).toBe(false); + }); it("admits simultaneous creators atomically without oversubscribing disk", async () => { const { root, repo, home, worktrees } = fixture(); const second = repository(join(root, "second")); @@ -197,32 +246,33 @@ describe("worktrees", { timeout: 20_000 }, () => { const record = worktrees.create(repo, "work", 10_000_000); git(repo, "worktree", "unlock", record.path); git(repo, "worktree", "lock", "--reason", "another owner", record.path); - expect(() => worktrees.release(record.id, [])).toThrow("ownership/lock changed"); + 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", () => { const { root, repo, worktrees } = fixture(); - const outer = worktrees.create(repo, "outer", 10_000_000); + const outer = 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 = worktrees.create(outer.path, "inner", 10_000_000); expect(inner.path).toBe(join(outer.path, ".worktrees", "inner")); - expect(() => worktrees.release(outer.id, [".worktrees"])).toThrow("scratch contains a Git worktree"); + expect(() => worktrees.release(outer.id)).toThrow("scratch contains a Git 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 = worktrees.create(repo, "other", 10_000_000, "HEAD", ["cache"]); const foreign = repository(join(root, "foreign")); - git(foreign, "worktree", "add", "--detach", join(outer.path, "cache", "agent")); - expect(() => worktrees.release(outer.id, ["cache"])).toThrow("scratch contains a Git worktree: cache"); - expect(existsSync(join(outer.path, "cache", "agent", "file"))).toBe(true); - expect(worktrees.status().records.map((record) => record.phase)).toEqual(["active", "active"]); + git(foreign, "worktree", "add", "--detach", join(other.path, "cache", "agent")); + expect(() => worktrees.release(other.id)).toThrow("scratch contains a Git worktree: cache"); + expect(existsSync(join(other.path, "cache", "agent", "file"))).toBe(true); + expect(worktrees.status().records.map((record) => record.phase)).toEqual(["active", "active", "active"]); }); it.skipIf(process.platform === "win32")("refuses a root replaced by a link to another checkout", () => { const { repo, worktrees } = fixture(); - const replaced = worktrees.create(repo, "first", 10_000_000); + const replaced = worktrees.create(repo, "first", 10_000_000, "HEAD", ["cache"]); const other = worktrees.create(repo, "second", 10_000_000); mkdirSync(join(other.path, "cache")); rmSync(replaced.path, { recursive: true }); symlinkSync(other.path, replaced.path); - expect(() => worktrees.release(replaced.id, ["cache"])).toThrow("ownership/lock changed"); + 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", () => { @@ -230,14 +280,137 @@ describe("worktrees", { timeout: 20_000 }, () => { vi.stubEnv("QUEST_WORKTREE_HOME", home); expect(run("worktree", "create", "cli", "--root", repo).code).toBe(2); expect(run("worktree", "capacity", "0").code).toBe(2); - const created = run("--root", repo, "worktree", "create", "cli", "--reserve", "10000000"); + const created = run("--root", repo, "worktree", "create", "cli", "--reserve", "10000000", "--scratch", "cache"); expect(created.code).toBe(0); const { path } = JSON.parse(created.stdout); mkdirSync(join(path, "cache")); - expect(run("worktree", "release", "cli", "--scratch", "cache").code).toBe(0); + expect(run("worktree", "release", "cli").code).toBe(0); expect(existsSync(join(path, "cache"))).toBe(false); const status = run("worktree", "status"); expect(status.code).toBe(0); expect(JSON.parse(status.stdout).records[0].phase).toBe("released"); }); }); + +describe("supervised runs", { timeout: 20_000 }, () => { + it("keeps scratch while any run is live and prunes it once the last one stops", async () => { + const { finish, scopes } = fakeScopes(); + const { repo, worktrees } = fixture(scopes); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + worktrees.release(record.id); + // A run restores a released checkout before it starts. + const first = worktrees.run(record.id, ["first"]); + const second = worktrees.run(record.id, ["second"]); + const units = worktrees.status().records[0].runs.map((run) => run.unit); + expect(units).toHaveLength(2); + expect(worktrees.status().records[0].phase).toBe("active"); + mkdirSync(join(record.path, "cache")); + expect(() => worktrees.release(record.id)).toThrow("supervised runs are live"); + finish.get(units[0])!(0); + expect(await first).toEqual({ code: 0, failure: undefined }); + expect(existsSync(join(record.path, "cache"))).toBe(true); + finish.get(units[1])!(7); + expect(await second).toEqual({ code: 7, failure: undefined }); + expect(existsSync(join(record.path, "cache"))).toBe(false); + expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); + }); + it("reconciles a crashed supervisor only once its scope is empty", () => { + const { live, scopes } = fakeScopes(); + 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)); + live.add("orphan.scope"); + 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); + live.delete("orphan.scope"); + 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", () => { + const { repo, home } = fixture(); + const worktrees = new Worktrees(home, { + check() {}, + live: () => { + throw new Error("no user manager"); + }, + run: () => Promise.resolve(0), + }); + 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)); + expect(worktrees.status().records[0]).toMatchObject({ + phase: "active", + failure: expect.stringContaining("no user manager"), + }); + expect(existsSync(join(record.path, "cache"))).toBe(true); + }); + it("reports a failed release as a retained reservation", async () => { + const { finish, scopes } = fakeScopes(); + const { repo, worktrees } = fixture(scopes); + const record = worktrees.create(repo, "work", 10_000_000, "HEAD", ["cache"]); + const running = worktrees.run(record.id, ["build"]); + mkdirSync(join(record.path, "cache")); + writeFileSync(join(record.path, "cache", "forced"), "tracked"); + git(record.path, "add", "-f", "cache/forced"); + finish.get(worktrees.status().records[0].runs[0].unit)!(0); + const result = await running; + expect(result.code).toBe(0); + expect(result.failure).toContain("tracked files: cache/forced"); + expect(worktrees.status().records[0]).toMatchObject({ phase: "active", runs: [] }); + }); + 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 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"); + expect(worktrees.status().records[0]).toMatchObject({ phase: "active", runs: [] }); + expect(existsSync(join(record.path, "cache"))).toBe(true); + }); + 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 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", + }); + 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); + }); + it.skipIf(process.platform === "linux")("fails explicitly without a supported backend", async () => { + const { repo, home } = fixture(); + vi.stubEnv("QUEST_WORKTREE_HOME", home); + expect(run("--root", repo, "worktree", "create", "cli", "--reserve", "10000000").code).toBe(0); + let stderr = ""; + const code = await main(["worktree", "run", "cli", "--", "git", "status"], { + stdout: () => {}, + stderr: (text) => { + stderr += text; + }, + }); + expect(code).toBe(1); + expect(stderr).toContain("needs Linux with a systemd user manager"); + expect(new Worktrees(home).status().records[0].runs).toEqual([]); + }); +}); From c5efae0a5d014185c84b43921240a535c04f81a9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 14:31:35 -0700 Subject: [PATCH 3/4] fix(worktree): refuse scratch naming the checkout on Windows resolve('/', '.') is a drive root on Windows, so '.' passed the check there and was refused only by the tracked-file check. Test the path's own components instead. Co-Authored-By: Claude Opus 5.5 --- src/cli/worktree.ts | 6 +++--- test/worktree.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli/worktree.ts b/src/cli/worktree.ts index 4d476d36..08e771e5 100644 --- a/src/cli/worktree.ts +++ b/src/cli/worktree.ts @@ -98,11 +98,11 @@ function checkouts(repository: string): { path: string; locked?: string }[] { }); } function childPath(path: string): string { + const parts = path.split(/[\\/]/); if ( - !path || isAbsolute(path) || - path.split(/[\\/]/).some((part) => part === ".." || part.toLowerCase() === ".git") || - resolve("/", path) === "/" + parts.some((part) => part === ".." || part.toLowerCase() === ".git") || + parts.every((part) => part === "" || part === ".") // Names the checkout itself. ) { throw new Error(`expected a relative path without .. or .git: ${path}`); } diff --git a/test/worktree.test.ts b/test/worktree.test.ts index e2a4663c..ac267651 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -95,7 +95,7 @@ describe("worktrees", { timeout: 20_000 }, () => { }); it("refuses scratch that is tracked, not ignored, or outside the checkout", () => { const { repo, worktrees } = fixture(); - for (const path of ["../outside", ".git", ".GIT", "/absolute", "."]) + for (const path of ["../outside", ".git", ".GIT", "/absolute", ".", "./", ""]) expect(() => worktrees.create(repo, "invalid", 10_000_000, "HEAD", [path])).toThrow("relative path"); let count = 0; const declare = (scratch: string) => { From 6fc09242196c908c5f40898a208f85c8c0d696d5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 15:27:35 -0700 Subject: [PATCH 4/4] fix(worktree): require cgroup.kill instead of waiting forever Before Linux 5.14 a scope's cgroup has no cgroup.kill, so leftover descendants were never killed and the supervisor waited on a populated scope forever. run now refuses such hosts up front, and kill() ignores a missing cgroup.kill only once the cgroup itself is gone, without ever creating the file. Retitle the worktree quest [M] and point the Ctrl-C follow-up at kixelated/quest#180. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- quest/m0/worktree.md | 5 +++-- src/cli/main.ts | 2 +- src/cli/supervise.ts | 30 ++++++++++++++++++++++++------ test/worktree.run.test.ts | 2 +- test/worktree.test.ts | 33 +++++++++++++++++++++++++++++++-- 6 files changed, 62 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index b8844f3a..249229af 100644 --- a/README.md +++ b/README.md @@ -428,7 +428,8 @@ job, such as an agent session or a script that installs, builds, and tests, in one run, since the next run starts without scratch. `run` owns processes through a systemd user scope per run, so it needs Linux -with a systemd 254+ user manager and unified cgroup v2; it fails on other hosts. +5.14+ with a systemd 254+ user manager and unified cgroup v2; it fails on other +hosts. 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 diff --git a/quest/m0/worktree.md b/quest/m0/worktree.md index 574e00f0..bff617d4 100644 --- a/quest/m0/worktree.md +++ b/quest/m0/worktree.md @@ -1,4 +1,4 @@ -# [S] Remove finished quest worktree checkouts +# [M] Remove finished quest worktree checkouts ## Goal @@ -38,7 +38,8 @@ live and refuses a checkout that still holds anything not already preserved. 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 in a separate gate/signal quest. + and planned separately in + [kixelated/quest#180](https://github.com/kixelated/quest/pull/180). - 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. diff --git a/src/cli/main.ts b/src/cli/main.ts index f320e73a..77d643e6 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -388,7 +388,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 with a systemd 254+ user manager and unified cgroup " + + "is empty, and releases the checkout. 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.", ) diff --git a/src/cli/supervise.ts b/src/cli/supervise.ts index bbdd72a6..93875f5e 100644 --- a/src/cli/supervise.ts +++ b/src/cli/supervise.ts @@ -2,7 +2,7 @@ // named once and never reused, so its cgroup holds every descendant that does not ask another // service manager to start it. Other hosts fail explicitly rather than guess from process groups. import { execFileSync, spawn } from "node:child_process"; -import { readFileSync, watch, writeFileSync } from "node:fs"; +import { closeSync, existsSync, constants as fs, openSync, readFileSync, watch, writeSync } from "node:fs"; import { constants, hostname } from "node:os"; import { join } from "node:path"; import type { Readable, Writable } from "node:stream"; @@ -62,7 +62,7 @@ export interface Scopes { export class Retained extends Error {} export const UNSUPPORTED = - "quest worktree run needs Linux with a systemd user manager (systemd 254 or later) and unified cgroup v2"; + "quest worktree run needs Linux 5.14 or later with a systemd user manager (systemd 254 or later) and unified cgroup v2"; /** * The shell proves it runs in the scope's own v2 cgroup, then waits for the supervisor before it @@ -116,18 +116,36 @@ function vacated(group: string): Promise { }); } +/** + * Whether this process's unified cgroup offers `cgroup.kill` (Linux 5.14), which stops every + * descendant in a scope at once. Without it a run's scope could never be emptied. + */ +export function killable(cgroups = "/sys/fs/cgroup", self = "/proc/self/cgroup"): boolean { + const unified = readFileSync(self, "utf8") + .split("\n") + .find((line) => line.startsWith("0::")); + return unified !== undefined && existsSync(join(cgroups, unified.slice(3), "cgroup.kill")); +} + /** SIGKILL every process in the cgroup; one already removed has none. */ -function kill(group: string): void { +export function kill(group: string): void { try { - writeFileSync(join(group, "cgroup.kill"), "1"); + // Never create the file: a missing one means the cgroup is gone or cannot be killed. + const fd = openSync(join(group, "cgroup.kill"), fs.O_WRONLY); + try { + writeSync(fd, "1"); + } finally { + closeSync(fd); + } } catch (error) { - if (!["ENOENT", "ENODEV"].includes((error as NodeJS.ErrnoException).code ?? "")) throw error; + const code = (error as NodeJS.ErrnoException).code ?? ""; + if (!["ENOENT", "ENODEV"].includes(code) || existsSync(group)) throw error; } } export const systemdScopes: Scopes = { check() { - if (process.platform !== "linux") throw new Error(UNSUPPORTED); + if (process.platform !== "linux" || !killable()) throw new Error(UNSUPPORTED); }, live(unit) { diff --git a/test/worktree.run.test.ts b/test/worktree.run.test.ts index a76c81ca..5e8953fc 100644 --- a/test/worktree.run.test.ts +++ b/test/worktree.run.test.ts @@ -1,4 +1,4 @@ -// Supervised runs against the host's real systemd user manager. On Linux these need systemd 254+ +// Supervised runs against the host's real systemd user manager. On Linux these need 5.14+, systemd 254+ // and unified cgroup v2; a host without them fails here rather than skipping. import { execFileSync, spawn, spawnSync } from "node:child_process"; import { once } from "node:events"; diff --git a/test/worktree.test.ts b/test/worktree.test.ts index ac267651..bedb486e 100644 --- a/test/worktree.test.ts +++ b/test/worktree.test.ts @@ -15,7 +15,7 @@ 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 } from "../src/cli/supervise"; +import { Retained, type Scopes, currentOwner, kill, killable } from "../src/cli/supervise"; import { main } from "../src/cli/main"; import { Worktrees, diskUsage } from "../src/cli/worktree"; import { run } from "./fixture"; @@ -410,7 +410,36 @@ describe("supervised runs", { timeout: 20_000 }, () => { }, }); expect(code).toBe(1); - expect(stderr).toContain("needs Linux with a systemd user manager"); + expect(stderr).toContain("needs Linux 5.14 or later with a systemd user manager"); expect(new Worktrees(home).status().records[0].runs).toEqual([]); }); }); + +describe("cgroup kill", () => { + it("requires cgroup.kill in this process's unified cgroup", () => { + const root = mkdtempSync(join(tmpdir(), "quest-cgroup-test-")); + roots.push(root); + const self = join(root, "self"); + const cgroups = join(root, "cgroup"); + mkdirSync(join(cgroups, "user.slice"), { recursive: true }); + writeFileSync(self, "0::/user.slice\n"); + // Before Linux 5.14 the cgroup exists without cgroup.kill, so a run could never empty its scope. + expect(killable(cgroups, self)).toBe(false); + writeFileSync(join(cgroups, "user.slice", "cgroup.kill"), ""); + expect(killable(cgroups, self)).toBe(true); + // A hybrid hierarchy lists v1 controllers, and /sys/fs/cgroup is not its unified mount. + writeFileSync(self, "1:name=systemd:/user.slice\n"); + expect(killable(cgroups, self)).toBe(false); + }); + + it("ignores a missing cgroup.kill only once the cgroup is gone", () => { + const root = mkdtempSync(join(tmpdir(), "quest-cgroup-test-")); + roots.push(root); + const group = join(root, "run.scope"); + mkdirSync(group); + expect(() => kill(group)).toThrow("ENOENT"); + expect(existsSync(join(group, "cgroup.kill"))).toBe(false); + rmSync(group, { recursive: true }); + expect(() => kill(group)).not.toThrow(); + }); +});