Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion migrations/000_combined.sql
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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);
16 changes: 16 additions & 0 deletions migrations/022_codebase_composite_identity.sql
Original file line number Diff line number Diff line change
@@ -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);
25 changes: 25 additions & 0 deletions packages/core/src/db/adapters/sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export class SqliteAdapter implements IDatabase {
private initSchema(): void {
this.createSchema();
this.migrateColumns();
this.migrateCodebaseIdentity();
}

/**
Expand Down Expand Up @@ -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.'
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/**
* Create all tables.
*
Expand Down Expand Up @@ -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');
}
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/db/codebases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,27 @@ export async function findCodebaseByPathPrefix(cwdPath: string): Promise<Codebas
return result.rows[0] || null;
}

/**
* Find a codebase by name and local path (composite identity).
* This is the preferred lookup for registration flows where path is known.
*/
export async function findCodebaseByNameAndPath(
name: string,
defaultCwd: string
): Promise<Codebase | null> {
const result = await pool.query<Codebase>(
'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<Codebase | null> {
const result = await pool.query<Codebase>(
'SELECT * FROM remote_agent_codebases WHERE name = $1 ORDER BY created_at DESC LIMIT 1',
Expand Down
146 changes: 104 additions & 42 deletions packages/core/src/handlers/clone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -38,6 +39,7 @@ mock.module('../db/codebases', () => ({
updateCodebaseCommands: mockUpdateCodebaseCommands,
findCodebaseByRepoUrl: mockFindCodebaseByRepoUrl,
findCodebaseByDefaultCwd: mockFindCodebaseByDefaultCwd,
findCodebaseByNameAndPath: mockFindCodebaseByNameAndPath,
findCodebaseByName: mockFindCodebaseByName,
updateCodebase: mockUpdateCodebase,
}));
Expand Down Expand Up @@ -100,6 +102,7 @@ function clearMocks(): void {
mockUpdateCodebaseCommands.mockReset();
mockFindCodebaseByRepoUrl.mockReset();
mockFindCodebaseByDefaultCwd.mockReset();
mockFindCodebaseByNameAndPath.mockReset();
mockFindCodebaseByName.mockReset();
mockUpdateCodebase.mockReset();
mockFindMarkdownFilesRecursive.mockReset();
Expand All @@ -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([]);
Expand Down Expand Up @@ -776,46 +780,78 @@ describe('normalizeRepoUrl (via cloneRepository)', () => {
});

// ────────────────────────────────────────────────────────────────────────────
describe('name-based deduplication', () => {
describe('composite identity deduplication', () => {
beforeEach(() => {
clearMocks();
restoreSpies();
setupSpies();
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<typeof makeCodebase>
);

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'))
return Promise.resolve({ stdout: 'https://github.com/owner/repo', stderr: '' });
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[]) => {
Expand All @@ -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<typeof makeCodebase>);

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',
Expand All @@ -876,18 +911,45 @@ 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,
{ repository_url?: string | null },
];
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);
});
});

// ────────────────────────────────────────────────────────────────────────────
Expand Down
Loading
Loading