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
1 change: 1 addition & 0 deletions migrations/000_combined.sql
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CREATE TABLE IF NOT EXISTS remote_agent_codebases (
name VARCHAR(255) NOT NULL,
repository_url VARCHAR(500),
default_cwd VARCHAR(500) NOT NULL,
default_branch TEXT DEFAULT 'main',
ai_assistant_type VARCHAR(20) DEFAULT 'claude',
allow_env_keys BOOLEAN NOT NULL DEFAULT FALSE,
commands JSONB DEFAULT '{}'::jsonb,
Expand Down
8 changes: 8 additions & 0 deletions migrations/022_add_default_branch_to_codebases.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Add default_branch column to remote_agent_codebases.
-- NULL means "not yet detected"; syncWorkspace falls back to auto-detection
-- (pre-existing behaviour). New clones set this via the branch-detect path in
-- clone.ts. Using no DEFAULT so existing rows stay NULL rather than being
-- silently set to 'main' (which could trigger an unwanted hard-reset for
-- managed clones on a non-main branch).
ALTER TABLE remote_agent_codebases
ADD COLUMN IF NOT EXISTS default_branch TEXT;
14 changes: 14 additions & 0 deletions packages/core/src/db/adapters/sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,20 @@ export class SqliteAdapter implements IDatabase {
} catch (e: unknown) {
getLog().warn({ err: e as Error }, 'db.sqlite_migration_session_columns_failed');
}

// Codebases columns
try {
const codebaseCols = this.db.prepare("PRAGMA table_info('remote_agent_codebases')").all() as {
name: string;
}[];
const codebaseColNames = new Set(codebaseCols.map(c => c.name));

if (!codebaseColNames.has('default_branch')) {
this.db.run('ALTER TABLE remote_agent_codebases ADD COLUMN default_branch TEXT');
}
} catch (e: unknown) {
getLog().warn({ err: e as Error }, 'db.sqlite_migration_codebases_columns_failed');
}
}

