diff --git a/docs/api-reference.md b/docs/api-reference.md index e572ee08..baf80194 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, `/.claude/settings.local.json`, the hooks file Codeman writes for every claude session, which would otherwise make `git worktree remove` refuse, and the agent skill copy (`/.claude/skills/codeman/`, `/.agents/skills/codeman/`: every skills folder the CLI registry names) that sessions get while `agentSkillEnabled` is on, for the same reason. - 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`. 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. +`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 worktree that is, sits inside, or holds a Docker case workspace is `400` too (git would run inside a folder the container writes, and the removal would delete that workspace under it). 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 c735daa3..043ea771 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -453,7 +453,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`). 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. +- ⚠️ **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. A managed worktree that is, sits inside, or holds a Docker case workspace (a Docker case linked to the worktree folder) is never removed (400): `git worktree remove` runs a status inside the tree and then deletes it, ignored files included, under the container. - ⚠️ **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). The agent skill copy (`/codeman/`, injected while `agentSkillEnabled` is on) gets a line too, for every skills folder the registry names (`capabilities.agentSkillDir`, passed in by the route as data). A symlinked `info` dir or `exclude` file is reported, never written through. - **Names are gated before git**: charset and length (Zod and `worktreeNameError()`), then `git check-ref-format --branch`, whose output must EQUAL the input (`--branch` expands `@{-1}`, which the charset gate already refuses). The base is a resolved commit (`rev-parse --verify ^{commit}`), passed after `--` with `--no-track`; an existing branch is refused (`-b`, never `-B`, which would reset it). Create and remove are serialized per repository. diff --git a/docs/wiki/Git-Worktrees.md b/docs/wiki/Git-Worktrees.md index aeeecc74..b0bda451 100644 --- a/docs/wiki/Git-Worktrees.md +++ b/docs/wiki/Git-Worktrees.md @@ -62,7 +62,9 @@ If a worktree's folder was deleted by hand, its row says **missing** and offers - **Docker cases and remote (SSH) cases.** A container can write to its workspace, and creating a worktree runs the repository's hooks and checkout filters on the Codeman machine, so git is - never run there. A remote case's repository is on another machine. + never run there. A remote case's repository is on another machine. For the same reason a + worktree that a Docker case uses as its workspace (or that holds one) is never removed from + here: remove the Docker case first. - A case folder that is not the top of a repository (a subfolder of one), a bare repository, or an empty repository with no commit yet. - Worktrees outside `.claude/worktrees/` (made by hand with `git worktree add`) are listed, but diff --git a/src/git-worktrees.ts b/src/git-worktrees.ts index d839429f..ba707f8d 100644 --- a/src/git-worktrees.ts +++ b/src/git-worktrees.ts @@ -46,7 +46,9 @@ * - git never runs for a case at or inside a Docker case workspace (or whose common git dir is * there): a container could plant a checkout filter or a hook, and `git worktree add` runs the * repository's post-checkout hook and filters on the HOST. On a local case that is acceptable, - * since the user's agent runs code there anyway. + * since the user's agent runs code there anyway. A managed worktree that is, sits inside, or + * holds a Docker case workspace is never removed either: `git worktree remove` runs a status + * inside the tree, then deletes it under the container. * - A repository that merely encloses the case and is the home folder or above (a dotfiles repo * in `$HOME`) is not the case's repository, and a case folder below a repository's top level * is refused: the worktrees would not be where the case is. @@ -485,6 +487,9 @@ const realOr = async (p: string): Promise => { const DOCKER_REFUSAL = 'Worktrees are not available for a folder inside a Docker case workspace: a container can write there, and git would run its hooks and filters on this machine.'; +const DOCKER_WORKTREE_REFUSAL = + 'This worktree is, or holds, a Docker case workspace: git would run inside a folder a container can write, and removing it would delete that workspace. Remove the Docker case first.'; + /** * The repository a case folder IS, or why it is not one Codeman makes worktrees for. Never runs * git when the folder (or the repository's common git dir) is at or inside a Docker case workspace. @@ -889,6 +894,17 @@ export async function removeCaseWorktree( // 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; + // The case passed the Docker rule, but the worktree folder itself can be a Docker case's + // workspace (a Docker case linked to it), or hold one. `git worktree remove` runs a status + // INSIDE the tree first (container-written files, checkout filters on this machine), then + // deletes the whole tree, ignored files included, under a container that still mounts it. + // A folder that is gone has neither problem: git only drops its record. + if (!found.missing) { + const dockerRoots = await Promise.all((opts.dockerWorkspaces ?? []).filter(Boolean).map(realOr)); + if (dockerRoots.some((root) => isPathWithin(found.path, root) || isPathWithin(root, found.path))) { + return fail('INVALID_INPUT', DOCKER_WORKTREE_REFUSAL); + } + } if (await req.isInUse(found.path)) { return fail( 'CONFLICT', diff --git a/src/web/routes/worktree-routes.ts b/src/web/routes/worktree-routes.ts index 7c0f5d02..884f9ceb 100644 --- a/src/web/routes/worktree-routes.ts +++ b/src/web/routes/worktree-routes.ts @@ -202,15 +202,13 @@ export function registerWorktreeRoutes( req: FastifyRequest, reply: FastifyReply, name: string - ): Promise<{ ok: true; repo: CaseRepository; casePath: string } | { ok: false; fail: Fail }> { + ): Promise<{ ok: true; repo: CaseRepository; casePath: string; dockerRoots: string[] } | { ok: false; fail: Fail }> { const folder = await resolveLocalCaseFolder(req, name); if (!folder.ok) return { ok: false, fail: failWith(reply, folder.code, folder.message) }; - const repo = await resolveCaseRepository(folder.casePath, { - git: deps.git, - dockerWorkspaces: await dockerWorkspaces(), - }); + const dockerRoots = await dockerWorkspaces(); + const repo = await resolveCaseRepository(folder.casePath, { git: deps.git, dockerWorkspaces: dockerRoots }); if (!repo.ok) return { ok: false, fail: failFrom(reply, repo) }; - return { ok: true, repo: repo.repo, casePath: folder.casePath }; + return { ok: true, repo: repo.repo, casePath: folder.casePath, dockerRoots }; } app.get('/api/cases/:name/worktrees', async (req, reply): Promise> => { @@ -282,7 +280,8 @@ export function registerWorktreeRoutes( isInUse: (path) => liveLocalSessions(ctx, resolved.casePath, resolved.repo.repoRoot).some((s) => isPathWithin(s.dir, path)), }, - { git: deps.git } + // The worktree folder itself must not be (or hold) a Docker case workspace either. + { git: deps.git, dockerWorkspaces: resolved.dockerRoots } ); if (!removed.ok) return failFrom(reply, removed); return { success: true, data: { removed: removed.removed } }; diff --git a/test/git-worktrees.test.ts b/test/git-worktrees.test.ts index 8872c185..f7c5fd13 100644 --- a/test/git-worktrees.test.ts +++ b/test/git-worktrees.test.ts @@ -5,7 +5,16 @@ * injected runner proving the Docker refusal never spawns git at all. Port: N/A. */ import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; @@ -23,6 +32,7 @@ import { resolveCaseRepository, resolveManagedWorktree, respellUnder, + runWorktreeGit, unquoteGitPath, worktreeBaseError, worktreeBranchError, @@ -351,6 +361,43 @@ describe('git worktrees on a real repository', () => { expect(git(repoDir, 'worktree', 'list', '--porcelain')).not.toContain('w1'); }); + it('never removes a worktree that is, or holds, a Docker case workspace', async () => { + const made = await createCaseWorktree(repo, { name: 'boxed' }); + if (!made.ok) throw new Error(made.message); + const wtPath = made.worktree.path; + mkdirSync(join(wtPath, 'sandbox')); + // The worktree IS a Docker case's workspace, or one sits inside it: refused before git runs, + // and nothing is deleted. git still lists it. + for (const dockerWorkspaces of [[wtPath], [join(wtPath, 'sandbox')]]) { + let spawned = 0; + const counting: WorktreeGitRunner = async (cwd, args, o) => { + if (args.includes('remove')) spawned++; + return runWorktreeGit(cwd, args, o); + }; + const r = await removeCaseWorktree( + repo, + { name: 'boxed', isInUse: () => false }, + { git: counting, dockerWorkspaces } + ); + expect(r).toMatchObject({ ok: false, code: 'INVALID_INPUT' }); + if (!r.ok) expect(r.message).toContain('Docker case workspace'); + expect(spawned).toBe(0); + expect(existsSync(join(wtPath, 'a.txt'))).toBe(true); + } + expect(git(repoDir, 'worktree', 'list', '--porcelain')).toContain('boxed'); + // An unrelated Docker workspace does not get in the way. + rmSync(join(wtPath, 'sandbox'), { recursive: true }); + const elsewhere = join(dir, 'other-box'); + mkdirSync(elsewhere); + const ok = await removeCaseWorktree( + repo, + { name: 'boxed', isInUse: () => false }, + { dockerWorkspaces: [elsewhere] } + ); + expect(ok.ok).toBe(true); + expect(existsSync(wtPath)).toBe(false); + }); + it('keeps an unmerged branch: git branch -d refuses, never -D', async () => { await createCaseWorktree(repo, { name: 'w2' }); const wtPath = join(repoDir, '.claude', 'worktrees', 'w2'); diff --git a/test/routes/worktree-routes.test.ts b/test/routes/worktree-routes.test.ts index 4c2331f1..bb6fae5e 100644 --- a/test/routes/worktree-routes.test.ts +++ b/test/routes/worktree-routes.test.ts @@ -301,6 +301,24 @@ describe('worktree routes: refusals', () => { expect(qs.json().error).toContain('Docker'); }); + it('refuses to remove a worktree a Docker case uses as its workspace', async () => { + expect((await create({ name: 'boxed' })).statusCode).toBe(201); + const wtPath = join(repo, '.claude', 'worktrees', 'boxed'); + await mkdir(getDataDir(), { recursive: true }); + await writeFile( + join(getDataDir(), 'docker-cases.json'), + JSON.stringify([{ name: 'boxcase', type: 'docker', hostId: 'd1', hostWorkspacePath: wtPath }]) + ); + // The local case itself is fine (listing and creating still work)... + expect((await list()).statusCode).toBe(200); + // ...but removing the folder the container mounts is refused, and nothing is deleted. + const res = await remove('boxed'); + expect(res.statusCode).toBe(400); + expect(res.json().error).toContain('Docker case workspace'); + expect(existsSync(join(wtPath, 'README.md'))).toBe(true); + expect(git(repo, 'worktree', 'list')).toContain('boxed'); + }); + it('refuses a remote case with a 400', async () => { await mkdir(getDataDir(), { recursive: true }); await writeFile(