diff --git a/memory/CARDS.md b/memory/CARDS.md index de1ed3ce..744c7827 100644 --- a/memory/CARDS.md +++ b/memory/CARDS.md @@ -83,12 +83,18 @@ Status: **in progress.** --- -## Slice 2 — worktree + branch GC / lifecycle (light) — `next` after Slice 1 - -A finished run reclaims its worktrees + `brunch/{run,slice}/*` refs instead of the -operator-owned cleanup `worktree.ts` documents. Ref-set depends on Slice 1's final -branch topology, so scope after it lands. Keep-on-failure for inspection; promoted -artifact survives GC. +## Slice 2 — worktree + branch GC / lifecycle (light) — `done` + +Branch `ka/fe-883-worktree-gc` (stacked on FE-883). `gcCookRun` (run-refs.ts, +commit bf43477f) reclaims the run's worktrees (run + nested slice/__epic__, +deepest-first) + the intermediate `brunch/slice//*` branches, keeping the +`brunch/run/` artifact branch and every other run untouched; realpath-safe +(macOS /var→/private/var). Wired into cook-cli: auto-GC on a **completed + +promoted** brownfield run, best-effort (never fails a good run); halted/conflicted +runs return earlier and keep their worktrees for inspection (keep-on-failure). +Decision: auto-GC (no flag) — "no leaks by default". Tests: run-refs.test.ts +(reclaim + unrelated-run-untouched). Gap: no end-to-end runCook test exercises the +auto-GC call (same gap as the promotion wiring). ## Slice 3 — per-slice build-cache write isolation (candidate) diff --git a/src/orchestrator/src/cook-cli.ts b/src/orchestrator/src/cook-cli.ts index 939a1ac7..f7540872 100644 --- a/src/orchestrator/src/cook-cli.ts +++ b/src/orchestrator/src/cook-cli.ts @@ -17,7 +17,7 @@ import type { CookBus } from './presenter.js'; import { resolveToolchain } from './project-profile.js'; import { landCookBranch, promoteGreenfieldRun } from './promote-run.js'; import { harvestCookRun } from './run-artifact.js'; -import { brunchRef } from './run-refs.js'; +import { brunchRef, gcCookRun } from './run-refs.js'; import { parseSpecId, resolveLatestSpecPlanPath, specPlanPath, specsRootDir } from './spec-plan-paths.js'; import { ToolchainTestRunner } from './test-runner.js'; import type { Plan, PlanMode } from './types.js'; @@ -612,6 +612,15 @@ export async function runCook(opts: CookOptions, bus: CookBus): Promise { })) { line(l); } + // Completed + promoted: reclaim the run's worktrees + intermediate slice + // branches (the brunch/run/ artifact branch is kept). Best-effort — + // cleanup must never fail a good run. Halted/conflicted runs returned + // earlier, so they keep their worktrees for inspection (keep-on-failure). + try { + gcCookRun({ sourceDir: sandbox.sourceDir, runId, runDir }); + } catch { + /* leave the run dir if cleanup hiccups; not worth failing a promoted run */ + } } catch (err) { const reason = `promotion failed: ${err instanceof Error ? err.message : String(err)}`; line(` ✗ ${reason}`); diff --git a/src/orchestrator/src/promote-run.test.ts b/src/orchestrator/src/promote-run.test.ts index aafaa45c..6ff18703 100644 --- a/src/orchestrator/src/promote-run.test.ts +++ b/src/orchestrator/src/promote-run.test.ts @@ -5,7 +5,7 @@ import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { landCookBranch, promoteBrownfieldRun, promoteGreenfieldRun } from './promote-run.js'; +import { landCookBranch, promoteGreenfieldRun } from './promote-run.js'; const dirs: string[] = []; const GIT_TEST_TIMEOUT_MS = 20_000; @@ -206,164 +206,12 @@ describe('promoteGreenfieldRun', () => { ); }); -describe('promoteBrownfieldRun', () => { - const id = ['-c', 'user.name=t', '-c', 'user.email=t@e']; - - // A user repo on `main` with a base commit, plus a brunch/run/ branch at the - // same base (as `git worktree add -b brunch/run/ … HEAD` would create). - function userRepo(): { dir: string; baseHead: string } { - const dir = mkdtempSync(join(tmpdir(), 'cook-userrepo-')); - dirs.push(dir); - execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: dir }); - writeFileSync(join(dir, 'app.ts'), 'export const v = 1;\n'); - writeFileSync(join(dir, '.gitignore'), 'node_modules/\n'); - execFileSync('git', ['add', '.'], { cwd: dir }); - execFileSync('git', [...id, 'commit', '-q', '-m', 'base'], { cwd: dir }); - execFileSync('git', ['branch', 'brunch/run/r1'], { cwd: dir }); - const baseHead = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(); - return { dir, baseHead }; - } - - // The composed cook result: a full tree (base + the cook delta). - function composedTree(): string { - const d = mkdtempSync(join(tmpdir(), 'cook-composed-')); - dirs.push(d); - writeFileSync(join(d, 'app.ts'), 'export const v = 2;\n'); // modified - writeFileSync(join(d, 'feature.ts'), 'export const f = true;\n'); // added - writeFileSync(join(d, '.gitignore'), 'node_modules/\n'); - mkdirSync(join(d, 'node_modules')); - writeFileSync(join(d, 'node_modules', 'dep.js'), 'junk\n'); // gitignored — must not land - return d; - } - - it( - 'commits the composed tree onto brunch/run/, leaving the active branch and working tree untouched', - () => { - const { dir, baseHead } = userRepo(); - const tree = composedTree(); - const branchesBefore = execFileSync('git', ['branch', '--list'], { cwd: dir, encoding: 'utf8' }); - - const result = promoteBrownfieldRun({ sourceDir: dir, sourceTreeDir: tree, runId: 'r1' }); - - // brunch/run/r1 advanced by one commit on top of the base. - expect(result.branch).toBe('brunch/run/r1'); - expect(result.commit).not.toBe(baseHead); - const parent = execFileSync('git', ['rev-parse', 'brunch/run/r1^'], { - cwd: dir, - encoding: 'utf8', - }).trim(); - expect(parent).toBe(baseHead); - - // The commit's tree carries the delta — and not the gitignored deps. - const files = execFileSync('git', ['ls-tree', '-r', '--name-only', 'brunch/run/r1'], { - cwd: dir, - encoding: 'utf8', - }); - expect(files).toContain('feature.ts'); - expect(files).toContain('app.ts'); - expect(files).not.toContain('node_modules'); - const appAtCook = execFileSync('git', ['show', 'brunch/run/r1:app.ts'], { cwd: dir, encoding: 'utf8' }); - expect(appAtCook).toContain('v = 2'); - - // The user's active branch (main), HEAD, working tree, and index are untouched. - expect(execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim()).toBe( - baseHead, - ); - expect( - execFileSync('git', ['symbolic-ref', '--short', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(), - ).toBe('main'); - expect(readFileSync(join(dir, 'app.ts'), 'utf8')).toContain('v = 1'); - expect(existsSync(join(dir, 'feature.ts'))).toBe(false); - expect(execFileSync('git', ['status', '--porcelain'], { cwd: dir, encoding: 'utf8' })).toBe(''); - // Only brunch/run/r1 moved — no stray branches. - expect(execFileSync('git', ['branch', '--list'], { cwd: dir, encoding: 'utf8' })).toBe(branchesBefore); - }, - GIT_TEST_TIMEOUT_MS, - ); - - it('throws when the brunch/run/ branch is absent (must be created by the worktree)', () => { - const { dir } = userRepo(); - const tree = composedTree(); - expect(() => promoteBrownfieldRun({ sourceDir: dir, sourceTreeDir: tree, runId: 'missing' })).toThrow( - /brunch\/run\/missing/, - ); - }); - - it( - 'works in the real linked-worktree topology — the live sandbox worktree is left to be discarded, the main checkout untouched', - () => { - // Mirror production: brunch/run/r1 exists *because* a linked worktree checked it out. - const dir = mkdtempSync(join(tmpdir(), 'cook-userrepo-')); - dirs.push(dir); - execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: dir }); - writeFileSync(join(dir, 'app.ts'), 'export const v = 1;\n'); - writeFileSync(join(dir, '.gitignore'), 'node_modules/\n'); - execFileSync('git', ['add', '.'], { cwd: dir }); - execFileSync('git', [...id, 'commit', '-q', '-m', 'base'], { cwd: dir }); - const wt = join(dir, 'wt'); - execFileSync('git', ['worktree', 'add', '-q', '-b', 'brunch/run/r1', wt, 'HEAD'], { cwd: dir }); - const baseHead = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(); - - const result = promoteBrownfieldRun({ sourceDir: dir, sourceTreeDir: composedTree(), runId: 'r1' }); - - // Only brunch/run/r1 moved (one commit on the base). - expect( - execFileSync('git', ['rev-parse', 'brunch/run/r1^'], { cwd: dir, encoding: 'utf8' }).trim(), - ).toBe(baseHead); - expect(execFileSync('git', ['show', 'brunch/run/r1:app.ts'], { cwd: dir, encoding: 'utf8' })).toContain( - 'v = 2', - ); - // The main checkout is wholly untouched. - expect(execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim()).toBe( - baseHead, - ); - expect( - execFileSync('git', ['symbolic-ref', '--short', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(), - ).toBe('main'); - expect(readFileSync(join(dir, 'app.ts'), 'utf8')).toContain('v = 1'); - // tracked files untouched (the linked `wt/` dir is an expected untracked entry). - expect( - execFileSync('git', ['status', '--porcelain', '--untracked-files=no'], { - cwd: dir, - encoding: 'utf8', - }), - ).toBe(''); - expect(result.commit).not.toBe(baseHead); - }, - GIT_TEST_TIMEOUT_MS, - ); - - it('stages tracked deletions — a file removed in the composed tree is removed in the cook commit', () => { - const dir = mkdtempSync(join(tmpdir(), 'cook-userrepo-')); - dirs.push(dir); - execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: dir }); - writeFileSync(join(dir, 'keep.ts'), 'keep\n'); - writeFileSync(join(dir, 'old.ts'), 'remove me\n'); - execFileSync('git', ['add', '.'], { cwd: dir }); - execFileSync('git', [...id, 'commit', '-q', '-m', 'base'], { cwd: dir }); - execFileSync('git', ['branch', 'brunch/run/r1'], { cwd: dir }); - - // Composed tree drops old.ts. - const tree = mkdtempSync(join(tmpdir(), 'cook-composed-')); - dirs.push(tree); - writeFileSync(join(tree, 'keep.ts'), 'keep\n'); - - promoteBrownfieldRun({ sourceDir: dir, sourceTreeDir: tree, runId: 'r1' }); - - const files = execFileSync('git', ['ls-tree', '-r', '--name-only', 'brunch/run/r1'], { - cwd: dir, - encoding: 'utf8', - }); - expect(files).toContain('keep.ts'); - expect(files).not.toContain('old.ts'); - }); -}); - describe('landCookBranch', () => { const id = ['-c', 'user.name=t', '-c', 'user.email=t@e']; // A user repo on `main` with one base commit and a promoted brunch/run/r1 branch - // (the composed result already committed on top of base via promoteBrownfieldRun). + // carrying the composed result one commit ahead of base (what a cook run leaves — + // built here via a throwaway worktree on the run branch, the shape landCookBranch merges). function repoWithPromotedCook(): { dir: string; baseHead: string; cookCommit: string } { const dir = mkdtempSync(join(tmpdir(), 'cook-land-')); dirs.push(dir); @@ -375,12 +223,15 @@ describe('landCookBranch', () => { execFileSync('git', ['branch', 'brunch/run/r1'], { cwd: dir }); const baseHead = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(); - const tree = mkdtempSync(join(tmpdir(), 'cook-land-tree-')); - dirs.push(tree); - writeFileSync(join(tree, 'app.ts'), 'export const v = 2;\n'); - writeFileSync(join(tree, 'feature.ts'), 'export const f = true;\n'); - writeFileSync(join(tree, '.gitignore'), 'node_modules/\n'); - const { commit } = promoteBrownfieldRun({ sourceDir: dir, sourceTreeDir: tree, runId: 'r1' }); + const wt = mkdtempSync(join(tmpdir(), 'cook-land-wt-')); + dirs.push(wt); + execFileSync('git', ['worktree', 'add', '-q', wt, 'brunch/run/r1'], { cwd: dir }); + writeFileSync(join(wt, 'app.ts'), 'export const v = 2;\n'); + writeFileSync(join(wt, 'feature.ts'), 'export const f = true;\n'); + execFileSync('git', ['add', '-A'], { cwd: wt }); + execFileSync('git', [...id, 'commit', '-q', '-m', 'cook: r1'], { cwd: wt }); + const commit = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: wt, encoding: 'utf8' }).trim(); + execFileSync('git', ['worktree', 'remove', '--force', wt], { cwd: dir }); return { dir, baseHead, cookCommit: commit }; } diff --git a/src/orchestrator/src/promote-run.ts b/src/orchestrator/src/promote-run.ts index 1bba3d8a..7fece799 100644 --- a/src/orchestrator/src/promote-run.ts +++ b/src/orchestrator/src/promote-run.ts @@ -1,7 +1,6 @@ import { execFileSync } from 'node:child_process'; -import { cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, realpathSync, rmSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { basename, isAbsolute, join, relative, resolve } from 'node:path'; +import { cpSync, existsSync, mkdirSync, readdirSync, realpathSync } from 'node:fs'; +import { basename, isAbsolute, relative, resolve } from 'node:path'; import { brunchRef } from './run-refs.js'; @@ -25,14 +24,6 @@ export type PromoteOptions = { force: boolean; }; -export type BrownfieldPromoteOptions = { - /** The user's repo root the brownfield cook ran against (a worktree of it). */ - sourceDir: string; - /** The composed final tree to land (from `promotionSourceDir`). */ - sourceTreeDir: string; - runId: string; -}; - function git(args: string[], cwd: string, env?: NodeJS.ProcessEnv): string { return execFileSync('git', args, { cwd, env, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); } @@ -142,68 +133,6 @@ export function promoteGreenfieldRun(opts: PromoteOptions): PromoteResult { return { target, branch, commit }; } -/** - * Land a completed *brownfield* run's composed tree onto the `brunch/run/` - * branch of the user's repo as one reviewable commit — the brownfield analogue - * of `promoteGreenfieldRun`. The brownfield sandbox was created with - * `git worktree add -b brunch/run/ … HEAD`, so the branch already exists at the - * base the run started from; this commits the result on top of it via plumbing - * (`commit-tree` + compare-and-swap `update-ref`) using a throwaway index and an - * external work-tree, so the user's real working tree, index, and active branch - * are never touched. Merging `brunch/run/` into the working branch stays the - * user's call — promotion never freelances into it. - */ -export function promoteBrownfieldRun(opts: BrownfieldPromoteOptions): PromoteResult { - const sourceDir = resolve(opts.sourceDir); - const sourceTreeDir = resolve(opts.sourceTreeDir); - const branch = brunchRef.run(opts.runId); - const ref = `refs/heads/${branch}`; - - // The branch must already exist (the sandbox branched it from HEAD); its tip is - // the parent we commit on top of and the CAS expected-value for update-ref. - let parent: string; - try { - parent = git(['rev-parse', '--verify', ref], sourceDir); - } catch { - throw new Error( - `Brownfield promotion expects an existing ${branch} branch in ${sourceDir} (created by the cook worktree).`, - ); - } - - // Absolute git dir so a throwaway index + external work-tree can target the - // user's object store without depending on cwd. - const gitDir = resolve(sourceDir, git(['rev-parse', '--git-dir'], sourceDir)); - const tmp = mkdtempSync(join(tmpdir(), 'brunch-promote-')); - const env: NodeJS.ProcessEnv = { ...process.env, GIT_INDEX_FILE: join(tmp, 'index') }; - const plumb = ['--git-dir', gitDir, '--work-tree', sourceTreeDir]; - try { - // Seed the index from the base, then stage the composed tree as the delta — - // adds, modifications, and deletions, all relative to the base commit. - git([...plumb, 'read-tree', parent], sourceDir, env); - git([...plumb, 'add', '-A'], sourceDir, env); - const tree = git(['--git-dir', gitDir, 'write-tree'], sourceDir, env); - const commit = git( - [ - ...COMMIT_IDENTITY, - '--git-dir', - gitDir, - 'commit-tree', - tree, - '-p', - parent, - '-m', - `cook: ${opts.runId}`, - ], - sourceDir, - env, - ); - git(['--git-dir', gitDir, 'update-ref', ref, commit, parent], sourceDir, env); - return { target: sourceDir, branch, commit }; - } finally { - rmSync(tmp, { recursive: true, force: true }); - } -} - /** * Merge a promoted `brunch/run/` branch into the repo's checked-out branch — the * opt-in counterpart to brownfield promotion's hands-off default. Promotion diff --git a/src/orchestrator/src/run-refs.test.ts b/src/orchestrator/src/run-refs.test.ts index 3f723f74..9912f091 100644 --- a/src/orchestrator/src/run-refs.test.ts +++ b/src/orchestrator/src/run-refs.test.ts @@ -1,11 +1,12 @@ import { execFileSync } from 'node:child_process'; import { mkdtempSync, rmSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { brunchRef, pruneWorktrees } from './run-refs.js'; +import { brunchRef, gcCookRun, pruneWorktrees } from './run-refs.js'; describe('brunchRef — the run ref namespace', () => { it('builds run and slice refs under sibling brunch/{run,slice} segments', () => { @@ -54,3 +55,61 @@ describe('pruneWorktrees', () => { ).not.toThrow(); }); }); + +describe('gcCookRun', () => { + let source: string; + + function git(...args: string[]): string { + return execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@t', ...args], { + cwd: source, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); + } + function branchExists(ref: string): boolean { + try { + git('rev-parse', '--verify', `refs/heads/${ref}`); + return true; + } catch { + return false; + } + } + /** A run rooted at .brunch/cook/runs//worktree with one nested slice worktree. */ + function seedRun(runId: string): { runDir: string } { + const runDir = join(source, '.brunch', 'cook', 'runs', runId); + const parent = join(runDir, 'worktree'); + git('worktree', 'add', '-q', '-b', brunchRef.run(runId), parent, 'HEAD'); + git('worktree', 'add', '-q', '-b', brunchRef.slice(runId, 'a'), join(parent, 'a'), brunchRef.run(runId)); + return { runDir }; + } + + beforeEach(() => { + source = mkdtempSync(join(tmpdir(), 'brunch-gc-')); + git('init', '-q', '-b', 'main'); + git('commit', '-q', '--allow-empty', '-m', 'base'); + }); + afterEach(() => rmSync(source, { recursive: true, force: true })); + + it('reclaims run worktrees + slice branches but keeps the run branch artifact', () => { + const { runDir } = seedRun('r1'); + const artifact = git('rev-parse', brunchRef.run('r1')); + + gcCookRun({ sourceDir: source, runId: 'r1', runDir }); + + expect(existsSync(runDir)).toBe(false); // worktree dirs gone + expect(branchExists(brunchRef.slice('r1', 'a'))).toBe(false); // intermediate slice branch deleted + expect(branchExists(brunchRef.run('r1'))).toBe(true); // the promoted artifact survives + expect(git('rev-parse', brunchRef.run('r1'))).toBe(artifact); + }); + + it('leaves an unrelated run untouched', () => { + const { runDir: r1Dir } = seedRun('r1'); + const { runDir: r2Dir } = seedRun('r2'); + + gcCookRun({ sourceDir: source, runId: 'r1', runDir: r1Dir }); + + expect(existsSync(r2Dir)).toBe(true); + expect(branchExists(brunchRef.slice('r2', 'a'))).toBe(true); + expect(branchExists(brunchRef.run('r2'))).toBe(true); + }); +}); diff --git a/src/orchestrator/src/run-refs.ts b/src/orchestrator/src/run-refs.ts index a878a8a7..ce066f8e 100644 --- a/src/orchestrator/src/run-refs.ts +++ b/src/orchestrator/src/run-refs.ts @@ -1,4 +1,6 @@ import { execFileSync } from 'node:child_process'; +import { realpathSync, rmSync } from 'node:fs'; +import { relative, resolve, sep } from 'node:path'; /** * Single source of truth for the git ref namespace a cook run owns. Sibling @@ -26,3 +28,57 @@ export const brunchRef = { export function pruneWorktrees(repoDir: string): void { execFileSync('git', ['worktree', 'prune'], { cwd: repoDir, stdio: ['ignore', 'pipe', 'pipe'] }); } + +function git(args: string[], cwd: string): string { + return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +} + +function gitOk(args: string[], cwd: string): void { + try { + git(args, cwd); + } catch { + /* best-effort cleanup — a missing worktree/branch is already the desired state */ + } +} + +function isInside(parent: string, child: string): boolean { + const rel = relative(parent, child); + return rel === '' || (!rel.startsWith('..') && !rel.startsWith(sep) && rel !== '..'); +} + +/** + * Reclaim a finished run's transient state: remove every worktree registered under + * `runDir` (the run worktree + its nested slice / `__epic__` worktrees), delete the + * intermediate `brunch/slice//*` branches (their work is folded into the run + * branch), and remove the on-disk `runDir`. The `brunch/run/` branch — the + * promoted artifact the user reviews/merges — is deliberately kept, as is every + * other run's state. Best-effort and idempotent: an already-gone worktree or branch + * is the desired end state, not an error. + */ +export function gcCookRun(opts: { sourceDir: string; runId: string; runDir: string }): void { + const sourceDir = resolve(opts.sourceDir); + // Canonicalize: `git worktree list` reports realpath-resolved paths (e.g. macOS + // /var → /private/var), so compare against a realpath'd runDir or nothing matches. + const runDir = realpathSync(resolve(opts.runDir)); + + // Worktrees nested under runDir, deepest first so a parent never blocks a child. + const worktreePaths = git(['worktree', 'list', '--porcelain'], sourceDir) + .split('\n') + .filter((l) => l.startsWith('worktree ')) + .map((l) => resolve(l.slice('worktree '.length))) + .filter((p) => isInside(runDir, p)) + .sort((a, b) => b.length - a.length); + for (const wt of worktreePaths) gitOk(['worktree', 'remove', '--force', wt], sourceDir); + pruneWorktrees(sourceDir); + + // Intermediate slice branches (folded into the run branch); keep brunch/run/. + // No trailing slash: for-each-ref matches a prefix at the /-boundary, so + // `refs/heads/brunch/slice/` covers every slice branch of this run. + const sliceBranchPrefix = `refs/heads/${brunchRef.slice(opts.runId, '').replace(/\/$/, '')}`; + const sliceBranches = git(['for-each-ref', '--format=%(refname:short)', sliceBranchPrefix], sourceDir) + .split('\n') + .filter(Boolean); + for (const branch of sliceBranches) gitOk(['branch', '-D', branch], sourceDir); + + rmSync(runDir, { recursive: true, force: true }); +}