From 37e748bd12ead447480dff32ac5bc6556663f404 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 25 Sep 2026 00:40:12 -0400 Subject: [PATCH 1/4] feat: gc removes worktrees whose branch is merged Plain gc reports every Stim-managed linked worktree whose branch is merged into origin/HEAD, and gc --delete removes it through worktree remove. Merge state comes from git: a merge commit, a rebase merge, or a squash merge detected by patch-id, after one bounded fetch per repository. Unknown state keeps the worktree. --- .../src/__tests__/gc-workspaces.test.ts | 169 +++++++++++++++++- packages/stim-cli/src/commands/gc.ts | 10 +- packages/stim-cli/src/commands/gc/report.ts | 26 +-- .../stim-cli/src/commands/gc/worktrees.ts | 109 +++++++++-- packages/stim-cli/src/commands/worktree.ts | 11 +- packages/stim-cli/src/guide/agent.ts | 5 +- packages/stim-cli/src/guide/cleanup.ts | 66 ++++--- packages/stim-cli/src/guide/facts.ts | 15 +- .../stim-cli/src/workspace/merge-state.ts | 127 +++++++++++++ website/docs/commands.md | 37 +++- website/docs/worktrees.md | 49 +++-- 11 files changed, 539 insertions(+), 85 deletions(-) create mode 100644 packages/stim-cli/src/workspace/merge-state.ts diff --git a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts index 613e3550e..cfdd0d261 100644 --- a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts +++ b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts @@ -18,6 +18,7 @@ import { runGc } from '../commands/gc.ts'; import { matchWorktreeEntry, removeWorktreeTarget } from '../commands/worktree.ts'; import { classifyWorkspaceDirs, listWorkspaceDirs, planWorkspaceOutputs } from '../commands/gc/workspaces.ts'; import { worktreeSkipReason, type WorktreeFacts } from '../commands/gc/worktrees.ts'; +import type { MergeState } from '../workspace/merge-state.ts'; import { getProject, saveConfig, upsertProject } from '../workspace/config.ts'; import { register } from '../cache/cache-manifest.ts'; import { ensureWorkspaceStorage, workspaceDir } from '../workspace/paths.ts'; @@ -457,13 +458,49 @@ describe('linked worktree sweep classification', () => { submodules: false, inUse: [], idleDays: 10, + merge: null, ...overrides, }); + const merged = (coversUnpushed = false): MergeState => ({ + merged: true, + into: 'origin/main', + head: 'abc', + coversUnpushed, + }); test('a clean, pushed, idle linked worktree is removable', () => { expect(worktreeSkipReason(linked(), 7)).toBe(null); }); + test('a merged worktree is removable without the idle rule, even when used today', () => { + expect(worktreeSkipReason(linked({ idleDays: 0, merge: merged() }), null)).toBe(null); + }); + + test('local-only commits block a merged worktree unless its upstream was deleted after the merge', () => { + expect(worktreeSkipReason(linked({ unpushed: ['abc wip'], merge: merged() }), null)?.code).toBe('unpushed'); + expect(worktreeSkipReason(linked({ unpushed: ['abc wip'], merge: merged(true) }), null)).toBe(null); + }); + + test.each([ + ['dirty', linked({ porcelain: ['?? notes.txt'], merge: merged() }), 'dirty'], + ['in use', linked({ inUse: ['its dev server supervisor (pid 1) is running'], merge: merged() }), 'in-use'], + ['locked', linked({ locked: true, merge: merged() }), 'locked'], + ['the source checkout', linked({ source: 'source', merge: merged() }), 'source-checkout'], + ['with submodules', linked({ submodules: true, merge: merged(true) }), 'submodules'], + [ + 'not merged', + linked({ merge: { merged: false, unknown: false, detail: 'not merged into origin/main' } }), + 'not-merged', + ], + [ + 'of unknown merge state', + linked({ merge: { merged: false, unknown: true, detail: 'merge state unknown: fetch failed' } }), + 'merge-unknown', + ], + ])('without --worktrees, a worktree %s is kept', (_name, facts, code) => { + expect(worktreeSkipReason(facts, null)?.code).toBe(code); + }); + test('pod install churn alone does not keep a worktree', () => { expect( worktreeSkipReason(linked({ porcelain: [' M ios/Podfile.lock', ' M ios/App.xcodeproj/project.pbxproj'] }), 7), @@ -583,14 +620,15 @@ test('gc --worktrees --json reports each worktree verdict with its idle threshol expect(byPath[realpathSync.native(worktrees.idle!)]).toEqual({ path: expect.any(String), idleDays: 10, + mergedInto: null, willRemove: true, reason: null, - detail: null, + detail: 'idle 10d', }); expect(byPath[realpathSync.native(worktrees.fresh!)]).toMatchObject({ willRemove: false, reason: 'recently-used', - detail: 'recently used 1d ago', + detail: expect.stringMatching(/^recently used 1d ago; merge state unknown: origin\/HEAD is not set/), }); expect(byPath[realpathSync.native(repo)]).toMatchObject({ willRemove: false, reason: 'source-checkout' }); expect(existsSync(worktrees.idle!)).toBe(true); @@ -675,6 +713,133 @@ test('worktree removal re-checks for new work under its removal locks', async () expect(errors.join('\n')).toMatch(/uncommitted changes or untracked files/); }, 30_000); +function repoWithMergedBranches() { + const repo = join(projects, 'origin-repo'); + const remote = join(projects, 'origin.git'); + const upstream = join(projects, 'upstream'); + const git = (args: string, cwd = repo) => execSync(`git ${args}`, { cwd, encoding: 'utf-8', timeout: 30_000 }); + const identity = (cwd: string) => { + git('config user.email test@example.com', cwd); + git('config user.name test', cwd); + }; + const commit = (cwd: string, file: string) => { + writeFileSync(join(cwd, file), file); + git(`add ${file}`, cwd); + git(`commit -q -m ${file}`, cwd); + }; + git(`init -q --bare -b main "${remote}"`, projects); + mkdirSync(repo); + git('init -q -b main'); + identity(repo); + git(`remote add origin "${remote}"`); + commit(repo, 'package.json'); + git('push -q -u origin main'); + git('remote set-head origin main'); + const worktrees: Record = {}; + for (const [name, commits] of [ + ['merged', 1], + ['squashed', 2], + ['fresh', 0], + ['open', 1], + ['dirty', 1], + ] as const) { + const path = join(projects, name); + git(`worktree add -q "${path}" -b ${name}`); + for (let i = 0; i < commits; i++) commit(path, `${name}-${i}.txt`); + if (commits) git(`push -q -u origin ${name}`, path); + worktrees[name] = realpathSync.native(path); + } + git(`clone -q "${remote}" "${upstream}"`, projects); + identity(upstream); + commit(upstream, 'main-moved-on.txt'); + git('merge -q --no-ff origin/merged -m merge-merged', upstream); + git('merge -q --no-ff origin/dirty -m merge-dirty', upstream); + git('merge -q --squash origin/squashed', upstream); + git('commit -q -m squash-squashed', upstream); + git('push -q origin main', upstream); + git('push -q origin --delete squashed', upstream); + git('update-ref -d refs/remotes/origin/squashed'); + writeFileSync(join(worktrees.dirty!, 'notes.txt'), 'wip'); + for (const path of Object.values(worktrees)) { + upsertProject(path, { metroPort: null }); + recordWorkspaceUse(path); + } + return { repo, remote, worktrees, git }; +} + +test('plain gc --delete removes merged worktrees, squash merges included, after fetching the default branch', async () => { + const { repo, worktrees, git } = repoWithMergedBranches(); + + const { payload } = await gcJson({}); + expect(payload.worktreeSweep).toBe(null); + const byPath = Object.fromEntries( + payload.sections.linkedWorktrees.map((w: { path: string }) => [realpathSync.native(w.path), w]), + ); + expect(Object.keys(byPath).toSorted()).toEqual(Object.values(worktrees).toSorted()); + expect(byPath[worktrees.merged!]).toMatchObject({ + mergedInto: 'origin/main', + willRemove: true, + reason: null, + detail: 'merged into origin/main', + }); + expect(byPath[worktrees.squashed!]).toMatchObject({ willRemove: true, detail: 'merged into origin/main' }); + expect(byPath[worktrees.fresh!]).toMatchObject({ + willRemove: false, + reason: 'not-merged', + detail: 'no commits of its own beyond origin/main', + }); + expect(byPath[worktrees.open!]).toMatchObject({ willRemove: false, reason: 'not-merged' }); + expect(byPath[worktrees.dirty!]).toMatchObject({ willRemove: false, reason: 'dirty' }); + + const output = await captureLog(() => runGc({ delete: true })); + expect(output).toContain(`Removed the worktree ${worktrees.merged} (merged into origin/main)`); + expect(output).toContain(`Removed the worktree ${worktrees.squashed} (merged into origin/main)`); + expect(existsSync(worktrees.merged!)).toBe(false); + expect(existsSync(worktrees.squashed!)).toBe(false); + for (const name of ['fresh', 'open', 'dirty']) expect(existsSync(worktrees[name]!)).toBe(true); + expect(existsSync(join(repo, 'package.json'))).toBe(true); + expect(git('branch --list squashed')).toContain('squashed'); + expect(process.exitCode).not.toBe(1); +}, 120_000); + +test('gc keeps a merged worktree when the fetch fails, and when its HEAD moves after the report', async () => { + const { remote, worktrees, git } = repoWithMergedBranches(); + git('fetch -q origin'); + renameSync(remote, `${remote}.moved`); + + const { payload } = await gcJson({ delete: true }); + const merged = payload.sections.linkedWorktrees.find( + (w: { path: string }) => realpathSync.native(w.path) === worktrees.merged, + ); + expect(merged).toMatchObject({ willRemove: false, reason: 'merge-unknown' }); + expect(merged.detail).toMatch(/^merge state unknown: git fetch origin main failed/); + expect(existsSync(worktrees.merged!)).toBe(true); + + renameSync(`${remote}.moved`, remote); + const lines: string[] = []; + const original = console.log; + const originalError = console.error; + console.error = () => {}; + console.log = (...args) => { + lines.push(args.join(' ')); + if (String(args[0]).startsWith('Linked worktrees')) { + writeFileSync(join(worktrees.merged!, 'late.txt'), 'x'); + git('add late.txt', worktrees.merged); + git('commit -q -m late', worktrees.merged); + git('push -q', worktrees.merged); + } + }; + try { + await runGc({ delete: true }); + } finally { + console.log = original; + console.error = originalError; + } + expect(existsSync(worktrees.merged!)).toBe(true); + expect(lines.join('\n')).toContain(`Kept the worktree ${worktrees.merged}: its HEAD moved since gc checked it`); + expect(existsSync(worktrees.squashed!)).toBe(false); +}, 120_000); + test('a worktree registered under a symlinked path still matches the path git reports', () => { const real = join(projects, 'real-worktree'); mkdirSync(real, { recursive: true }); diff --git a/packages/stim-cli/src/commands/gc.ts b/packages/stim-cli/src/commands/gc.ts index 25073806d..9d2fe6818 100644 --- a/packages/stim-cli/src/commands/gc.ts +++ b/packages/stim-cli/src/commands/gc.ts @@ -357,7 +357,7 @@ export async function collectGcReport( now, exclude: [...workspaceDirs.orphaned.map((entry) => entry.dir), ...deadProjects.map(workspaceDir)], }), - worktreeSweep: worktrees ? collectWorktreeSweep({ olderThan, now }) : null, + worktreeSweep: collectWorktreeSweep({ idle: worktrees, olderThan, now }), cacheScope: null, olderThan, all, @@ -537,9 +537,7 @@ async function runGcCore(opts: RunGcOptions, deps: GcDependencies): Promise !w.skipped); - const idle = sweep.defaulted - ? `idle ${sweep.olderThan}d or more (the default without --older-than)` - : `idle ${sweep.olderThan}d or more`; + const idle = !sweep.idle + ? '' + : sweep.idle.defaulted + ? ` or idle ${sweep.idle.olderThan}d or more (the default without --older-than)` + : ` or idle ${sweep.idle.olderThan}d or more`; const lines = [ - `Linked worktrees (${removable.length} removable, ${sweep.worktrees.length - removable.length} kept) - clean, pushed, ${idle}:`, + `Linked worktrees (${removable.length} removable, ${sweep.worktrees.length - removable.length} kept) - clean, pushed, merged into the default branch${idle}:`, ]; for (const w of sweep.worktrees) { const age = w.idleDays === null ? '' : ` (idle ${w.idleDays}d)`; lines.push(` ${w.path}${age}`); lines.push( - w.skipped ? ` kept: ${w.skipped}` : ' would be REMOVED by `stim worktree remove`', + w.skipped + ? ` kept: ${w.skipped}` + : ` would be REMOVED by \`stim worktree remove\`: ${worktreeRemovalReason(w)}`, ); } if (removable.length) { lines.push( - ' --delete runs it without --force; use and idleness are re-checked under its removal locks.', + ' --delete runs it without --force; use, idleness and HEAD are re-checked under its removal locks.', ); } return lines; @@ -428,9 +432,10 @@ export interface GcJsonSections { linkedWorktrees: { path: string; idleDays: number | null; + mergedInto: string | null; willRemove: boolean; reason: WorktreeSkipCode | null; - detail: string | null; + detail: string; }[]; parkedSimulators: ParkedSimReport[]; parkedEmulators: ParkedAvdReport[]; @@ -530,9 +535,10 @@ export function gcReportSections({ linkedWorktrees: (worktreeSweep?.worktrees ?? []).map((w) => ({ path: w.path, idleDays: w.idleDays, + mergedInto: w.merge?.merged ? w.merge.into : null, willRemove: w.skipCode === null, reason: w.skipCode, - detail: w.skipped, + detail: w.skipped ?? worktreeRemovalReason(w), })), parkedSimulators: parkedSims.map(({ udid, name, model, runtime, parkedAt, bytes, listed }) => ({ udid, diff --git a/packages/stim-cli/src/commands/gc/worktrees.ts b/packages/stim-cli/src/commands/gc/worktrees.ts index a0a3c241b..deed7d6b6 100644 --- a/packages/stim-cli/src/commands/gc/worktrees.ts +++ b/packages/stim-cli/src/commands/gc/worktrees.ts @@ -5,11 +5,14 @@ import { plural } from '../../command-output.ts'; import { loadConfig } from '../../workspace/config.ts'; import { workspaceInUse } from '../../workspace/in-use.ts'; import { workspaceLastUsed } from '../../workspace/workspace-state.ts'; +import { fetchDefaultBranch, mergeState, type MergeState } from '../../workspace/merge-state.ts'; import { dirtyPaths, + gitCommonDir, hasPopulatedSubmodules, hasUncommittedWork, listWorktrees, + resolveFullRef, sourceCheckoutOf, unpushedCommits, } from '../../workspace/worktree.ts'; @@ -29,6 +32,7 @@ export interface WorktreeFacts { submodules: boolean; inUse: string[]; idleDays: number | null; + merge: MergeState | null; } export type WorktreeSkipCode = @@ -43,6 +47,8 @@ export type WorktreeSkipCode = | 'unpushed-unchecked' | 'unpushed' | 'submodules' + | 'not-merged' + | 'merge-unknown' | 'last-use-unknown' | 'recently-used'; @@ -55,21 +61,28 @@ interface WorktreeCandidate { path: string; keys: string[]; idleDays: number | null; + merge: MergeState | null; skipCode: WorktreeSkipCode | null; skipped: string | null; } export interface WorktreeSweep { - olderThan: number; - defaulted: boolean; + idle: { olderThan: number; defaulted: boolean } | null; worktrees: WorktreeCandidate[]; } +const MERGE_DECIDES: ReadonlySet = new Set([ + 'unpushed', + 'not-merged', + 'last-use-unknown', + 'recently-used', +]); + function skip(code: WorktreeSkipCode, text: string): WorktreeSkip { return { code, text }; } -export function worktreeSkipReason(facts: WorktreeFacts, olderThan: number): WorktreeSkip | null { +export function worktreeSkipReason(facts: WorktreeFacts, olderThan: number | null): WorktreeSkip | null { if (facts.bare) return skip('bare-repository', 'bare repository'); if (typeof facts.source === 'object') { return skip('source-checkout-unknown', `source checkout unknown: ${facts.source.refusal}`); @@ -82,15 +95,27 @@ export function worktreeSkipReason(facts: WorktreeFacts, olderThan: number): Wor return skip('dirty', 'dirty: uncommitted changes or untracked files'); } if (facts.unpushed === null) return skip('unpushed-unchecked', 'unpushed commits could not be checked'); - if (facts.unpushed.length) { + const merged = facts.merge?.merged ? facts.merge : null; + if (facts.unpushed.length && !merged?.coversUnpushed) { return skip('unpushed', `unpushed: ${plural(facts.unpushed.length, 'commit')} on no remote or other branch`); } if (facts.submodules) return skip('submodules', 'initialized submodules'); - if (facts.idleDays === null) return skip('last-use-unknown', 'recently used: its last use is unknown'); - if (facts.idleDays < olderThan) return skip('recently-used', `recently used ${facts.idleDays}d ago`); + if (merged) return null; + const notMerged = facts.merge && !facts.merge.merged ? facts.merge : null; + if (olderThan === null) { + return skip(notMerged?.unknown ? 'merge-unknown' : 'not-merged', notMerged?.detail ?? 'not merged'); + } + const also = notMerged ? `; ${notMerged.detail}` : ''; + if (facts.idleDays === null) return skip('last-use-unknown', `recently used: its last use is unknown${also}`); + if (facts.idleDays < olderThan) return skip('recently-used', `recently used ${facts.idleDays}d ago${also}`); return null; } +/** Why gc removes a worktree it did not skip: `merged into origin/main` or `idle 9d`. */ +export function worktreeRemovalReason(candidate: Pick): string { + return candidate.merge?.merged ? `merged into ${candidate.merge.into}` : `idle ${candidate.idleDays ?? 0}d`; +} + function lastUsedOf(keys: readonly string[]): number { const times = keys.map(workspaceLastUsed).filter(Number.isFinite); return times.length ? Math.max(...times) : NaN; @@ -118,8 +143,39 @@ function candidateRoots(): string[] { return [...new Set([...registered, ...recorded])].filter((root) => existsSync(root)).toSorted(); } -export function collectWorktreeSweep({ olderThan, now }: { olderThan: number | null; now: number }): WorktreeSweep { - const days = olderThan ?? DEFAULT_WORKTREE_IDLE_DAYS; +function checkMergeStates(pending: { candidate: WorktreeCandidate; facts: WorktreeFacts }[], idle: number | null) { + const repos = new Map(); + for (const entry of pending) { + const common = gitCommonDir(entry.candidate.path) ?? entry.candidate.path; + repos.set(common, [...(repos.get(common) ?? []), entry]); + } + for (const entries of repos.values()) { + const target = fetchDefaultBranch(entries[0]!.candidate.path); + for (const { candidate, facts } of entries) { + const merge: MergeState = + 'error' in target + ? { merged: false, unknown: true, detail: `merge state unknown: ${target.error}` } + : mergeState(candidate.path, target); + const verdict = worktreeSkipReason({ ...facts, merge }, idle); + Object.assign(candidate, { merge, skipCode: verdict?.code ?? null, skipped: verdict?.text ?? null }); + } + } +} + +/** + * Classifies each Stim-managed linked worktree. A merged one is removable; with `idle`, so is one unused for + * `olderThan` days. Merge state is checked only where it decides the verdict, after one fetch per repository. + */ +export function collectWorktreeSweep({ + idle, + olderThan, + now, +}: { + idle: boolean; + olderThan: number | null; + now: number; +}): WorktreeSweep { + const days = idle ? (olderThan ?? DEFAULT_WORKTREE_IDLE_DAYS) : null; const groups = new Map(); const outside: WorktreeCandidate[] = []; for (const root of candidateRoots()) { @@ -129,6 +185,7 @@ export function collectWorktreeSweep({ olderThan, now }: { olderThan: number | n path: root, keys: [root], idleDays: null, + merge: null, skipCode: 'not-a-worktree', skipped: 'not inside a git worktree', }); @@ -137,6 +194,7 @@ export function collectWorktreeSweep({ olderThan, now }: { olderThan: number | n groups.set(entry.path, [...(groups.get(entry.path) ?? []), root]); } const worktrees: WorktreeCandidate[] = []; + const pending: { candidate: WorktreeCandidate; facts: WorktreeFacts }[] = []; for (const [path, roots] of groups) { const entries = listWorktrees(path); const entry = matchWorktreeEntry(entries, path); @@ -154,11 +212,25 @@ export function collectWorktreeSweep({ olderThan, now }: { olderThan: number | n submodules: linked && hasPopulatedSubmodules(path), inUse: linked ? inUseOf(keys, { managedLocks: true }) : [], idleDays, + merge: null, }; const verdict = worktreeSkipReason(facts, days); - worktrees.push({ path, keys, idleDays, skipCode: verdict?.code ?? null, skipped: verdict?.text ?? null }); + const candidate: WorktreeCandidate = { + path, + keys, + idleDays, + merge: null, + skipCode: verdict?.code ?? null, + skipped: verdict?.text ?? null, + }; + if (verdict && MERGE_DECIDES.has(verdict.code)) pending.push({ candidate, facts }); + worktrees.push(candidate); } - return { olderThan: days, defaulted: olderThan === null, worktrees: [...worktrees, ...outside] }; + checkMergeStates(pending, days); + const listed = [...worktrees, ...outside].filter( + (w) => idle || (w.skipCode !== 'not-a-worktree' && w.skipCode !== 'source-checkout'), + ); + return { idle: days === null ? null : { olderThan: days, defaulted: olderThan === null }, worktrees: listed }; } export async function removeWorktrees( @@ -176,21 +248,28 @@ export async function removeWorktrees( ...inUseOf(lockedKeys, { managedLocks: false }), ...inUseOf(unlocked, { managedLocks: true }), ].map((r) => `in use: ${r}`); - const idleDays = idleDaysOf(keys, now); - if (idleDays === null || idleDays < sweep.olderThan) { - reasons.push(`used ${idleDays === null ? 'at an unknown time' : `${idleDays}d ago`} since gc checked it`); + if (candidate.merge?.merged) { + if (resolveFullRef(candidate.path, 'HEAD') !== candidate.merge.head) { + reasons.push('its HEAD moved since gc checked it'); + } + } else { + const idleDays = idleDaysOf(keys, now); + if (idleDays === null || !sweep.idle || idleDays < sweep.idle.olderThan) { + reasons.push(`used ${idleDays === null ? 'at an unknown time' : `${idleDays}d ago`} since gc checked it`); + } } kept = reasons; return reasons; }; let removed = false; try { - removed = await removeWorktreeTarget(candidate.path, { linkedOnly: true, guard }); + const mergedHead = candidate.merge?.merged && candidate.merge.coversUnpushed ? candidate.merge.head : undefined; + removed = await removeWorktreeTarget(candidate.path, { linkedOnly: true, guard, mergedHead }); } catch (error) { console.error(chalk.red(`Could not remove ${candidate.path}: ${(error as Error)?.message || String(error)}`)); } if (removed) { - console.log(chalk.green(`Removed the worktree ${candidate.path}`)); + console.log(chalk.green(`Removed the worktree ${candidate.path} (${worktreeRemovalReason(candidate)})`)); } else if (kept.length) { console.log(chalk.yellow(`Kept the worktree ${candidate.path}: ${kept.join('; ')}`)); } else { diff --git a/packages/stim-cli/src/commands/worktree.ts b/packages/stim-cli/src/commands/worktree.ts index bae568ee5..074b5ed74 100644 --- a/packages/stim-cli/src/commands/worktree.ts +++ b/packages/stim-cli/src/commands/worktree.ts @@ -599,6 +599,8 @@ interface RemoveOptions { force?: boolean; linkedOnly?: boolean; guard?: (lockedKeys: readonly string[]) => string[]; + /** A HEAD whose change gc proved is on the default branch; its local-only commits do not block removal. */ + mergedHead?: string; } interface RemovalInspection { @@ -628,13 +630,14 @@ function canonicalExistingPath(target: string): string { return resolve(realpathSync(existing), ...missing); } -function inspectRemoval(path: string): RemovalInspection { +function inspectRemoval(path: string, mergedHead?: string): RemovalInspection { const gitAnswered = hasUncommittedWork(path); const allDirty = gitAnswered ? dirtyPaths(path, { limit: Infinity }) : []; const { lines: dirtyLines, restore: podChurn } = excludePodChurn(allDirty); const dirty = gitAnswered === null ? null : dirtyLines.length > 0; const unpushed = unpushedCommits(path); - const blockers = removalBlockers({ dirty, unpushed }); + const merged = Boolean(mergedHead) && resolveFullRef(path, 'HEAD') === mergedHead; + const blockers = removalBlockers({ dirty, unpushed: merged && unpushed ? [] : unpushed }); if (hasPopulatedSubmodules(path)) blockers.push('initialized submodules, which git removes only with --force'); return { dirtyLines, podChurn, unpushed, blockers }; } @@ -832,7 +835,7 @@ async function runRemove(target: string | undefined, opts: RemoveOptions, onRemo const branch = entry.branch; const ownsBranch = Boolean(branch && project?.worktreeBranchOwned === true && project.worktreeBranch === branch); const approvedBranchSha = ownsBranch ? resolveFullRef(path, 'HEAD') : null; - const inspection = inspectRemoval(path); + const inspection = inspectRemoval(path, opts.mergedHead); if (inspection.blockers.length && !opts.force) { printRemovalRefusal(path, inspection); return; @@ -853,7 +856,7 @@ async function runRemove(target: string | undefined, opts: RemoveOptions, onRemo await withManagedRemoteWorktreeRemovalLock(path, () => withReclaimLocks(path, async (lockedKeys) => { if (opts.guard?.(lockedKeys).length) return; - const current = inspectRemoval(path); + const current = inspectRemoval(path, opts.mergedHead); if (current.blockers.length && !opts.force) { printRemovalRefusal(path, current); return; diff --git a/packages/stim-cli/src/guide/agent.ts b/packages/stim-cli/src/guide/agent.ts index 548508cc4..ad84b855f 100644 --- a/packages/stim-cli/src/guide/agent.ts +++ b/packages/stim-cli/src/guide/agent.ts @@ -206,8 +206,9 @@ Ask the user before these actions: existing Stim ownership record is deleted only when it has no unique commits. - worktree remove --force, because it also discards uncommitted and untracked files. -- gc --delete, because it deletes orphaned resources and clears the build - outputs of every workspace not in use. Run stim gc --json first and show +- gc --delete, because it deletes orphaned resources, removes clean linked + worktrees whose branch is merged, and clears the build outputs of every + workspace not in use. Run stim gc --json first and show the user the entries its sections list (guide facts gc). gc --delete --cache all empties the shared build caches and those outputs instead, and gc --delete --cache workspaces clears only the outputs; both inspect diff --git a/packages/stim-cli/src/guide/cleanup.ts b/packages/stim-cli/src/guide/cleanup.ts index ca9dd57de..6c2d732f3 100644 --- a/packages/stim-cli/src/guide/cleanup.ts +++ b/packages/stim-cli/src/guide/cleanup.ts @@ -18,21 +18,24 @@ WHAT RECLAIMS AN OWNED DEVICE (\`guide lifecycle pool\`); deletes them when parking is disabled or their setup cannot be verified stim gc --delete sweeps devices Stim created that no project - references (\`guide cleanup gc\`), and clears - verified parked simulators and emulators + references (\`guide cleanup gc\`), clears + verified parked simulators and emulators, and + runs \`stim worktree remove\` on every clean, + Stim-managed linked worktree whose branch is merged stim gc --delete --older-than also reaps the device of a workspace no Stim command has used in that long, even though the project is still on disk stim gc --delete --worktrees - runs \`stim worktree remove\` on every clean, idle, - Stim-managed linked worktree (\`guide cleanup gc\`) + also runs \`stim worktree remove\` on every clean, + idle, Stim-managed linked worktree + (\`guide cleanup gc\`) stim gc --idle shuts DOWN (never deletes) owned devices with no driver, claim or activity for that long \`worktree remove\` and \`gc --delete\` are the only two commands that delete; -\`gc --delete --worktrees\` deletes only through \`worktree remove\`. \`gc +\`gc --delete\` deletes worktrees only through \`worktree remove\`. \`gc --delete\` also clears workspace build outputs (\`guide cleanup disk\`) and orphaned workspace directories, never a checkout. \`stim stop\` shuts a device DOWN and leaves it assigned, which is what makes returning to a branch cost a @@ -86,23 +89,46 @@ IN USE existence cannot be read (a permission error) is never treated as deleted. SWEEPING FINISHED WORKTREES - \`gc --worktrees\` is opt-in; no cache or age flag implies it. It looks at - every registered project root and every workspace.json root, grouped by - git worktree, and reports each worktree with the reason it is kept: - source checkout, bare, locked, in use, dirty (untracked files count; pod - install churn alone does not), unpushed (commits no remote-tracking ref or - other local branch reaches), initialized submodules, or recently used. - Idle means no recorded use for --older-than days, 7 without it; a worktree - whose last use is unknown is kept. --cache with --worktrees is refused with - STIM_BAD_ARG; run them separately. - stim gc --worktrees --older-than 3 # report only - stim gc --delete --worktrees --older-than 3 # remove the clean idle ones + Every \`gc\` without --cache looks at every registered project root and + every workspace.json root, grouped by git worktree, and reports each linked + worktree with the reason it is removed or kept. A worktree is finished when + its branch is merged into the default branch. \`--worktrees\` also removes + one that is idle: no recorded use for --older-than days, 7 without it; a + worktree whose last use is unknown is kept. Both need the same clean state; + a worktree is kept when it is the source checkout, bare, locked, in use, + dirty (untracked files count; pod install churn alone does not), unpushed + (commits no remote-tracking ref or other local branch reaches), or has + initialized submodules. Without --worktrees, the report leaves out the + source checkout and roots outside git. + + MERGED means, after one \`git fetch origin \` per repository + (30s timeout, no credential prompt), with the default branch taken from + origin/HEAD: + - HEAD is reachable from origin/ through a merge. A HEAD on the + default branch's first-parent line has no commits of its own and is not + merged. + - every commit since the merge base has a patch-equivalent commit on the + default branch (a rebase merge), or the whole change since the merge base + is patch-equivalent to one commit there (a squash merge). + Merge state comes from git alone, not from a hosting service. A squash + merge whose content changed during the merge (a conflict resolution, a + suggested edit) does not match and is kept. When origin/HEAD is not set, + the fetch fails, or git cannot answer, the state is unknown and the + worktree is kept (reason merge-unknown, with the remedy). A squash-merged + branch whose upstream was deleted after the merge has commits only it + reaches; they do not block removal, because their content is on the + default branch, and the branch is kept. + stim gc # report merged worktrees + stim gc --delete # remove them + stim gc --delete --worktrees --older-than 3 # also the clean idle ones + --cache with --worktrees is refused with STIM_BAD_ARG; run them separately. With --delete it runs the \`stim worktree remove\` pipeline, never --force, on each removable worktree. That pipeline re-inspects the worktree and - re-checks use and idleness under the removal locks, then parks devices and - handles the branch exactly as a manual \`stim worktree remove\`. A worktree - that became busy or recently used since the report is kept with the reason. - A worktree that fails is reported, gc exits 1, and the sweep continues. + re-checks use under the removal locks, and idleness for an idle worktree + or an unchanged HEAD for a merged one, then parks devices and handles the + branch exactly as a manual \`stim worktree remove\`. A worktree that + changed since the report is kept with the reason. A worktree that fails is + reported, gc exits 1, and the sweep continues. IDLE DEVICES Plain \`gc\` lists booted owned simulators and emulators whose \`status\` diff --git a/packages/stim-cli/src/guide/facts.ts b/packages/stim-cli/src/guide/facts.ts index ff45a084f..7e167f07c 100644 --- a/packages/stim-cli/src/guide/facts.ts +++ b/packages/stim-cli/src/guide/facts.ts @@ -459,7 +459,8 @@ RULES olderThan the --older-than days, or null worktreeSweep null without --worktrees; otherwise { olderThan, defaulted }: the idle days a linked worktree needs, and whether that is - the default 7 because --older-than was not given + the default 7 because --older-than was not given. Merged + worktrees are swept either way actionable true when --delete with the same flags reclaims something failures null on a dry run without --idle; otherwise the entries it could not delete or shut down @@ -470,8 +471,12 @@ RULES orphanedPorts { project, label, port } orphanedWorkspaces { dir, projectRoot, bytes } --delete removes the whole workspace directory - linkedWorktrees { path, idleDays, willRemove, reason, detail } - only with --worktrees + linkedWorktrees { path, idleDays, mergedInto, willRemove, + reason, detail } mergedInto is the default + branch HEAD is merged into ("origin/main"), + or null; detail says why it is removed + ("merged into origin/main", "idle 9d") or kept. + Without --worktrees, only linked worktrees parkedSimulators { udid, name, model, runtime, parkedAt, bytes, listed } parkedEmulators { name, systemImage, parkedAt, bytes, listed } @@ -515,13 +520,15 @@ RULES reason is null for an entry --delete acts on, and otherwise a stable code to branch on; detail is the text line, for the user. Never parse detail. + A linked worktree's detail is never null. workspaceBuildOutputs unresolved | in-use | last-use-unknown | recently-used linkedWorktrees not-a-worktree | bare-repository | source-checkout-unknown | source-checkout | locked | in-use | status-unreadable | dirty | unpushed-unchecked | unpushed | submodules | - last-use-unknown | recently-used + not-merged | merge-unknown | last-use-unknown | + recently-used These print the error contract instead and exit 1: diff --git a/packages/stim-cli/src/workspace/merge-state.ts b/packages/stim-cli/src/workspace/merge-state.ts new file mode 100644 index 000000000..05d5b678e --- /dev/null +++ b/packages/stim-cli/src/workspace/merge-state.ts @@ -0,0 +1,127 @@ +import { getExecutor } from '../exec.ts'; + +const FETCH_TIMEOUT_MS = 30_000; +const GIT_TIMEOUT_MS = 60_000; +const REMOTE_PREFIX = 'refs/remotes/origin/'; + +// `git commit-tree` needs an identity and a date; fixed values keep the +// synthetic squash commit identical across runs, so repeated checks add no +// new objects. +const SQUASH_IDENTITY = { + GIT_AUTHOR_NAME: 'stim', + GIT_AUTHOR_EMAIL: 'stim@localhost', + GIT_AUTHOR_DATE: '1000000000 +0000', + GIT_COMMITTER_NAME: 'stim', + GIT_COMMITTER_EMAIL: 'stim@localhost', + GIT_COMMITTER_DATE: '1000000000 +0000', +}; + +export interface DefaultBranch { + ref: string; + name: string; +} + +export type MergeState = + | { merged: true; into: string; head: string; coversUnpushed: boolean } + | { merged: false; unknown: boolean; detail: string }; + +function failure(error: unknown): string { + const { stderr, message } = error as { stderr?: unknown; message?: string }; + const text = String(stderr ?? '').trim() || String(message ?? error); + return text.split('\n')[0] ?? text; +} + +/** + * Fetches the default branch that `origin/HEAD` names into its remote-tracking ref, once, bounded by a timeout, and + * never prompting for credentials. + */ +export function fetchDefaultBranch(repo: string): DefaultBranch | { error: string } { + const exec = getExecutor(); + const ref = exec.runFileQuiet('git', ['-C', repo, 'symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'])?.trim(); + if (!ref?.startsWith(REMOTE_PREFIX)) { + return { error: `origin/HEAD is not set; run \`git -C ${repo} remote set-head origin --auto\`` }; + } + const branch = ref.slice(REMOTE_PREFIX.length); + try { + exec.runFile( + 'git', + [ + '-C', + repo, + 'fetch', + '--quiet', + '--no-tags', + '--no-recurse-submodules', + 'origin', + `+refs/heads/${branch}:${ref}`, + ], + { timeoutMs: FETCH_TIMEOUT_MS, env: { GIT_TERMINAL_PROMPT: '0' } }, + ); + } catch (error) { + return { error: `git fetch origin ${branch} failed: ${failure(error)}` }; + } + return { ref, name: `origin/${branch}` }; +} + +function notMerged(detail: string): MergeState { + return { merged: false, unknown: false, detail }; +} + +function upstreamGone(path: string): boolean { + const exec = getExecutor(); + const branch = exec.runFileQuiet('git', ['-C', path, 'symbolic-ref', '--quiet', 'HEAD'])?.trim(); + if (!branch?.startsWith('refs/heads/')) return false; + const track = exec.runFileQuiet('git', [ + '-C', + path, + 'for-each-ref', + '--format=%(upstream)%00%(upstream:track)', + branch, + ]); + const [upstream, state] = (track ?? '').trim().split('\0'); + return Boolean(upstream) && state === '[gone]'; +} + +/** + * Whether the worktree's HEAD is merged into `target`. The signals, all local git: + * - HEAD is an ancestor of the default branch but not on its first-parent line, so a merge commit brought it in. A + * HEAD on the first-parent line is a branch with no commits of its own and counts as not merged. + * - Every commit since the merge base has a patch-equivalent commit on the default branch (a rebase merge). + * - The whole change since the merge base is patch-equivalent to one commit on the default branch (a squash merge). + * Anything git cannot answer is unknown, never merged. `coversUnpushed` is true for a patch-equivalent HEAD whose + * upstream branch was deleted: its commits exist only locally, but their content is on the default branch. + */ +export function mergeState(path: string, { ref, name }: DefaultBranch): MergeState { + const git = (args: string[], env?: Record): string => + getExecutor().runFile('git', ['-C', path, ...args], { timeoutMs: GIT_TIMEOUT_MS, env }); + try { + const head = git(['rev-parse', '--verify', 'HEAD^{commit}']); + const base = git(['merge-base', head, ref]); + if (base === head) { + const mainline = + git(['rev-parse', ref]) === head || + git(['rev-list', '--first-parent', '--parents', `${head}..${ref}`]) + .split('\n') + .some((line) => line.split(' ')[1] === head); + if (mainline) return notMerged(`no commits of its own beyond ${name}`); + return { merged: true, into: name, head, coversUnpushed: false }; + } + const equivalent = (tip: string): boolean => { + const lines = git(['cherry', ref, tip, base]).split('\n').filter(Boolean); + return lines.length > 0 && lines.every((line) => line.startsWith('- ')); + }; + const patchEquivalent = () => ({ merged: true as const, into: name, head, coversUnpushed: upstreamGone(path) }); + if (equivalent(head)) return patchEquivalent(); + const tree = git(['rev-parse', `${head}^{tree}`]); + if (tree !== git(['rev-parse', `${base}^{tree}`])) { + const squashed = git( + ['commit-tree', '--no-gpg-sign', tree, '-p', base, '-m', 'stim gc squash-merge check'], + SQUASH_IDENTITY, + ); + if (equivalent(squashed)) return patchEquivalent(); + } + return notMerged(`not merged into ${name}`); + } catch (error) { + return { merged: false, unknown: true, detail: `merge state unknown: ${failure(error)}` }; + } +} diff --git a/website/docs/commands.md b/website/docs/commands.md index 0a57cc944..b371b2857 100644 --- a/website/docs/commands.md +++ b/website/docs/commands.md @@ -613,9 +613,12 @@ worktree locked with `git worktree lock` is refused until you unlock it. stim gc [--delete] [--older-than ] [--cache ] [--worktrees] [--idle ] [--json] ``` -Reports stale workspace entries, orphaned workspace directories, orphaned -owned devices and remote sessions, stale locks, and shared cache sizes. It does -not change anything without `--delete`. +Reports stale workspace entries, orphaned workspace directories, clean linked +worktrees whose branch is merged, orphaned owned devices and remote sessions, +stale locks, and shared cache sizes. It does not change anything without +`--delete`. See +[removing finished worktrees in bulk](./worktrees.md#remove-finished-worktrees-in-bulk) +for how gc decides that a branch is merged. An orphaned device is one Stim created that no workspace references. Other devices whose names start with `stim-` appear under "Unrecognized stim-\* @@ -642,10 +645,10 @@ workspace keeps its state, logs, devices and ports. See outputs with `all`. `workspaces` clears only the workspace build outputs. Devices and project entries are not inspected, so a scoped run empties caches and reaps nothing. -- `--worktrees` also reports every clean, idle linked worktree that has a Stim - workspace, and why each other one is kept. With `--delete` it runs - `stim worktree remove` without `--force` on each of them. Idle means unused - for `--older-than` days, or 7 days without that option. It cannot be +- `--worktrees` also selects every clean, idle linked worktree that has a Stim + workspace, not only the merged ones plain `gc` selects. With `--delete` gc + runs `stim worktree remove` without `--force` on each of them. Idle means + unused for `--older-than` days, or 7 days without that option. It cannot be combined with `--cache`. See [removing finished worktrees in bulk](./worktrees.md#remove-finished-worktrees-in-bulk). - `--idle ` shuts down owned simulators and emulators whose @@ -678,10 +681,26 @@ prints, for example: "deadProjects": [{ "path": "/path/to/removed-app" }], "orphanedWorkspaces": [{ "dir": "~/.stim/workspaces/old--1a2b", "projectRoot": "/path/to/old", "bytes": 52428800 }], "linkedWorktrees": [ - { "path": "/path/to/feature", "idleDays": 12, "willRemove": true, "reason": null, "detail": null }, + { + "path": "/path/to/feature", + "idleDays": 12, + "mergedInto": null, + "willRemove": true, + "reason": null, + "detail": "idle 12d" + }, + { + "path": "/path/to/shipped", + "idleDays": 0, + "mergedInto": "origin/main", + "willRemove": true, + "reason": null, + "detail": "merged into origin/main" + }, { "path": "/path/to/wip", "idleDays": 20, + "mergedInto": null, "willRemove": false, "reason": "dirty", "detail": "dirty: uncommitted changes or untracked files" @@ -706,7 +725,7 @@ prints, for example: The example omits the empty sections. `reason` is `null` for an entry `--delete` acts on and otherwise a stable code; `detail` is the text the report prints. `bytes` is `null` when the size is unknown. `worktreeSweep` is `null` without -`--worktrees`. With `--delete`, `mode` is `"delete"`, the sections list what +`--worktrees`, which still reports merged worktrees. With `--delete`, `mode` is `"delete"`, the sections list what the run acted on, and `failures` counts the entries it could not delete. A nonzero count exits with status 1. Run `stim gc --json` again to see what is left. `idle` is the `--idle` duration in milliseconds or `null`, and with diff --git a/website/docs/worktrees.md b/website/docs/worktrees.md index 11656195f..b89eb6a9e 100644 --- a/website/docs/worktrees.md +++ b/website/docs/worktrees.md @@ -216,24 +216,47 @@ alone. See [named server ports](./dev-server-and-logs.md#named-server-ports). ## Remove finished worktrees in bulk -`gc --worktrees` lists every linked worktree that has a Stim workspace and says -why each one is kept: source checkout, bare, locked, in use, dirty (untracked -files count), unpushed, initialized submodules, or recently used. A worktree is -idle when no Stim command has used it for `--older-than` days, or 7 days -without that option. With `--delete`, gc runs `stim worktree remove` without -`--force` on each removable worktree. That command checks the worktree again -before removing it and handles devices and branches as it does when you run it -yourself. A worktree that fails is reported and the others still run. A -worktree removed with `git worktree remove` or `rm -rf` leaves its Stim -workspace directory behind; plain `gc --delete` removes those. +`gc` lists every linked worktree that has a Stim workspace and says why each +one is removed or kept. A worktree is finished when its branch is merged into +the default branch, and plain `gc --delete` removes it. `--worktrees` also +removes a worktree that is idle: no Stim command has used it for +`--older-than` days, or 7 days without that option. Either way, gc keeps a +worktree that is the source checkout, bare, locked, in use, dirty (untracked +files count), unpushed, or has initialized submodules. With `--delete`, gc +runs `stim worktree remove` without `--force` on each removable worktree. That +command checks the worktree again before removing it and handles devices and +branches as it does when you run it yourself. A worktree that fails is +reported and the others still run. A worktree removed with +`git worktree remove` or `rm -rf` leaves its Stim workspace directory behind; +plain `gc --delete` removes those. + +To decide that a branch is merged, gc runs one `git fetch origin ` per +repository, with a 30-second timeout, and takes the default branch from +`origin/HEAD`. The branch counts as merged when: + +- a merge brought its HEAD into the default branch. A branch with no commits + of its own is not merged. +- each of its commits has a patch-equivalent commit on the default branch, as + after a rebase merge. +- its whole change is patch-equivalent to one commit on the default branch, as + after a squash merge. + +gc asks git only, not GitHub. A squash merge whose content changed while +merging, such as a conflict resolution, does not match, and gc keeps that +worktree. When `origin/HEAD` is not set, the fetch fails, or git cannot answer, +gc keeps the worktree and reports why. After a squash merge that deleted the +remote branch, the branch's commits exist only locally; gc still removes the +worktree, because their content is on the default branch, and keeps the +branch. Ask your agent: ```text -Run `stim gc --worktrees --older-than 3` and show me which worktrees it would -remove and why it keeps the others. Do not pass --delete until I confirm. +Run `stim gc --json` and show me which merged worktrees it would remove and +why it keeps the others. Do not pass --delete until I confirm. ``` From aee6d26e8b51e524e9a353e54e8fcd048ba916c4 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 25 Sep 2026 00:50:10 -0400 Subject: [PATCH 2/4] fix: require a commit made on the branch and no merges before calling it merged A branch cut from a merged branch no longer counts as merged, the rebase-merge check skips branches with merge commits, a branch with no net change is not merged, and gc skips the fetch when the checkout fetched in the last 10 minutes. --- .../src/__tests__/gc-workspaces.test.ts | 39 ++++++--- .../stim-cli/src/commands/gc/worktrees.ts | 28 ++++--- packages/stim-cli/src/guide/cleanup.ts | 36 ++++---- packages/stim-cli/src/guide/facts.ts | 3 +- .../stim-cli/src/workspace/merge-state.ts | 82 +++++++++++++------ website/docs/worktrees.md | 33 ++++---- 6 files changed, 140 insertions(+), 81 deletions(-) diff --git a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts index cfdd0d261..928e705d0 100644 --- a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts +++ b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts @@ -739,6 +739,8 @@ function repoWithMergedBranches() { for (const [name, commits] of [ ['merged', 1], ['squashed', 2], + ['rebased', 1], + ['evil', 1], ['fresh', 0], ['open', 1], ['dirty', 1], @@ -749,6 +751,9 @@ function repoWithMergedBranches() { if (commits) git(`push -q -u origin ${name}`, path); worktrees[name] = realpathSync.native(path); } + const followup = join(projects, 'followup'); + git(`worktree add -q "${followup}" -b followup merged`); + worktrees.followup = realpathSync.native(followup); git(`clone -q "${remote}" "${upstream}"`, projects); identity(upstream); commit(upstream, 'main-moved-on.txt'); @@ -756,9 +761,16 @@ function repoWithMergedBranches() { git('merge -q --no-ff origin/dirty -m merge-dirty', upstream); git('merge -q --squash origin/squashed', upstream); git('commit -q -m squash-squashed', upstream); + git('cherry-pick origin/rebased', upstream); + git('cherry-pick origin/evil', upstream); git('push -q origin main', upstream); - git('push -q origin --delete squashed', upstream); - git('update-ref -d refs/remotes/origin/squashed'); + git('fetch -q origin main', worktrees.evil); + git('merge -q --no-ff --no-commit origin/main', worktrees.evil); + commit(worktrees.evil!, 'not-on-main.txt'); + for (const gone of ['squashed', 'rebased', 'evil']) { + git(`push -q origin --delete ${gone}`, upstream); + git(`update-ref -d refs/remotes/origin/${gone}`); + } writeFileSync(join(worktrees.dirty!, 'notes.txt'), 'wip'); for (const path of Object.values(worktrees)) { upsertProject(path, { metroPort: null }); @@ -783,28 +795,33 @@ test('plain gc --delete removes merged worktrees, squash merges included, after detail: 'merged into origin/main', }); expect(byPath[worktrees.squashed!]).toMatchObject({ willRemove: true, detail: 'merged into origin/main' }); - expect(byPath[worktrees.fresh!]).toMatchObject({ - willRemove: false, - reason: 'not-merged', - detail: 'no commits of its own beyond origin/main', - }); + expect(byPath[worktrees.rebased!]).toMatchObject({ willRemove: true, detail: 'merged into origin/main' }); + for (const name of ['fresh', 'followup']) { + expect(byPath[worktrees[name]!]).toMatchObject({ + willRemove: false, + reason: 'not-merged', + detail: 'no commits of its own beyond origin/main', + }); + } + expect(byPath[worktrees.evil!]).toMatchObject({ willRemove: false, reason: 'unpushed', mergedInto: null }); expect(byPath[worktrees.open!]).toMatchObject({ willRemove: false, reason: 'not-merged' }); expect(byPath[worktrees.dirty!]).toMatchObject({ willRemove: false, reason: 'dirty' }); const output = await captureLog(() => runGc({ delete: true })); expect(output).toContain(`Removed the worktree ${worktrees.merged} (merged into origin/main)`); expect(output).toContain(`Removed the worktree ${worktrees.squashed} (merged into origin/main)`); - expect(existsSync(worktrees.merged!)).toBe(false); - expect(existsSync(worktrees.squashed!)).toBe(false); - for (const name of ['fresh', 'open', 'dirty']) expect(existsSync(worktrees[name]!)).toBe(true); + for (const name of ['merged', 'squashed', 'rebased']) expect(existsSync(worktrees[name]!)).toBe(false); + for (const name of ['fresh', 'followup', 'evil', 'open', 'dirty']) expect(existsSync(worktrees[name]!)).toBe(true); expect(existsSync(join(repo, 'package.json'))).toBe(true); expect(git('branch --list squashed')).toContain('squashed'); expect(process.exitCode).not.toBe(1); }, 120_000); test('gc keeps a merged worktree when the fetch fails, and when its HEAD moves after the report', async () => { - const { remote, worktrees, git } = repoWithMergedBranches(); + const { repo, remote, worktrees, git } = repoWithMergedBranches(); git('fetch -q origin'); + const stale = new Date(Date.now() - 11 * 60_000); + utimesSync(join(repo, '.git', 'FETCH_HEAD'), stale, stale); renameSync(remote, `${remote}.moved`); const { payload } = await gcJson({ delete: true }); diff --git a/packages/stim-cli/src/commands/gc/worktrees.ts b/packages/stim-cli/src/commands/gc/worktrees.ts index deed7d6b6..d22ae8cd3 100644 --- a/packages/stim-cli/src/commands/gc/worktrees.ts +++ b/packages/stim-cli/src/commands/gc/worktrees.ts @@ -8,7 +8,6 @@ import { workspaceLastUsed } from '../../workspace/workspace-state.ts'; import { fetchDefaultBranch, mergeState, type MergeState } from '../../workspace/merge-state.ts'; import { dirtyPaths, - gitCommonDir, hasPopulatedSubmodules, hasUncommittedWork, listWorktrees, @@ -143,14 +142,17 @@ function candidateRoots(): string[] { return [...new Set([...registered, ...recorded])].filter((root) => existsSync(root)).toSorted(); } -function checkMergeStates(pending: { candidate: WorktreeCandidate; facts: WorktreeFacts }[], idle: number | null) { - const repos = new Map(); - for (const entry of pending) { - const common = gitCommonDir(entry.candidate.path) ?? entry.candidate.path; - repos.set(common, [...(repos.get(common) ?? []), entry]); - } - for (const entries of repos.values()) { - const target = fetchDefaultBranch(entries[0]!.candidate.path); +interface PendingMerge { + candidate: WorktreeCandidate; + facts: WorktreeFacts; + repo: string; +} + +function checkMergeStates(pending: PendingMerge[], idle: number | null, now: number): void { + const repos = new Map(); + for (const entry of pending) repos.set(entry.repo, [...(repos.get(entry.repo) ?? []), entry]); + for (const [repo, entries] of repos) { + const target = fetchDefaultBranch(repo, now); for (const { candidate, facts } of entries) { const merge: MergeState = 'error' in target @@ -194,7 +196,7 @@ export function collectWorktreeSweep({ groups.set(entry.path, [...(groups.get(entry.path) ?? []), root]); } const worktrees: WorktreeCandidate[] = []; - const pending: { candidate: WorktreeCandidate; facts: WorktreeFacts }[] = []; + const pending: PendingMerge[] = []; for (const [path, roots] of groups) { const entries = listWorktrees(path); const entry = matchWorktreeEntry(entries, path); @@ -223,10 +225,12 @@ export function collectWorktreeSweep({ skipCode: verdict?.code ?? null, skipped: verdict?.text ?? null, }; - if (verdict && MERGE_DECIDES.has(verdict.code)) pending.push({ candidate, facts }); + if (verdict && MERGE_DECIDES.has(verdict.code) && !('refusal' in source)) { + pending.push({ candidate, facts, repo: source.path }); + } worktrees.push(candidate); } - checkMergeStates(pending, days); + checkMergeStates(pending, days, now); const listed = [...worktrees, ...outside].filter( (w) => idle || (w.skipCode !== 'not-a-worktree' && w.skipCode !== 'source-checkout'), ); diff --git a/packages/stim-cli/src/guide/cleanup.ts b/packages/stim-cli/src/guide/cleanup.ts index 6c2d732f3..bf3bbbb1c 100644 --- a/packages/stim-cli/src/guide/cleanup.ts +++ b/packages/stim-cli/src/guide/cleanup.ts @@ -101,23 +101,25 @@ SWEEPING FINISHED WORKTREES initialized submodules. Without --worktrees, the report leaves out the source checkout and roots outside git. - MERGED means, after one \`git fetch origin \` per repository - (30s timeout, no credential prompt), with the default branch taken from - origin/HEAD: - - HEAD is reachable from origin/ through a merge. A HEAD on the - default branch's first-parent line has no commits of its own and is not - merged. - - every commit since the merge base has a patch-equivalent commit on the - default branch (a rebase merge), or the whole change since the merge base - is patch-equivalent to one commit there (a squash merge). - Merge state comes from git alone, not from a hosting service. A squash - merge whose content changed during the merge (a conflict resolution, a - suggested edit) does not match and is kept. When origin/HEAD is not set, - the fetch fails, or git cannot answer, the state is unknown and the - worktree is kept (reason merge-unknown, with the remedy). A squash-merged - branch whose upstream was deleted after the merge has commits only it - reaches; they do not block removal, because their content is on the - default branch, and the branch is kept. + MERGED means, with the default branch taken from origin/HEAD, after a + \`git fetch origin \` per repository (30s timeout, no credential + prompt; skipped when that checkout fetched in the last 10 minutes): + - HEAD is reachable from origin/ through a merge commit, and the + branch's reflog shows a commit made on it. A branch with no commit of + its own -- fresh, or cut from another branch -- is not merged. + - the branch changes the tree, and either it has no merge commits and every + commit since the merge base has a patch-equivalent commit on the default + branch (a rebase merge), or its whole change since the merge base is + patch-equivalent to one commit there (a squash merge). + Merge state comes from git alone, not from a hosting service. Patch + equivalence is git's patch-id, which ignores whitespace. A squash merge + whose content changed during the merge (a conflict resolution, a suggested + edit) does not match and is kept, and so is a fast-forwarded branch. When + origin/HEAD is not set, the fetch fails, or git cannot answer, the state is + unknown and the worktree is kept (reason merge-unknown, with the remedy). A + squash- or rebase-merged branch whose upstream was deleted after the merge + has commits only it reaches; they do not block removal, because their + change is on the default branch, and the branch is kept. stim gc # report merged worktrees stim gc --delete # remove them stim gc --delete --worktrees --older-than 3 # also the clean idle ones diff --git a/packages/stim-cli/src/guide/facts.ts b/packages/stim-cli/src/guide/facts.ts index 7e167f07c..f8246e0f2 100644 --- a/packages/stim-cli/src/guide/facts.ts +++ b/packages/stim-cli/src/guide/facts.ts @@ -476,7 +476,8 @@ RULES branch HEAD is merged into ("origin/main"), or null; detail says why it is removed ("merged into origin/main", "idle 9d") or kept. - Without --worktrees, only linked worktrees + Without --worktrees, the source checkout and + roots outside git are left out parkedSimulators { udid, name, model, runtime, parkedAt, bytes, listed } parkedEmulators { name, systemImage, parkedAt, bytes, listed } diff --git a/packages/stim-cli/src/workspace/merge-state.ts b/packages/stim-cli/src/workspace/merge-state.ts index 05d5b678e..4ff6b9dfb 100644 --- a/packages/stim-cli/src/workspace/merge-state.ts +++ b/packages/stim-cli/src/workspace/merge-state.ts @@ -1,6 +1,8 @@ +import { statSync } from 'node:fs'; import { getExecutor } from '../exec.ts'; const FETCH_TIMEOUT_MS = 30_000; +const FETCH_FRESH_MS = 10 * 60_000; const GIT_TIMEOUT_MS = 60_000; const REMOTE_PREFIX = 'refs/remotes/origin/'; @@ -31,17 +33,29 @@ function failure(error: unknown): string { return text.split('\n')[0] ?? text; } +function fetchedRecently(repo: string, now: number): boolean { + const path = getExecutor() + .runFileQuiet('git', ['-C', repo, 'rev-parse', '--path-format=absolute', '--git-path', 'FETCH_HEAD']) + ?.trim(); + try { + return Boolean(path) && now - statSync(path!).mtimeMs < FETCH_FRESH_MS; + } catch { + return false; + } +} + /** - * Fetches the default branch that `origin/HEAD` names into its remote-tracking ref, once, bounded by a timeout, and - * never prompting for credentials. + * Fetches the default branch that `origin/HEAD` names into its remote-tracking ref, bounded by a timeout and never + * prompting for credentials. It skips the fetch when the checkout fetched anything in the last 10 minutes. */ -export function fetchDefaultBranch(repo: string): DefaultBranch | { error: string } { +export function fetchDefaultBranch(repo: string, now: number = Date.now()): DefaultBranch | { error: string } { const exec = getExecutor(); const ref = exec.runFileQuiet('git', ['-C', repo, 'symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'])?.trim(); if (!ref?.startsWith(REMOTE_PREFIX)) { return { error: `origin/HEAD is not set; run \`git -C ${repo} remote set-head origin --auto\`` }; } const branch = ref.slice(REMOTE_PREFIX.length); + if (fetchedRecently(repo, now)) return { ref, name: `origin/${branch}` }; try { exec.runFile( 'git', @@ -67,11 +81,14 @@ function notMerged(detail: string): MergeState { return { merged: false, unknown: false, detail }; } -function upstreamGone(path: string): boolean { - const exec = getExecutor(); - const branch = exec.runFileQuiet('git', ['-C', path, 'symbolic-ref', '--quiet', 'HEAD'])?.trim(); - if (!branch?.startsWith('refs/heads/')) return false; - const track = exec.runFileQuiet('git', [ +function currentBranch(path: string): string | null { + const ref = getExecutor().runFileQuiet('git', ['-C', path, 'symbolic-ref', '--quiet', 'HEAD'])?.trim(); + return ref?.startsWith('refs/heads/') ? ref : null; +} + +function upstreamGone(path: string, branch: string | null): boolean { + if (!branch) return false; + const track = getExecutor().runFileQuiet('git', [ '-C', path, 'for-each-ref', @@ -82,44 +99,59 @@ function upstreamGone(path: string): boolean { return Boolean(upstream) && state === '[gone]'; } +function committedOn(path: string, branch: string | null): boolean { + if (!branch) return false; + const subjects = getExecutor().runFileQuiet('git', ['-C', path, 'reflog', 'show', '--format=%gs', branch]) ?? ''; + return subjects.split('\n').some((subject) => /^(commit|cherry-pick|rebase|revert)\b/.test(subject)); +} + /** * Whether the worktree's HEAD is merged into `target`. The signals, all local git: - * - HEAD is an ancestor of the default branch but not on its first-parent line, so a merge commit brought it in. A - * HEAD on the first-parent line is a branch with no commits of its own and counts as not merged. - * - Every commit since the merge base has a patch-equivalent commit on the default branch (a rebase merge). - * - The whole change since the merge base is patch-equivalent to one commit on the default branch (a squash merge). - * Anything git cannot answer is unknown, never merged. `coversUnpushed` is true for a patch-equivalent HEAD whose - * upstream branch was deleted: its commits exist only locally, but their content is on the default branch. + * - HEAD is an ancestor of the default branch, off its first-parent line, and the branch's reflog shows a commit made + * on it, so a merge commit brought the branch's own work in. A branch with no commit of its own is not merged. + * - The branch changes the tree, has no merge commits, and every commit since the merge base has a patch-equivalent + * commit on the default branch (a rebase merge). + * - The branch changes the tree and its whole change since the merge base is patch-equivalent to one commit on the + * default branch (a squash merge). + * Patch equivalence is git's patch-id, which ignores whitespace. Anything git cannot answer is unknown, never merged. + * `coversUnpushed` is true for a patch-equivalent HEAD whose upstream branch was deleted: its commits exist only + * locally, but their change is on the default branch. */ export function mergeState(path: string, { ref, name }: DefaultBranch): MergeState { const git = (args: string[], env?: Record): string => getExecutor().runFile('git', ['-C', path, ...args], { timeoutMs: GIT_TIMEOUT_MS, env }); + const noOwnCommits = notMerged(`no commits of its own beyond ${name}`); try { const head = git(['rev-parse', '--verify', 'HEAD^{commit}']); const base = git(['merge-base', head, ref]); + const branch = currentBranch(path); if (base === head) { const mainline = git(['rev-parse', ref]) === head || git(['rev-list', '--first-parent', '--parents', `${head}..${ref}`]) .split('\n') .some((line) => line.split(' ')[1] === head); - if (mainline) return notMerged(`no commits of its own beyond ${name}`); + if (mainline || !committedOn(path, branch)) return noOwnCommits; return { merged: true, into: name, head, coversUnpushed: false }; } + const tree = git(['rev-parse', `${head}^{tree}`]); + if (tree === git(['rev-parse', `${base}^{tree}`])) return notMerged(`no net change beyond ${name}`); const equivalent = (tip: string): boolean => { const lines = git(['cherry', ref, tip, base]).split('\n').filter(Boolean); return lines.length > 0 && lines.every((line) => line.startsWith('- ')); }; - const patchEquivalent = () => ({ merged: true as const, into: name, head, coversUnpushed: upstreamGone(path) }); - if (equivalent(head)) return patchEquivalent(); - const tree = git(['rev-parse', `${head}^{tree}`]); - if (tree !== git(['rev-parse', `${base}^{tree}`])) { - const squashed = git( - ['commit-tree', '--no-gpg-sign', tree, '-p', base, '-m', 'stim gc squash-merge check'], - SQUASH_IDENTITY, - ); - if (equivalent(squashed)) return patchEquivalent(); - } + const patchEquivalent = () => ({ + merged: true as const, + into: name, + head, + coversUnpushed: upstreamGone(path, branch), + }); + if (!git(['rev-list', '--merges', `${base}..${head}`]) && equivalent(head)) return patchEquivalent(); + const squashed = git( + ['commit-tree', '--no-gpg-sign', tree, '-p', base, '-m', 'stim gc squash-merge check'], + SQUASH_IDENTITY, + ); + if (equivalent(squashed)) return patchEquivalent(); return notMerged(`not merged into ${name}`); } catch (error) { return { merged: false, unknown: true, detail: `merge state unknown: ${failure(error)}` }; diff --git a/website/docs/worktrees.md b/website/docs/worktrees.md index b89eb6a9e..a48aa1847 100644 --- a/website/docs/worktrees.md +++ b/website/docs/worktrees.md @@ -235,24 +235,27 @@ reported and the others still run. A worktree removed with `git worktree remove` or `rm -rf` leaves its Stim workspace directory behind; plain `gc --delete` removes those. -To decide that a branch is merged, gc runs one `git fetch origin ` per -repository, with a 30-second timeout, and takes the default branch from -`origin/HEAD`. The branch counts as merged when: - -- a merge brought its HEAD into the default branch. A branch with no commits - of its own is not merged. -- each of its commits has a patch-equivalent commit on the default branch, as - after a rebase merge. +To decide that a branch is merged, gc takes the default branch from +`origin/HEAD` and runs `git fetch origin ` once per repository, with a +30-second timeout. It skips the fetch when that checkout fetched in the last 10 +minutes. The branch counts as merged when: + +- a merge commit brought its HEAD into the default branch, and the branch's + reflog shows a commit made on it. A branch with no commits of its own, such + as one just created or cut from another branch, is not merged. +- it has no merge commits and each of its commits has a patch-equivalent + commit on the default branch, as after a rebase merge. - its whole change is patch-equivalent to one commit on the default branch, as after a squash merge. -gc asks git only, not GitHub. A squash merge whose content changed while -merging, such as a conflict resolution, does not match, and gc keeps that -worktree. When `origin/HEAD` is not set, the fetch fails, or git cannot answer, -gc keeps the worktree and reports why. After a squash merge that deleted the -remote branch, the branch's commits exist only locally; gc still removes the -worktree, because their content is on the default branch, and keeps the -branch. +gc asks git only, not GitHub. Patch equivalence is git's patch-id, which +ignores whitespace. A squash merge whose content changed while merging, such +as a conflict resolution, does not match, and gc keeps that worktree; so does +a branch that was fast-forwarded into the default branch. When `origin/HEAD` is +not set, the fetch fails, or git cannot answer, gc keeps the worktree and +reports why. After a squash or rebase merge that deleted the remote branch, the +branch's commits exist only locally; gc still removes the worktree, because +their change is on the default branch, and keeps the branch. Ask your agent: From dd0b5635733de34c5cbba287350a6a7730b57ab2 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 25 Sep 2026 00:53:54 -0400 Subject: [PATCH 3/4] fix: compare verbatim patch ids and re-check HEAD before removing the checkout Merge detection pipes diffs to git patch-id --verbatim, so a whitespace-only difference no longer matches, and writes no synthetic commit. worktree remove keeps the checkout when its HEAD moves while devices are reclaimed. --- .../src/__tests__/gc-workspaces.test.ts | 20 +++++- packages/stim-cli/src/commands/gc/report.ts | 6 +- packages/stim-cli/src/commands/worktree.ts | 8 +++ packages/stim-cli/src/exec.ts | 5 +- packages/stim-cli/src/guide/cleanup.ts | 29 +++++---- .../stim-cli/src/workspace/merge-state.ts | 63 +++++++++---------- website/docs/worktrees.md | 23 +++---- 7 files changed, 88 insertions(+), 66 deletions(-) diff --git a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts index 928e705d0..302497acb 100644 --- a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts +++ b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts @@ -751,6 +751,13 @@ function repoWithMergedBranches() { if (commits) git(`push -q -u origin ${name}`, path); worktrees[name] = realpathSync.native(path); } + const spaced = join(projects, 'spaced'); + git(`worktree add -q "${spaced}" -b spaced`); + writeFileSync(join(spaced, 'value.txt'), 'a b\n'); + git('add value.txt', spaced); + git('commit -q -m spaced', spaced); + git('push -q -u origin spaced', spaced); + worktrees.spaced = realpathSync.native(spaced); const followup = join(projects, 'followup'); git(`worktree add -q "${followup}" -b followup merged`); worktrees.followup = realpathSync.native(followup); @@ -763,11 +770,14 @@ function repoWithMergedBranches() { git('commit -q -m squash-squashed', upstream); git('cherry-pick origin/rebased', upstream); git('cherry-pick origin/evil', upstream); + writeFileSync(join(upstream, 'value.txt'), 'ab\n'); + git('add value.txt', upstream); + git('commit -q -m value-without-the-space', upstream); git('push -q origin main', upstream); git('fetch -q origin main', worktrees.evil); git('merge -q --no-ff --no-commit origin/main', worktrees.evil); commit(worktrees.evil!, 'not-on-main.txt'); - for (const gone of ['squashed', 'rebased', 'evil']) { + for (const gone of ['squashed', 'rebased', 'evil', 'spaced']) { git(`push -q origin --delete ${gone}`, upstream); git(`update-ref -d refs/remotes/origin/${gone}`); } @@ -803,7 +813,9 @@ test('plain gc --delete removes merged worktrees, squash merges included, after detail: 'no commits of its own beyond origin/main', }); } - expect(byPath[worktrees.evil!]).toMatchObject({ willRemove: false, reason: 'unpushed', mergedInto: null }); + for (const name of ['evil', 'spaced']) { + expect(byPath[worktrees[name]!]).toMatchObject({ willRemove: false, reason: 'unpushed', mergedInto: null }); + } expect(byPath[worktrees.open!]).toMatchObject({ willRemove: false, reason: 'not-merged' }); expect(byPath[worktrees.dirty!]).toMatchObject({ willRemove: false, reason: 'dirty' }); @@ -811,7 +823,9 @@ test('plain gc --delete removes merged worktrees, squash merges included, after expect(output).toContain(`Removed the worktree ${worktrees.merged} (merged into origin/main)`); expect(output).toContain(`Removed the worktree ${worktrees.squashed} (merged into origin/main)`); for (const name of ['merged', 'squashed', 'rebased']) expect(existsSync(worktrees[name]!)).toBe(false); - for (const name of ['fresh', 'followup', 'evil', 'open', 'dirty']) expect(existsSync(worktrees[name]!)).toBe(true); + for (const name of ['fresh', 'followup', 'evil', 'spaced', 'open', 'dirty']) { + expect(existsSync(worktrees[name]!)).toBe(true); + } expect(existsSync(join(repo, 'package.json'))).toBe(true); expect(git('branch --list squashed')).toContain('squashed'); expect(process.exitCode).not.toBe(1); diff --git a/packages/stim-cli/src/commands/gc/report.ts b/packages/stim-cli/src/commands/gc/report.ts index 367ecac53..489034219 100644 --- a/packages/stim-cli/src/commands/gc/report.ts +++ b/packages/stim-cli/src/commands/gc/report.ts @@ -389,10 +389,10 @@ function worktreeSweepLines(sweep: WorktreeSweep | null): string[] { const idle = !sweep.idle ? '' : sweep.idle.defaulted - ? ` or idle ${sweep.idle.olderThan}d or more (the default without --older-than)` - : ` or idle ${sweep.idle.olderThan}d or more`; + ? `, or clean, pushed and idle ${sweep.idle.olderThan}d or more (the default without --older-than)` + : `, or clean, pushed and idle ${sweep.idle.olderThan}d or more`; const lines = [ - `Linked worktrees (${removable.length} removable, ${sweep.worktrees.length - removable.length} kept) - clean, pushed, merged into the default branch${idle}:`, + `Linked worktrees (${removable.length} removable, ${sweep.worktrees.length - removable.length} kept) - clean and merged into the default branch${idle}:`, ]; for (const w of sweep.worktrees) { const age = w.idleDays === null ? '' : ` (idle ${w.idleDays}d)`; diff --git a/packages/stim-cli/src/commands/worktree.ts b/packages/stim-cli/src/commands/worktree.ts index 074b5ed74..500ce6ff5 100644 --- a/packages/stim-cli/src/commands/worktree.ts +++ b/packages/stim-cli/src/commands/worktree.ts @@ -861,11 +861,19 @@ async function runRemove(target: string | undefined, opts: RemoveOptions, onRemo printRemovalRefusal(path, current); return; } + const inspectedHead = resolveFullRef(path, 'HEAD'); const result = await reclaimAll(path, lockedKeys, { preserveRootProject: true }); if (result.keptEntries.length) { reportRetainedResources(path, result); return; } + if (!opts.force && resolveFullRef(path, 'HEAD') !== inspectedHead) { + console.error(chalk.red(`Refusing to remove ${path}: its HEAD moved while its environment was reclaimed.`)); + console.error(chalk.dim(`The directory and Stim ownership record for ${path} were kept.`)); + printRemovalCleanup(result, true); + process.exitCode = 1; + return; + } restorePodChurn(path, current.podChurn); try { removeWorktree(path, { from: source.path, force: opts.force }); diff --git a/packages/stim-cli/src/exec.ts b/packages/stim-cli/src/exec.ts index 818bcf2f8..09eef3172 100644 --- a/packages/stim-cli/src/exec.ts +++ b/packages/stim-cli/src/exec.ts @@ -8,6 +8,8 @@ interface ExecOptions { cwd?: string; env?: Record; omitEnv?: readonly string[]; + /** Text written to the child's stdin; `runFile` only. */ + input?: string; } export interface Executor { @@ -51,7 +53,7 @@ const defaultExecutor: Executor = { // refuses .cmd/.bat files and shebang scripts without a shell, and every // package bin (eas, agent-device) is one of those. The throw matches // execFileSync's, so callers keep reading status, stdout and stderr off it. - runFile(file, args = [], { timeoutMs, killSignal, cwd, env, omitEnv } = {}) { + runFile(file, args = [], { timeoutMs, killSignal, cwd, env, omitEnv, input } = {}) { const opts: Parameters[2] = { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], @@ -60,6 +62,7 @@ const defaultExecutor: Executor = { if (timeoutMs) opts.timeout = timeoutMs; if (killSignal) opts.killSignal = killSignal; if (cwd) opts.cwd = cwd; + if (input !== undefined) opts.input = input; if (env || omitEnv?.length) { const childEnv = { ...process.env, ...env }; for (const key of omitEnv ?? []) delete childEnv[key]; diff --git a/packages/stim-cli/src/guide/cleanup.ts b/packages/stim-cli/src/guide/cleanup.ts index bf3bbbb1c..fddfa8b14 100644 --- a/packages/stim-cli/src/guide/cleanup.ts +++ b/packages/stim-cli/src/guide/cleanup.ts @@ -108,18 +108,20 @@ SWEEPING FINISHED WORKTREES branch's reflog shows a commit made on it. A branch with no commit of its own -- fresh, or cut from another branch -- is not merged. - the branch changes the tree, and either it has no merge commits and every - commit since the merge base has a patch-equivalent commit on the default - branch (a rebase merge), or its whole change since the merge base is - patch-equivalent to one commit there (a squash merge). - Merge state comes from git alone, not from a hosting service. Patch - equivalence is git's patch-id, which ignores whitespace. A squash merge - whose content changed during the merge (a conflict resolution, a suggested - edit) does not match and is kept, and so is a fast-forwarded branch. When - origin/HEAD is not set, the fetch fails, or git cannot answer, the state is - unknown and the worktree is kept (reason merge-unknown, with the remedy). A - squash- or rebase-merged branch whose upstream was deleted after the merge - has commits only it reaches; they do not block removal, because their - change is on the default branch, and the branch is kept. + commit since the merge base has the same \`git patch-id --verbatim\` as a + commit on the default branch (a rebase merge), or its whole diff since the + merge base has the same verbatim patch id as a commit there, compared on + the files the branch changes (a squash merge). + Merge state comes from git alone, not from a hosting service. A squash + merge whose content changed during the merge (a conflict resolution, a + suggested edit, even whitespace) does not match and is kept, and so is a + fast-forwarded branch. When origin/HEAD is not set, the fetch fails, or git + cannot answer, the state is unknown and never counts as merged (reason + merge-unknown, with the remedy); with --worktrees an idle worktree is still + removed. A squash- or rebase-merged branch whose upstream is gone (deleted + after the merge and pruned locally) has commits only it reaches; they do + not block removal, because their change is on the default branch, and the + branch is kept. stim gc # report merged worktrees stim gc --delete # remove them stim gc --delete --worktrees --older-than 3 # also the clean idle ones @@ -128,7 +130,8 @@ SWEEPING FINISHED WORKTREES on each removable worktree. That pipeline re-inspects the worktree and re-checks use under the removal locks, and idleness for an idle worktree or an unchanged HEAD for a merged one, then parks devices and handles the - branch exactly as a manual \`stim worktree remove\`. A worktree that + branch exactly as a manual \`stim worktree remove\`, which keeps the + checkout when its HEAD moves while devices are reclaimed. A worktree that changed since the report is kept with the reason. A worktree that fails is reported, gc exits 1, and the sweep continues. diff --git a/packages/stim-cli/src/workspace/merge-state.ts b/packages/stim-cli/src/workspace/merge-state.ts index 4ff6b9dfb..435a6d741 100644 --- a/packages/stim-cli/src/workspace/merge-state.ts +++ b/packages/stim-cli/src/workspace/merge-state.ts @@ -6,18 +6,6 @@ const FETCH_FRESH_MS = 10 * 60_000; const GIT_TIMEOUT_MS = 60_000; const REMOTE_PREFIX = 'refs/remotes/origin/'; -// `git commit-tree` needs an identity and a date; fixed values keep the -// synthetic squash commit identical across runs, so repeated checks add no -// new objects. -const SQUASH_IDENTITY = { - GIT_AUTHOR_NAME: 'stim', - GIT_AUTHOR_EMAIL: 'stim@localhost', - GIT_AUTHOR_DATE: '1000000000 +0000', - GIT_COMMITTER_NAME: 'stim', - GIT_COMMITTER_EMAIL: 'stim@localhost', - GIT_COMMITTER_DATE: '1000000000 +0000', -}; - export interface DefaultBranch { ref: string; name: string; @@ -109,17 +97,23 @@ function committedOn(path: string, branch: string | null): boolean { * Whether the worktree's HEAD is merged into `target`. The signals, all local git: * - HEAD is an ancestor of the default branch, off its first-parent line, and the branch's reflog shows a commit made * on it, so a merge commit brought the branch's own work in. A branch with no commit of its own is not merged. - * - The branch changes the tree, has no merge commits, and every commit since the merge base has a patch-equivalent - * commit on the default branch (a rebase merge). - * - The branch changes the tree and its whole change since the merge base is patch-equivalent to one commit on the - * default branch (a squash merge). - * Patch equivalence is git's patch-id, which ignores whitespace. Anything git cannot answer is unknown, never merged. - * `coversUnpushed` is true for a patch-equivalent HEAD whose upstream branch was deleted: its commits exist only - * locally, but their change is on the default branch. + * - The branch changes the tree, has no merge commits, and every commit since the merge base has the same + * `git patch-id --verbatim` as a commit on the default branch (a rebase merge). + * - The branch changes the tree and its whole diff since the merge base has the same verbatim patch id as a commit on + * the default branch, limited to the files the branch changes (a squash merge). + * Anything git cannot answer is unknown, never merged. `coversUnpushed` is true for a patch-equivalent HEAD whose + * upstream branch was deleted: its commits exist only locally, but their change is on the default branch. */ export function mergeState(path: string, { ref, name }: DefaultBranch): MergeState { - const git = (args: string[], env?: Record): string => - getExecutor().runFile('git', ['-C', path, ...args], { timeoutMs: GIT_TIMEOUT_MS, env }); + const git = (args: string[], input?: string): string => + getExecutor().runFile('git', ['--literal-pathspecs', '-C', path, ...args], { timeoutMs: GIT_TIMEOUT_MS, input }); + const patchIds = (patch: string): string[] => + patch + ? git(['patch-id', '--verbatim'], patch) + .split('\n') + .flatMap((line) => line.split(' ')[0] || []) + : []; + const diffOptions = ['--no-color', '--no-ext-diff']; const noOwnCommits = notMerged(`no commits of its own beyond ${name}`); try { const head = git(['rev-parse', '--verify', 'HEAD^{commit}']); @@ -134,24 +128,23 @@ export function mergeState(path: string, { ref, name }: DefaultBranch): MergeSta if (mainline || !committedOn(path, branch)) return noOwnCommits; return { merged: true, into: name, head, coversUnpushed: false }; } - const tree = git(['rev-parse', `${head}^{tree}`]); - if (tree === git(['rev-parse', `${base}^{tree}`])) return notMerged(`no net change beyond ${name}`); - const equivalent = (tip: string): boolean => { - const lines = git(['cherry', ref, tip, base]).split('\n').filter(Boolean); - return lines.length > 0 && lines.every((line) => line.startsWith('- ')); - }; - const patchEquivalent = () => ({ - merged: true as const, + const files = git(['diff', '--name-only', '-z', base, head]).split('\0').filter(Boolean); + if (!files.length) return notMerged(`no net change beyond ${name}`); + const log = (range: string, pathspec: string[] = []) => + git(['log', '-p', '--no-merges', ...diffOptions, '--format=commit %H', range, '--', ...pathspec]); + const upstream = new Set(patchIds(log(`${base}..${ref}`, files))); + const patchEquivalent = (): MergeState => ({ + merged: true, into: name, head, coversUnpushed: upstreamGone(path, branch), }); - if (!git(['rev-list', '--merges', `${base}..${head}`]) && equivalent(head)) return patchEquivalent(); - const squashed = git( - ['commit-tree', '--no-gpg-sign', tree, '-p', base, '-m', 'stim gc squash-merge check'], - SQUASH_IDENTITY, - ); - if (equivalent(squashed)) return patchEquivalent(); + const squash = patchIds(git(['diff', ...diffOptions, base, head])); + if (squash.length === 1 && upstream.has(squash[0]!)) return patchEquivalent(); + if (!git(['rev-list', '--merges', `${base}..${head}`])) { + const own = patchIds(log(`${base}..${head}`)); + if (own.length && own.every((id) => upstream.has(id))) return patchEquivalent(); + } return notMerged(`not merged into ${name}`); } catch (error) { return { merged: false, unknown: true, detail: `merge state unknown: ${failure(error)}` }; diff --git a/website/docs/worktrees.md b/website/docs/worktrees.md index a48aa1847..a34104f83 100644 --- a/website/docs/worktrees.md +++ b/website/docs/worktrees.md @@ -243,17 +243,18 @@ minutes. The branch counts as merged when: - a merge commit brought its HEAD into the default branch, and the branch's reflog shows a commit made on it. A branch with no commits of its own, such as one just created or cut from another branch, is not merged. -- it has no merge commits and each of its commits has a patch-equivalent - commit on the default branch, as after a rebase merge. -- its whole change is patch-equivalent to one commit on the default branch, as - after a squash merge. - -gc asks git only, not GitHub. Patch equivalence is git's patch-id, which -ignores whitespace. A squash merge whose content changed while merging, such -as a conflict resolution, does not match, and gc keeps that worktree; so does -a branch that was fast-forwarded into the default branch. When `origin/HEAD` is -not set, the fetch fails, or git cannot answer, gc keeps the worktree and -reports why. After a squash or rebase merge that deleted the remote branch, the +- it has no merge commits and each of its commits has the same + `git patch-id --verbatim` as a commit on the default branch, as after a + rebase merge. +- its whole diff has the same verbatim patch id as a commit on the default + branch, compared on the files the branch changes, as after a squash merge. + +gc asks git only, not GitHub. A squash merge whose content changed while +merging, such as a conflict resolution or even a whitespace edit, does not +match, and gc keeps that worktree; so does a branch that was fast-forwarded +into the default branch. When `origin/HEAD` is not set, the fetch fails, or git +cannot answer, gc does not treat the branch as merged and reports why. After a +squash or rebase merge whose remote branch was deleted and pruned locally, the branch's commits exist only locally; gc still removes the worktree, because their change is on the default branch, and keeps the branch. From 9800da1f87838b985a21429a4d9d66cd31ab7261 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Fri, 25 Sep 2026 01:03:18 -0400 Subject: [PATCH 4/4] fix: keep trailing whitespace in patch ids and require a reflog commit HEAD contains Patch text reaches git patch-id untrimmed, a reflog commit from an earlier life of a reused branch name no longer counts, renames keep both paths in the pathspec, and worktree remove snapshots HEAD before its final inspection. --- .../src/__tests__/gc-workspaces.test.ts | 21 +++++++---- packages/stim-cli/src/commands/worktree.ts | 2 +- packages/stim-cli/src/exec.ts | 6 ++-- packages/stim-cli/src/guide/cleanup.ts | 5 +-- .../stim-cli/src/workspace/merge-state.ts | 35 +++++++++++++------ website/docs/worktrees.md | 7 ++-- 6 files changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts index 302497acb..a8f668fa2 100644 --- a/packages/stim-cli/src/__tests__/gc-workspaces.test.ts +++ b/packages/stim-cli/src/__tests__/gc-workspaces.test.ts @@ -753,7 +753,7 @@ function repoWithMergedBranches() { } const spaced = join(projects, 'spaced'); git(`worktree add -q "${spaced}" -b spaced`); - writeFileSync(join(spaced, 'value.txt'), 'a b\n'); + writeFileSync(join(spaced, 'value.txt'), 'ab \n'); git('add value.txt', spaced); git('commit -q -m spaced', spaced); git('push -q -u origin spaced', spaced); @@ -761,9 +761,19 @@ function repoWithMergedBranches() { const followup = join(projects, 'followup'); git(`worktree add -q "${followup}" -b followup merged`); worktrees.followup = realpathSync.native(followup); + const reused = join(projects, 'reused'); + git(`worktree add -q "${reused}" -b reused`); + commit(reused, 'reused-earlier-life.txt'); + git(`worktree remove "${reused}"`); + git(`worktree add -q -B reused "${reused}" merged`); + worktrees.reused = realpathSync.native(reused); git(`clone -q "${remote}" "${upstream}"`, projects); identity(upstream); commit(upstream, 'main-moved-on.txt'); + git('push -q origin main', upstream); + git('fetch -q origin main', worktrees.evil); + git('merge -q --no-ff --no-commit origin/main', worktrees.evil); + commit(worktrees.evil!, 'not-on-main.txt'); git('merge -q --no-ff origin/merged -m merge-merged', upstream); git('merge -q --no-ff origin/dirty -m merge-dirty', upstream); git('merge -q --squash origin/squashed', upstream); @@ -772,11 +782,8 @@ function repoWithMergedBranches() { git('cherry-pick origin/evil', upstream); writeFileSync(join(upstream, 'value.txt'), 'ab\n'); git('add value.txt', upstream); - git('commit -q -m value-without-the-space', upstream); + git('commit -q -m value-without-the-trailing-spaces', upstream); git('push -q origin main', upstream); - git('fetch -q origin main', worktrees.evil); - git('merge -q --no-ff --no-commit origin/main', worktrees.evil); - commit(worktrees.evil!, 'not-on-main.txt'); for (const gone of ['squashed', 'rebased', 'evil', 'spaced']) { git(`push -q origin --delete ${gone}`, upstream); git(`update-ref -d refs/remotes/origin/${gone}`); @@ -806,7 +813,7 @@ test('plain gc --delete removes merged worktrees, squash merges included, after }); expect(byPath[worktrees.squashed!]).toMatchObject({ willRemove: true, detail: 'merged into origin/main' }); expect(byPath[worktrees.rebased!]).toMatchObject({ willRemove: true, detail: 'merged into origin/main' }); - for (const name of ['fresh', 'followup']) { + for (const name of ['fresh', 'followup', 'reused']) { expect(byPath[worktrees[name]!]).toMatchObject({ willRemove: false, reason: 'not-merged', @@ -823,7 +830,7 @@ test('plain gc --delete removes merged worktrees, squash merges included, after expect(output).toContain(`Removed the worktree ${worktrees.merged} (merged into origin/main)`); expect(output).toContain(`Removed the worktree ${worktrees.squashed} (merged into origin/main)`); for (const name of ['merged', 'squashed', 'rebased']) expect(existsSync(worktrees[name]!)).toBe(false); - for (const name of ['fresh', 'followup', 'evil', 'spaced', 'open', 'dirty']) { + for (const name of ['fresh', 'followup', 'reused', 'evil', 'spaced', 'open', 'dirty']) { expect(existsSync(worktrees[name]!)).toBe(true); } expect(existsSync(join(repo, 'package.json'))).toBe(true); diff --git a/packages/stim-cli/src/commands/worktree.ts b/packages/stim-cli/src/commands/worktree.ts index 500ce6ff5..84264cdaa 100644 --- a/packages/stim-cli/src/commands/worktree.ts +++ b/packages/stim-cli/src/commands/worktree.ts @@ -856,12 +856,12 @@ async function runRemove(target: string | undefined, opts: RemoveOptions, onRemo await withManagedRemoteWorktreeRemovalLock(path, () => withReclaimLocks(path, async (lockedKeys) => { if (opts.guard?.(lockedKeys).length) return; + const inspectedHead = resolveFullRef(path, 'HEAD'); const current = inspectRemoval(path, opts.mergedHead); if (current.blockers.length && !opts.force) { printRemovalRefusal(path, current); return; } - const inspectedHead = resolveFullRef(path, 'HEAD'); const result = await reclaimAll(path, lockedKeys, { preserveRootProject: true }); if (result.keptEntries.length) { reportRetainedResources(path, result); diff --git a/packages/stim-cli/src/exec.ts b/packages/stim-cli/src/exec.ts index 09eef3172..431112782 100644 --- a/packages/stim-cli/src/exec.ts +++ b/packages/stim-cli/src/exec.ts @@ -10,6 +10,8 @@ interface ExecOptions { omitEnv?: readonly string[]; /** Text written to the child's stdin; `runFile` only. */ input?: string; + /** `runFile` only: return stdout as written, without trimming surrounding whitespace. */ + untrimmed?: boolean; } export interface Executor { @@ -53,7 +55,7 @@ const defaultExecutor: Executor = { // refuses .cmd/.bat files and shebang scripts without a shell, and every // package bin (eas, agent-device) is one of those. The throw matches // execFileSync's, so callers keep reading status, stdout and stderr off it. - runFile(file, args = [], { timeoutMs, killSignal, cwd, env, omitEnv, input } = {}) { + runFile(file, args = [], { timeoutMs, killSignal, cwd, env, omitEnv, input, untrimmed } = {}) { const opts: Parameters[2] = { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], @@ -75,7 +77,7 @@ const defaultExecutor: Executor = { const message = `Command failed: ${[file, ...args].join(' ')}${stderr ? `\n${stderr}` : ''}`; throw Object.assign(new Error(message), result); } - return String(result.stdout).trim(); + return untrimmed ? String(result.stdout) : String(result.stdout).trim(); }, runFileAsync(file, args = [], { timeoutMs, killSignal, cwd, env, omitEnv } = {}) { const command = [file, ...args].join(' '); diff --git a/packages/stim-cli/src/guide/cleanup.ts b/packages/stim-cli/src/guide/cleanup.ts index fddfa8b14..009d7bad8 100644 --- a/packages/stim-cli/src/guide/cleanup.ts +++ b/packages/stim-cli/src/guide/cleanup.ts @@ -105,8 +105,9 @@ SWEEPING FINISHED WORKTREES \`git fetch origin \` per repository (30s timeout, no credential prompt; skipped when that checkout fetched in the last 10 minutes): - HEAD is reachable from origin/ through a merge commit, and the - branch's reflog shows a commit made on it. A branch with no commit of - its own -- fresh, or cut from another branch -- is not merged. + branch's reflog shows a commit made on it that HEAD contains. A branch + with no commit of its own -- fresh, cut from another branch, or reset + onto one -- is not merged. - the branch changes the tree, and either it has no merge commits and every commit since the merge base has the same \`git patch-id --verbatim\` as a commit on the default branch (a rebase merge), or its whole diff since the diff --git a/packages/stim-cli/src/workspace/merge-state.ts b/packages/stim-cli/src/workspace/merge-state.ts index 435a6d741..73daac9b5 100644 --- a/packages/stim-cli/src/workspace/merge-state.ts +++ b/packages/stim-cli/src/workspace/merge-state.ts @@ -87,16 +87,24 @@ function upstreamGone(path: string, branch: string | null): boolean { return Boolean(upstream) && state === '[gone]'; } -function committedOn(path: string, branch: string | null): boolean { +function committedOn(path: string, branch: string | null, head: string): boolean { if (!branch) return false; - const subjects = getExecutor().runFileQuiet('git', ['-C', path, 'reflog', 'show', '--format=%gs', branch]) ?? ''; - return subjects.split('\n').some((subject) => /^(commit|cherry-pick|rebase|revert)\b/.test(subject)); + const exec = getExecutor(); + const entries = exec.runFileQuiet('git', ['-C', path, 'reflog', 'show', '--format=%H %gs', branch]) ?? ''; + return entries.split('\n').some((entry) => { + const [sha = '', ...subject] = entry.split(' '); + return ( + /^(commit|cherry-pick|rebase|revert)\b/.test(subject.join(' ')) && + exec.runFileQuiet('git', ['-C', path, 'merge-base', '--is-ancestor', sha, head]) !== null + ); + }); } /** * Whether the worktree's HEAD is merged into `target`. The signals, all local git: * - HEAD is an ancestor of the default branch, off its first-parent line, and the branch's reflog shows a commit made - * on it, so a merge commit brought the branch's own work in. A branch with no commit of its own is not merged. + * on it that HEAD contains, so a merge commit brought the branch's own work in. A branch with no commit of its own + * is not merged. * - The branch changes the tree, has no merge commits, and every commit since the merge base has the same * `git patch-id --verbatim` as a commit on the default branch (a rebase merge). * - The branch changes the tree and its whole diff since the merge base has the same verbatim patch id as a commit on @@ -107,9 +115,14 @@ function committedOn(path: string, branch: string | null): boolean { export function mergeState(path: string, { ref, name }: DefaultBranch): MergeState { const git = (args: string[], input?: string): string => getExecutor().runFile('git', ['--literal-pathspecs', '-C', path, ...args], { timeoutMs: GIT_TIMEOUT_MS, input }); - const patchIds = (patch: string): string[] => - patch - ? git(['patch-id', '--verbatim'], patch) + const patch = (args: string[]): string => + getExecutor().runFile('git', ['--literal-pathspecs', '-C', path, ...args], { + timeoutMs: GIT_TIMEOUT_MS, + untrimmed: true, + }); + const patchIds = (text: string): string[] => + text + ? git(['patch-id', '--verbatim'], text) .split('\n') .flatMap((line) => line.split(' ')[0] || []) : []; @@ -125,13 +138,13 @@ export function mergeState(path: string, { ref, name }: DefaultBranch): MergeSta git(['rev-list', '--first-parent', '--parents', `${head}..${ref}`]) .split('\n') .some((line) => line.split(' ')[1] === head); - if (mainline || !committedOn(path, branch)) return noOwnCommits; + if (mainline || !committedOn(path, branch, head)) return noOwnCommits; return { merged: true, into: name, head, coversUnpushed: false }; } - const files = git(['diff', '--name-only', '-z', base, head]).split('\0').filter(Boolean); + const files = git(['diff', '--name-only', '--no-renames', '-z', base, head]).split('\0').filter(Boolean); if (!files.length) return notMerged(`no net change beyond ${name}`); const log = (range: string, pathspec: string[] = []) => - git(['log', '-p', '--no-merges', ...diffOptions, '--format=commit %H', range, '--', ...pathspec]); + patch(['log', '-p', '--no-merges', ...diffOptions, '--format=commit %H', range, '--', ...pathspec]); const upstream = new Set(patchIds(log(`${base}..${ref}`, files))); const patchEquivalent = (): MergeState => ({ merged: true, @@ -139,7 +152,7 @@ export function mergeState(path: string, { ref, name }: DefaultBranch): MergeSta head, coversUnpushed: upstreamGone(path, branch), }); - const squash = patchIds(git(['diff', ...diffOptions, base, head])); + const squash = patchIds(patch(['diff', ...diffOptions, base, head])); if (squash.length === 1 && upstream.has(squash[0]!)) return patchEquivalent(); if (!git(['rev-list', '--merges', `${base}..${head}`])) { const own = patchIds(log(`${base}..${head}`)); diff --git a/website/docs/worktrees.md b/website/docs/worktrees.md index a34104f83..ed7095e28 100644 --- a/website/docs/worktrees.md +++ b/website/docs/worktrees.md @@ -241,8 +241,11 @@ To decide that a branch is merged, gc takes the default branch from minutes. The branch counts as merged when: - a merge commit brought its HEAD into the default branch, and the branch's - reflog shows a commit made on it. A branch with no commits of its own, such - as one just created or cut from another branch, is not merged. + reflog shows a commit made on it that HEAD contains. A branch with no commits + of its own, such as one just created, cut from another branch, or reset onto + one, is not merged. + The next two apply only to a branch that changes the tree: + - it has no merge commits and each of its commits has the same `git patch-id --verbatim` as a commit on the default branch, as after a rebase merge.