diff --git a/migrations/000_combined.sql b/migrations/000_combined.sql index 176963b40e..c432bc76bc 100644 --- a/migrations/000_combined.sql +++ b/migrations/000_combined.sql @@ -1,5 +1,5 @@ -- Remote Coding Agent - Combined Schema --- Version: Combined (final state after migrations 001-020) +-- Version: Combined (final state after migrations 001-022) -- Description: Complete database schema (idempotent - safe to run multiple times) -- -- 8 Tables: @@ -312,3 +312,7 @@ ALTER TABLE remote_agent_sessions -- From migration 021: allow_env_keys on codebases ALTER TABLE remote_agent_codebases ADD COLUMN IF NOT EXISTS allow_env_keys BOOLEAN NOT NULL DEFAULT FALSE; + +-- From migration 022: composite codebase identity (name + path) +CREATE UNIQUE INDEX IF NOT EXISTS idx_codebases_name_cwd + ON remote_agent_codebases (name, default_cwd); diff --git a/migrations/022_codebase_composite_identity.sql b/migrations/022_codebase_composite_identity.sql new file mode 100644 index 0000000000..98a0f725d3 --- /dev/null +++ b/migrations/022_codebase_composite_identity.sql @@ -0,0 +1,16 @@ +-- Make codebase identity composite: (name, default_cwd). +-- Multiple local clones of the same remote now get distinct codebase_id values, +-- preventing conversations, sessions, env vars, and isolation environments from +-- leaking across clones. +-- +-- Existing single-clone installs are unaffected — the unique index only +-- prevents future duplicate (name, path) pairs, and the application layer +-- handles name-only lookups for backward compatibility. +-- +-- Pre-check for duplicates (run before applying if unsure): +-- SELECT name, default_cwd, COUNT(*) FROM remote_agent_codebases +-- GROUP BY name, default_cwd HAVING COUNT(*) > 1; +-- If duplicates exist, merge or delete the extra rows before running this migration. + +CREATE UNIQUE INDEX IF NOT EXISTS idx_codebases_name_cwd + ON remote_agent_codebases (name, default_cwd); diff --git a/packages/core/src/db/adapters/sqlite.ts b/packages/core/src/db/adapters/sqlite.ts index 485706d040..e2fd0e25f3 100644 --- a/packages/core/src/db/adapters/sqlite.ts +++ b/packages/core/src/db/adapters/sqlite.ts @@ -153,6 +153,7 @@ export class SqliteAdapter implements IDatabase { private initSchema(): void { this.createSchema(); this.migrateColumns(); + this.migrateCodebaseIdentity(); } /** @@ -217,6 +218,26 @@ export class SqliteAdapter implements IDatabase { } } + /** + * Add a unique index on (name, default_cwd) to codebases so that multiple + * local clones of the same remote get distinct codebase_id values. + * Uses CREATE UNIQUE INDEX IF NOT EXISTS — idempotent for databases that + * already have the index (new installs get it from createSchema). + */ + private migrateCodebaseIdentity(): void { + try { + this.db.run( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_codebases_name_cwd ON remote_agent_codebases (name, default_cwd)' + ); + } catch (e: unknown) { + const err = e as Error; + getLog().error({ err }, 'db.sqlite_migration_codebase_identity_failed'); + throw new Error( + 'Failed to enforce unique codebase identity. Resolve duplicate (name, default_cwd) rows in remote_agent_codebases before restarting.' + ); + } + } + /** * Create all tables. * @@ -375,6 +396,10 @@ export class SqliteAdapter implements IDatabase { ON remote_agent_sessions(parent_session_id); CREATE INDEX IF NOT EXISTS idx_sessions_conversation_started ON remote_agent_sessions(conversation_id, started_at DESC); + + -- From PG migration 022: composite codebase identity (name + path) + CREATE UNIQUE INDEX IF NOT EXISTS idx_codebases_name_cwd + ON remote_agent_codebases (name, default_cwd); `); getLog().info('db.sqlite_schema_initialized'); } diff --git a/packages/core/src/db/codebases.ts b/packages/core/src/db/codebases.ts index f3947fb6c1..572f64e4e2 100644 --- a/packages/core/src/db/codebases.ts +++ b/packages/core/src/db/codebases.ts @@ -115,6 +115,27 @@ export async function findCodebaseByPathPrefix(cwdPath: string): Promise { + const result = await pool.query( + 'SELECT * FROM remote_agent_codebases WHERE name = $1 AND default_cwd = $2 LIMIT 1', + [name, defaultCwd] + ); + return result.rows[0] || null; +} + +/** + * Find a codebase by name only. + * When multiple rows share the same name (different local paths), returns the + * most recently created one. Callers that have path context should prefer + * {@link findCodebaseByNameAndPath} for unambiguous lookup. + */ export async function findCodebaseByName(name: string): Promise { const result = await pool.query( 'SELECT * FROM remote_agent_codebases WHERE name = $1 ORDER BY created_at DESC LIMIT 1', diff --git a/packages/core/src/handlers/clone.test.ts b/packages/core/src/handlers/clone.test.ts index c913c1a78c..a1a7460525 100644 --- a/packages/core/src/handlers/clone.test.ts +++ b/packages/core/src/handlers/clone.test.ts @@ -29,6 +29,7 @@ const mockGetCodebaseCommands = mock(() => Promise.resolve({})); const mockUpdateCodebaseCommands = mock(() => Promise.resolve()); const mockFindCodebaseByRepoUrl = mock(() => Promise.resolve(null)); const mockFindCodebaseByDefaultCwd = mock(() => Promise.resolve(null)); +const mockFindCodebaseByNameAndPath = mock(() => Promise.resolve(null)); const mockFindCodebaseByName = mock(() => Promise.resolve(null)); const mockUpdateCodebase = mock(() => Promise.resolve()); @@ -38,6 +39,7 @@ mock.module('../db/codebases', () => ({ updateCodebaseCommands: mockUpdateCodebaseCommands, findCodebaseByRepoUrl: mockFindCodebaseByRepoUrl, findCodebaseByDefaultCwd: mockFindCodebaseByDefaultCwd, + findCodebaseByNameAndPath: mockFindCodebaseByNameAndPath, findCodebaseByName: mockFindCodebaseByName, updateCodebase: mockUpdateCodebase, })); @@ -100,6 +102,7 @@ function clearMocks(): void { mockUpdateCodebaseCommands.mockReset(); mockFindCodebaseByRepoUrl.mockReset(); mockFindCodebaseByDefaultCwd.mockReset(); + mockFindCodebaseByNameAndPath.mockReset(); mockFindCodebaseByName.mockReset(); mockUpdateCodebase.mockReset(); mockFindMarkdownFilesRecursive.mockReset(); @@ -113,6 +116,7 @@ function clearMocks(): void { mockUpdateCodebaseCommands.mockResolvedValue(undefined); mockFindCodebaseByRepoUrl.mockResolvedValue(null); mockFindCodebaseByDefaultCwd.mockResolvedValue(null); + mockFindCodebaseByNameAndPath.mockResolvedValue(null); mockFindCodebaseByName.mockResolvedValue(null); mockUpdateCodebase.mockResolvedValue(undefined); mockFindMarkdownFilesRecursive.mockResolvedValue([]); @@ -776,7 +780,7 @@ describe('normalizeRepoUrl (via cloneRepository)', () => { }); // ──────────────────────────────────────────────────────────────────────────── -describe('name-based deduplication', () => { +describe('composite identity deduplication', () => { beforeEach(() => { clearMocks(); restoreSpies(); @@ -784,15 +788,48 @@ describe('name-based deduplication', () => { delete process.env.GH_TOKEN; }); - test('should return existing codebase when registering same owner/repo via different path', async () => { - // Existing codebase registered via clone (managed path) + test('should create distinct codebase when same remote is registered from a different path', async () => { + // First clone exists at a managed path — but composite lookup (name + new path) returns null + spyExecFileAsync.mockImplementation((cmd: string, args: string[]) => { + if (args.includes('rev-parse')) return Promise.resolve({ stdout: '.git', stderr: '' }); + if (args.includes('get-url')) + return Promise.resolve({ stdout: 'https://github.com/owner/repo', stderr: '' }); + return Promise.resolve({ stdout: '', stderr: '' }); + }); + mockFindCodebaseByDefaultCwd.mockResolvedValueOnce(null); + // Composite lookup: no match for (owner/repo, /home/user/repo) + mockFindCodebaseByNameAndPath.mockResolvedValueOnce(null); + // Name-only lookup: finds existing, but path differs and is NOT managed + mockFindCodebaseByName.mockResolvedValueOnce( + makeCodebase({ + id: 'existing-id', + name: 'owner/repo', + default_cwd: '/home/user/other-checkout', + }) + ); + // New codebase created for the distinct clone + mockCreateCodebase.mockResolvedValueOnce( + makeCodebase({ + id: 'new-clone-id', + name: 'owner/repo', + default_cwd: '/home/user/repo', + }) as ReturnType + ); + + const result = await registerRepository('/home/user/repo'); + + expect(result.alreadyExisted).toBe(false); + expect(result.codebaseId).toBe('new-clone-id'); + expect(mockCreateCodebase.mock.calls.length).toBe(1); + }); + + test('should reuse existing codebase when re-registering the same path', async () => { const existingCodebase = makeCodebase({ id: 'existing-id', name: 'owner/repo', repository_url: 'https://github.com/owner/repo', - default_cwd: '/home/test/.archon/workspaces/owner/repo/source', + default_cwd: '/home/user/repo', }); - // registerRepository: rev-parse succeeds, path not in DB, remote URL returns owner/repo spyExecFileAsync.mockImplementation((cmd: string, args: string[]) => { if (args.includes('rev-parse')) return Promise.resolve({ stdout: '.git', stderr: '' }); if (args.includes('get-url')) @@ -800,22 +837,21 @@ describe('name-based deduplication', () => { return Promise.resolve({ stdout: '', stderr: '' }); }); mockFindCodebaseByDefaultCwd.mockResolvedValueOnce(null); - // Name-based lookup finds existing codebase - mockFindCodebaseByName.mockResolvedValueOnce(existingCodebase); + // Composite lookup: exact match for (owner/repo, /home/user/repo) + mockFindCodebaseByNameAndPath.mockResolvedValueOnce(existingCodebase); const result = await registerRepository('/home/user/repo'); expect(result.alreadyExisted).toBe(true); expect(result.codebaseId).toBe('existing-id'); - // createCodebase should NOT be called expect(mockCreateCodebase.mock.calls.length).toBe(0); }); - test('should update default_cwd to local path when local is registered after clone', async () => { - const existingCodebase = makeCodebase({ + test('should upgrade managed path to local path when local is registered after clone', async () => { + const managedCodebase = makeCodebase({ id: 'existing-id', name: 'owner/repo', - repository_url: 'https://github.com/owner/repo', + repository_url: null, default_cwd: '/home/test/.archon/workspaces/owner/repo/source', }); spyExecFileAsync.mockImplementation((cmd: string, args: string[]) => { @@ -824,45 +860,44 @@ describe('name-based deduplication', () => { return Promise.resolve({ stdout: 'https://github.com/owner/repo', stderr: '' }); return Promise.resolve({ stdout: '', stderr: '' }); }); + // access(): command folder at local path succeeds + spyFsAccess.mockImplementation((path: string) => { + const normalized = typeof path === 'string' ? path.replace(/\\/g, '/') : ''; + if (normalized.includes('.archon/commands')) { + return Promise.resolve(undefined); + } + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + }); + mockFindMarkdownFilesRecursive.mockResolvedValue([ + { commandName: 'deploy', relativePath: 'deploy.md' }, + ]); mockFindCodebaseByDefaultCwd.mockResolvedValueOnce(null); - mockFindCodebaseByName.mockResolvedValueOnce(existingCodebase); + // Composite lookup: no match (name matches but path differs) + mockFindCodebaseByNameAndPath.mockResolvedValueOnce(null); + // Name-only lookup finds the managed-path codebase + mockFindCodebaseByName.mockResolvedValueOnce(managedCodebase); const result = await registerRepository('/home/user/repo'); - // updateCodebase should be called with the local path + // Should upgrade the existing managed record to the local path + expect(result.alreadyExisted).toBe(true); + expect(result.codebaseId).toBe('existing-id'); + expect(result.defaultCwd).toBe('/home/user/repo'); + // Batched update: default_cwd + repository_url in one call expect(mockUpdateCodebase.mock.calls.length).toBe(1); - const updateArgs = mockUpdateCodebase.mock.calls[0] as [string, { default_cwd?: string }]; + const updateArgs = mockUpdateCodebase.mock.calls[0] as [ + string, + { default_cwd?: string; repository_url?: string | null }, + ]; expect(updateArgs[0]).toBe('existing-id'); expect(updateArgs[1].default_cwd).toBe('/home/user/repo'); - expect(result.defaultCwd).toBe('/home/user/repo'); - }); - - test('should not downgrade default_cwd from local to managed path', async () => { - // Existing codebase registered via local path - const existingCodebase = makeCodebase({ - id: 'existing-id', - name: 'owner/repo', - repository_url: 'https://github.com/owner/repo', - default_cwd: '/home/user/repo', - }); - // Clone same repo — name-based lookup finds existing - // .git does NOT exist (proceed to clone), but name dedup catches it - mockFindCodebaseByName.mockResolvedValueOnce(existingCodebase); - mockCreateCodebase.mockResolvedValueOnce(makeCodebase() as ReturnType); - - const result = await cloneRepository('https://github.com/owner/repo'); - - // default_cwd should stay as local path (managed path is NOT "better") - expect(result.defaultCwd).toBe('/home/user/repo'); - // updateCodebase should NOT be called with default_cwd (no downgrade) - if (mockUpdateCodebase.mock.calls.length > 0) { - const updateArgs = mockUpdateCodebase.mock.calls[0] as [string, { default_cwd?: string }]; - expect(updateArgs[1].default_cwd).toBeUndefined(); - } + expect(updateArgs[1].repository_url).toBe('https://github.com/owner/repo'); + // Commands loaded from new local path + expect(result.commandCount).toBe(1); + expect(mockUpdateCodebaseCommands.mock.calls.length).toBe(1); }); test('should fill in repository_url on existing codebase if missing', async () => { - // Existing codebase registered locally without remote URL const existingCodebase = makeCodebase({ id: 'existing-id', name: 'owner/repo', @@ -876,11 +911,11 @@ describe('name-based deduplication', () => { return Promise.resolve({ stdout: '', stderr: '' }); }); mockFindCodebaseByDefaultCwd.mockResolvedValueOnce(null); - mockFindCodebaseByName.mockResolvedValueOnce(existingCodebase); + // Composite lookup: exact match (same name AND same path) + mockFindCodebaseByNameAndPath.mockResolvedValueOnce(existingCodebase); await registerRepository('/home/user/repo'); - // updateCodebase should be called with repository_url expect(mockUpdateCodebase.mock.calls.length).toBe(1); const updateArgs = mockUpdateCodebase.mock.calls[0] as [ string, @@ -888,6 +923,33 @@ describe('name-based deduplication', () => { ]; expect(updateArgs[1].repository_url).toBe('https://github.com/owner/repo'); }); + + test('backward compat: existing single-clone installs found by path continue to work', async () => { + // User has ~/myproject registered as "owner/repo" — the directory name + // doesn't match the remote-derived name. findCodebaseByDefaultCwd catches it. + const existingCodebase = makeCodebase({ + id: 'legacy-id', + name: 'owner/repo', + default_cwd: '/home/user/myproject', + }); + spyExecFileAsync.mockImplementation((cmd: string, args: string[]) => { + if (args.includes('rev-parse')) return Promise.resolve({ stdout: '.git', stderr: '' }); + if (args.includes('get-url')) + return Promise.resolve({ stdout: 'https://github.com/owner/repo', stderr: '' }); + return Promise.resolve({ stdout: '', stderr: '' }); + }); + // Path-based lookup finds it immediately — registerRepoAtPath is never reached + mockFindCodebaseByDefaultCwd.mockResolvedValueOnce(existingCodebase); + + const result = await registerRepository('/home/user/myproject'); + + expect(result.alreadyExisted).toBe(true); + expect(result.codebaseId).toBe('legacy-id'); + // Neither composite nor name lookup should be called + expect(mockFindCodebaseByNameAndPath.mock.calls.length).toBe(0); + expect(mockFindCodebaseByName.mock.calls.length).toBe(0); + expect(mockCreateCodebase.mock.calls.length).toBe(0); + }); }); // ──────────────────────────────────────────────────────────────────────────── diff --git a/packages/core/src/handlers/clone.ts b/packages/core/src/handlers/clone.ts index 366a951b8a..928db25d4d 100644 --- a/packages/core/src/handlers/clone.ts +++ b/packages/core/src/handlers/clone.ts @@ -65,18 +65,16 @@ async function registerRepoAtPath( } } - // Check if a codebase with this name already exists (dedup by project identity) - const existing = await codebaseDb.findCodebaseByName(name); + // Dedup by composite identity (name + path). + // 1. Exact (name, path) match → re-registration of the same project + // 2. Same name, different path → distinct clone, create a new row + // 3. Backward compat: existing single-clone installs where the directory + // name doesn't match the remote-derived name are handled by + // registerRepository's findCodebaseByDefaultCwd guard (checked before + // reaching this function). + const existing = await codebaseDb.findCodebaseByNameAndPath(name, targetPath); if (existing) { - // Determine if the new path is "better" (local > archon-managed clone) - const isNewPathLocal = !targetPath.includes('/.archon/workspaces/'); - const isExistingPathManaged = existing.default_cwd.includes('/.archon/workspaces/'); - const shouldUpdateCwd = isNewPathLocal && isExistingPathManaged; - const updates: { default_cwd?: string; repository_url?: string | null } = {}; - if (shouldUpdateCwd) { - updates.default_cwd = targetPath; - } // Fill in repository_url if the existing record doesn't have one if (!existing.repository_url && repositoryUrl) { updates.repository_url = repositoryUrl; @@ -86,10 +84,9 @@ async function registerRepoAtPath( } // Still reload commands for the existing codebase - const effectiveCwd = shouldUpdateCwd ? targetPath : existing.default_cwd; let commandsLoaded = 0; for (const folder of getCommandFolderSearchPaths()) { - const commandPath = join(effectiveCwd, folder); + const commandPath = join(existing.default_cwd, folder); try { await access(commandPath); } catch { @@ -113,13 +110,65 @@ async function registerRepoAtPath( return { codebaseId: existing.id, name: existing.name, - repositoryUrl: existing.repository_url, - defaultCwd: shouldUpdateCwd ? targetPath : existing.default_cwd, + repositoryUrl: existing.repository_url ?? repositoryUrl, + defaultCwd: existing.default_cwd, commandCount: commandsLoaded, alreadyExisted: true, }; } + // Check if a name-only match exists with a managed path that should be + // upgraded to the local path (archon-managed clone → local checkout). + const nameMatch = await codebaseDb.findCodebaseByName(name); + if (nameMatch) { + const isNewPathLocal = !targetPath.includes('/.archon/workspaces/'); + const isExistingPathManaged = nameMatch.default_cwd.includes('/.archon/workspaces/'); + if (isNewPathLocal && isExistingPathManaged) { + // Upgrade managed clone to local path (single identity, new path) + const updates: { default_cwd: string; repository_url?: string | null } = { + default_cwd: targetPath, + }; + if (!nameMatch.repository_url && repositoryUrl) { + updates.repository_url = repositoryUrl; + } + await codebaseDb.updateCodebase(nameMatch.id, updates); + + // Reload commands from the new local path + let commandsLoaded = 0; + for (const folder of getCommandFolderSearchPaths()) { + const commandPath = join(targetPath, folder); + try { + await access(commandPath); + } catch { + continue; + } + const markdownFiles = await findMarkdownFilesRecursive(commandPath); + if (markdownFiles.length > 0) { + const commands = { ...(await codebaseDb.getCodebaseCommands(nameMatch.id)) }; + markdownFiles.forEach(({ commandName, relativePath }) => { + commands[commandName] = { + path: join(folder, relativePath), + description: `From ${folder}`, + }; + }); + await codebaseDb.updateCodebaseCommands(nameMatch.id, commands); + commandsLoaded = markdownFiles.length; + break; + } + } + + return { + codebaseId: nameMatch.id, + name: nameMatch.name, + repositoryUrl: nameMatch.repository_url ?? repositoryUrl, + defaultCwd: targetPath, + commandCount: commandsLoaded, + alreadyExisted: true, + }; + } + // Same name, different local path → distinct clone, fall through to create + } + // No existing codebase — create new const codebase = await codebaseDb.createCodebase({ name,