/**
Expand Down
27 changes: 23 additions & 4 deletions packages/core/src/db/codebases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('codebases', () => {
name: 'test-project',
repository_url: 'https://github.com/user/repo',
default_cwd: '/workspace/test-project',
default_branch: 'main',
ai_assistant_type: 'claude',
commands: { plan: { path: '.claude/commands/plan.md', description: 'Plan feature' } },
created_at: new Date(),
Expand All @@ -54,8 +55,8 @@ describe('codebases', () => {

expect(result).toEqual(mockCodebase);
expect(mockQuery).toHaveBeenCalledWith(
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *',
['test-project', 'https://github.com/user/repo', '/workspace/test-project', 'claude']
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *',
['test-project', 'https://github.com/user/repo', '/workspace/test-project', null, 'claude']
);
});

Expand All @@ -73,8 +74,25 @@ describe('codebases', () => {

expect(result).toEqual(codebaseWithoutOptional);
expect(mockQuery).toHaveBeenCalledWith(
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *',
['test-project', null, '/workspace/test-project', 'claude']
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *',
['test-project', null, '/workspace/test-project', null, 'claude']
);
});

test('creates codebase with explicit default_branch', async () => {
mockQuery.mockResolvedValueOnce(
createQueryResult([{ ...mockCodebase, default_branch: 'develop' }])
);

await createCodebase({
name: 'test-project',
default_cwd: '/workspace/test-project',
default_branch: 'develop',
});

expect(mockQuery).toHaveBeenCalledWith(
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *',
['test-project', null, '/workspace/test-project', 'develop', 'claude']
);
});

Expand Down Expand Up @@ -296,6 +314,7 @@ describe('codebases', () => {
id: 'cb-123',
name: 'test-repo',
default_cwd: '/workspace/test-repo',
default_branch: null,
ai_assistant_type: 'claude',
repository_url: null,
commands: {},
Expand Down
11 changes: 9 additions & 2 deletions packages/core/src/db/codebases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@ export async function createCodebase(data: {
name: string;
repository_url?: string;
default_cwd: string;
default_branch?: string;
ai_assistant_type?: string;
}): Promise<Codebase> {
const assistantType = data.ai_assistant_type ?? 'claude';
const result = await pool.query<Codebase>(
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, ai_assistant_type) VALUES ($1, $2, $3, $4) RETURNING *',
[data.name, data.repository_url ?? null, data.default_cwd, assistantType]
'INSERT INTO remote_agent_codebases (name, repository_url, default_cwd, default_branch, ai_assistant_type) VALUES ($1, $2, $3, $4, $5) RETURNING *',
[
data.name,
data.repository_url ?? null,
data.default_cwd,
data.default_branch ?? null,
assistantType,
]
);
if (!result.rows[0]) {
throw new Error('Failed to create codebase: INSERT succeeded but no row returned');
Expand Down
66 changes: 66 additions & 0 deletions packages/core/src/handlers/clone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ function makeCodebase(
name: string;
repository_url: string | null;
default_cwd: string;
default_branch: string | null;
ai_assistant_type: string;
}> = {}
): object {
Expand All @@ -139,6 +140,7 @@ function makeCodebase(
name: 'owner/repo',
repository_url: 'https://github.com/owner/repo',
default_cwd: '/home/test/.archon/workspaces/owner/repo/source',
default_branch: null,
ai_assistant_type: 'claude',
commands: {},
created_at: new Date(),
Expand Down Expand Up @@ -301,6 +303,70 @@ describe('cloneRepository', () => {
});
});

// ── Branch detection after clone ──────────────────────────────────────
describe('branch detection after clone', () => {
test('stores detected branch in createCodebase call', async () => {
spyExecFileAsync.mockImplementation((_cmd: string, args: string[]) => {
if (args[0] === '-C' && args[2] === 'rev-parse') {
return Promise.resolve({ stdout: 'master\n', stderr: '' });
}
return Promise.resolve({ stdout: '', stderr: '' });
});
mockCreateCodebase.mockResolvedValueOnce(makeCodebase() as ReturnType<typeof makeCodebase>);

await cloneRepository('https://github.com/owner/repo');

const createArg = mockCreateCodebase.mock.calls[0]?.[0] as { default_branch?: string };
expect(createArg.default_branch).toBe('master');
});

test('stores undefined (null in DB) when HEAD is detached', async () => {
spyExecFileAsync.mockImplementation((_cmd: string, args: string[]) => {
if (args[0] === '-C' && args[2] === 'rev-parse') {
return Promise.resolve({ stdout: 'HEAD\n', stderr: '' });
}
return Promise.resolve({ stdout: '', stderr: '' });
});
mockCreateCodebase.mockResolvedValueOnce(makeCodebase() as ReturnType<typeof makeCodebase>);

await cloneRepository('https://github.com/owner/repo');

const createArg = mockCreateCodebase.mock.calls[0]?.[0] as { default_branch?: string };
expect(createArg.default_branch).toBeUndefined();
});

test('does not throw and stores undefined when rev-parse fails', async () => {
spyExecFileAsync.mockImplementation((_cmd: string, args: string[]) => {
if (args[0] === '-C' && args[2] === 'rev-parse') {
return Promise.reject(new Error('not a git repo'));
}
return Promise.resolve({ stdout: '', stderr: '' });
});
mockCreateCodebase.mockResolvedValueOnce(makeCodebase() as ReturnType<typeof makeCodebase>);

const result = await cloneRepository('https://github.com/owner/repo');
expect(result).toBeDefined();

const createArg = mockCreateCodebase.mock.calls[0]?.[0] as { default_branch?: string };
expect(createArg.default_branch).toBeUndefined();
});

test('stores undefined when rev-parse returns empty string', async () => {
spyExecFileAsync.mockImplementation((_cmd: string, args: string[]) => {
if (args[0] === '-C' && args[2] === 'rev-parse') {
return Promise.resolve({ stdout: '', stderr: '' });
}
return Promise.resolve({ stdout: '', stderr: '' });
});
mockCreateCodebase.mockResolvedValueOnce(makeCodebase() as ReturnType<typeof makeCodebase>);

await cloneRepository('https://github.com/owner/repo');

const createArg = mockCreateCodebase.mock.calls[0]?.[0] as { default_branch?: string };
expect(createArg.default_branch).toBeUndefined();
});
});

// ── Already-cloned directory ───────────────────────────────────────────
describe('pre-existing clone', () => {
beforeEach(() => {
Expand Down
30 changes: 28 additions & 2 deletions packages/core/src/handlers/clone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export interface RegisterResult {
async function registerRepoAtPath(
targetPath: string,
name: string,
repositoryUrl: string | null
repositoryUrl: string | null,
defaultBranch?: string
): Promise<RegisterResult> {
// Auto-detect assistant type based on SDK folder conventions.
// Built-in providers use well-known folders (.claude/, .codex/).
Expand Down Expand Up @@ -125,6 +126,7 @@ async function registerRepoAtPath(
name,
repository_url: repositoryUrl ?? undefined,
default_cwd: targetPath,
default_branch: defaultBranch,
ai_assistant_type: suggestedAssistant,
});

Expand Down Expand Up @@ -279,7 +281,31 @@ export async function cloneRepository(repoUrl: string): Promise<RegisterResult>
await execFileAsync('git', ['config', '--global', '--add', 'safe.directory', targetPath]);
getLog().debug({ path: targetPath }, 'safe_directory_added');

const result = await registerRepoAtPath(targetPath, `${ownerName}/${repoName}`, workingUrl);
// Detect the actual branch checked out after clone (may differ from 'main' for repos with
// a non-default HEAD). Non-fatal: if detection fails, NULL is stored and syncWorkspace
// auto-detects the branch at runtime (pre-existing behavior).
let detectedBranch: string | undefined;
try {
const { stdout } = await execFileAsync(
'git',
['-C', targetPath, 'rev-parse', '--abbrev-ref', 'HEAD'],
{ timeout: 5000 }
);
const branch = stdout.trim();
if (branch && branch !== 'HEAD') {
detectedBranch = branch;
}
} catch (err) {
// Non-fatal — store NULL so syncWorkspace falls back to auto-detection
getLog().debug({ path: targetPath, err }, 'clone.branch_detect_failed');
}

const result = await registerRepoAtPath(
targetPath,
`${ownerName}/${repoName}`,
workingUrl,
detectedBranch
);
getLog().info({ url: workingUrl, targetPath }, 'clone_completed');
return result;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/orchestrator/orchestrator-agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ function makeCodebase(name: string, id = `id-${name}`): Codebase {
name,
repository_url: null,
default_cwd: `/repos/${name}`,
default_branch: null,
ai_assistant_type: 'claude',
commands: {},
created_at: new Date(),
Expand Down Expand Up @@ -830,6 +831,7 @@ function makeCodebaseForSync() {
name: 'test-repo',
repository_url: 'https://github.com/test/repo',
default_cwd: '/repos/test-repo',
default_branch: null,
ai_assistant_type: 'claude',
commands: {},
created_at: new Date(),
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/orchestrator/orchestrator-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { toError } from '../utils/error';
import { getAgentProvider, getProviderCapabilities } from '@archon/providers';
import { getArchonHome, getArchonWorkspacesPath } from '@archon/paths';
import { syncArchonToWorktree } from '../utils/worktree-sync';
import { syncWorkspace, toRepoPath } from '@archon/git';
import { syncWorkspace, toRepoPath, toBranchName } from '@archon/git';
import type { WorkspaceSyncResult } from '@archon/git';
import { discoverWorkflowsWithConfig } from '@archon/workflows/workflow-discovery';
import { findWorkflow } from '@archon/workflows/router';
Expand Down Expand Up @@ -410,7 +410,10 @@ async function discoverAllWorkflows(conversation: Conversation): Promise<Discove
const isManagedClone = codebase.default_cwd
.replace(/\\/g, '/')
.startsWith(getArchonWorkspacesPath().replace(/\\/g, '/'));
syncResult = await syncWorkspace(toRepoPath(codebase.default_cwd), undefined, {
const configuredBranch = codebase.default_branch
? toBranchName(codebase.default_branch)
: undefined;
syncResult = await syncWorkspace(toRepoPath(codebase.default_cwd), configuredBranch, {
resetAfterFetch: isManagedClone,
});
Comment on lines +413 to 418
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Null default_branch silently falls back to auto-detect — same root bug for unset rows.

When codebase.default_branch is null, configuredBranch is undefined and syncWorkspace re-enters the auto-detect path (git symbolic-ref refs/remotes/origin/HEAD → typically main). For managed clones (resetAfterFetch: true) this will still do git reset --hard origin/main — the exact behavior issue #1273 is trying to avoid.

This works for codebases created via the updated clone path (branch is detected and persisted), but two scenarios still hit the bug:

  • Rows that existed before migration 022 (defaulted to 'main' — see my comment on the migration).
  • Codebases created via /register-project (handleRegisterProject at Line 1300 doesn't pass default_branch, so null is persisted).

Consider either (a) detecting HEAD at /register-project time too, or (b) when default_branch is null on a managed clone, detecting the local branch via git rev-parse --abbrev-ref HEAD before syncing rather than resolving via origin/HEAD, and refusing the reset if they disagree.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/orchestrator/orchestrator-agent.ts` around lines 413 - 418,
When codebase.default_branch is null we must avoid falling back to origin/HEAD
for managed clones; change the logic around configuredBranch (the variable
computed from codebase.default_branch before calling syncWorkspace) so that if
isManagedClone is true and codebase.default_branch is null you run a local
branch detection (git rev-parse --abbrev-ref HEAD via the existing git helper or
a small helper) and set configuredBranch to that local branch; if local branch
disagrees with origin/HEAD, disable or refuse the hard reset (e.g., flip
resetAfterFetch to false or return an error) before calling
syncWorkspace(toRepoPath(...), configuredBranch, { resetAfterFetch:
isManagedClone }); alternatively also populate default_branch during
handleRegisterProject so new rows never persist null.

getLog().debug(
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/orchestrator/orchestrator-isolation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,10 @@ function makeCodebase(overrides?: Partial<Codebase>): Codebase {
return {
id: 'cb-1',
name: 'test-repo',
repository_url: null,
default_cwd: '/workspace/test-repo',
default_branch: null,
ai_assistant_type: 'claude',
commands: {},
created_at: new Date(),
updated_at: new Date(),
Expand Down
Loading
Loading