diff --git a/docs/api-reference.md b/docs/api-reference.md index 81c920d1..e572ee08 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1040,7 +1040,7 @@ A local case whose folder is the top of a git repository can keep **worktrees** - `name`: 1 to 64 characters of `A-Z a-z 0-9 . _ -`, starting with a letter or digit, no `..`, not ending in `.` or `.lock`. `400` otherwise. - `branch`: the NEW branch, default `worktree-`. Letters, digits, `.`, `_`, `-`, `/`, then `git check-ref-format --branch` must accept it unchanged. An existing branch is `409 ALREADY_EXISTS` (never reset), and so is a name whose folder already exists. - `base`: where the branch starts, default `HEAD` (the case's current commit): a branch, tag or commit, resolved before anything is written (`400` when it is not a commit, which includes an empty repository). -- `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. +- `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. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index b56cdde9..c735daa3 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -455,7 +455,7 @@ Further detail: the synchronous clone request is bounded by `GIT_CLONE_TIMEOUT_M - ⚠️ **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. +- ⚠️ **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. - **Sessions start through the EXISTING create paths**, never a new spawn path: `POST /api/quick-start` with `worktree` (resolved with the same guards; hooks, the agent skill, the model override and the session all target the worktree, and a case that does not exist is a 404, never a scaffold), or `POST /api/sessions` with the worktree as `workingDir` (what the Run button uses for claude and shell). The frontend launches through `run({ worktree })` with a temporary `_runMode` swap, tabs named `w-`. - The unified session list names a live session's worktree from its folder layout (`worktreeFromPath`), so a codex or shell session gets the worktree pill before (or without) a transcript record. diff --git a/docs/wiki/Git-Worktrees.md b/docs/wiki/Git-Worktrees.md index 7d028ad5..aeeecc74 100644 --- a/docs/wiki/Git-Worktrees.md +++ b/docs/wiki/Git-Worktrees.md @@ -16,9 +16,11 @@ Claude Code recognizes, and the other way round. Codeman's Recent Sessions and S lists mark a session that runs in one with a `⑂ ` pill. The main checkout does not show the worktrees as untracked: Codeman adds `/.claude/worktrees/` -(and `/.claude/settings.local.json`, the hooks file it writes for every Claude session) to the -repository's local `.git/info/exclude` when git does not ignore them already. Your tracked -`.gitignore` is never touched. +(and `/.claude/settings.local.json`, the hooks file it writes for every Claude session, plus the +`codeman` folder it writes when **Codeman Agent Skill** is on) to the repository's local +`.git/info/exclude` when git does not ignore them already. Without that, those files would make +every worktree look modified and git would refuse to remove it. Your tracked `.gitignore` is +never touched. ## Creating one and starting a session in it diff --git a/src/git-worktrees.ts b/src/git-worktrees.ts index 9696e733..d839429f 100644 --- a/src/git-worktrees.ts +++ b/src/git-worktrees.ts @@ -18,7 +18,9 @@ * already ignore it. `/.claude/settings.local.json` gets the same treatment: Codeman installs its * hooks there for every claude session, and an untracked copy in a worktree makes * `git worktree remove` refuse forever (Claude Code itself ignores that file through the global - * excludes file, which is not guaranteed to be set up on a given machine). + * excludes file, which is not guaranteed to be set up on a given machine). The copy of Codeman's + * agent skill (`/codeman/`, injected while `agentSkillEnabled` is on) is excluded for + * the same reason, for every skills folder the registry names (`WorktreeOptions.agentSkillDirs`). * * Split like the other git modules: a PURE half (name and branch validation, `worktree list` * parsing, exclude text, the managed-path rule) unit-tested without git, and an IO half that @@ -88,6 +90,19 @@ export const WORKTREES_EXCLUDE_LINE = '/.claude/worktrees/'; /** The `info/exclude` line for the hooks file Codeman writes into every claude workspace. */ export const SETTINGS_LOCAL_EXCLUDE_LINE = '/.claude/settings.local.json'; +/** A workspace-relative skills folder (`.claude/skills`, `.agents/skills`): plain segments only. */ +const SKILLS_DIR_RE = /^[A-Za-z0-9._-]+(?:\/[A-Za-z0-9._-]+)*$/; + +/** + * The `info/exclude` line for the copy of Codeman's agent skill in `skillsDir` of a workspace + * (`/codeman/`, written into the folder a session runs in while `agentSkillEnabled` + * is on), or null for a folder that is not a plain relative path. + */ +export function agentSkillExcludeLine(skillsDir: string): string | null { + if (!SKILLS_DIR_RE.test(skillsDir) || skillsDir.split('/').some((s) => s === '.' || s === '..')) return null; + return `/${skillsDir}/codeman/`; +} + // ─── Pure: names ───────────────────────────────────────────────────────────── const WORKTREE_NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; @@ -301,7 +316,7 @@ export function excludeAppendText(existing: string, lines: readonly string[]): { if (added.length === 0) return { text: '', added }; const lead = existing.length > 0 && !existing.endsWith('\n') ? '\n' : ''; return { - text: `${lead}# Added by Codeman: git worktrees and the per-workspace hooks file stay local\n${added.join('\n')}\n`, + text: `${lead}# Added by Codeman: git worktrees and the files Codeman writes into each workspace stay local\n${added.join('\n')}\n`, added, }; } @@ -450,6 +465,13 @@ export interface WorktreeOptions { dockerWorkspaces?: string[]; /** Overrides `os.homedir()` for the unrelated-ancestor rule (tests). */ home?: string; + /** + * Every workspace skills folder a session can get Codeman's agent skill in (the registry's + * `capabilities.agentSkillDir` values, passed in as data by the caller). A create excludes + * `/codeman/` for each, like the hooks file: a session in a worktree gets an untracked + * copy there, and `git worktree remove` would refuse that worktree forever. + */ + agentSkillDirs?: readonly string[]; } const realOr = async (p: string): Promise => { @@ -797,11 +819,17 @@ export async function createCaseWorktree( return fail('OPERATION_FAILED', `Could not inspect ${target}: ${(err as Error).message}`); } + const skillProbes: Array<{ line: string; probe: string }> = []; + for (const dir of new Set(opts.agentSkillDirs ?? [])) { + const line = agentSkillExcludeLine(dir); + if (line) skillProbes.push({ line, probe: `${dir}/codeman` }); + } const excluded = await ensureExcluded( repo, [ { line: WORKTREES_EXCLUDE_LINE, probe: `${WORKTREES_DIR_SEGMENTS.join('/')}/${req.name}` }, { line: SETTINGS_LOCAL_EXCLUDE_LINE, probe: '.claude/settings.local.json' }, + ...skillProbes, ], git ); diff --git a/src/web/routes/worktree-routes.ts b/src/web/routes/worktree-routes.ts index b27e1150..7c0f5d02 100644 --- a/src/web/routes/worktree-routes.ts +++ b/src/web/routes/worktree-routes.ts @@ -29,6 +29,8 @@ import { readDockerCases } from '../../docker-hosts.js'; import { readRemoteCases } from '../../remote-hosts.js'; import { dataPath, getDataDir } from '../../config/instance.js'; import { isMultiUserMode } from '../../config/multiuser.js'; +import { listClis } from '../../config/cli-registry/registry.js'; +import { agentSkillDir } from '../../hooks-config.js'; import { describeUnknownPath, probePathKind } from '../../utils/index.js'; import { createCaseWorktree, @@ -65,6 +67,19 @@ const SAFE_CASE_NAME = /^[a-zA-Z0-9_-]+$/; const defaultDockerWorkspaces = async (): Promise => (await readDockerCases(getDataDir()).catch(() => [])).map((c) => c.hostWorkspacePath).filter(Boolean); +/** + * Every workspace skills folder a session of some registry CLI gets Codeman's agent skill in + * (`capabilities.agentSkillDir`), read from the registry at call time as data, never by CLI id. + */ +const registryAgentSkillDirs = (): string[] => { + const dirs = new Set(); + for (const cli of listClis()) { + const dir = agentSkillDir(cli.capabilities); + if (dir) dirs.add(dir); + } + return [...dirs]; +}; + /** A live session in a worktree, as the list reports it to a caller who may see it. */ export interface WorktreeSessionRef { id: string; @@ -239,7 +254,10 @@ export function registerWorktreeRoutes( if (nameError) return failWith(reply, ApiErrorCode.INVALID_INPUT, nameError); const resolved = await caseRepository(req, reply, name); if (!resolved.ok) return resolved.fail; - const created = await createCaseWorktree(resolved.repo, body, { git: deps.git }); + const created = await createCaseWorktree(resolved.repo, body, { + git: deps.git, + agentSkillDirs: registryAgentSkillDirs(), + }); if (!created.ok) return failFrom(reply, created); reply.code(201); return { success: true, data: { worktree: created.worktree } }; diff --git a/test/git-worktrees.test.ts b/test/git-worktrees.test.ts index 324a03ae..8872c185 100644 --- a/test/git-worktrees.test.ts +++ b/test/git-worktrees.test.ts @@ -10,6 +10,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { + agentSkillExcludeLine, createCaseWorktree, defaultWorktreeBranch, excludeAppendText, @@ -167,6 +168,13 @@ describe('excludeAppendText', () => { it('is a no-op when every line is present', () => { expect(excludeAppendText('/.claude/worktrees/\n', ['/.claude/worktrees/'])).toEqual({ text: '', added: [] }); }); + it('agentSkillExcludeLine names the skill copy of a plain relative skills folder only', () => { + expect(agentSkillExcludeLine('.claude/skills')).toBe('/.claude/skills/codeman/'); + expect(agentSkillExcludeLine('.agents/skills')).toBe('/.agents/skills/codeman/'); + for (const bad of ['', '/abs', 'a/../b', 'a//b', '.', 'x/', 'a b', '*', '!neg']) { + expect(agentSkillExcludeLine(bad), bad).toBeNull(); + } + }); }); describe('gitRefusalText', () => { diff --git a/test/routes/worktree-routes.test.ts b/test/routes/worktree-routes.test.ts index ccc1181a..4c2331f1 100644 --- a/test/routes/worktree-routes.test.ts +++ b/test/routes/worktree-routes.test.ts @@ -22,6 +22,7 @@ import { CASES_DIR } from '../../src/web/route-helpers.js'; import { getDataDir } from '../../src/config/instance.js'; import { userCasesDir } from '../../src/config/multiuser.js'; import { Session } from '../../src/session.js'; +import { applyAgentSkill } from '../../src/hooks-config.js'; import type { WorktreeGitRunner } from '../../src/git-worktrees.js'; const ENV = { @@ -182,6 +183,31 @@ describe('worktree routes: happy path', () => { expect(readFileSync(join(wtPath, 'wip.txt'), 'utf-8')).toBe('work'); }); + it('removes a worktree its sessions got the agent skill in (agentSkillEnabled on)', async () => { + // The agent skill is injected into the folder a session runs in, so a worktree session gets + // an untracked `/codeman/`, which used to make `git worktree remove` refuse forever. + (h.ctx.getAgentSkillEnabled as unknown as { mockResolvedValue(v: boolean): void }).mockResolvedValue(true); + expect((await create({ name: 'skilled' })).statusCode).toBe(201); + const wtPath = join(repo, '.claude', 'worktrees', 'skilled'); + const qs = await h.app.inject({ + method: 'POST', + url: '/api/quick-start', + payload: { caseName: 'wtcase', mode: 'claude', worktree: 'skilled' }, + }); + expect(qs.statusCode).toBe(200); + h.ctx.sessions.delete(qs.json().data.sessionId); + expect(existsSync(join(wtPath, '.claude', 'skills', 'codeman', 'SKILL.md'))).toBe(true); + // The other skills directory (codex, gemini and the rest scan `.agents/skills`), written by + // the same function quick-start calls; those CLIs are not installed here. + await applyAgentSkill(wtPath, true, '.agents/skills'); + expect(existsSync(join(wtPath, '.agents', 'skills', 'codeman', 'SKILL.md'))).toBe(true); + expect(git(wtPath, 'status', '--porcelain')).toBe(''); + expect(git(repo, 'status', '--porcelain')).toBe(''); + const res = await remove('skilled'); + expect(res.statusCode).toBe(200); + expect(existsSync(wtPath)).toBe(false); + }); + 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');