mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
fix(worktrees): a session spelled through the case's symlink blocks removal
The removal guard and the per-worktree session count compared each session's workingDir lexically against the worktree paths git lists, which are real paths. POST /api/sessions stores workingDir as it was spelled, so a session started in a worktree of a linked case whose folder is a symlink (a repository on another disk, macOS /tmp, a symlinked cases folder), named through that link, counted for no worktree, and DELETE /api/cases/:name/worktrees/:worktree removed the folder under the running agent (reproduced: 200 instead of 409). Each session folder under the case path is now respelled onto the repository's real top level before the comparison (respellUnder, pure and lexical, so no filesystem call per session). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
3952963bad
commit
0ce6e09725
4 files changed
+85
-9
No files matched your search
@@ -173,6 +173,21 @@ export function isPathWithin(child: string, root: string): boolean {
|
||||
return c === r || c.startsWith(r === sep ? r : r + sep);
|
||||
}
|
||||
|
||||
/**
|
||||
* `path` as the repository spells it: a path under `alias` (the case folder as its case resolves
|
||||
* it, which can run through a symlink) moved under `root` (the repository's real top level), else
|
||||
* `path` unchanged. Lexical, so it costs no filesystem call per session. Sessions keep the
|
||||
* `workingDir` they were created with, while the worktree paths git lists are real, so without
|
||||
* this a session spelled through the case's link counted for no worktree and did not block its
|
||||
* removal.
|
||||
*/
|
||||
export function respellUnder(path: string, alias: string, root: string): string {
|
||||
const p = resolve(path);
|
||||
const a = resolve(alias);
|
||||
if (a === resolve(root) || !isPathWithin(p, a)) return p;
|
||||
return join(root, p.slice(a.length));
|
||||
}
|
||||
|
||||
// ─── Pure: `git worktree list --porcelain` ───────────────────────────────────
|
||||
|
||||
export interface ParsedWorktree {
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
managedWorktreesDir,
|
||||
removeCaseWorktree,
|
||||
resolveCaseRepository,
|
||||
respellUnder,
|
||||
worktreeNameError,
|
||||
type CaseRepository,
|
||||
type CaseWorktree,
|
||||
@@ -151,12 +152,22 @@ async function resolveLocalCaseFolder(
|
||||
return { ok: true, casePath };
|
||||
}
|
||||
|
||||
/** Live local sessions, with the folder each runs in. Remote sessions have no local folder. */
|
||||
function liveLocalSessions(ctx: SessionPort): Array<{ id: string; name: string; owner?: string; dir: string }> {
|
||||
/**
|
||||
* Live local sessions, with the folder each runs in as the case's repository spells it
|
||||
* (`respellUnder`: a folder named through the case's own path, which can run through a symlink,
|
||||
* is moved onto the repository's real top level, where git lists the worktrees). Remote sessions
|
||||
* have no local folder.
|
||||
*/
|
||||
function liveLocalSessions(
|
||||
ctx: SessionPort,
|
||||
casePath: string,
|
||||
repoRoot: string
|
||||
): Array<{ id: string; name: string; owner?: string; dir: string }> {
|
||||
const out: Array<{ id: string; name: string; owner?: string; dir: string }> = [];
|
||||
for (const session of ctx.sessions.values()) {
|
||||
if (session.remote || !session.workingDir || !session.workingDir.startsWith('/')) continue;
|
||||
out.push({ id: session.id, name: session.name, owner: session.owner, dir: session.workingDir });
|
||||
const dir = respellUnder(session.workingDir, casePath, repoRoot);
|
||||
out.push({ id: session.id, name: session.name, owner: session.owner, dir });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
@@ -168,12 +179,15 @@ export function registerWorktreeRoutes(
|
||||
): void {
|
||||
const dockerWorkspaces = deps.dockerWorkspaces ?? defaultDockerWorkspaces;
|
||||
|
||||
/** Case folder → its repository, or the refusal already rendered. */
|
||||
/**
|
||||
* Case folder → its repository, or the refusal already rendered. `casePath` is the folder as
|
||||
* the case resolves it (possibly through a symlink); `repo.repoRoot` is its real path.
|
||||
*/
|
||||
async function caseRepository(
|
||||
req: FastifyRequest,
|
||||
reply: FastifyReply,
|
||||
name: string
|
||||
): Promise<{ ok: true; repo: CaseRepository } | { ok: false; fail: Fail }> {
|
||||
): Promise<{ ok: true; repo: CaseRepository; casePath: 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, {
|
||||
@@ -181,14 +195,14 @@ export function registerWorktreeRoutes(
|
||||
dockerWorkspaces: await dockerWorkspaces(),
|
||||
});
|
||||
if (!repo.ok) return { ok: false, fail: failFrom(reply, repo) };
|
||||
return { ok: true, repo: repo.repo };
|
||||
return { ok: true, repo: repo.repo, casePath: folder.casePath };
|
||||
}
|
||||
|
||||
app.get('/api/cases/:name/worktrees', async (req, reply): Promise<ApiResponse<CaseWorktreeList>> => {
|
||||
const { name } = req.params as { name: string };
|
||||
const resolved = await caseRepository(req, reply, name);
|
||||
if (!resolved.ok) return resolved.fail;
|
||||
const { repo } = resolved;
|
||||
const { repo, casePath } = resolved;
|
||||
const listed = await listCaseWorktrees(repo, { git: deps.git });
|
||||
if (!listed.ok) return failFrom(reply, listed);
|
||||
|
||||
@@ -196,7 +210,7 @@ export function registerWorktreeRoutes(
|
||||
// managed worktree is not also counted against the main checkout that contains it.
|
||||
const user = getAuthUser(req);
|
||||
const rows: CaseWorktreeInfo[] = listed.worktrees.map((wt) => ({ ...wt, sessionCount: 0, sessions: [] }));
|
||||
for (const session of liveLocalSessions(ctx)) {
|
||||
for (const session of liveLocalSessions(ctx, casePath, repo.repoRoot)) {
|
||||
let best: CaseWorktreeInfo | undefined;
|
||||
for (const row of rows) {
|
||||
if (!isPathWithin(session.dir, row.path)) continue;
|
||||
@@ -247,7 +261,8 @@ export function registerWorktreeRoutes(
|
||||
deleteBranch: query.deleteBranch === 'true' || query.deleteBranch === '1',
|
||||
// Any owner's session blocks: removing the folder under a running agent is the damage
|
||||
// this refusal exists to prevent, whoever started it.
|
||||
isInUse: (path) => liveLocalSessions(ctx).some((s) => isPathWithin(s.dir, path)),
|
||||
isInUse: (path) =>
|
||||
liveLocalSessions(ctx, resolved.casePath, resolved.repo.repoRoot).some((s) => isPathWithin(s.dir, path)),
|
||||
},
|
||||
{ git: deps.git }
|
||||
);
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
removeCaseWorktree,
|
||||
resolveCaseRepository,
|
||||
resolveManagedWorktree,
|
||||
respellUnder,
|
||||
unquoteGitPath,
|
||||
worktreeBaseError,
|
||||
worktreeBranchError,
|
||||
@@ -103,6 +104,15 @@ describe('paths', () => {
|
||||
expect(isPathWithin('/r/.claude/worktrees/a/src', '/r/.claude/worktrees/a')).toBe(true);
|
||||
expect(isPathWithin('/r/.claude/worktrees/ab', '/r/.claude/worktrees/a')).toBe(false);
|
||||
});
|
||||
it('respellUnder moves a path named through the case link onto the real repository root', () => {
|
||||
expect(respellUnder('/link/.claude/worktrees/a', '/link', '/real')).toBe('/real/.claude/worktrees/a');
|
||||
expect(respellUnder('/link/.claude/worktrees/a/src/', '/link', '/real')).toBe('/real/.claude/worktrees/a/src');
|
||||
expect(respellUnder('/link', '/link', '/real')).toBe('/real');
|
||||
// Not under the alias (a segment match, never a string prefix), or no alias at all: unchanged.
|
||||
expect(respellUnder('/linked/x', '/link', '/real')).toBe('/linked/x');
|
||||
expect(respellUnder('/real/.claude/worktrees/a', '/link', '/real')).toBe('/real/.claude/worktrees/a');
|
||||
expect(respellUnder('/real/x', '/real', '/real')).toBe('/real/x');
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseWorktreeList', () => {
|
||||
|
||||
@@ -311,6 +311,42 @@ describe('worktree routes: refusals', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('worktree routes: a case reached through a symlink', () => {
|
||||
// A linked case's folder can be a symlink (a repository on another disk, macOS's /tmp), and
|
||||
// POST /api/sessions stores `workingDir` as it was spelled. The worktree list and the removal
|
||||
// guard compare paths lexically, so a session spelled through the link has to be mapped onto
|
||||
// the repository's real path first, or it is invisible to both.
|
||||
it('counts and protects a session whose folder names the worktree through the case link', async () => {
|
||||
const link = join(CASES_DIR, 'repo-link');
|
||||
symlinkSync(repo, link);
|
||||
await mkdir(getDataDir(), { recursive: true });
|
||||
await writeFile(join(getDataDir(), 'linked-cases.json'), JSON.stringify({ linkedviasym: link }));
|
||||
const made = await create({ name: 'sym' }, 'linkedviasym');
|
||||
expect(made.statusCode).toBe(201);
|
||||
const real = join(repo, '.claude', 'worktrees', 'sym');
|
||||
expect(made.json().data.worktree.path).toBe(real);
|
||||
|
||||
// The same worktree, spelled through the link.
|
||||
const spelled = join(link, '.claude', 'worktrees', 'sym');
|
||||
const res = await h.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/sessions',
|
||||
payload: { workingDir: spelled, mode: 'shell', name: 's1-sym' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data.session.workingDir).toBe(spelled);
|
||||
|
||||
const busy = await remove('sym', '', 'linkedviasym');
|
||||
expect(busy.statusCode).toBe(409);
|
||||
expect(busy.json().error).toContain('live session');
|
||||
expect(existsSync(real)).toBe(true);
|
||||
|
||||
const row = (await list('linkedviasym')).json().data.worktrees.find((w: { name?: string }) => w.name === 'sym');
|
||||
expect(row).toMatchObject({ managed: true, sessionCount: 1 });
|
||||
expect(row.sessions).toEqual([{ id: res.json().data.session.id, name: 's1-sym' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('worktree routes: multi-user ownership', () => {
|
||||
it('resolves a non-admin’s own case space only, never a linked case or another user’s case', async () => {
|
||||
process.env.CODEMAN_MULTIUSER = '1';
|
||||
|
||||
Reference in new issue
Block a user