diff --git a/docs/api-reference.md b/docs/api-reference.md index aa4d5cc8..81c920d1 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1043,7 +1043,7 @@ A local case whose folder is the top of a git repository can keep **worktrees** - `excludeAdded` lists the lines appended to the repository's local `info/exclude` (in the real common git dir, never a tracked `.gitignore`) when git did not already ignore them: `/.claude/worktrees/`, so the main checkout does not show the worktrees as untracked, and `/.claude/settings.local.json`, the hooks file Codeman writes for every claude session, which would otherwise make `git worktree remove` refuse. - A symlinked `.claude` or `.claude/worktrees` directory is refused (`400`): a repository cannot redirect the worktree outside itself. -`DELETE /api/cases/:name/worktrees/:worktree[?deleteBranch=true]` removes a managed worktree with `git worktree remove` **without `--force`**: git refuses a worktree with modified or untracked files (or a locked one), and that refusal is the answer, `409 CONFLICT` with git's own words in `error`. A live session at or inside the worktree is `409` as well. With `deleteBranch=true` the branch is deleted with `git branch -d`, never `-D`, so an unmerged branch is kept: the response is still `200`, with `branchDeleted: false` and git's reason in `branchError`. Response: `{ removed: { name, path, branch, branchDeleted, branchError? } }`. A folder git does not list as a worktree of this repository, or a symlink in its place, is `404`/`400`. +`DELETE /api/cases/:name/worktrees/:worktree[?deleteBranch=true]` removes a managed worktree with `git worktree remove` **without `--force`**: git refuses a worktree with modified or untracked files (or a locked one), and that refusal is the answer, `409 CONFLICT` with git's own words in `error`. A live session at or inside the worktree is `409` as well. With `deleteBranch=true` the branch is deleted with `git branch -d`, never `-D`, so an unmerged branch is kept: the response is still `200`, with `branchDeleted: false` and git's reason in `branchError`. Response: `{ removed: { name, path, branch, branchDeleted, branchError? } }`. A folder git does not list as a worktree of this repository, or a symlink in its place, is `404`/`400`. A managed worktree whose folder is gone (deleted by hand; the list reports it `prunable`) is still removable: git then only drops its record, which is what frees the name for a new worktree. A session never starts in one. **Starting a session in a worktree** goes through the normal create paths: `POST /api/quick-start` takes `worktree: ""` next to `caseName` and runs the session (and the hooks, the agent skill and `modelOverride`) in the worktree folder; the response adds `worktree` and `workingDir`. It is refused for remote and Docker cases (`400`), for a worktree that is not a registered, managed one (`404`), and for a case that does not exist (`404`: a worktree launch never scaffolds a case). `POST /api/sessions` takes the worktree path as `workingDir` like any other folder. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 0a81b119..b56cdde9 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -452,7 +452,7 @@ Further detail: the synchronous clone request is bounded by `GIT_CLONE_TIMEOUT_M **Git worktrees** (case picker → **Git worktrees…**, the phone case sheet's **Worktrees**, Close Session's **Kill & remove worktree**): a LOCAL case whose folder is the top of a git repository keeps worktrees at `/.claude/worktrees/` on a new branch (default `worktree-`), so parallel agents each get their own checkout instead of sharing the case folder. That is Claude Code's own `--worktree` layout (measured in 2.1.296: same path, branch `worktree-`, names of `[A-Za-z0-9._-]`, at most 64), and Codeman's names are a strict subset of Claude's. Core in `src/git-worktrees.ts` (pure name/branch gates, `worktree list --porcelain` parsing, exclude text; IO behind an injectable runner); routes in `routes/worktree-routes.ts` (`GET`/`POST /api/cases/:name/worktrees`, `DELETE /api/cases/:name/worktrees/:worktree`). -- ⚠️ **Only `/.claude/worktrees/` is ever created or removed.** Every component is `lstat`ed (a symlinked `.claude`, `worktrees` or worktree folder is refused, never followed) and the real path must equal the lexical one, so a repository cannot redirect a create or remove outside itself; removal also requires git to list the folder as a worktree of THIS repository. A worktree anywhere else is listed read-only (`managed: false`). +- ⚠️ **Only `/.claude/worktrees/` is ever created or removed.** Every component is `lstat`ed (a symlinked `.claude`, `worktrees` or worktree folder is refused, never followed) and the real path must equal the lexical one, so a repository cannot redirect a create or remove outside itself; removal also requires git to list the folder as a worktree of THIS repository. A worktree anywhere else is listed read-only (`managed: false`). A managed worktree whose folder is GONE stays removable (`resolveManagedWorktree({ allowMissing })`, removal only, never a session start: git lists it at exactly the managed path, no component on the way is a symlink, and `git worktree remove` then only drops git's record), since otherwise its name could never be used again. - ⚠️ **git never runs for a case at or inside a Docker case workspace** (or whose common git dir is there), and Docker and remote cases are refused with a 400 before any git, the same rule as the git status indicator: `git worktree add` runs the repository's post-checkout hook and checkout filters ON THE HOST. That is acceptable for a local case, where the user's agent already runs code. - ⚠️ **Removal is `git worktree remove` WITHOUT `--force`**, refused (409) while any live session (any owner) runs at or inside the worktree, and git's own refusal (modified or untracked files, a lock) is the answer, shown through `textContent`. The optional branch delete is `git branch -d`, never `-D`: an unmerged branch survives and the response says why (`branchError`). - ⚠️ **The main checkout must not see the worktrees.** When `git check-ignore` says they are not ignored, `/.claude/worktrees/` is appended to the repository's LOCAL `/info/exclude` (never a tracked `.gitignore`), and so is `/.claude/settings.local.json`: Codeman writes its hooks there for every claude session, and an untracked copy in a worktree would make `git worktree remove` refuse forever (Claude Code covers that file with the global excludes file, which a machine may not have). A symlinked `info` dir or `exclude` file is reported, never written through. diff --git a/docs/wiki/Git-Worktrees.md b/docs/wiki/Git-Worktrees.md index 563aed9b..7d028ad5 100644 --- a/docs/wiki/Git-Worktrees.md +++ b/docs/wiki/Git-Worktrees.md @@ -53,6 +53,9 @@ work first, then remove again. A worktree a live session still runs in is refuse commits are not merged yet. The worktree is still removed, and the message says the branch was kept. +If a worktree's folder was deleted by hand, its row says **missing** and offers only +**Remove…**: that clears the record git keeps of it, so the name can be used for a new worktree. + ## What is not supported - **Docker cases and remote (SSH) cases.** A container can write to its workspace, and creating diff --git a/src/git-worktrees.ts b/src/git-worktrees.ts index 6b1d83ec..9696e733 100644 --- a/src/git-worktrees.ts +++ b/src/git-worktrees.ts @@ -594,36 +594,58 @@ async function prepareWorktreesDir(repoRoot: string): Promise<{ ok: true; dir: s * The managed worktree `name` of `repo` as it is on disk now: every component a real directory * (no symlinks), the real path the expected one, and git listing it as a worktree of this * repository. Used by removal and by starting a session in it. + * + * `allowMissing` (removal only, never a session start) also accepts a worktree whose folder is + * GONE (deleted by hand, or by an agent) while git still lists it, as prunable, at exactly the + * managed path: no component on the way may be a symlink or a file, and `missing` says so. + * Without it such a worktree was a dead end: it could not be removed, and its name could not be + * used again (the branch exists, then git refuses "a missing but already registered worktree"). */ export async function resolveManagedWorktree( repo: CaseRepository, name: string, - opts: WorktreeOptions = {} -): Promise<{ ok: true; path: string; worktree: CaseWorktree } | WorktreeFailure> { + opts: WorktreeOptions & { allowMissing?: boolean } = {} +): Promise<{ ok: true; path: string; worktree: CaseWorktree; missing: boolean } | WorktreeFailure> { const nameError = worktreeNameError(name); if (nameError) return fail('INVALID_INPUT', nameError); const target = managedWorktreePath(repo.repoRoot, name); let cur = repo.repoRoot; + let missing = false; for (const segment of [...WORKTREES_DIR_SEGMENTS, name]) { cur = join(cur, segment); const state = await lstatState(cur); - if (state === 'absent') return fail('NOT_FOUND', `No worktree named "${name}" in this case.`); + if (state === 'absent') { + if (!opts.allowMissing) return fail('NOT_FOUND', `No worktree named "${name}" in this case.`); + // Nothing below an absent folder exists, so nothing below it can redirect anything. + missing = true; + break; + } if (state === 'symlink') { return fail('INVALID_INPUT', `${cur} is a symlink. Codeman only manages worktrees that are real directories.`); } if (state === 'other') return fail('INVALID_INPUT', `${cur} is not a directory.`); } - if ((await realOr(target)) !== target) { + if (!missing && (await realOr(target)) !== target) { return fail('INVALID_INPUT', `${target} does not resolve to itself; refusing to use it.`); } const listed = await listCaseWorktrees(repo, opts); if (!listed.ok) return listed; for (const wt of listed.worktrees) { if (wt.main) continue; + if (missing) { + // Lexical only: the folder is gone, so there is no real path to compare. git never calls a + // LOCKED worktree prunable (prune skips it), so a locked one is taken too, and the caller's + // lock check then refuses it with git's lock reason. + if (wt.path === target && (wt.prunable || wt.locked)) { + return { ok: true, path: target, worktree: { ...wt, managed: true, name }, missing: true }; + } + continue; + } if (wt.path === target || (await realOr(wt.path)) === target) { - return { ok: true, path: target, worktree: { ...wt, managed: true, name } }; + return { ok: true, path: target, worktree: { ...wt, managed: true, name }, missing: false }; } } + if (missing) return fail('NOT_FOUND', `No worktree named "${name}" in this case.`); return fail( 'NOT_FOUND', `${target} is not a registered git worktree of this repository (a leftover folder, or one git no longer knows about).` @@ -835,7 +857,9 @@ export async function removeCaseWorktree( ): Promise<{ ok: true; removed: RemovedWorktree } | WorktreeFailure> { const git = opts.git ?? runWorktreeGit; return withRepoLock(repo.repoRoot, async () => { - const found = await resolveManagedWorktree(repo, req.name, opts); + // A worktree whose folder is gone is removable too: `git worktree remove` then only drops + // git's record of it (measured on git 2.43: exit 0, nothing on disk touched). + const found = await resolveManagedWorktree(repo, req.name, { ...opts, allowMissing: true }); if (!found.ok) return found; if (await req.isInUse(found.path)) { return fail( diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index 3106dedd..7cedde3e 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -297,6 +297,8 @@ 'Remove…': '移除…', 'git worktree remove refuses while the worktree has uncommitted or untracked files. Codeman never forces it.': '工作树中有未提交或未跟踪的文件时，git worktree remove 会拒绝执行。{name} 从不强制移除。', + 'Its folder is gone. Removing it clears the record git keeps of it, so the name can be used again.': + '它的文件夹已不存在。移除它会清除 git 保存的记录，这个名称便可再次使用。', 'Remove worktree': '移除工作树', 'Kill & remove worktree': '终止并移除工作树', 'git refuses while the worktree has uncommitted or untracked files; nothing is forced': diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 6e22e416..df26df9e 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -5229,21 +5229,26 @@ Object.assign(CodemanApp.prototype, { badge(`${wt.sessionCount} session${wt.sessionCount === 1 ? '' : 's'}`, 'worktree-badge-live'); } - if (wt.managed && !wt.prunable) { + if (wt.managed) { const actions = document.createElement('span'); actions.className = 'worktree-row-actions'; - const open = document.createElement('button'); - open.type = 'button'; - open.className = 'wt-btn wt-btn-sm'; - open.textContent = 'Open'; - open.title = 'Start a session in this worktree'; - open.addEventListener('click', () => void this.openWorktreeSession(wt.name)); + // A worktree whose folder is gone (`prunable`) cannot host a session, but Remove still + // clears git's record of it, which is what frees its name for a new worktree. + if (!wt.prunable) { + const open = document.createElement('button'); + open.type = 'button'; + open.className = 'wt-btn wt-btn-sm'; + open.textContent = 'Open'; + open.title = 'Start a session in this worktree'; + open.addEventListener('click', () => void this.openWorktreeSession(wt.name)); + actions.appendChild(open); + } const remove = document.createElement('button'); remove.type = 'button'; remove.className = 'wt-btn wt-btn-sm wt-btn-danger'; remove.textContent = 'Remove…'; remove.addEventListener('click', () => this.confirmWorktreeRemoval(wt.name)); - actions.append(open, remove); + actions.appendChild(remove); head.appendChild(actions); } row.appendChild(head); @@ -5266,8 +5271,9 @@ Object.assign(CodemanApp.prototype, { box.className = 'worktree-remove-confirm'; const note = document.createElement('p'); note.className = 'worktree-remove-note'; - note.textContent = - 'git worktree remove refuses while the worktree has uncommitted or untracked files. Codeman never forces it.'; + note.textContent = wt.prunable + ? 'Its folder is gone. Removing it clears the record git keeps of it, so the name can be used again.' + : 'git worktree remove refuses while the worktree has uncommitted or untracked files. Codeman never forces it.'; box.appendChild(note); let checkbox = null; diff --git a/test/git-worktrees.test.ts b/test/git-worktrees.test.ts index 9e318155..324a03ae 100644 --- a/test/git-worktrees.test.ts +++ b/test/git-worktrees.test.ts @@ -380,4 +380,50 @@ describe('git worktrees on a real repository', () => { expect(r).toMatchObject({ ok: false, code: 'CONFLICT' }); if (!r.ok) expect(r.message).toContain('agent busy'); }); + + it('removes a worktree whose folder was deleted by hand, so its name can be used again', async () => { + const made = await createCaseWorktree(repo, { name: 'gone' }); + if (!made.ok) throw new Error(made.message); + rmSync(made.worktree.path, { recursive: true, force: true }); + const listed = await listCaseWorktrees(repo); + expect(listed.ok && listed.worktrees.find((w) => w.name === 'gone')).toMatchObject({ + managed: true, + prunable: true, + }); + // A session never starts in it; only removal accepts a missing folder. + expect(await resolveManagedWorktree(repo, 'gone')).toMatchObject({ ok: false, code: 'NOT_FOUND' }); + // Before the fix this was NOT_FOUND, and both ways of reusing the name failed. + const r = await removeCaseWorktree(repo, { name: 'gone', deleteBranch: true, isInUse: () => false }); + expect(r).toMatchObject({ ok: true, removed: { name: 'gone', branch: 'worktree-gone', branchDeleted: true } }); + expect(git(repoDir, 'worktree', 'list', '--porcelain')).not.toContain('gone'); + expect((await createCaseWorktree(repo, { name: 'gone' })).ok).toBe(true); + }); + + it('refuses a missing worktree that a live session still names, or that is locked', async () => { + const made = await createCaseWorktree(repo, { name: 'held' }); + if (!made.ok) throw new Error(made.message); + git(repoDir, 'worktree', 'lock', '--reason', 'on a removable disk', made.worktree.path); + rmSync(made.worktree.path, { recursive: true, force: true }); + expect(await removeCaseWorktree(repo, { name: 'held', isInUse: () => true })).toMatchObject({ + ok: false, + code: 'CONFLICT', + }); + const locked = await removeCaseWorktree(repo, { name: 'held', isInUse: () => false }); + expect(locked).toMatchObject({ ok: false, code: 'CONFLICT' }); + if (!locked.ok) expect(locked.message).toContain('removable disk'); + }); + + it('never follows a symlink on the way to a missing worktree', async () => { + const made = await createCaseWorktree(repo, { name: 'via' }); + if (!made.ok) throw new Error(made.message); + // The whole worktrees folder replaced by a link to somewhere else. + const elsewhere = join(dir, 'elsewhere'); + mkdirSync(elsewhere); + rmSync(join(repoDir, '.claude', 'worktrees'), { recursive: true, force: true }); + symlinkSync(elsewhere, join(repoDir, '.claude', 'worktrees')); + expect(await removeCaseWorktree(repo, { name: 'via', isInUse: () => false })).toMatchObject({ + ok: false, + code: 'INVALID_INPUT', + }); + }); }); diff --git a/test/routes/worktree-routes.test.ts b/test/routes/worktree-routes.test.ts index ae7298e2..ccc1181a 100644 --- a/test/routes/worktree-routes.test.ts +++ b/test/routes/worktree-routes.test.ts @@ -182,6 +182,26 @@ describe('worktree routes: happy path', () => { expect(readFileSync(join(wtPath, 'wip.txt'), 'utf-8')).toBe('work'); }); + it('removes a worktree whose folder was deleted by hand and frees its name', async () => { + await create({ name: 'gone' }); + const wtPath = join(repo, '.claude', 'worktrees', 'gone'); + await rm(wtPath, { recursive: true, force: true }); + const row = (await list()).json().data.worktrees.find((w: { name?: string }) => w.name === 'gone'); + expect(row).toMatchObject({ managed: true, prunable: true }); + // A session cannot start in it. + const qs = await h.app.inject({ + method: 'POST', + url: '/api/quick-start', + payload: { caseName: 'wtcase', mode: 'shell', worktree: 'gone' }, + }); + expect(qs.statusCode).toBe(404); + const res = await remove('gone', '?deleteBranch=true'); + expect(res.statusCode).toBe(200); + expect(res.json().data.removed).toMatchObject({ name: 'gone', branchDeleted: true }); + expect(git(repo, 'worktree', 'list')).not.toContain('gone'); + expect((await create({ name: 'gone' })).statusCode).toBe(201); + }); + it('lets a POST /api/sessions create path use the worktree as workingDir', async () => { await create({ name: 'viasessions' }); const wtPath = join(repo, '.claude', 'worktrees', 'viasessions'); diff --git a/test/worktree-modal-i18n.test.ts b/test/worktree-modal-i18n.test.ts index 935b9031..18022681 100644 --- a/test/worktree-modal-i18n.test.ts +++ b/test/worktree-modal-i18n.test.ts @@ -118,6 +118,7 @@ describe('the git worktree UI in zh-CN', () => { 'Start a session in this worktree', 'Remove…', 'git worktree remove refuses while the worktree has uncommitted or untracked files. Codeman never forces it.', + 'Its folder is gone. Removing it clears the record git keeps of it, so the name can be used again.', 'Also delete its branch (git branch -d keeps a branch that is not merged)', 'Remove worktree', 'Cancel', diff --git a/test/worktree-ui.test.ts b/test/worktree-ui.test.ts index 4dc9840b..4702af30 100644 --- a/test/worktree-ui.test.ts +++ b/test/worktree-ui.test.ts @@ -13,6 +13,7 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import vm from 'node:vm'; +import { JSDOM } from 'jsdom'; import { describe, expect, it, vi } from 'vitest'; const SESSION_UI = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8'); @@ -149,6 +150,35 @@ describe('launching in a worktree', () => { expect(app._runMode).toBe('claude'); }); + it('a worktree whose folder is gone offers Remove but not Open, with a note saying what Remove does', () => { + const dom = new JSDOM(''); + const CodemanApp = function CodemanApp(this: any) {}; + const context = vm.createContext({ + CodemanApp, + localStorage: { getItem: () => null, setItem: () => {} }, + document: dom.window.document, + console, + setTimeout, + }); + vm.runInContext(SESSION_UI, context, { filename: 'session-ui.js' }); + const app = new (CodemanApp as any)(); + app._shortenHomePath = (p: string) => p; + const row = (wt: Record) => + app._buildWorktreeRow({ branch: 'worktree-x', path: '/r/.claude/worktrees/x', name: 'x', ...wt }); + const buttons = (el: HTMLElement) => [...el.querySelectorAll('button')].map((b) => b.textContent); + + expect(buttons(row({ managed: true, prunable: false }))).toEqual(['Open', 'Remove…']); + expect(buttons(row({ managed: true, prunable: true }))).toEqual(['Remove…']); + expect(buttons(row({ managed: false, prunable: true }))).toEqual([]); + + app._worktreeRemoveConfirm = 'x'; + const missingNote = row({ managed: true, prunable: true }).querySelector('.worktree-remove-note')!.textContent; + expect(missingNote).toContain('folder is gone'); + const liveNote = row({ managed: true, prunable: false }).querySelector('.worktree-remove-note')!.textContent; + expect(liveNote).toContain('never forces'); + dom.window.close(); + }); + it('closing offers worktree removal only for a session in a managed worktree', () => { const { app } = load(async () => json({})); expect(app._managedWorktreeOfSession({ workingDir: WT.path })).toEqual(WT);