From 5e0a9603063f6607bac1875b77da75dafae4b4db Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 22:39:07 -0700 Subject: [PATCH 1/5] chore(quest): claim quest/m0/gate-headroom.md Quest-Claim: 264968c8b3e93113 From 98fb0333f8d8993214c29cdbedfcb30acc7b159b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 23:09:48 -0700 Subject: [PATCH 2/5] feat(gate): admit at once while the host has headroom Each admission from the last minute counts as one more unit of load and one command's memory budget. Commands start at once while load and memory still fit counting those, one at a time SETTLE_MS after the previous admission while only the raw measures fit, and otherwise wait. Admissions live in a lock-free ledger of numbered versions published by hard-linking a complete draft, so concurrent callers never exceed the rule, a burst always elects a winner, and a crashed caller blocks no one. Every caller of one user on one host shares ~/.cache/quest/admissions/, whatever its environment or login session. A terminal Ctrl-C now reaches the child of `quest gate` and `quest worktree run` once: SIGINT is not forwarded while Quest is in the terminal's foreground process group; SIGHUP and SIGTERM always are. Co-Authored-By: Claude Opus 5.5 --- README.md | 23 ++- quest/m0/README.md | 1 - quest/m0/gate-headroom.md | 99 ----------- quest/m0/worktree.md | 3 - scripts/gate-bench.sh | 20 ++- src/cli/gate.ts | 217 +++++++++++++++++++----- src/cli/interrupt.ts | 41 +++++ src/cli/main.ts | 6 +- src/cli/supervise.ts | 6 +- test/fixture.ts | 29 ++++ test/gate.test.ts | 340 ++++++++++++++++++++++++++++---------- 11 files changed, 533 insertions(+), 252 deletions(-) delete mode 100644 quest/m0/gate-headroom.md create mode 100644 src/cli/interrupt.ts diff --git a/README.md b/README.md index e4355494..9c6b353e 100644 --- a/README.md +++ b/README.md @@ -380,12 +380,17 @@ available memory is below one command's budget: 4 GiB, or a quarter of RAM on smaller hosts, where RAM is capped by any container memory limit. On Linux, available memory includes reclaimable host and cgroup file cache and is bounded by the remaining budget of each enclosing memory cgroup. Load and memory count -work from every session and user. The load average lags new work, so the gate -admits at most one waiting command per 5-second window per host, never in -consecutive windows, rather than releasing every waiter at once: a burst of -waiters starts about 10 seconds apart. Windows reports no load -average and macOS counts reclaimable cache as used memory, so those signals are -skipped there. +work from every session and user. Windows reports no load average and macOS +counts reclaimable cache as used memory, so those signals are skipped there. + +Both measures lag new work, so each command admitted in the last minute counts +as one more unit of load and takes one budget from available memory. While the +host has room even counting those, commands start at once: a burst of gated +test runs on an idle host starts together. Once only the raw load and memory +have room, commands start one at a time, 5 seconds after the previous admission. +A user's admissions on one host are recorded under +`~/.cache/quest/admissions/`, whatever the caller's environment or +login session. Admission is the gate's only job: it holds no slot or lock while the command runs and does not track the command's descendants. A nested invocation runs @@ -396,8 +401,10 @@ This repository's `just check`, `just test`, `just worker-check`, and `just worker-test` run through the gate. Arguments, working directory, environment and terminal streams pass through to -the executable, and waiting happens before it starts. There is no fairness -guarantee or timeout: a command waits as long as the host stays busy. +the executable, and waiting happens before it starts. A terminal's Ctrl-C +reaches the command once, directly; SIGTERM and SIGHUP sent to Quest are +forwarded. There is no fairness guarantee or timeout: a command waits as long as +the host stays busy. ## Headless agents diff --git a/quest/m0/README.md b/quest/m0/README.md index 8387cfbe..23b57535 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -35,7 +35,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/gate-headroom.md b/quest/m0/gate-headroom.md deleted file mode 100644 index 6dfe24a4..00000000 --- a/quest/m0/gate-headroom.md +++ /dev/null @@ -1,99 +0,0 @@ -# [M] Admit gated commands at once while the host has room - -## Goal - -`quest gate` adds no noticeable delay while the host has room, and still -spaces admissions out as the host nears its CPU or memory limit. A burst of -gated `just test` runs on an idle host starts together, instead of about -10 seconds apart. - -## Plan - -Decided 2026-10-21 (user) after the contention measurements in -[kixelated/quest#177](https://github.com/kixelated/quest/pull/177), reproducible -with `scripts/gate-bench.sh`. On a 24-core host, Worker suites only timed out -at about 8 suites per CPU, where the gate prevented the failures at about -1.3× the ungated makespan. With room to spare, the gate only added waiting: a -burst of 8 took 104 s gated versus 19 s ungated. - -- **Admit with headroom (user).** The load average and free memory both lag - new work, so count each admission from the last minute as one more unit of - load and as one command's memory budget taken from available memory. Then: - 1. Admit at once while load plus recent admissions stays under the CPU - count, and available memory minus the recent admissions' budgets still - covers one command. - 2. Once recent admissions tip either measure over, but the load alone is at - or under the CPU count and available memory alone covers one command, - admit one command at a time, `SETTLE_MS` apart. Spaced admissions count - as recent too. - 3. Otherwise wait, as today. - - This reverses the 2026-10-15 "no adaptive scheduler" decision. Memory - admission stays, because heavier repositories (Rust builds) are what risk - OOM; counting recent admissions against it keeps a burst from passing on - one shared free-memory sample. Rejected: fixing the spacing only, taking - the gate out of `just check` and `just test`, and keeping today's delay. -- **Admission requirements.** Today a claim is one `O_EXCL` file per 5-second - window, and each admission deletes the older ones; the replacement for - `Host.claim(window)` is left to the implementation, which must meet these: - - Count and claim atomically across processes, with several admissions per - window and each admission remembered for a minute. - - Never admit past the three-band rule, and always admit some waiter when - the rule has room. - - Recover from a caller that crashes mid-admission on every platform the - gate runs on, without blocking admission indefinitely. - - Bound the effect of a wall-clock step in either direction to about one - record lifetime. - - Space band-2 admissions at least `SETTLE_MS` apart, measured from the - previous admission's time (decision 8 in - [kixelated/quest#128](https://github.com/kixelated/quest/pull/128)), with - waiters waking when that interval ends rather than at the next window - boundary, so queued commands stop starting about 10 s apart. - - Update the README's "Gate expensive commands" admission paragraph and the - `admit()` doc comment, which both describe today's behaviour. -- **Hazards to test.** Each came up in review of the plan in - [kixelated/quest#180](https://github.com/kixelated/quest/pull/180): - - A burst passing one shared free-memory sample, so every caller fits a - budget that only one does. - - A burst that all reads "0 recent" before any of them records. - - Two admissions milliseconds apart across a window boundary. - - Expired records leaving holes in numbered slots, so a slot number - undercounts recent admissions. - - Every caller in an over-capacity burst recording, seeing no room, and - withdrawing, then repeating that together forever. - - A dead lock holder's PID reused on macOS or Windows, where only Linux - records a process start time, keeping the lock held indefinitely. - - A backward clock step leaving records dated far in the future, which - count as recent indefinitely or are discarded while still valid. - - A forward clock step past the record lifetime, expiring records early and - dropping their spacing and reservations. - - `performance.now()` differing per process, and `CLOCK_MONOTONIC` - restarting at boot while the fallback claim directory persists. -- **Stable claim directory.** Callers with and without `XDG_RUNTIME_DIR` use - different claim directories today. Every caller of one user on one host - must use the same claim location, one that never switches with the - caller's environment or with login sessions, and that stays per-host when - home directories are shared. `gate-bench.sh overhead` needs another way to - get a fresh directory. Hazards to test: callers with and without - `XDG_RUNTIME_DIR`, and `/run/user/` appearing or disappearing as logind - sessions start and end while background callers keep running. -- **Ctrl-C once.** In both `quest gate` and `quest worktree run` - (`src/cli/supervise.ts`), a terminal Ctrl-C reaches the child directly and - again through the forwarder. Share one fix between them. On Unix, while - Quest is in the terminal's foreground process group, skip forwarding only - SIGINT, which the terminal also delivers to the child; always forward SIGHUP - and SIGTERM, which may be sent to Quest alone. A SIGINT sent with `kill` to - Quest alone is then not forwarded. On Windows, - Ctrl-C reaches every process on the console and Node's `child.kill()` - terminates the child forcefully, so `quest gate` waits for the child - instead of forwarding; `quest worktree run` is Linux-only. -- Re-run `scripts/gate-bench.sh` for the idle-burst and 8× cases and record - both results in the PR. Expect the pinned 8× case to look ungated: it - confines 16 suites to two CPUs with `taskset`, while headroom is measured - across the host, so most start at once and the timeouts from - [kixelated/quest#177](https://github.com/kixelated/quest/pull/177) likely - return. That follows from the decision and is not a regression. - -## Related - -- [Agent lifecycle integration](/quest/m0/worktree-agents.md) - removes agent-count throttles once resource control is in place diff --git a/quest/m0/worktree.md b/quest/m0/worktree.md index bbe0857a..e8571bb3 100644 --- a/quest/m0/worktree.md +++ b/quest/m0/worktree.md @@ -36,9 +36,6 @@ live and refuses a checkout that still holds anything not already preserved. 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. diff --git a/scripts/gate-bench.sh b/scripts/gate-bench.sh index 1d081dbc..ab0c1eb8 100755 --- a/scripts/gate-bench.sh +++ b/scripts/gate-bench.sh @@ -2,8 +2,8 @@ # Measure `quest gate` under contention. Run inside `nix develop`. # # scripts/gate-bench.sh overhead [runs] -# Admission cost: `quest gate true` with a fresh claim directory, so no -# window is ever taken, against the same call with the gate bypassed. +# Admission cost: `quest gate true` with the gate bypassed, and admission +# alone against a fresh ledger, so no earlier run counts as recent. # scripts/gate-bench.sh gated|ungated [cpus] # Start Worker test suites at once, each in its own detached # worktree of HEAD, through the gate or bypassing it. `cpus` is a taskset @@ -45,17 +45,23 @@ stats() { } overhead() { - local runs=${1:-20} i start bypass=() gated=() + local runs=${1:-20} i start bypass=() admit=() cd "$root" for ((i = 0; i < runs; i++)); do start=$(now_ms) QUEST_GATE_ADMITTED=1 bun run --silent quest gate true bypass+=($(($(now_ms) - start))) - start=$(now_ms) - XDG_RUNTIME_DIR="$work/claims.$i" bun run --silent quest gate true - gated+=($(($(now_ms) - start))) done - echo "runs=$runs $(stats bypass_ms "${bypass[@]}")$(stats gated_ms "${gated[@]}")" + # Every caller shares one ledger, so time admission itself against a fresh one per run. + # shellcheck disable=SC2016 # Bun expands it. + mapfile -t admit < <(BENCH_GATE="$root/src/cli/gate.ts" BENCH_WORK="$work" BENCH_RUNS="$runs" bun --eval ' + const { admit, fileHost, hostLimits } = await import(process.env.BENCH_GATE); + for (let i = 0; i < Number(process.env.BENCH_RUNS); i++) { + const start = performance.now(); + await admit(hostLimits(), fileHost(`${process.env.BENCH_WORK}/ledger.${i}`), new AbortController().signal); + console.log(Math.round(performance.now() - start)); + }') + echo "runs=$runs $(stats bypass_ms "${bypass[@]}")$(stats admit_ms "${admit[@]}")" } contend() { diff --git a/src/cli/gate.ts b/src/cli/gate.ts index f4d17a97..91276368 100644 --- a/src/cli/gate.ts +++ b/src/cli/gate.ts @@ -2,10 +2,24 @@ // for CPU load and free memory before starting the command. The gate owns no // descendants and holds nothing while the command runs. import { spawn } from "node:child_process"; -import { closeSync, existsSync, mkdirSync, openSync, readdirSync, readFileSync, rmSync } from "node:fs"; -import { availableParallelism, constants, cpus, homedir, hostname, loadavg, totalmem } from "node:os"; +import { randomUUID } from "node:crypto"; +import { linkSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { availableParallelism, constants, cpus, homedir, hostname, loadavg, totalmem, userInfo } from "node:os"; import { dirname, join, relative, resolve, sep } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; +import { forwards } from "./interrupt"; + +/** One admitted command: a unique id and the wall-clock time it was admitted. */ +export interface Admission { + id: string; + at: number; +} + +/** One version of the host's admission ledger. Versions only grow; 0 is the empty ledger. */ +export interface Ledger { + version: number; + admissions: Admission[]; +} /** The machine-wide inputs to load admission, replaceable in tests. */ export interface Host { @@ -13,9 +27,15 @@ export interface Host { load(): number; /** Bytes available to a new command without swapping. */ memory(): number; + /** Wall-clock milliseconds, shared by every process on the host. */ now(): number; - /** Atomically claim an admission window; false when another caller took it. */ - claim(window: number): boolean; + /** The newest ledger. */ + ledger(): Ledger; + /** + * Atomically publish `admissions` as the version after `base`. False when another caller + * published since `base` was read, so the caller reads the ledger again and decides afresh. + */ + publish(base: Ledger, admissions: Admission[]): boolean; sleep(ms: number, signal: AbortSignal): Promise; } @@ -29,11 +49,17 @@ export interface Limits { export type Waiting = "load" | "memory"; /** - * Admissions across all callers are at least this far apart. The load average - * lags new work, so waiters that all see it fall must not start together. + * Admissions that only fit the host's raw load and memory, not its headroom, + * start at least this far apart, so each one's load can begin to show. */ export const SETTLE_MS = 5000; +/** + * How long an admission counts against the host's headroom. The 1-minute load + * average and available memory take about this long to reflect a new command. + */ +export const RECENT_MS = 60_000; + /** The memory one gated build or test stack is assumed to need. */ export const COMMAND_MEMORY = 4 * 2 ** 30; @@ -124,19 +150,74 @@ export function linuxMemory(proc = "/proc", cgroups = "/sys/fs/cgroup"): { total return { total, available }; } +/** The account's home directory from the user database, not `$HOME`, which callers may override. */ +function accountHome(): string { + try { + return userInfo().homedir; + } catch { + return homedir(); // An account missing from the user database, as in some containers. + } +} + /** - * Claims belong to this host, since a home directory may be shared between - * machines: the user's runtime directory where one exists, otherwise a - * per-host cache directory. + * Every caller of one user on one host shares this directory, whatever its + * environment or login sessions. It is named for the host, since a home + * directory may be shared between machines. */ -export function claimDirectory(env = process.env, home = homedir(), host = hostname()): string { - return env.XDG_RUNTIME_DIR - ? join(env.XDG_RUNTIME_DIR, "quest", "gate") - : join(home, ".cache", "quest", "gate", host); +export function claimDirectory(home = accountHome(), host = hostname()): string { + return join(home, ".cache", "quest", "admissions", host); +} + +/** A version cut short, as by power loss, is read as empty: its commands ended with the host. */ +function parse(text: string): Admission[] { + let value: unknown; + try { + value = JSON.parse(text); + } catch { + return []; + } + const valid = + Array.isArray(value) && + value.every((entry: Partial) => typeof entry?.id === "string" && Number.isFinite(entry?.at)); + return valid ? (value as Admission[]) : []; +} + +/** Remove a file that another caller may already have removed, or on Windows may still hold open. */ +function remove(file: string) { + try { + rmSync(file, { force: true }); + } catch (error) { + if (!["EPERM", "EBUSY"].includes((error as NodeJS.ErrnoException).code ?? "")) throw error; + } } -/** Claims are empty files, named by window, in one per-user, per-host directory. */ +/** + * The ledger is a directory of numbered versions, each the JSON list of recent + * admissions. A caller publishes version N + 1 by linking a complete draft to + * that name, which only one caller can create. No lock is held, so a caller + * that dies at any point leaves nothing that blocks the others. + */ export function fileHost(directory: string, now = Date.now): Host { + const path = (version: number) => join(directory, String(version)); + const ledger = (): Ledger => { + for (;;) { + let names: string[]; + try { + names = readdirSync(directory); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return { version: 0, admissions: [] }; + throw error; + } + const version = names.reduce((max, name) => (/^\d+$/.test(name) ? Math.max(max, Number(name)) : max), 0); + if (version === 0) return { version, admissions: [] }; + try { + return { version, admissions: parse(readFileSync(path(version), "utf8")) }; + } catch (error) { + // Superseded and pruned since the listing. + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + } + }; return { load: () => loadavg()[0], // macOS counts reclaimable cache as used, so only its load is gated. @@ -147,28 +228,42 @@ export function fileHost(directory: string, now = Date.now): Host { ? linuxMemory().available : process.availableMemory(), now, - claim(window) { - if (window !== Math.floor(now() / SETTLE_MS)) return false; + ledger, + publish(base, admissions) { mkdirSync(directory, { recursive: true }); - const file = join(directory, String(window)); + const version = base.version + 1; + const draft = join(directory, `${randomUUID()}.tmp`); + writeFileSync(draft, JSON.stringify(admissions)); try { - closeSync(openSync(file, "wx")); + linkSync(draft, path(version)); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "EEXIST") return false; + // EEXIST: another caller published first. ENOENT: a prune took the draft after a clock step. + if (["EEXIST", "ENOENT"].includes((error as NodeJS.ErrnoException).code ?? "")) return false; throw error; + } finally { + remove(draft); } - // The previous window's claim may have admitted a command moments ago, - // so admissions stay a full window apart. Only an admitted claim removes - // older ones, and none can be admitted while this one exists, so that - // claim is still here. A descheduled caller may also have created an old - // claim after cleanup; it must not admit work from that expired sample. - // A refused claim is removed, so it never holds back the next window. - if (existsSync(join(directory, String(window - 1))) || window !== Math.floor(now() / SETTLE_MS)) { - rmSync(file, { force: true }); + // A caller that stalled after reading can recreate a version already pruned below the + // newest. That copy is outside the ledger's history, so it admitted no one. + const newest = ledger(); + const added = admissions.filter((admission) => !base.admissions.some(({ id }) => id === admission.id)); + const kept = + newest.version === version || + (added.length > 0 && + added.every((admission) => newest.admissions.some(({ id }) => id === admission.id))); + if (!kept) { + remove(path(version)); return false; } + // Older versions, and drafts left by callers that died before publishing. for (const name of readdirSync(directory)) { - if (Number(name) < window) rmSync(join(directory, name), { force: true }); + const file = join(directory, name); + try { + if (/^\d+$/.test(name) ? Number(name) < newest.version : statSync(file).mtimeMs < now() - RECENT_MS) + remove(file); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } } return true; }, @@ -177,19 +272,53 @@ export function fileHost(directory: string, now = Date.now): Host { } /** - * Wait until the host has room for a command and this caller wins the current - * window. No fairness promise, adaptive scheduler, or admission timeout. + * Wait until the host has room for a command, then record its admission. The + * load average and available memory lag new work, so each admission from the + * last minute counts as one more unit of load and as one command's memory + * taken from available memory: + * + * 1. While load and memory have room even counting those, admit at once. + * 2. While only the raw load and memory have room, admit one command at a + * time, SETTLE_MS after the previous admission. + * 3. Otherwise wait. + * + * Each caller decides against the newest ledger and publishes its admission + * atomically, so concurrent callers never exceed the rule and one always wins + * while it has room. No fairness promise or admission timeout. */ export async function admit(limits: Limits, host: Host, signal: AbortSignal, waiting?: (reason: Waiting) => void) { + const id = randomUUID(); for (;;) { signal.throwIfAborted(); + const ledger = host.ledger(); + // Sampled after the read, so only a backward clock step dates an admission in the future. const now = host.now(); - const window = Math.floor(now / SETTLE_MS); - if (host.load() > limits.load) waiting?.("load"); - else if (host.memory() < limits.memory) waiting?.("memory"); - else if (host.claim(window)) return; - // Waiters wake together at the boundary, and exactly one claims the window. - await host.sleep((window + 1) * SETTLE_MS - now, signal); + const recent = ledger.admissions.filter((admission) => admission.at > now - RECENT_MS); + if (recent.some((admission) => admission.at > now)) { + // Date those now, so they still count and space admissions but expire a minute after + // the step. A forward step can only expire admissions up to a minute early. + host.publish( + ledger, + recent.map((admission) => ({ id: admission.id, at: Math.min(admission.at, now) })), + ); + continue; + } + const load = host.load(); + const memory = host.memory(); + let wait = SETTLE_MS; + if (load > limits.load) waiting?.("load"); + else if (memory < limits.memory) waiting?.("memory"); + else { + const headroom = + load + recent.length < limits.load && memory - recent.length * limits.memory >= limits.memory; + const last = Math.max(-Infinity, ...recent.map((admission) => admission.at)); + wait = headroom ? 0 : last + SETTLE_MS - now; + if (wait <= 0) { + if (host.publish(ledger, [...recent, { id, at: now }])) return; + continue; + } + } + await host.sleep(wait, signal); } } @@ -198,8 +327,9 @@ const ADMITTED = "QUEST_GATE_ADMITTED"; /** * Run a command in place, with inherited streams and its exact exit status. - * Aborting with a signal name forwards that signal, so the command's own - * handling of it runs; any other reason sends SIGTERM. + * Aborting with a signal name forwards that signal unless the command already + * has it (see `forwards`), so the command's own handling of it runs; any other + * reason sends SIGTERM. */ export function run(command: string[], signal: AbortSignal): Promise { return new Promise((resolve, reject) => { @@ -207,12 +337,11 @@ export function run(command: string[], signal: AbortSignal): Promise { env: { ...process.env, [ADMITTED]: "1" }, stdio: "inherit", }); - const cancel = () => - child.kill( - typeof signal.reason === "string" && signal.reason in constants.signals - ? (signal.reason as NodeJS.Signals) - : "SIGTERM", - ); + const cancel = () => { + const name = signal.reason; + if (typeof name !== "string" || !(name in constants.signals)) child.kill("SIGTERM"); + else if (forwards(name as NodeJS.Signals)) child.kill(name as NodeJS.Signals); + }; signal.addEventListener("abort", cancel, { once: true }); if (signal.aborted) cancel(); child.once("error", (error) => { diff --git a/src/cli/interrupt.ts b/src/cli/interrupt.ts new file mode 100644 index 00000000..b5cfe38e --- /dev/null +++ b/src/cli/interrupt.ts @@ -0,0 +1,41 @@ +// Signal forwarding to a child that runs in Quest's own process group, shared by `quest gate` +// and `quest worktree run`. +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; + +/** Whether /proc//stat's process group (field 5) is its terminal's foreground group (field 8). */ +export function foregroundStat(stat: string): boolean { + // The command name in parentheses may hold spaces. + const fields = stat.slice(stat.lastIndexOf(")") + 2).split(" "); + return fields[2] === fields[5]; +} + +/** Whether this process is in its controlling terminal's foreground process group. */ +function foreground(): boolean { + if (process.platform === "linux") return foregroundStat(readFileSync("/proc/self/stat", "utf8")); + try { + const [terminal, group] = execFileSync("ps", ["-o", "tpgid=,pgid=", "-p", String(process.pid)], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }) + .trim() + .split(/\s+/); + return terminal === group; + } catch { + return false; // Forward, as for a process without a terminal. + } +} + +/** + * Whether to pass a signal Quest received on to its child. A terminal's Ctrl-C + * sends SIGINT to the whole foreground process group, child included, so + * forwarding it would deliver it twice. SIGHUP and SIGTERM may be sent to Quest + * alone, so they always pass; so does SIGINT while Quest is in the background. + * A SIGINT sent with `kill` to a foreground Quest alone is therefore not + * forwarded. Windows delivers Ctrl-C to every process on the console and has + * no signal to forward: `child.kill()` terminates the child outright. + */ +export function forwards(name: NodeJS.Signals): boolean { + if (process.platform === "win32") return false; + return name !== "SIGINT" || !foreground(); +} diff --git a/src/cli/main.ts b/src/cli/main.ts index 44c2a2d9..818ca72a 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -76,8 +76,10 @@ export function main(argv: string[], out: Output): number | Promise { .summary("Run an expensive command once the host has room for it") .description( "Wait while the 1-minute load average exceeds the host's CPU count or available memory is below " + - "min(4 GiB, RAM/4), admitting one waiting command every 5 seconds. Nested calls run immediately. " + - "Limits come from the host; there is no configuration.", + "min(4 GiB, RAM/4). Each admission in the last minute counts as one more unit of load and one " + + "budget less memory: commands start at once while those still fit, and 5 seconds apart while " + + "only the raw measures do. Nested calls run immediately. Limits come from the host; there is " + + "no configuration.", ) .argument("", "Executable to run") .argument("[args...]", "Arguments forwarded verbatim") diff --git a/src/cli/supervise.ts b/src/cli/supervise.ts index 93875f5e..607b9822 100644 --- a/src/cli/supervise.ts +++ b/src/cli/supervise.ts @@ -6,6 +6,7 @@ import { closeSync, existsSync, constants as fs, openSync, readFileSync, watch, import { constants, hostname } from "node:os"; import { join } from "node:path"; import type { Readable, Writable } from "node:stream"; +import { forwards } from "./interrupt"; /** A process, identified so that a reused PID never looks like its predecessor. */ export interface Owner { @@ -191,8 +192,9 @@ export const systemdScopes: Scopes = { 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. + if (admitted) { + if (forwards(name)) child.kill(name); + } else admission.destroy(); // The shell reads end-of-file and never starts the command. }; const handlers = { SIGINT: () => interrupt("SIGINT"), diff --git a/test/fixture.ts b/test/fixture.ts index 0b0b21ea..630a83ec 100644 --- a/test/fixture.ts +++ b/test/fixture.ts @@ -1,6 +1,8 @@ // A minimal but complete quest tree on disk: root epic -> milestone -> // epic -> two quests, one blocking the other. +import { spawn } from "node:child_process"; +import { once } from "node:events"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -181,3 +183,30 @@ export function runOn( if (typeof code !== "number") throw new Error("use await main for asynchronous CLI commands"); return { code, stdout, stderr }; } + +/** + * Run `command` as the foreground job of a new pseudo-terminal, through util-linux `script`, + * and type `keys` once the terminal shows "ready". Resolves with everything the terminal showed. + */ +export async function onTerminal( + command: string[], + keys: string, + env: NodeJS.ProcessEnv = process.env, +): Promise<{ code: number | null; output: string }> { + const quoted = command.map((word) => `'${word.replaceAll("'", "'\\''")}'`).join(" "); + const child = spawn("script", ["--quiet", "--return", "--flush", "--command", quoted, "/dev/null"], { + env: { ...env, SHELL: "/bin/sh" }, + stdio: ["pipe", "pipe", "inherit"], + }); + let output = ""; + let typed = false; + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (data: string) => { + output += data; + if (typed || !output.includes("ready")) return; + typed = true; + child.stdin.write(keys); + }); + const [code] = await once(child, "exit"); + return { code, output }; +} diff --git a/test/gate.test.ts b/test/gate.test.ts index bc70af9a..ac8c667a 100644 --- a/test/gate.test.ts +++ b/test/gate.test.ts @@ -1,6 +1,9 @@ -import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { spawn } from "node:child_process"; +import { once } from "node:events"; +import { mkdirSync, readdirSync, renameSync, utimesSync, writeFileSync } from "node:fs"; import { constants } from "node:os"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; import { afterEach, expect, test, vi } from "vitest"; import { admit, @@ -11,11 +14,13 @@ import { type Host, hostLimits, linuxMemory, + RECENT_MS, run as runCommand, SETTLE_MS, } from "../src/cli/gate"; +import { forwards, foregroundStat } from "../src/cli/interrupt"; import { gateBoundary, main } from "../src/cli/main"; -import { run, tempDir } from "./fixture"; +import { onTerminal, run, tempDir } from "./fixture"; const GiB = 2 ** 30; @@ -30,88 +35,220 @@ test("limits derive from the host's CPUs and memory", () => { expect(hostLimits().load).toBeGreaterThanOrEqual(1); }); -test("load and memory pressure wait, then admit one caller per settle window", async () => { - const events: string[] = []; - const taken = new Set([4]); // Another waiter won the first window once the host had room. - let now = 2 * SETTLE_MS + 500; - let load = 9; - let memory = 0; - const host: Host = { - load: () => load, - memory: () => memory, - now: () => now, - claim: (window) => { - events.push(`claim ${window}`); - if (taken.has(window)) return false; - taken.add(window); - return true; - }, +/** A real ledger in `directory`, with load, memory, and a clock the test controls. */ +function testHost( + directory: string, + state: { now: number; load: number; memory: number }, + events: string[] = [], +): Host { + return { + ...fileHost(directory, () => state.now), + load: () => state.load, + memory: () => state.memory, sleep: async (ms) => { events.push(`sleep ${ms}`); - now += ms; - if (load > 4) load = 3; - else memory = GiB; + state.now += ms; }, }; - const signal = new AbortController().signal; +} + +const signal = new AbortController().signal; + +test("an idle host admits a burst at once, then spaces admissions from the previous one", async () => { + const directory = tempDir(); + const state = { now: 5 * SETTLE_MS - 1, load: 1, memory: Infinity }; + const limits = { load: 4, memory: GiB }; + const sleeps = async () => { + const events: string[] = []; + await admit(limits, testHost(directory, state, events), signal); + return events; + }; + // Load 1 plus 0, 1, and 2 recent admissions stays under 4 CPUs. + for (let i = 0; i < 3; i++) expect(await sleeps()).toEqual([]); + // Now only the raw load has room: one at a time, waking as the interval ends, + // even when it crosses what used to be a window boundary. + state.now += 1; + expect(await sleeps()).toEqual([`sleep ${SETTLE_MS - 1}`]); + expect(await sleeps()).toEqual([`sleep ${SETTLE_MS}`]); + // Every admission counts as recent for a minute. + state.now += RECENT_MS; + expect(await sleeps()).toEqual([]); +}); + +test("each recent admission takes one command's memory from a shared free-memory sample", async () => { + const directory = tempDir(); + // Memory has not yet reflected the first build, so every caller reads 6 GiB. + const state = { now: 0, load: 0, memory: 6 * GiB }; + const limits = { load: 24, memory: 4 * GiB }; + const events: string[] = []; + await admit(limits, testHost(directory, state, events), signal); + expect(events).toEqual([]); + await admit(limits, testHost(directory, state, events), signal); + expect(events).toEqual([`sleep ${SETTLE_MS}`]); +}); + +test("load and memory pressure wait whatever the ledger holds", async () => { + const events: string[] = []; + const state = { now: 0, load: 9, memory: 0 }; + const host = testHost(tempDir(), state, events); + const sleep = host.sleep; + host.sleep = async (ms, signal) => { + await sleep(ms, signal); + if (state.load > 4) state.load = 3; + else state.memory = GiB; + }; await admit({ load: 4, memory: GiB }, host, signal, (reason) => events.push(reason)); - expect(events).toEqual([ - "load", - `sleep ${SETTLE_MS - 500}`, - "memory", - `sleep ${SETTLE_MS}`, - "claim 4", - `sleep ${SETTLE_MS}`, - "claim 5", - ]); + expect(events).toEqual(["load", `sleep ${SETTLE_MS}`, "memory", `sleep ${SETTLE_MS}`]); + expect(host.ledger().admissions).toEqual([{ id: expect.any(String), at: 2 * SETTLE_MS }]); }); -test("a window admits one claim and cleanup never admits an expired sample again", () => { - const directory = join(tempDir(), "claims"); - let now = 5 * SETTLE_MS; - const host = fileHost(directory, () => now); - expect(host.claim(5)).toBe(true); - expect(host.claim(5)).toBe(false); - now = 7 * SETTLE_MS; - expect(host.claim(7)).toBe(true); - expect(existsSync(join(directory, "5"))).toBe(false); - // A waiter sampled 5 before descheduling, then resumes after cleanup. - expect(host.claim(5)).toBe(false); - expect(existsSync(join(directory, "5"))).toBe(false); - expect(host.load()).toBeGreaterThanOrEqual(0); - expect(host.memory()).toBeGreaterThan(0); -}); - -test("a claim that crosses rollover during filesystem work does not admit", () => { - const directory = join(tempDir(), "claims"); - const successor = fileHost(directory, () => 6 * SETTLE_MS); - let calls = 0; - const delayed = fileHost(directory, () => { - if (++calls === 1) return 5 * SETTLE_MS; - // The pending claim may still admit, so the next window must wait for it. - expect(successor.claim(6)).toBe(false); - return 6 * SETTLE_MS; - }); - expect(delayed.claim(5)).toBe(false); - // The expired claim admitted nothing and is gone, so the next window is free. - expect(existsSync(join(directory, "5"))).toBe(false); - expect(successor.claim(6)).toBe(true); +test("a burst that all read the ledger before any records admits only what the rule allows", async () => { + const directory = tempDir(); + const state = { now: 0, load: 1, memory: Infinity }; + const limits = { load: 2, memory: GiB }; + const events: string[] = []; + const late = testHost(directory, state, events); + const publish = late.publish; + let raced = false; + late.publish = (base, admissions) => { + // Another caller read the same empty ledger and publishes first. + if (!raced) { + raced = true; + const other = testHost(directory, state); + expect(other.publish(other.ledger(), [{ id: "other", at: state.now }])).toBe(true); + } + return publish(base, admissions); + }; + await admit(limits, late, signal); + // The late caller saw the other admission once its publish failed, and spaced itself. + expect(events).toEqual([`sleep ${SETTLE_MS}`]); + expect(late.ledger().admissions.map(({ at }) => at)).toEqual([0, SETTLE_MS]); +}); + +test("a stalled caller cannot publish into a version already pruned", () => { + const directory = tempDir(); + const host = fileHost(directory, () => 0); + expect(host.publish(host.ledger(), [{ id: "a", at: 0 }])).toBe(true); + const stale = host.ledger(); + for (const id of ["b", "c"]) { + const base = host.ledger(); + expect(host.publish(base, [...base.admissions, { id, at: 0 }])).toBe(true); + } + expect(readdirSync(directory)).toEqual(["3"]); + // Version 2 is free again, but outside the ledger's history. + expect(host.publish(stale, [...stale.admissions, { id: "stale", at: 0 }])).toBe(false); + expect(readdirSync(directory)).toEqual(["3"]); + expect(host.ledger().admissions.map(({ id }) => id)).toEqual(["a", "b", "c"]); +}); + +test("a caller that dies mid-admission blocks no one past a minute", async () => { + const directory = tempDir(); + // Drafts age by file time, so this clock is the real one. + const state = { now: Date.now(), load: 0, memory: Infinity }; + const host = testHost(directory, state); + // One caller died a minute ago after publishing, another before it could link its draft. + expect(host.publish(host.ledger(), [{ id: "dead", at: state.now - RECENT_MS }])).toBe(true); + const draft = join(directory, "dead.tmp"); + writeFileSync(draft, "["); + utimesSync(draft, new Date(state.now - 2 * RECENT_MS), new Date(state.now - 2 * RECENT_MS)); + writeFileSync(join(directory, "live.tmp"), "["); + const events: string[] = []; + await admit({ load: 1, memory: 1 }, testHost(directory, state, events), signal); + expect(events).toEqual([]); + // The stale draft is gone; one still being written stays. + expect(readdirSync(directory).sort()).toEqual(["2", "live.tmp"]); + // A version cut short, as by power loss, reads as empty rather than wedging every caller. + writeFileSync(join(directory, "100"), ""); + expect(host.ledger()).toEqual({ version: 100, admissions: [] }); }); -test("admissions stay a full window apart across a window boundary", () => { - const directory = join(tempDir(), "claims"); - let now = 5 * SETTLE_MS - 1; +test("expired admissions do not count, wherever they sit in the ledger", async () => { + const directory = tempDir(); + const now = 10 * RECENT_MS; const host = fileHost(directory, () => now); - expect(host.claim(4)).toBe(true); - // A waiter at the next boundary, 1 ms later, must not start alongside it. - now = 5 * SETTLE_MS; - expect(host.claim(5)).toBe(false); - // The refused claim is removed: it admitted nothing, so it holds back no one. - expect(existsSync(join(directory, "5"))).toBe(false); - expect(host.claim(5)).toBe(false); - now = 6 * SETTLE_MS; - expect(host.claim(6)).toBe(true); - expect(existsSync(join(directory, "4"))).toBe(false); + const ages = [RECENT_MS, 1, RECENT_MS + 1, 2]; + host.publish( + host.ledger(), + ages.map((age, i) => ({ id: String(i), at: now - age })), + ); + const events: string[] = []; + await admit({ load: 3, memory: 1 }, testHost(directory, { now, load: 0, memory: Infinity }, events), signal); + // Two recent admissions plus load 0 stays under 3, so this one starts at once. + expect(events).toEqual([]); + expect(host.ledger().admissions.map(({ id }) => id)).toEqual(["1", "3", expect.any(String)]); +}); + +test("a backward clock step dates future admissions now, and they expire a minute later", async () => { + const directory = tempDir(); + const state = { now: RECENT_MS, load: 0, memory: Infinity }; + const host = testHost(directory, state); + // Admitted just before the clock stepped back by ten minutes. + host.publish(host.ledger(), [{ id: "before", at: state.now + 10 * RECENT_MS }]); + const events: string[] = []; + await admit({ load: 1, memory: 1 }, testHost(directory, state, events), signal); + // It still spaces the next admission, from the step rather than from the future. + expect(events).toEqual([`sleep ${SETTLE_MS}`]); + expect(host.ledger().admissions).toEqual([ + { id: "before", at: RECENT_MS }, + { id: expect.any(String), at: RECENT_MS + SETTLE_MS }, + ]); + state.now += RECENT_MS + SETTLE_MS; + events.length = 0; + await admit({ load: 1, memory: 1 }, testHost(directory, state, events), signal); + expect(events).toEqual([]); +}); + +test("a forward clock step forgets at most the last minute of admissions", async () => { + const directory = tempDir(); + const state = { now: 0, load: 0, memory: Infinity }; + const limits = { load: 1, memory: 1 }; + await admit(limits, testHost(directory, state), signal); + state.now += 10 * RECENT_MS; + const events: string[] = []; + // The step expires the earlier admission early, so this one starts at once… + await admit(limits, testHost(directory, state, events), signal); + expect(events).toEqual([]); + // …but admissions after the step count and space as usual. + await admit(limits, testHost(directory, state, events), signal); + expect(events).toEqual([`sleep ${SETTLE_MS}`]); +}); + +test("concurrent processes never admit past the headroom", { timeout: 30_000 }, async () => { + const directory = tempDir(); + const ledger = join(directory, "ledger"); + const go = join(directory, "go"); + const gate = fileURLToPath(new URL("../src/cli/gate.ts", import.meta.url)); + // Each caller waits for the others, then races for one of 4 CPUs' headroom until a shared deadline. + const script = ` + const { existsSync, readFileSync } = await import("node:fs"); + const { admit, fileHost } = await import(${JSON.stringify(gate)}); + console.log("ready"); + while (!existsSync(${JSON.stringify(go)})) await new Promise((resolve) => setTimeout(resolve, 2)); + const deadline = Number(readFileSync(${JSON.stringify(go)}, "utf8")); + const host = { ...fileHost(${JSON.stringify(ledger)}), load: () => 0, memory: () => Infinity }; + const signal = AbortSignal.timeout(Math.max(0, deadline - Date.now())); + await admit({ load: 4, memory: 1 }, host, signal).then(() => console.log("admitted"), () => {}); + `; + const callers = Array.from({ length: 12 }, () => { + const child = spawn(process.execPath, ["-e", script], { stdio: ["ignore", "pipe", "inherit"] }); + let stdout = ""; + child.stdout.setEncoding("utf8"); + const ready = new Promise((resolve) => + child.stdout.on("data", (data: string) => { + stdout += data; + if (stdout.includes("ready\n")) resolve(); + }), + ); + return { ready, exited: once(child, "exit").then(([code]) => ({ code, stdout })) }; + }); + await Promise.all(callers.map(({ ready }) => ready)); + // The next admission is due SETTLE_MS after the last; the deadline comes sooner. + writeFileSync(`${go}.tmp`, String(Date.now() + SETTLE_MS / 2)); + renameSync(`${go}.tmp`, go); + const results = await Promise.all(callers.map(({ exited }) => exited)); + expect(results.map(({ code }) => code)).toEqual(Array(12).fill(0)); + expect(results.filter(({ stdout }) => stdout.includes("admitted"))).toHaveLength(4); + expect(fileHost(ledger).ledger().admissions).toHaveLength(4); }); test.each(["v1", "v2"])("Linux memory counts reclaimable cache and bounds %s container headroom", (version) => { @@ -165,22 +302,24 @@ test("Linux memory reads the container cgroup mounted as the root", () => { expect(linuxMemory(proc, cgroups)).toEqual({ total: 64 * GiB, available: 24 * GiB }); }); -test("admission claims stay on this host", () => { - expect(claimDirectory({ XDG_RUNTIME_DIR: "/run/user/1000" }, "/home/u", "a")).toBe( - join("/run/user/1000", "quest", "gate"), - ); - // A home directory shared between machines keeps each host's claims apart. - expect(claimDirectory({}, "/home/u", "a")).toBe(join("/home/u", ".cache", "quest", "gate", "a")); - expect(claimDirectory({}, "/home/u", "b")).not.toBe(claimDirectory({}, "/home/u", "a")); +test("every caller on this host shares one ledger, whatever its environment or sessions", () => { + // A home directory shared between machines keeps each host's admissions apart. + expect(claimDirectory("/home/u", "a")).toBe(join("/home/u", ".cache", "quest", "admissions", "a")); + expect(claimDirectory("/home/u", "b")).not.toBe(claimDirectory("/home/u", "a")); + // A login session's runtime directory comes and goes; an overridden HOME is still this account. + const shared = claimDirectory(); + vi.stubEnv("XDG_RUNTIME_DIR", "/run/user/1000"); + expect(claimDirectory()).toBe(shared); + vi.stubEnv("XDG_RUNTIME_DIR", undefined); + vi.stubEnv("HOME", tempDir()); + expect(claimDirectory()).toBe(shared); }); test("cancelling a waiting caller rejects admission", async () => { const controller = new AbortController(); const busy: Host = { + ...fileHost(tempDir()), load: () => 9, - memory: () => Infinity, - now: () => 0, - claim: () => true, sleep: async () => controller.abort(new Error("cancelled")), }; await expect(admit({ load: 1, memory: 1 }, busy, controller.signal)).rejects.toThrow("cancelled"); @@ -207,9 +346,38 @@ test("a command cancelled before listener registration still stops", async () => // Windows has no POSIX signals to forward; kill() always terminates there. test.skipIf(process.platform === "win32")("cancellation forwards the received signal to the command", async () => { const controller = new AbortController(); - controller.abort("SIGINT"); + controller.abort("SIGHUP"); const result = await runCommand([process.execPath, "-e", "setInterval(() => {}, 1000)"], controller.signal); - expect(result).toBe(128 + constants.signals.SIGINT); + expect(result).toBe(128 + constants.signals.SIGHUP); +}); + +test.skipIf(process.platform === "win32")("only a terminal's own SIGINT is left to reach the child", () => { + // Signals that may be sent to Quest alone always pass. + expect(forwards("SIGHUP")).toBe(true); + expect(forwards("SIGTERM")).toBe(true); + // pid (comm with spaces) state ppid pgrp session tty_nr tpgid ... + expect(foregroundStat("42 (a ) b) S 1 42 42 34816 42 4194560")).toBe(true); + expect(foregroundStat("42 (a ) b) S 1 42 42 34816 77 4194560")).toBe(false); + // No controlling terminal. + expect(foregroundStat("42 (quest) S 1 42 42 0 -1 4194560")).toBe(false); +}); + +test.runIf(process.platform === "linux")("a terminal Ctrl-C reaches the gated command once", async () => { + const child = join(tempDir(), "child.js"); + writeFileSync( + child, + `let interrupts = 0; + process.on("SIGINT", () => interrupts++); + console.log("ready"); + setTimeout(() => console.log("interrupts=" + interrupts), 1000);`, + ); + const bin = fileURLToPath(new URL("../src/cli/bin.ts", import.meta.url)); + const result = await onTerminal([process.execPath, bin, "gate", process.execPath, child], "\x03", { + ...process.env, + QUEST_GATE_ADMITTED: "1", + }); + expect(result.output).toContain("interrupts=1"); + expect(result.code).toBe(128 + constants.signals.SIGINT); }); test("the gated command keeps its own flags, even ones Quest also parses", () => { From f13f8c629f7e3bebc3c7896229d9ec9446a5bc3b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 23:13:45 -0700 Subject: [PATCH 3/5] test(worktree): a terminal Ctrl-C reaches a supervised command once Co-Authored-By: Claude Opus 5.5 --- test/worktree.run.test.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/worktree.run.test.ts b/test/worktree.run.test.ts index 5e8953fc..a85ad475 100644 --- a/test/worktree.run.test.ts +++ b/test/worktree.run.test.ts @@ -6,7 +6,7 @@ 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"; +import { onTerminal, tempDir } from "./fixture"; const root = resolve(import.meta.dirname, ".."); @@ -124,4 +124,23 @@ describe.runIf(process.platform === "linux")("supervised runs under systemd", { expect(worktrees.status().records[0]).toMatchObject({ phase: "released", runs: [] }); expect(existsSync(join(record.path, "cache"))).toBe(false); }); + + it("lets a terminal Ctrl-C reach the command once", async () => { + const { dir, home } = fixture(); + const child = join(dir, "child.js"); + writeFileSync( + child, + `let interrupts = 0; + process.on("SIGINT", () => interrupts++); + console.log("ready"); + setTimeout(() => console.log("interrupts=" + interrupts), 1000);`, + ); + const command = ["bun", `--config=${join(root, "bunfig.toml")}`, join(root, "src/cli/bin.ts")]; + const result = await onTerminal([...command, "worktree", "run", "work", "--", "bun", child], "\x03", { + ...process.env, + QUEST_WORKTREE_HOME: home, + }); + expect(result.output).toContain("interrupts=1"); + expect(result.code).toBe(130); + }); }); From f11d5ab2c279b1052caf8f0372bd32d34759db39 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 08:59:27 -0700 Subject: [PATCH 4/5] fix(gate): record fresh admissions and find the home without $HOME A caller stalled for SETTLE_MS or more between sampling the host and publishing now decides again, and every admission records the time it was published rather than the time it was sampled. Bun's os.userInfo() prefers $HOME, so the ledger directory followed the caller's environment. The account's home now comes from a child process without $HOME, and an account missing from the user database uses /tmp/quest- instead of falling back to $HOME. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +++- src/cli/gate.ts | 46 +++++++++++++++++++++++++++++++++------------- test/gate.test.ts | 41 ++++++++++++++++++++++++++++++++++------- 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 9c6b353e..1500a957 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,9 @@ test runs on an idle host starts together. Once only the raw load and memory have room, commands start one at a time, 5 seconds after the previous admission. A user's admissions on one host are recorded under `~/.cache/quest/admissions/`, whatever the caller's environment or -login session. +login session. The home directory comes from the user database, not `$HOME`; +an account missing from it, as in arbitrary-UID containers, uses +`/tmp/quest-/admissions/`. Admission is the gate's only job: it holds no slot or lock while the command runs and does not track the command's descendants. A nested invocation runs diff --git a/src/cli/gate.ts b/src/cli/gate.ts index 91276368..c4104ca2 100644 --- a/src/cli/gate.ts +++ b/src/cli/gate.ts @@ -1,10 +1,10 @@ // Host-wide command admission, sized from the host itself: every platform waits // for CPU load and free memory before starting the command. The gate owns no // descendants and holds nothing while the command runs. -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { linkSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; -import { availableParallelism, constants, cpus, homedir, hostname, loadavg, totalmem, userInfo } from "node:os"; +import { availableParallelism, constants, cpus, hostname, loadavg, totalmem, userInfo } from "node:os"; import { dirname, join, relative, resolve, sep } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; import { forwards } from "./interrupt"; @@ -150,22 +150,39 @@ export function linuxMemory(proc = "/proc", cgroups = "/sys/fs/cgroup"): { total return { total, available }; } -/** The account's home directory from the user database, not `$HOME`, which callers may override. */ -function accountHome(): string { - try { - return userInfo().homedir; - } catch { - return homedir(); // An account missing from the user database, as in some containers. - } +/** + * The account's home directory from the user database, not `$HOME`, which + * callers may override; null for an account missing from it. Bun's `userInfo()` + * prefers `$HOME`, so a child without one looks the account up. + */ +export function accountHome(): string | null { + if (process.platform === "win32") return userInfo().homedir; + const script = `try { + process.stdout.write(require("node:os").userInfo().homedir); + } catch (error) { + if (error.code !== "ENOENT") throw error; + }`; + // A compiled Quest runs the script as Bun would. The root holds no project config. + const lookup = spawnSync(process.execPath, ["-e", script], { + cwd: "/", + env: { BUN_BE_BUN: "1" }, + encoding: "utf8", + }); + if (lookup.error) throw lookup.error; + if (lookup.status !== 0) throw new Error(`gate cannot look up the account's home: ${lookup.stderr.trim()}`); + return lookup.stdout || null; } /** * Every caller of one user on one host shares this directory, whatever its * environment or login sessions. It is named for the host, since a home - * directory may be shared between machines. + * directory may be shared between machines. An account missing from the user + * database, as in arbitrary-UID containers, has no home of its own, so it + * shares one keyed by its UID. */ -export function claimDirectory(home = accountHome(), host = hostname()): string { - return join(home, ".cache", "quest", "admissions", host); +export function claimDirectory(home = accountHome(), host = hostname(), uid = process.getuid?.()): string { + const root = home === null ? join("/tmp", `quest-${uid}`) : join(home, ".cache", "quest"); + return join(root, "admissions", host); } /** A version cut short, as by power loss, is read as empty: its commands ended with the host. */ @@ -314,7 +331,10 @@ export async function admit(limits: Limits, host: Host, signal: AbortSignal, wai const last = Math.max(-Infinity, ...recent.map((admission) => admission.at)); wait = headroom ? 0 : last + SETTLE_MS - now; if (wait <= 0) { - if (host.publish(ledger, [...recent, { id, at: now }])) return; + // A caller descheduled since sampling would record a stale time, which lets the + // next one skip its spacing; decide again once the stall reaches that interval. + const at = host.now(); + if (at - now < SETTLE_MS && host.publish(ledger, [...recent, { id, at }])) return; continue; } } diff --git a/test/gate.test.ts b/test/gate.test.ts index ac8c667a..9347b865 100644 --- a/test/gate.test.ts +++ b/test/gate.test.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { once } from "node:events"; import { mkdirSync, readdirSync, renameSync, utimesSync, writeFileSync } from "node:fs"; import { constants } from "node:os"; @@ -125,6 +125,25 @@ test("a burst that all read the ledger before any records admits only what the r expect(late.ledger().admissions.map(({ at }) => at)).toEqual([0, SETTLE_MS]); }); +test("a caller stalled after sampling the host decides again and records when it published", async () => { + const directory = tempDir(); + const state = { now: 0, load: 0, memory: Infinity }; + const limits = { load: 1, memory: 1 }; + const stalled = testHost(directory, state); + let stalls = 1; + stalled.memory = () => { + // Descheduled between reading the clock and publishing. + if (stalls-- > 0) state.now += SETTLE_MS; + return state.memory; + }; + await admit(limits, stalled, signal); + expect(stalled.ledger().admissions).toEqual([{ id: expect.any(String), at: SETTLE_MS }]); + // The next caller spaces itself from the publication, not from the stale sample. + const events: string[] = []; + await admit(limits, testHost(directory, state, events), signal); + expect(events).toEqual([`sleep ${SETTLE_MS}`]); +}); + test("a stalled caller cannot publish into a version already pruned", () => { const directory = tempDir(); const host = fileHost(directory, () => 0); @@ -306,13 +325,21 @@ test("every caller on this host shares one ledger, whatever its environment or s // A home directory shared between machines keeps each host's admissions apart. expect(claimDirectory("/home/u", "a")).toBe(join("/home/u", ".cache", "quest", "admissions", "a")); expect(claimDirectory("/home/u", "b")).not.toBe(claimDirectory("/home/u", "a")); - // A login session's runtime directory comes and goes; an overridden HOME is still this account. + // An account missing from the user database shares a directory keyed by its UID. + expect(claimDirectory(null, "a", 1234)).toBe(join("/tmp", "quest-1234", "admissions", "a")); + // A login session's runtime directory comes and goes; an overridden HOME is still this + // account. Runtimes may read both once at startup, so each caller is a new process. + const gate = fileURLToPath(new URL("../src/cli/gate.ts", import.meta.url)); + const script = `const { claimDirectory } = await import(${JSON.stringify(gate)}); console.log(claimDirectory());`; + const directory = (env: NodeJS.ProcessEnv) => + spawnSync(process.execPath, ["-e", script], { + env: { ...process.env, ...env }, + encoding: "utf8", + }).stdout.trim(); const shared = claimDirectory(); - vi.stubEnv("XDG_RUNTIME_DIR", "/run/user/1000"); - expect(claimDirectory()).toBe(shared); - vi.stubEnv("XDG_RUNTIME_DIR", undefined); - vi.stubEnv("HOME", tempDir()); - expect(claimDirectory()).toBe(shared); + expect(directory({ XDG_RUNTIME_DIR: "/run/user/1000" })).toBe(shared); + expect(directory({ XDG_RUNTIME_DIR: undefined })).toBe(shared); + expect(directory({ HOME: tempDir() })).toBe(shared); }); test("cancelling a waiting caller rejects admission", async () => { From c7137825ff25f6adeccbb3219040f0823e7f09a2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 09:27:47 -0700 Subject: [PATCH 5/5] fix(gate): trust only a private UID directory under /tmp Another local user can create /tmp/quest- first. Create it with mode 0700 and refuse it unless it is a real directory, owned by the UID, that no one else can write. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- src/cli/gate.ts | 23 +++++++++++++++++++---- test/gate.test.ts | 32 +++++++++++++++++++++++++++++--- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index c1d508ff..09e3bc65 100644 --- a/README.md +++ b/README.md @@ -399,7 +399,8 @@ A user's admissions on one host are recorded under `~/.cache/quest/admissions/`, whatever the caller's environment or login session. The home directory comes from the user database, not `$HOME`; an account missing from it, as in arbitrary-UID containers, uses -`/tmp/quest-/admissions/`. +`/tmp/quest-/admissions/`, and refuses that directory unless only +its UID can write it. Admission is the gate's only job: it holds no slot or lock while the command runs and does not track the command's descendants. A nested invocation runs diff --git a/src/cli/gate.ts b/src/cli/gate.ts index c4104ca2..01d151ad 100644 --- a/src/cli/gate.ts +++ b/src/cli/gate.ts @@ -3,7 +3,7 @@ // descendants and holds nothing while the command runs. import { spawn, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { linkSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { linkSync, lstatSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { availableParallelism, constants, cpus, hostname, loadavg, totalmem, userInfo } from "node:os"; import { dirname, join, relative, resolve, sep } from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; @@ -178,10 +178,25 @@ export function accountHome(): string | null { * environment or login sessions. It is named for the host, since a home * directory may be shared between machines. An account missing from the user * database, as in arbitrary-UID containers, has no home of its own, so it - * shares one keyed by its UID. + * shares one in `tmp` keyed by its UID. */ -export function claimDirectory(home = accountHome(), host = hostname(), uid = process.getuid?.()): string { - const root = home === null ? join("/tmp", `quest-${uid}`) : join(home, ".cache", "quest"); +export function claimDirectory( + home = accountHome(), + host = hostname(), + uid = process.getuid?.(), + tmp = "/tmp", +): string { + if (home !== null) return join(home, ".cache", "quest", "admissions", host); + // Other users can create this name first, so it must be a directory only this UID can write. + const root = join(tmp, `quest-${uid}`); + try { + mkdirSync(root, { mode: 0o700 }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + } + const stat = lstatSync(root); + if (!stat.isDirectory() || stat.uid !== uid || (stat.mode & 0o022) !== 0) + throw new Error(`gate cannot trust ${root}: it must be a directory that only UID ${uid} can write`); return join(root, "admissions", host); } diff --git a/test/gate.test.ts b/test/gate.test.ts index 9347b865..5031e59a 100644 --- a/test/gate.test.ts +++ b/test/gate.test.ts @@ -1,6 +1,15 @@ import { spawn, spawnSync } from "node:child_process"; import { once } from "node:events"; -import { mkdirSync, readdirSync, renameSync, utimesSync, writeFileSync } from "node:fs"; +import { + chmodSync, + mkdirSync, + readdirSync, + renameSync, + statSync, + symlinkSync, + utimesSync, + writeFileSync, +} from "node:fs"; import { constants } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -325,8 +334,6 @@ test("every caller on this host shares one ledger, whatever its environment or s // A home directory shared between machines keeps each host's admissions apart. expect(claimDirectory("/home/u", "a")).toBe(join("/home/u", ".cache", "quest", "admissions", "a")); expect(claimDirectory("/home/u", "b")).not.toBe(claimDirectory("/home/u", "a")); - // An account missing from the user database shares a directory keyed by its UID. - expect(claimDirectory(null, "a", 1234)).toBe(join("/tmp", "quest-1234", "admissions", "a")); // A login session's runtime directory comes and goes; an overridden HOME is still this // account. Runtimes may read both once at startup, so each caller is a new process. const gate = fileURLToPath(new URL("../src/cli/gate.ts", import.meta.url)); @@ -342,6 +349,25 @@ test("every caller on this host shares one ledger, whatever its environment or s expect(directory({ HOME: tempDir() })).toBe(shared); }); +// An account missing from the user database has a UID, which Windows lacks. +test.skipIf(process.platform === "win32")("an account without a home shares a private directory by UID", () => { + const uid = process.getuid!(); + const tmp = tempDir(); + const root = join(tmp, `quest-${uid}`); + expect(claimDirectory(null, "a", uid, tmp)).toBe(join(root, "admissions", "a")); + expect(statSync(root).mode & 0o777).toBe(0o700); + expect(claimDirectory(null, "a", uid, tmp)).toBe(join(root, "admissions", "a")); + // Another user may create the name first, writable by them or owned by them. + chmodSync(root, 0o777); + expect(() => claimDirectory(null, "a", uid, tmp)).toThrow("only UID"); + chmodSync(root, 0o700); + expect(() => claimDirectory(null, "a", uid + 1, tmp)).toThrow("only UID"); + // Or point it elsewhere. + const elsewhere = tempDir(); + symlinkSync(root, join(elsewhere, `quest-${uid}`)); + expect(() => claimDirectory(null, "a", uid, elsewhere)).toThrow("only UID"); +}); + test("cancelling a waiting caller rejects admission", async () => { const controller = new AbortController(); const busy: Host = {