fix: Worktree get() matches wrong agent due to ambiguous endsWith lookup
The `get(id)` method on SimpleGitWorktreeManager used `path.endsWith(id)` to find worktrees. Since all agents working on the same project create worktrees with the same project name suffix (e.g., "codewalk-district"), cleanup for one agent could match and delete another agent's worktree. Fix: match on `basename(worktreesDir)/id` so each manager's lookups are scoped to its own worktree base directory.
This commit is contained in:
@@ -453,6 +453,58 @@ describe('SimpleGitWorktreeManager', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ==========================================================================
|
||||||
|
// Cross-Agent Isolation
|
||||||
|
// ==========================================================================
|
||||||
|
|
||||||
|
describe('cross-agent isolation', () => {
|
||||||
|
it('get() only matches worktrees in its own worktreesDir', async () => {
|
||||||
|
// Simulate two agents with separate worktree base dirs but same repo
|
||||||
|
const agentADir = path.join(repoPath, 'workdirs', 'agent-a');
|
||||||
|
const agentBDir = path.join(repoPath, 'workdirs', 'agent-b');
|
||||||
|
await mkdir(agentADir, { recursive: true });
|
||||||
|
await mkdir(agentBDir, { recursive: true });
|
||||||
|
|
||||||
|
const managerA = new SimpleGitWorktreeManager(repoPath, undefined, agentADir);
|
||||||
|
const managerB = new SimpleGitWorktreeManager(repoPath, undefined, agentBDir);
|
||||||
|
|
||||||
|
// Both create worktrees with the same id (project name)
|
||||||
|
await managerA.create('my-project', 'agent/agent-a');
|
||||||
|
await managerB.create('my-project', 'agent/agent-b');
|
||||||
|
|
||||||
|
// Each manager should only see its own worktree
|
||||||
|
const wtA = await managerA.get('my-project');
|
||||||
|
const wtB = await managerB.get('my-project');
|
||||||
|
|
||||||
|
expect(wtA).not.toBeNull();
|
||||||
|
expect(wtB).not.toBeNull();
|
||||||
|
expect(wtA!.path).toContain('agent-a');
|
||||||
|
expect(wtB!.path).toContain('agent-b');
|
||||||
|
expect(wtA!.path).not.toBe(wtB!.path);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('remove() only removes worktrees in its own worktreesDir', async () => {
|
||||||
|
const agentADir = path.join(repoPath, 'workdirs', 'agent-a');
|
||||||
|
const agentBDir = path.join(repoPath, 'workdirs', 'agent-b');
|
||||||
|
await mkdir(agentADir, { recursive: true });
|
||||||
|
await mkdir(agentBDir, { recursive: true });
|
||||||
|
|
||||||
|
const managerA = new SimpleGitWorktreeManager(repoPath, undefined, agentADir);
|
||||||
|
const managerB = new SimpleGitWorktreeManager(repoPath, undefined, agentBDir);
|
||||||
|
|
||||||
|
await managerA.create('my-project', 'agent/agent-a');
|
||||||
|
await managerB.create('my-project', 'agent/agent-b');
|
||||||
|
|
||||||
|
// Remove agent A's worktree
|
||||||
|
await managerA.remove('my-project');
|
||||||
|
|
||||||
|
// Agent B's worktree should still exist
|
||||||
|
const wtB = await managerB.get('my-project');
|
||||||
|
expect(wtB).not.toBeNull();
|
||||||
|
expect(wtB!.path).toContain('agent-b');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ==========================================================================
|
// ==========================================================================
|
||||||
// Edge Cases
|
// Edge Cases
|
||||||
// ==========================================================================
|
// ==========================================================================
|
||||||
|
|||||||
@@ -9,6 +9,7 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import path from 'node:path';
|
import path from 'node:path';
|
||||||
|
import { realpathSync } from 'node:fs';
|
||||||
import { simpleGit, type SimpleGit } from 'simple-git';
|
import { simpleGit, type SimpleGit } from 'simple-git';
|
||||||
import type { EventBus } from '../events/types.js';
|
import type { EventBus } from '../events/types.js';
|
||||||
import type {
|
import type {
|
||||||
@@ -159,8 +160,14 @@ export class SimpleGitWorktreeManager implements WorktreeManager {
|
|||||||
* Finds worktree by matching path ending with id.
|
* Finds worktree by matching path ending with id.
|
||||||
*/
|
*/
|
||||||
async get(id: string): Promise<Worktree | null> {
|
async get(id: string): Promise<Worktree | null> {
|
||||||
|
const expectedSuffix = path.join(path.basename(this.worktreesDir), id);
|
||||||
const worktrees = await this.list();
|
const worktrees = await this.list();
|
||||||
return worktrees.find((wt) => wt.path.endsWith(id)) ?? null;
|
// Match on the worktreesDir + id suffix to avoid cross-agent collisions.
|
||||||
|
// Multiple agents may have worktrees ending with the same project name
|
||||||
|
// (e.g., ".../agent-A/codewalk-district" vs ".../agent-B/codewalk-district").
|
||||||
|
// We match on basename(worktreesDir)/id to handle symlink differences
|
||||||
|
// (e.g., macOS /var → /private/var) while still being unambiguous.
|
||||||
|
return worktrees.find((wt) => wt.path.endsWith(expectedSuffix)) ?? null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user