mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
fix(worktrees): a worktree whose folder was deleted can still be removed
When a managed worktree's folder is deleted by hand (a user in a file manager, an agent's rm -rf), git keeps its record and lists it as prunable. Codeman then had no way out: the dialog showed the row as "missing" with no action, DELETE answered "No worktree named ...", and the name could never be used again (the branch still existed, and with another branch git refused "a missing but already registered worktree"). Reproduced on git 2.43; git's own `git worktree remove` on the missing path drops the record with exit 0. Removal (only removal, never a session start) now accepts a worktree whose folder is gone when git lists it at exactly the managed path, as prunable or locked, and no component on the way is a symlink or a file. The live-session and lock refusals still apply. The dialog offers Remove... (not Open) on such a row, with a note saying what it does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
0ce6e09725
commit
64fea4ff84
10 files changed
+150
-18
No files matched your search
@@ -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: "<name>"` 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.
|
||||
|
||||
|
||||
@@ -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 `<repo>/.claude/worktrees/<name>` on a new branch (default `worktree-<name>`), 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-<name>`, 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 `<repo>/.claude/worktrees/<valid-name>` 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 `<repo>/.claude/worktrees/<valid-name>` 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 `<common git dir>/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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+30
-6
@@ -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(
|
||||
|
||||
@@ -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':
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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('<!doctype html><body></body>');
|
||||
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<string, unknown>) =>
|
||||
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);
|
||||
|
||||
Reference in new issue
Block a user