From f250a1091df42df40d20a3f0bfe16fe8605bfae3 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:08:06 -0700 Subject: [PATCH 1/7] feat(seats): a git worktree per supervised run, cleaned up on the run's verdict ctx.workspace = {repo, base?, root?} gives a run a detached worktree under ~/.jellymoon/worktrees/ (root injectable), locked with the reason "jellymoon run " and marked by /.json. The adapter gets its path as ctx.cwd. At settle a clean worktree (no changes, no untracked files, HEAD still at base) is unlocked and removed; anything else stays locked and its path is returned. An abandoned run (adapter ignored the cancel) keeps its worktree locked for sweepWorktrees({root, isRunLive}), which releases dead runs' locks, removes only clean ones, and never touches an unmarked or foreign-locked worktree. Adapters declaring nativeWorktree get ctx.workspace unchanged and no helper worktree. Base/head SHAs ride as an optional `workspace` field on attempt.started and run.settled (record v1 allows optional fields; no new kind); the host path stays out of the record. Released from hold by Cofounder (Jellymoon ruling 1, 10-27). Rebased onto main (squashed): record-v1.md keeps main's harness.switched row and rules beside this change's workspace fields. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/README.md | 2 + packages/seats/docs/record-v1.md | 12 +- packages/seats/src/contract.js | 2 + packages/seats/src/index.js | 3 + packages/seats/src/supervisor.js | 39 +++++- packages/seats/src/workspace.js | 178 ++++++++++++++++++++++++ packages/seats/test/workspace.test.js | 186 ++++++++++++++++++++++++++ 7 files changed, 417 insertions(+), 5 deletions(-) create mode 100644 packages/seats/src/workspace.js create mode 100644 packages/seats/test/workspace.test.js diff --git a/packages/seats/README.md b/packages/seats/README.md index 522df333f3..ef76af155f 100644 --- a/packages/seats/README.md +++ b/packages/seats/README.md @@ -23,4 +23,6 @@ It is private and is not published on its own. `@commonlyai/cli` depends on it a The extraction plan is D62, extract-in-place. +**Worktrees.** A run given `ctx.workspace = { repo, base? }` runs in its own detached, locked git worktree under `~/.jellymoon/worktrees/` (`src/workspace.js`). At settle a clean one is removed and anything else is kept locked, with its path in the result; `sweepWorktrees({ root, isRunLive })` releases the worktrees of runs that died without settling. An adapter whose CLI has its own worktree flag declares `nativeWorktree: true` and gets `ctx.workspace` instead. + **Run record.** The trace store writes each run as a versioned, hash-chained JSONL record. It covers who granted which agent what access for which task, who approved each call, what was attempted, and the result. A tool outside this package can read, verify and export it to OTLP. The format is specified in [`docs/record-v1.md`](docs/record-v1.md), and a generated sample is in [`docs/examples/`](docs/examples/). diff --git a/packages/seats/docs/record-v1.md b/packages/seats/docs/record-v1.md index 9af247a22f..a31336ddbc 100644 --- a/packages/seats/docs/record-v1.md +++ b/packages/seats/docs/record-v1.md @@ -255,10 +255,10 @@ request, and the OTLP export places it there. | kind | data | default actor | |---|---|---| -| `attempt.started` | `{attempt, adapter, cli?: {name, version}, sessionId?}` | run | +| `attempt.started` | `{attempt, adapter, cli?: {name, version}, sessionId?, workspace?: {base}}` | run | | `session.opened` | `{method, sessionId, requestedSessionId?, fallback?}` | `system` | | `attempt.settled` | `{attempt, outcome, failure?: {kind, detail}}` | `system` | -| `run.settled` | `{outcome, failure?, durationMs, attempts}` | `system` | +| `run.settled` | `{outcome, failure?, durationMs, attempts, workspace?: {base, head, removed, kept?}}` | `system` | | `harness.switched` | `{from, to, reason}` | `system` | - `outcome` is `ok`, `failed`, `stuck` or `cancelled`. @@ -275,6 +275,14 @@ request, and the OTLP export places it there. nor session/load`, or `session/ answered error ` with the JSON-RPC error's integer code; - no other field is allowed. +- `workspace` is set when the run had its own git worktree (`src/workspace.js`): + - `base` is the commit SHA it was checked out at; + - `head` is its HEAD when the run settled, or `null` if that could not be read; + - `removed` says whether it was removed at settle; + - `kept` says why not: `dirty` (tracked changes or untracked files), + `committed` (HEAD moved off base), `abandoned` (the adapter ignored the + cancel, so its process may still be running) or `error`. + The worktree's path is not recorded. - `harness.switched` comes before the `attempt.started` of an attempt that runs on another entry of the seat's run config than the one the seat was on (`src/harness-plan.js`): diff --git a/packages/seats/src/contract.js b/packages/seats/src/contract.js index 3980951746..c300abed29 100644 --- a/packages/seats/src/contract.js +++ b/packages/seats/src/contract.js @@ -10,6 +10,8 @@ * @property {() => Promise<{ path: string, version: string } | null>} detect * @property {(environment: object) => Promise} [validateEnvironment] refuse before spawn * @property {(prompt: string, ctx: object) => Promise} spawn + * @property {boolean} [nativeWorktree] the CLI makes its own git worktree (Claude Code `--worktree`, + * Codex `--worktree`) from ctx.workspace; the supervisor then makes none (workspace.js) * * @typedef {object} SpawnResult * @property {string} text diff --git a/packages/seats/src/index.js b/packages/seats/src/index.js index 47e30b6def..501c7c684d 100644 --- a/packages/seats/src/index.js +++ b/packages/seats/src/index.js @@ -20,3 +20,6 @@ export { } from './record/format.js'; export { toOtlpJson } from './record/otel.js'; export { superviseRun, SUPERVISOR_DEFAULTS, DEFAULT_RETRY_ON } from './supervisor.js'; +export { + prepareWorktree, settleWorktree, sweepWorktrees, usesWorktreeHelper, defaultWorktreeRoot, lockReasonOf, +} from './workspace.js'; diff --git a/packages/seats/src/supervisor.js b/packages/seats/src/supervisor.js index 7bfd471b1a..69fb565bc1 100644 --- a/packages/seats/src/supervisor.js +++ b/packages/seats/src/supervisor.js @@ -18,6 +18,7 @@ import { RunFailure, runFailure, failureKindOf, FAILURE_KINDS } from './failures.js'; import { redactKnownCredentials, scrubAdapterFailure, spawnCredentials } from './upstream-refusal.js'; import { killProcessTree, trackSupervisedGroup, OWN_PROCESS_GROUP } from './process-group.js'; +import { prepareWorktree, settleWorktree, usesWorktreeHelper } from './workspace.js'; export const SUPERVISOR_DEFAULTS = Object.freeze({ // Longer than the quiet stretches a healthy turn has: a long-context first @@ -239,6 +240,9 @@ const runAttempt = ({ adapter, prompt, ctx, run, settings, attempt, sessionId, e * @param {object} [args.ctx] passed to adapter.spawn; `trace`, `signal` and `onSpawn` are wrapped, * and `ownProcessGroup` is set to true. * `ctx.signal`, when given, cancels the run (outcome `cancelled`). + * `ctx.workspace = {repo, base?, root?}` runs the turn in its own git worktree (workspace.js): + * the adapter gets its path as `ctx.cwd` and no `workspace`, unless it declares + * `nativeWorktree`, in which case it gets `workspace` unchanged and makes its own. * @param {ReturnType} args.store * @param {object} [args.policy] * @param {number} [args.policy.stallMs] no-progress window; 0 turns it off @@ -253,7 +257,8 @@ const runAttempt = ({ adapter, prompt, ctx, run, settings, attempt, sessionId, e * @param {object} [args.actor] the run's own party, {type, id, owner?, onBehalfOf?}; * entries the run writes without naming an actor (run.started, tool calls, * permission requests) record this one instead of null - * @returns {Promise<{ runId: string, outcome: string, attempts: number, result?: object, failure?: Error }>} + * @returns {Promise<{ runId: string, outcome: string, attempts: number, result?: object, failure?: Error, + * workspace?: { path, base, head, removed, kept? } }>} */ export const superviseRun = async ({ adapter, prompt, ctx = {}, store, policy = {}, runId, label, taskId, podId, actor, @@ -274,12 +279,24 @@ export const superviseRun = async ({ let attempt = 0; let sessionId = ctx.sessionId || null; let last = { failure: null }; + let worktree = null; + let workspace = null; + let spawnCtx = ctx; try { + if (usesWorktreeHelper(adapter, ctx)) { + worktree = prepareWorktree({ ...ctx.workspace, runId: run.runId }); + const { workspace: _consumed, ...rest } = ctx; + spawnCtx = { ...rest, cwd: worktree.path }; + } for (;;) { attempt += 1; - run.emit('run.started', { adapter: adapter.name, ...(sessionId ? { sessionId } : {}), attempt }); - last = await runAttempt({ adapter, prompt, ctx, run, settings, attempt, sessionId, external }); + run.emit('run.started', { + adapter: adapter.name, ...(sessionId ? { sessionId } : {}), attempt, ...(worktree ? { workspace: { base: worktree.base } } : {}), + }); + last = await runAttempt({ + adapter, prompt, ctx: spawnCtx, run, settings, attempt, sessionId, external, + }); if (!last.failure || external?.aborted || !shouldRetry(last.failure, attempt, settings)) break; sessionId = sessionIdOf(last.failure) || sessionId; run.emit('liveness.recovery', { action: 'retry', attempt: attempt + 1 }); @@ -289,6 +306,15 @@ export const superviseRun = async ({ last = { failure: error }; } + // Every verdict but an abandoned run means no process of this run is left, + // so a clean worktree goes now; an abandoned one waits, locked, for the sweep. + if (worktree) { + try { + workspace = settleWorktree(worktree, { abandoned: !!last.failure?.adapterIgnoredCancel }); + } catch { + workspace = { path: worktree.path, base: worktree.base, head: null, removed: false, kept: 'error' }; + } + } const outcome = outcomeOf(last.failure); const credentials = spawnCredentials(ctx, [adapter.providerKeyEnv].filter(Boolean)); try { @@ -297,6 +323,12 @@ export const superviseRun = async ({ ...(last.failure ? { failure: failureData(last.failure, credentials) } : {}), durationMs: Date.now() - startedAt, attempts: attempt, + // The host path stays out of the record; the caller gets it below. + ...(workspace ? { + workspace: { + base: workspace.base, head: workspace.head, removed: workspace.removed, ...(workspace.kept ? { kept: workspace.kept } : {}), + }, + } : {}), }); } catch { /* the outcome is still returned */ } return { @@ -304,5 +336,6 @@ export const superviseRun = async ({ outcome, attempts: attempt, ...(last.failure ? { failure: last.failure } : { result: last.result }), + ...(workspace ? { workspace } : {}), }; }; diff --git a/packages/seats/src/workspace.js b/packages/seats/src/workspace.js new file mode 100644 index 0000000000..47b1bf15ca --- /dev/null +++ b/packages/seats/src/workspace.js @@ -0,0 +1,178 @@ +/** + * A plain git worktree per run, and the one thing git does not do on its own: + * tie the worktree's lifetime to the supervisor's verdict on the run. + * + * A run that asks for `ctx.workspace = { repo, base? }` gets a detached + * worktree at `/`, checked out at `base` and locked with the + * reason `jellymoon run `. A marker beside it, `/.json`, + * says this package made it; nothing here touches a worktree without one. + * + * When the run settles, a clean worktree is unlocked and removed. Clean means + * no tracked change, no untracked file (ignored files don't count, as with + * `git worktree remove`), and HEAD still at base. A commit the agent made on + * the detached HEAD lives only in that worktree, so a moved HEAD keeps it too. + * Anything else stays where it is, still locked, and its path is reported. + * + * A run whose wrapper died never settles. `sweepWorktrees` finds those through + * the caller's `isRunLive`, releases the lock, and removes the worktree only + * when it is clean. + * + * An adapter whose CLI makes its own worktree (Claude Code `--worktree`, + * Codex `--worktree`) declares `nativeWorktree: true` and gets `ctx.workspace` + * unchanged; none is made here. + */ +import { execFileSync } from 'node:child_process'; +import { + existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, writeFileSync, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +export const defaultWorktreeRoot = () => join(homedir(), '.jellymoon', 'worktrees'); +export const lockReasonOf = (runId) => `jellymoon run ${runId}`; + +const RUN_ID = /^[A-Za-z0-9_-]{6,64}$/; +const MARKER_SUFFIX = '.json'; + +// A caller's GIT_DIR or GIT_WORK_TREE (a hook, say) would point every call +// below at the wrong repository. +const gitEnv = () => { + const env = { ...process.env }; + for (const name of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR']) delete env[name]; + return env; +}; +const git = (dir, args) => execFileSync('git', ['-C', dir, ...args], { + encoding: 'utf8', env: gitEnv(), stdio: ['ignore', 'pipe', 'pipe'], +}).trim(); + +/** Whether this run gets a helper worktree: it asked for one and its CLI makes none. */ +export const usesWorktreeHelper = (adapter, ctx) => !!ctx?.workspace && !adapter?.nativeWorktree; + +const markerPath = (root, runId) => join(root, `${runId}${MARKER_SUFFIX}`); + +/** `git worktree list --porcelain` as { path → { head, locked } }; `locked` is the reason, or null. */ +const listWorktrees = (repo) => { + const out = new Map(); + for (const block of git(repo, ['worktree', 'list', '--porcelain']).split('\n\n')) { + const fields = {}; + for (const line of block.split('\n')) { + const space = line.indexOf(' '); + fields[space < 0 ? line : line.slice(0, space)] = space < 0 ? '' : line.slice(space + 1); + } + if (fields.worktree) out.set(fields.worktree, { head: fields.HEAD || null, locked: 'locked' in fields ? fields.locked : null }); + } + return out; +}; + +/** null when the worktree is clean against base, else why it is kept. */ +const keepReason = (path, base) => { + if (git(path, ['status', '--porcelain', '--untracked-files=all'])) return 'dirty'; + if (git(path, ['rev-parse', 'HEAD']) !== base) return 'committed'; + return null; +}; + +/** Unlock and remove; git refuses a worktree with changes, a second check behind keepReason. */ +const removeWorktree = (repo, path) => { + git(repo, ['worktree', 'unlock', path]); + git(repo, ['worktree', 'remove', path]); +}; + +/** + * Make the run's worktree. The marker is written first, so a crash between + * the two leaves a marker the sweep can clear, never an unmarked worktree. + * + * @param {{ repo: string, base?: string, runId: string, root?: string }} args + * @returns {{ runId, repo, root, path, base }} `base` is the resolved commit SHA + */ +export const prepareWorktree = ({ repo, base = 'HEAD', runId, root = defaultWorktreeRoot() } = {}) => { + if (typeof repo !== 'string' || !repo) throw new Error('workspace.repo must be a path to a git repository'); + if (!RUN_ID.test(String(runId))) throw new Error('a worktree needs a runId of 6-64 characters of [A-Za-z0-9_-]'); + const top = realpathSync(git(repo, ['rev-parse', '--show-toplevel'])); + const baseSha = git(top, ['rev-parse', '--verify', '--end-of-options', `${base}^{commit}`]); + mkdirSync(root, { recursive: true, mode: 0o700 }); + const realRoot = realpathSync(root); + const path = join(realRoot, runId); + const marker = markerPath(realRoot, runId); + if (existsSync(path) || existsSync(marker)) throw new Error(`a worktree for run ${runId} already exists`); + writeFileSync(marker, `${JSON.stringify({ runId, repo: top, path, base: baseSha, pid: process.pid })}\n`, { mode: 0o600, flag: 'wx' }); + try { + git(top, ['worktree', 'add', '--detach', '--lock', '--reason', lockReasonOf(runId), path, baseSha]); + } catch (error) { + rmSync(marker, { force: true }); + throw error; + } + return { runId, repo: top, root: realRoot, path, base: baseSha }; +}; + +/** + * The run is over: remove a clean worktree, keep anything else locked. + * `abandoned` is the supervisor's word that the adapter never let go of the + * run, so its process may still be writing here; the worktree is then kept + * locked for the sweep, whatever its state. + * + * @returns {{ path, base, head, removed: boolean, kept?: 'dirty'|'committed'|'abandoned' }} + */ +export const settleWorktree = (worktree, { abandoned = false } = {}) => { + const { repo, root, path, base, runId } = worktree; + const head = git(path, ['rev-parse', 'HEAD']); + const kept = abandoned ? 'abandoned' : keepReason(path, base); + if (kept) return { path, base, head, removed: false, kept }; + removeWorktree(repo, path); + rmSync(markerPath(root, runId), { force: true }); + return { path, base, head, removed: true }; +}; + +const readMarker = (file) => { + try { + const marker = JSON.parse(readFileSync(file, 'utf8')); + return marker && RUN_ID.test(String(marker.runId)) && marker.repo && marker.path ? marker : null; + } catch { return null; } +}; + +/** + * Release the worktrees of runs that are no longer live. Only a marked + * worktree, listed by its marker's repository and locked (if at all) with this + * package's reason, is touched; a worktree under `root` with no marker is + * never read. + * + * @param {{ root?: string, isRunLive: (runId: string, marker: object) => boolean | Promise }} args + * @returns {Promise>} + * `unlocked`: the lock is released and the worktree kept, with the reason + */ +export const sweepWorktrees = async ({ root = defaultWorktreeRoot(), isRunLive } = {}) => { + if (typeof isRunLive !== 'function') throw new Error('sweepWorktrees needs isRunLive(runId)'); + if (!existsSync(root)) return []; + const realRoot = realpathSync(root); + const report = []; + for (const name of readdirSync(realRoot).filter((file) => file.endsWith(MARKER_SUFFIX)).sort()) { + const file = join(realRoot, name); + const marker = readMarker(file); + // A marker names the worktree beside it, never one elsewhere. + if (!marker || name !== `${marker.runId}${MARKER_SUFFIX}` || marker.path !== join(realRoot, marker.runId)) continue; + const { runId, repo, path } = marker; + if (await isRunLive(runId, marker)) { report.push({ runId, path, action: 'live' }); continue; } + try { + const entry = listWorktrees(repo).get(path); + if (!entry || !existsSync(path)) { + // The directory is gone (or was never made). Unlocking lets git's own + // pruning drop its record; `git worktree prune` here would reach + // every stale worktree of the repository, not only this one. + if (entry && entry.locked === lockReasonOf(runId)) git(repo, ['worktree', 'unlock', path]); + rmSync(file, { force: true }); + report.push({ runId, path, action: 'pruned' }); + continue; + } + if (entry.locked !== null && entry.locked !== lockReasonOf(runId)) { report.push({ runId, path, action: 'skipped' }); continue; } + if (entry.locked !== null) git(repo, ['worktree', 'unlock', path]); + const kept = keepReason(path, marker.base); + if (kept) { report.push({ runId, path, action: 'unlocked', kept }); continue; } + git(repo, ['worktree', 'remove', path]); + rmSync(file, { force: true }); + report.push({ runId, path, action: 'removed' }); + } catch { + // A repository moved or deleted since: leave it for a person. + report.push({ runId, path, action: 'skipped' }); + } + } + return report; +}; diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js new file mode 100644 index 0000000000..b918467b12 --- /dev/null +++ b/packages/seats/test/workspace.test.js @@ -0,0 +1,186 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { + existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { superviseRun } from '../src/supervisor.js'; +import { createTraceStore } from '../src/trace/store.js'; +import { + prepareWorktree, settleWorktree, sweepWorktrees, lockReasonOf, +} from '../src/workspace.js'; +import * as seats from '../src/index.js'; + +// Every git call in this file, the module's included, reads no host config: +// a global hooksPath or signing rule would otherwise run on the test commits. +const sandbox = realpathSync(mkdtempSync(join(tmpdir(), 'seats-wt-'))); +const emptyConfig = join(sandbox, 'gitconfig'); +writeFileSync(emptyConfig, ''); +process.env.GIT_CONFIG_GLOBAL = emptyConfig; +process.env.GIT_CONFIG_NOSYSTEM = '1'; +process.on('exit', () => rmSync(sandbox, { recursive: true, force: true })); + +const git = (dir, ...args) => execFileSync('git', ['-C', dir, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); + +/** A throwaway repo with one commit, a worktree root, and a trace store, all under one temp dir. */ +const setup = () => { + const dir = mkdtempSync(join(sandbox, 'case-')); + const repo = join(dir, 'repo'); + execFileSync('git', ['init', '-q', repo]); + git(repo, 'config', 'user.name', 'test'); + git(repo, 'config', 'user.email', 'test@example.invalid'); + writeFileSync(join(repo, 'README'), 'one\n'); + git(repo, 'add', 'README'); + git(repo, 'commit', '-q', '-m', 'one'); + return { + repo, root: join(dir, 'worktrees'), store: createTraceStore({ root: join(dir, 'trace') }), base: git(repo, 'rev-parse', 'HEAD'), + }; +}; + +/** { path → locked reason | null } for the repo's worktrees. */ +const worktreesOf = (repo) => Object.fromEntries(git(repo, 'worktree', 'list', '--porcelain').split('\n\n').map((block) => { + const lines = block.split('\n'); + const locked = lines.find((line) => line === 'locked' || line.startsWith('locked ')); + return [lines[0].slice('worktree '.length), locked === undefined ? null : locked.slice('locked '.length)]; +})); + +/** An adapter that runs `work(cwd)` in the directory it was given. */ +const working = (work, calls = []) => ({ + name: 'worker', + async spawn(prompt, ctx) { + calls.push(ctx); + work(ctx.cwd); + return { text: 'done' }; + }, +}); + +test('a clean worktree is removed after the run settles, and base/head land in the record', async () => { + const { repo, root, store, base } = setup(); + const calls = []; + const out = await superviseRun({ + adapter: working(() => {}, calls), prompt: 'look only', store, runId: 'run-clean-1', ctx: { workspace: { repo, root } }, + }); + assert.equal(out.outcome, 'ok'); + const path = join(realpathSync(root), 'run-clean-1'); + assert.equal(calls[0].cwd, path); + assert.equal(calls[0].workspace, undefined, 'the helper consumed the workspace'); + assert.deepEqual(out.workspace, { path, base, head: base, removed: true }); + assert.equal(existsSync(path), false); + assert.equal(existsSync(`${path}.json`), false, 'the marker goes with it'); + assert.equal(worktreesOf(repo)[path], undefined); + + const entries = store.readRun('run-clean-1'); + assert.deepEqual(entries.find((e) => e.type === 'run.started').data.workspace, { base }); + assert.deepEqual(entries.find((e) => e.type === 'run.settled').data.workspace, { base, head: base, removed: true }); + assert.equal(store.verifyRun('run-clean-1').ok, true); +}); + +test('a dirty worktree is kept, still locked, and its path reported', async () => { + const { repo, root, store, base } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => writeFileSync(join(cwd, 'new-file'), 'x')), prompt: 'write', store, runId: 'run-dirty-1', ctx: { workspace: { repo, root } }, + }); + const path = join(realpathSync(root), 'run-dirty-1'); + assert.deepEqual(out.workspace, { path, base, head: base, removed: false, kept: 'dirty' }); + assert.equal(existsSync(join(path, 'new-file')), true); + assert.equal(worktreesOf(repo)[path], lockReasonOf('run-dirty-1')); + assert.deepEqual(store.readRun('run-dirty-1').find((e) => e.type === 'run.settled').data.workspace, + { base, head: base, removed: false, kept: 'dirty' }); +}); + +test('a commit on the detached HEAD keeps the worktree, with the new head recorded', async () => { + const { repo, root, store, base } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => { + writeFileSync(join(cwd, 'README'), 'two\n'); + git(cwd, 'commit', '-q', '-am', 'two'); + }), + prompt: 'commit', + store, + runId: 'run-commit-1', + ctx: { workspace: { repo, root } }, + }); + assert.equal(out.workspace.kept, 'committed'); + assert.equal(out.workspace.base, base); + assert.notEqual(out.workspace.head, base); + assert.equal(out.workspace.head, git(out.workspace.path, 'rev-parse', 'HEAD')); +}); + +test('the sweep releases a dead run\'s lock, removes it when clean, and leaves an unmarked worktree alone', async () => { + const { repo, root } = setup(); + // Two runs whose wrapper died: prepared, never settled. + const clean = prepareWorktree({ repo, root, runId: 'run-dead-clean' }); + const dirty = prepareWorktree({ repo, root, runId: 'run-dead-dirty' }); + const live = prepareWorktree({ repo, root, runId: 'run-still-live' }); + writeFileSync(join(dirty.path, 'scratch'), 'x'); + // Someone else's worktree under the same root, locked, with no marker. + const stranger = join(realpathSync(root), 'stranger'); + git(repo, 'worktree', 'add', '-q', '--detach', '--lock', '--reason', 'not ours', stranger); + + const report = await sweepWorktrees({ root, isRunLive: (runId) => runId === 'run-still-live' }); + assert.deepEqual(report.map(({ runId, action, kept }) => ({ runId, action, ...(kept ? { kept } : {}) })), [ + { runId: 'run-dead-clean', action: 'removed' }, + { runId: 'run-dead-dirty', action: 'unlocked', kept: 'dirty' }, + { runId: 'run-still-live', action: 'live' }, + ]); + const after = worktreesOf(repo); + assert.equal(existsSync(clean.path), false); + assert.equal(after[clean.path], undefined); + assert.equal(after[dirty.path], null, 'unlocked and kept'); + assert.equal(existsSync(join(dirty.path, 'scratch')), true); + assert.equal(after[live.path], lockReasonOf('run-still-live')); + assert.equal(after[stranger], 'not ours'); +}); + +test('a marker whose worktree is locked by someone else is skipped', async () => { + const { repo, root } = setup(); + const wt = prepareWorktree({ repo, root, runId: 'run-relocked' }); + git(repo, 'worktree', 'unlock', wt.path); + git(repo, 'worktree', 'lock', '--reason', 'a person is looking', wt.path); + const report = await sweepWorktrees({ root, isRunLive: () => false }); + assert.deepEqual(report.map((r) => r.action), ['skipped']); + assert.equal(worktreesOf(repo)[wt.path], 'a person is looking'); +}); + +test('an adapter with a native worktree flag gets the workspace and no helper worktree', async () => { + const { repo, root, store } = setup(); + const calls = []; + const adapter = { ...working(() => {}, calls), nativeWorktree: true }; + const out = await superviseRun({ + adapter, prompt: 'p', store, runId: 'run-native-1', ctx: { workspace: { repo, root } }, + }); + assert.equal(out.outcome, 'ok'); + assert.deepEqual(calls[0].workspace, { repo, root }); + assert.equal(calls[0].cwd, undefined); + assert.equal(out.workspace, undefined); + assert.equal(existsSync(root), false); + assert.equal(Object.keys(worktreesOf(repo)).length, 1, 'only the main worktree'); + assert.equal(store.readRun('run-native-1').find((e) => e.type === 'run.settled').data.workspace, undefined); +}); + +test('an abandoned run keeps its worktree locked for the sweep', () => { + const { repo, root, base } = setup(); + const wt = prepareWorktree({ repo, root, runId: 'run-abandon' }); + assert.deepEqual(settleWorktree(wt, { abandoned: true }), { + path: wt.path, base, head: base, removed: false, kept: 'abandoned', + }); + assert.equal(worktreesOf(repo)[wt.path], lockReasonOf('run-abandon')); +}); + +test('a workspace that cannot be made settles the run as failed, without spawning', async () => { + const { root, store } = setup(); + const calls = []; + const out = await superviseRun({ + adapter: working(() => {}, calls), prompt: 'p', store, runId: 'run-norepo', ctx: { workspace: { repo: join(root, 'missing'), root } }, + }); + assert.equal(out.outcome, 'failed'); + assert.equal(calls.length, 0); + assert.deepEqual(store.readRun('run-norepo').map((e) => e.type), ['run.settled']); +}); + +test('the worktree API is exported from the package entry', () => { + assert.equal(seats.sweepWorktrees, sweepWorktrees); + assert.equal(seats.prepareWorktree, prepareWorktree); +}); From 4d54c7ec5439561a78e7da05bf85e694c9396062 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:09:20 -0700 Subject: [PATCH 2/7] test(seats): cover the sweep's pruned and committed branches, and the supervisor's abandoned wiring - a worktree whose directory is gone is pruned: its marker goes, our lock is released, and a lock someone else took stays; - a dead run's worktree that moved off its base is kept, unlocked, as committed; - superviseRun with an adapter that ignores the cancel leaves the worktree locked as abandoned, with its marker, for the sweep. Each fails under its mutant (abandoned: false; unlock regardless of reason; no committed check). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/test/workspace.test.js | 58 +++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js index b918467b12..036e5a7f47 100644 --- a/packages/seats/test/workspace.test.js +++ b/packages/seats/test/workspace.test.js @@ -134,6 +134,43 @@ test('the sweep releases a dead run\'s lock, removes it when clean, and leaves a assert.equal(after[stranger], 'not ours'); }); +test('the sweep prunes a marker whose worktree is gone, releasing only its own lock', async () => { + const { repo, root } = setup(); + const ours = prepareWorktree({ repo, root, runId: 'run-gone-ours' }); + const relocked = prepareWorktree({ repo, root, runId: 'run-gone-relocked' }); + git(repo, 'worktree', 'unlock', relocked.path); + git(repo, 'worktree', 'lock', '--reason', 'a person is looking', relocked.path); + rmSync(ours.path, { recursive: true, force: true }); + rmSync(relocked.path, { recursive: true, force: true }); + + const report = await sweepWorktrees({ root, isRunLive: () => false }); + assert.deepEqual(report.map(({ runId, action }) => ({ runId, action })), [ + { runId: 'run-gone-ours', action: 'pruned' }, + { runId: 'run-gone-relocked', action: 'pruned' }, + ]); + const realRoot = realpathSync(root); + assert.equal(existsSync(join(realRoot, 'run-gone-ours.json')), false); + assert.equal(existsSync(join(realRoot, 'run-gone-relocked.json')), false); + const after = worktreesOf(repo); + assert.equal(after[ours.path], null, 'our lock released, so git can prune the record'); + assert.equal(after[relocked.path], 'a person is looking', 'a lock that is not ours stays'); +}); + +test('the sweep keeps a dead run\'s worktree that moved off its base, unlocked', async () => { + const { repo, root, base } = setup(); + const wt = prepareWorktree({ repo, root, runId: 'run-dead-committed' }); + writeFileSync(join(wt.path, 'README'), 'two\n'); + git(wt.path, 'commit', '-q', '-am', 'two'); + assert.notEqual(git(wt.path, 'rev-parse', 'HEAD'), base); + + const report = await sweepWorktrees({ root, isRunLive: () => false }); + assert.deepEqual(report.map(({ runId, action, kept }) => ({ runId, action, kept })), [ + { runId: 'run-dead-committed', action: 'unlocked', kept: 'committed' }, + ]); + assert.equal(existsSync(wt.path), true); + assert.equal(worktreesOf(repo)[wt.path], null); +}); + test('a marker whose worktree is locked by someone else is skipped', async () => { const { repo, root } = setup(); const wt = prepareWorktree({ repo, root, runId: 'run-relocked' }); @@ -169,6 +206,27 @@ test('an abandoned run keeps its worktree locked for the sweep', () => { assert.equal(worktreesOf(repo)[wt.path], lockReasonOf('run-abandon')); }); +test('a run whose adapter ignores the cancel keeps its worktree locked as abandoned', async () => { + const { repo, root, store } = setup(); + // Never settles and never looks at its signal: the supervisor gives up after + // the grace period, and the process may still be writing in the worktree. + const adapter = { name: 'deaf', spawn: () => new Promise(() => {}) }; + const out = await superviseRun({ + adapter, + prompt: 'p', + store, + runId: 'run-deaf-1', + ctx: { workspace: { repo, root } }, + policy: { stallMs: 0, timeoutMs: 100, cancelGraceMs: 50 }, + }); + assert.equal(out.failure.adapterIgnoredCancel, true); + assert.equal(out.workspace.kept, 'abandoned'); + assert.equal(out.workspace.removed, false); + assert.equal(existsSync(out.workspace.path), true); + assert.equal(worktreesOf(repo)[out.workspace.path], lockReasonOf('run-deaf-1')); + assert.equal(existsSync(join(realpathSync(root), 'run-deaf-1.json')), true, 'the marker stays for the sweep'); +}); + test('a workspace that cannot be made settles the run as failed, without spawning', async () => { const { root, store } = setup(); const calls = []; From 9172ee5664747164aa4e4a2ba2f6a933cb02faea Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:21:00 -0700 Subject: [PATCH 3/7] test(seats): isolate the worktree tests from a caller's git env; cover a tracked-file edit GIT_DIR, GIT_WORK_TREE and the other repo-location variables are cleared before any git call, so a run from a git hook cannot touch the caller's repo (the suite passes with GIT_DIR=/nonexistent exported). A new case edits the tracked README without committing and expects the worktree kept, locked, as dirty; it fails if keepReason counts only untracked files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/test/workspace.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js index 036e5a7f47..1e3e0b1777 100644 --- a/packages/seats/test/workspace.test.js +++ b/packages/seats/test/workspace.test.js @@ -20,6 +20,8 @@ const emptyConfig = join(sandbox, 'gitconfig'); writeFileSync(emptyConfig, ''); process.env.GIT_CONFIG_GLOBAL = emptyConfig; process.env.GIT_CONFIG_NOSYSTEM = '1'; +// Run from a git hook, these would point every call at the caller's repo, -C or not. +for (const name of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR', 'GIT_OBJECT_DIRECTORY', 'GIT_ALTERNATE_OBJECT_DIRECTORIES', 'GIT_NAMESPACE', 'GIT_PREFIX']) delete process.env[name]; process.on('exit', () => rmSync(sandbox, { recursive: true, force: true })); const git = (dir, ...args) => execFileSync('git', ['-C', dir, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); @@ -90,6 +92,16 @@ test('a dirty worktree is kept, still locked, and its path reported', async () = { base, head: base, removed: false, kept: 'dirty' }); }); +test('an uncommitted edit to a tracked file keeps the worktree, still locked', async () => { + const { repo, root, store, base } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => writeFileSync(join(cwd, 'README'), 'edited\n')), prompt: 'edit', store, runId: 'run-tracked-1', ctx: { workspace: { repo, root } }, + }); + const path = join(realpathSync(root), 'run-tracked-1'); + assert.deepEqual(out.workspace, { path, base, head: base, removed: false, kept: 'dirty' }); + assert.equal(worktreesOf(repo)[path], lockReasonOf('run-tracked-1')); +}); + test('a commit on the detached HEAD keeps the worktree, with the new head recorded', async () => { const { repo, root, store, base } = setup(); const out = await superviseRun({ From 51a7c3fe0e68f60765113fb1b87d924dccf49cd0 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:33:34 -0700 Subject: [PATCH 4/7] fix(seats): a run whose only output is gitignored keeps its worktree keepReason read `git status --porcelain`, which hides ignored files, and `git worktree remove` deletes them. A run that built into dist/ looked clean, lost its output with the worktree, and was reported as removed. The check now passes --ignored, and a test pins a dist/ build kept 'dirty' and locked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/src/workspace.js | 8 ++++++-- packages/seats/test/workspace.test.js | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/seats/src/workspace.js b/packages/seats/src/workspace.js index 47b1bf15ca..e99fd61e51 100644 --- a/packages/seats/src/workspace.js +++ b/packages/seats/src/workspace.js @@ -64,9 +64,13 @@ const listWorktrees = (repo) => { return out; }; -/** null when the worktree is clean against base, else why it is kept. */ +/** + * null when the worktree is clean against base, else why it is kept. Ignored + * files count: a build into a gitignored dist/ is still the run's output, and + * `git worktree remove` deletes ignored files without asking. + */ const keepReason = (path, base) => { - if (git(path, ['status', '--porcelain', '--untracked-files=all'])) return 'dirty'; + if (git(path, ['status', '--porcelain', '--untracked-files=all', '--ignored'])) return 'dirty'; if (git(path, ['rev-parse', 'HEAD']) !== base) return 'committed'; return null; }; diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js index 1e3e0b1777..fc46ac078d 100644 --- a/packages/seats/test/workspace.test.js +++ b/packages/seats/test/workspace.test.js @@ -2,7 +2,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; import { - existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync, + existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync, } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -34,7 +34,8 @@ const setup = () => { git(repo, 'config', 'user.name', 'test'); git(repo, 'config', 'user.email', 'test@example.invalid'); writeFileSync(join(repo, 'README'), 'one\n'); - git(repo, 'add', 'README'); + writeFileSync(join(repo, '.gitignore'), 'dist/\n'); + git(repo, 'add', 'README', '.gitignore'); git(repo, 'commit', '-q', '-m', 'one'); return { repo, root: join(dir, 'worktrees'), store: createTraceStore({ root: join(dir, 'trace') }), base: git(repo, 'rev-parse', 'HEAD'), @@ -92,6 +93,23 @@ test('a dirty worktree is kept, still locked, and its path reported', async () = { base, head: base, removed: false, kept: 'dirty' }); }); +test('output written only to a gitignored dir keeps the worktree, still locked', async () => { + // `git worktree remove` deletes ignored files, so a build into dist/ would + // vanish with the worktree and the report would call the run clean. + const { repo, root, store, base } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => { + mkdirSync(join(cwd, 'dist')); + writeFileSync(join(cwd, 'dist', 'out.js'), 'built\n'); + }), + prompt: 'build', store, runId: 'run-ignored-1', ctx: { workspace: { repo, root } }, + }); + const path = join(realpathSync(root), 'run-ignored-1'); + assert.deepEqual(out.workspace, { path, base, head: base, removed: false, kept: 'dirty' }); + assert.equal(existsSync(join(path, 'dist', 'out.js')), true); + assert.equal(worktreesOf(repo)[path], lockReasonOf('run-ignored-1')); +}); + test('an uncommitted edit to a tracked file keeps the worktree, still locked', async () => { const { repo, root, store, base } = setup(); const out = await superviseRun({ From 0d973740133a98e2aa5b3b182a9a2f5e3c5b1246 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:40:45 -0700 Subject: [PATCH 5/7] fix(seats): list an ignored or untracked directory as one line in the keep check With --untracked-files=all, --ignored listed every file in node_modules, and a large tree overran execFileSync's 1 MiB buffer, so the check threw instead of reporting 'dirty'. "normal" lists each such directory once. A test builds ~1 MiB of ignored paths and expects the worktree kept and locked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/src/workspace.js | 6 ++++-- packages/seats/test/workspace.test.js | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/seats/src/workspace.js b/packages/seats/src/workspace.js index e99fd61e51..308ab3c2dc 100644 --- a/packages/seats/src/workspace.js +++ b/packages/seats/src/workspace.js @@ -67,10 +67,12 @@ const listWorktrees = (repo) => { /** * null when the worktree is clean against base, else why it is kept. Ignored * files count: a build into a gitignored dist/ is still the run's output, and - * `git worktree remove` deletes ignored files without asking. + * `git worktree remove` deletes ignored files without asking. Untracked + * "normal" lists an untracked or ignored directory as one line; "all" would + * list every file in node_modules and overrun execFileSync's buffer. */ const keepReason = (path, base) => { - if (git(path, ['status', '--porcelain', '--untracked-files=all', '--ignored'])) return 'dirty'; + if (git(path, ['status', '--porcelain', '--untracked-files=normal', '--ignored'])) return 'dirty'; if (git(path, ['rev-parse', 'HEAD']) !== base) return 'committed'; return null; }; diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js index fc46ac078d..802363e49f 100644 --- a/packages/seats/test/workspace.test.js +++ b/packages/seats/test/workspace.test.js @@ -110,6 +110,23 @@ test('output written only to a gitignored dir keeps the worktree, still locked', assert.equal(worktreesOf(repo)[path], lockReasonOf('run-ignored-1')); }); +test('a large ignored tree is reported as one line, so the check does not overrun its buffer', async () => { + // ~5,000 paths of ~220 bytes: listed file by file, status would pass the + // 1 MiB execFileSync buffer and throw instead of reporting 'dirty'. + const { repo, root, store, base } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => { + const deep = join(cwd, 'dist', 'm'.repeat(200)); + mkdirSync(deep, { recursive: true }); + for (let i = 0; i < 5000; i += 1) writeFileSync(join(deep, `f${i}`), ''); + }), + prompt: 'install', store, runId: 'run-ignored-big', ctx: { workspace: { repo, root } }, + }); + const path = join(realpathSync(root), 'run-ignored-big'); + assert.deepEqual(out.workspace, { path, base, head: base, removed: false, kept: 'dirty' }); + assert.equal(worktreesOf(repo)[path], lockReasonOf('run-ignored-big')); +}); + test('an uncommitted edit to a tracked file keeps the worktree, still locked', async () => { const { repo, root, store, base } = setup(); const out = await superviseRun({ From a0c382569b13c1161c485fbc14b506f92cc04ac3 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:20:19 -0700 Subject: [PATCH 6/7] docs(seats): the workspace header describes the keep check as it now is Clean now means no ignored file as well, because `git worktree remove` deletes ignored files. The header still said they didn't count. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/src/workspace.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/seats/src/workspace.js b/packages/seats/src/workspace.js index 308ab3c2dc..13a2c3f188 100644 --- a/packages/seats/src/workspace.js +++ b/packages/seats/src/workspace.js @@ -8,9 +8,11 @@ * says this package made it; nothing here touches a worktree without one. * * When the run settles, a clean worktree is unlocked and removed. Clean means - * no tracked change, no untracked file (ignored files don't count, as with - * `git worktree remove`), and HEAD still at base. A commit the agent made on - * the detached HEAD lives only in that worktree, so a moved HEAD keeps it too. + * no tracked change, no untracked file, no ignored file, and HEAD still at + * base. Ignored files count because `git worktree remove` deletes them without + * asking, so a build into a gitignored dist/ would go with the worktree. A + * commit the agent made on the detached HEAD lives only in that worktree, so a + * moved HEAD keeps it too. * Anything else stays where it is, still locked, and its path is reported. * * A run whose wrapper died never settles. `sweepWorktrees` finds those through From a60b87a5c0944826e30651f03b954c74780e5eb9 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:31:13 -0700 Subject: [PATCH 7/7] fix(seats): pin the sweep's path guard with a test; answer the worktree review - A forged marker whose path names a worktree outside the root is never touched. With the guard removed the test fails and the outside worktree is really removed, which is what Sable's mutation showed. - gitEnv also scrubs GIT_OBJECT_DIRECTORY, the alternates, GIT_NAMESPACE and GIT_PREFIX. - A failed remove puts the run's lock back, so a kept worktree is never left unlocked. - The OTLP export carries the run's workspace. - A skipped sweep entry keeps git's error, one line. - A missing worktree locked by someone else keeps its marker. - An old git gets a clear "needs git 2.36" error. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DRK3nqxjeAV7xL8uhg --- packages/seats/docs/record-v1.md | 4 +- packages/seats/src/record/otel.js | 3 ++ packages/seats/src/workspace.js | 52 +++++++++++++----- packages/seats/test/record-otel.test.js | 19 +++++++ packages/seats/test/workspace.test.js | 72 ++++++++++++++++++++++++- 5 files changed, 135 insertions(+), 15 deletions(-) diff --git a/packages/seats/docs/record-v1.md b/packages/seats/docs/record-v1.md index a31336ddbc..8d4291ca5e 100644 --- a/packages/seats/docs/record-v1.md +++ b/packages/seats/docs/record-v1.md @@ -629,7 +629,9 @@ to that commit. `error.type` is Stable. `seats.attempt.outcome`, `seats.adapter`, `seats.cli.name`, `seats.usage.context_tokens`, `seats.usage.context_size` (the last known values in the attempt), - `seats.cli.version`, `seats.agent.owner`, `seats.agent.on_behalf_of.id`, + `seats.cli.version`, `seats.workspace.base` (attempt span, and the run + span with `seats.workspace.head`, `.removed` and `.kept` from + `run.settled`), `seats.agent.owner`, `seats.agent.on_behalf_of.id`, `seats.agent.on_behalf_of.role`, `seats.tool.kind`, `seats.tool.surface` and `seats.tool.args_digest`; - on every event, `seats.seq` and `seats.actor.type`, `.id`, `.owner`, diff --git a/packages/seats/src/record/otel.js b/packages/seats/src/record/otel.js index 6d59ff26f7..264b77ea07 100644 --- a/packages/seats/src/record/otel.js +++ b/packages/seats/src/record/otel.js @@ -148,6 +148,7 @@ const spansOfRun = (entries) => { ['seats.agent.on_behalf_of.id', entry.actor?.onBehalfOf?.id], ['seats.agent.on_behalf_of.role', entry.actor?.onBehalfOf?.role], ['seats.attempt.number', data.attempt], ['seats.adapter', data.adapter], ['seats.cli.name', data.cli?.name], ['seats.cli.version', data.cli?.version], + ['seats.workspace.base', data.workspace?.base], ]), }); spans.push(s); @@ -160,6 +161,8 @@ const spansOfRun = (entries) => { closeAttempt(entry.ts); } else if (kind === 'run.settled') { run.attributes.push(...attrs([['seats.run.outcome', data.outcome], ['seats.run.attempts', data.attempts]])); + // The run's own worktree, when it had one: base, head, removed, kept. + if (data.workspace && typeof data.workspace === 'object') run.attributes.push(...flatten(data.workspace, 'seats.workspace')); if (FAILED_OUTCOMES.has(data.outcome)) { const errorType = data.failure?.kind || data.outcome; fail(run, errorType); diff --git a/packages/seats/src/workspace.js b/packages/seats/src/workspace.js index 13a2c3f188..6b9f219c05 100644 --- a/packages/seats/src/workspace.js +++ b/packages/seats/src/workspace.js @@ -37,10 +37,15 @@ const RUN_ID = /^[A-Za-z0-9_-]{6,64}$/; const MARKER_SUFFIX = '.json'; // A caller's GIT_DIR or GIT_WORK_TREE (a hook, say) would point every call -// below at the wrong repository. +// below at the wrong repository; GIT_OBJECT_DIRECTORY and the alternates would +// read and write objects somewhere else. +const SCRUBBED_GIT_ENV = [ + 'GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR', + 'GIT_OBJECT_DIRECTORY', 'GIT_ALTERNATE_OBJECT_DIRECTORIES', 'GIT_NAMESPACE', 'GIT_PREFIX', +]; const gitEnv = () => { const env = { ...process.env }; - for (const name of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR']) delete env[name]; + for (const name of SCRUBBED_GIT_ENV) delete env[name]; return env; }; const git = (dir, args) => execFileSync('git', ['-C', dir, ...args], { @@ -79,10 +84,28 @@ const keepReason = (path, base) => { return null; }; -/** Unlock and remove; git refuses a worktree with changes, a second check behind keepReason. */ -const removeWorktree = (repo, path) => { +/** + * Unlock and remove; git refuses a worktree with changes, a second check + * behind keepReason. git will not remove a locked worktree without a double + * --force, which is never used here, so the unlock has to come first; when + * the remove then fails (a submodule, say), the lock goes back on with the + * same reason, so a kept worktree is never left unlocked. + */ +const removeWorktree = (repo, path, reason) => { git(repo, ['worktree', 'unlock', path]); - git(repo, ['worktree', 'remove', path]); + try { + git(repo, ['worktree', 'remove', path]); + } catch (error) { + try { git(repo, ['worktree', 'lock', '--reason', reason, path]); } catch { /* the remove error is the one to report */ } + throw error; + } +}; + +/** git's own complaint, one line and bounded, for a report a person reads. */ +const gitErrorOf = (error) => { + const text = String(error?.stderr || error?.message || error || '').trim(); + const line = text.split('\n').find((l) => l.trim()) || 'unknown error'; + return line.length > 300 ? `${line.slice(0, 300)}…` : line; }; /** @@ -107,6 +130,8 @@ export const prepareWorktree = ({ repo, base = 'HEAD', runId, root = defaultWork git(top, ['worktree', 'add', '--detach', '--lock', '--reason', lockReasonOf(runId), path, baseSha]); } catch (error) { rmSync(marker, { force: true }); + // `worktree add --reason` is git 2.36+; say so rather than pass on "unknown option". + if (/unknown option.*reason/.test(String(error?.stderr || ''))) throw new Error('a run worktree needs git 2.36 or later (worktree add --reason)'); throw error; } return { runId, repo: top, root: realRoot, path, base: baseSha }; @@ -125,7 +150,7 @@ export const settleWorktree = (worktree, { abandoned = false } = {}) => { const head = git(path, ['rev-parse', 'HEAD']); const kept = abandoned ? 'abandoned' : keepReason(path, base); if (kept) return { path, base, head, removed: false, kept }; - removeWorktree(repo, path); + removeWorktree(repo, path, lockReasonOf(runId)); rmSync(markerPath(root, runId), { force: true }); return { path, base, head, removed: true }; }; @@ -144,8 +169,9 @@ const readMarker = (file) => { * never read. * * @param {{ root?: string, isRunLive: (runId: string, marker: object) => boolean | Promise }} args - * @returns {Promise>} + * @returns {Promise>} * `unlocked`: the lock is released and the worktree kept, with the reason + * `skipped`: left alone, its marker kept; `error` is git's message when a git call failed */ export const sweepWorktrees = async ({ root = defaultWorktreeRoot(), isRunLive } = {}) => { if (typeof isRunLive !== 'function') throw new Error('sweepWorktrees needs isRunLive(runId)'); @@ -164,8 +190,10 @@ export const sweepWorktrees = async ({ root = defaultWorktreeRoot(), isRunLive } if (!entry || !existsSync(path)) { // The directory is gone (or was never made). Unlocking lets git's own // pruning drop its record; `git worktree prune` here would reach - // every stale worktree of the repository, not only this one. - if (entry && entry.locked === lockReasonOf(runId)) git(repo, ['worktree', 'unlock', path]); + // every stale worktree of the repository, not only this one. A lock + // someone else put on it is theirs to release, so the marker stays. + if (entry && entry.locked !== null && entry.locked !== lockReasonOf(runId)) { report.push({ runId, path, action: 'skipped' }); continue; } + if (entry && entry.locked !== null) git(repo, ['worktree', 'unlock', path]); rmSync(file, { force: true }); report.push({ runId, path, action: 'pruned' }); continue; @@ -177,9 +205,9 @@ export const sweepWorktrees = async ({ root = defaultWorktreeRoot(), isRunLive } git(repo, ['worktree', 'remove', path]); rmSync(file, { force: true }); report.push({ runId, path, action: 'removed' }); - } catch { - // A repository moved or deleted since: leave it for a person. - report.push({ runId, path, action: 'skipped' }); + } catch (error) { + // A repository moved or deleted since, or git refused: leave it for a person. + report.push({ runId, path, action: 'skipped', error: gitErrorOf(error) }); } } return report; diff --git a/packages/seats/test/record-otel.test.js b/packages/seats/test/record-otel.test.js index b1aa6fd44d..0ace8166dc 100644 --- a/packages/seats/test/record-otel.test.js +++ b/packages/seats/test/record-otel.test.js @@ -256,3 +256,22 @@ test('session.opened is an event that names the attempt\'s conversation; context assert.deepEqual(second.attributes.filter((a) => a.key === 'gen_ai.conversation.id').map((a) => a.value.stringValue), ['ses-9']); assert.equal(attrOf(second, 'seats.usage.context_tokens'), undefined); }); + +test('a run\'s worktree reaches the export: base on the attempt, base/head/removed/kept on the run', () => { + const base = { v: 1, schema: 'seats.record', runId: 'run-wt-00001', actor: null, prev: null }; + const sha = (c) => c.repeat(40); + const entries = [ + { ...base, seq: 1, ts: '2026-10-25T12:00:00.000Z', kind: 'attempt.started', data: { attempt: 1, workspace: { base: sha('a') } } }, + { ...base, seq: 2, ts: '2026-10-25T12:00:01.000Z', kind: 'run.settled', data: { outcome: 'ok', attempts: 1, workspace: { base: sha('a'), head: sha('b'), removed: false, kept: 'committed' } } }, + ]; + const [run, attempt] = spansOf(toOtlpJson(entries)); + assert.equal(attrOf(attempt, 'seats.workspace.base').stringValue, sha('a')); + assert.equal(attrOf(run, 'seats.workspace.base').stringValue, sha('a')); + assert.equal(attrOf(run, 'seats.workspace.head').stringValue, sha('b')); + assert.equal(attrOf(run, 'seats.workspace.removed').boolValue, false); + assert.equal(attrOf(run, 'seats.workspace.kept').stringValue, 'committed'); + // A head that could not be read is left out, not exported as null. + const unread = spansOf(toOtlpJson([entries[0], { ...entries[1], data: { outcome: 'ok', attempts: 1, workspace: { base: sha('a'), head: null, removed: false, kept: 'error' } } }])); + assert.equal(attrOf(unread[0], 'seats.workspace.head'), undefined); + assert.equal(attrOf(unread[0], 'seats.workspace.kept').stringValue, 'error'); +}); diff --git a/packages/seats/test/workspace.test.js b/packages/seats/test/workspace.test.js index 802363e49f..da7f3b5f7f 100644 --- a/packages/seats/test/workspace.test.js +++ b/packages/seats/test/workspace.test.js @@ -193,16 +193,84 @@ test('the sweep prunes a marker whose worktree is gone, releasing only its own l const report = await sweepWorktrees({ root, isRunLive: () => false }); assert.deepEqual(report.map(({ runId, action }) => ({ runId, action })), [ { runId: 'run-gone-ours', action: 'pruned' }, - { runId: 'run-gone-relocked', action: 'pruned' }, + { runId: 'run-gone-relocked', action: 'skipped' }, ]); const realRoot = realpathSync(root); assert.equal(existsSync(join(realRoot, 'run-gone-ours.json')), false); - assert.equal(existsSync(join(realRoot, 'run-gone-relocked.json')), false); + assert.equal(existsSync(join(realRoot, 'run-gone-relocked.json')), true, 'a lock that is not ours keeps the marker'); const after = worktreesOf(repo); assert.equal(after[ours.path], null, 'our lock released, so git can prune the record'); assert.equal(after[relocked.path], 'a person is looking', 'a lock that is not ours stays'); }); +test('a forged marker naming a worktree outside the root is never touched', async () => { + const { repo, root, base } = setup(); + // A clean, unlocked worktree outside the root: everything the sweep would remove. + const outside = join(realpathSync(join(repo, '..')), 'outside'); + git(repo, 'worktree', 'add', '-q', '--detach', outside); + mkdirSync(root, { recursive: true }); + const realRoot = realpathSync(root); + writeFileSync(join(realRoot, 'run-forged.json'), `${JSON.stringify({ + runId: 'run-forged', repo, path: outside, base, + })}\n`); + + const report = await sweepWorktrees({ root, isRunLive: () => false }); + assert.deepEqual(report.filter((r) => r.runId === 'run-forged' || r.path === outside), []); + assert.equal(existsSync(outside), true); + assert.equal(worktreesOf(repo)[outside], null, 'still registered, still unlocked'); + assert.equal(existsSync(join(realRoot, 'run-forged.json')), true); +}); + +/** Make git refuse to remove a clean worktree, as it does once a submodule is set up in it. */ +const withSubmoduleDir = (path) => mkdirSync(join(git(path, 'rev-parse', '--absolute-git-dir'), 'modules')); + +test('a clean worktree git refuses to remove goes back under its lock', async () => { + const { repo, root } = setup(); + const wt = prepareWorktree({ repo, root, runId: 'run-refused' }); + withSubmoduleDir(wt.path); + assert.throws(() => settleWorktree(wt), /submodules/); + assert.equal(existsSync(wt.path), true); + assert.equal(worktreesOf(repo)[wt.path], lockReasonOf('run-refused'), 'relocked with our reason'); + assert.equal(existsSync(`${wt.path}.json`), true, 'the marker stays for the sweep'); +}); + +test('a supervised run whose worktree git refuses to remove reports kept: error and stays locked', async () => { + const { repo, root, store } = setup(); + const out = await superviseRun({ + adapter: working((cwd) => withSubmoduleDir(cwd)), prompt: 'p', store, runId: 'run-refused-2', ctx: { workspace: { repo, root } }, + }); + assert.equal(out.workspace.kept, 'error'); + assert.equal(worktreesOf(repo)[out.workspace.path], lockReasonOf('run-refused-2')); +}); + +test('a sweep entry for a git failure carries git\'s message', async () => { + const { repo, root } = setup(); + const wt = prepareWorktree({ repo, root, runId: 'run-sweep-refused' }); + withSubmoduleDir(wt.path); + const report = await sweepWorktrees({ root, isRunLive: () => false }); + assert.equal(report.length, 1); + assert.equal(report[0].action, 'skipped'); + assert.match(report[0].error, /submodules cannot be moved or removed/); + assert.ok(!report[0].error.includes('\n')); + assert.equal(existsSync(wt.path), true); + assert.equal(existsSync(`${wt.path}.json`), true); +}); + +test('a caller\'s GIT_OBJECT_DIRECTORY and alternates do not reach the worktree calls', () => { + const { repo, root, base } = setup(); + const elsewhere = mkdtempSync(join(sandbox, 'objects-')); + process.env.GIT_OBJECT_DIRECTORY = elsewhere; + process.env.GIT_ALTERNATE_OBJECT_DIRECTORIES = elsewhere; + try { + const wt = prepareWorktree({ repo, root, runId: 'run-env-objects' }); + assert.equal(wt.base, base); + assert.equal(settleWorktree(wt).removed, true); + } finally { + delete process.env.GIT_OBJECT_DIRECTORY; + delete process.env.GIT_ALTERNATE_OBJECT_DIRECTORIES; + } +}); + test('the sweep keeps a dead run\'s worktree that moved off its base, unlocked', async () => { const { repo, root, base } = setup(); const wt = prepareWorktree({ repo, root, runId: 'run-dead-committed' });