Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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;
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
67 changes: 67 additions & 0 deletions packages/core/src/orchestrator/orchestrator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,17 @@ mock.module('../utils/worktree-sync', () => ({
syncArchonToWorktree: mockSyncArchonToWorktree,
}));

// Git workspace sync mock
const mockSyncWorkspace = mock(() => Promise.resolve({ fetched: true, reset: false }));
const mockToRepoPath = mock((p: string) => p);
const mockToBranchName = mock((b: string) => b);

mock.module('@archon/git', () => ({
syncWorkspace: mockSyncWorkspace,
toRepoPath: mockToRepoPath,
toBranchName: mockToBranchName,
}));
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Orchestrator (isolation & dispatch) mocks
const mockValidateAndResolveIsolation = mock(() =>
Promise.resolve({ status: 'existing', cwd: '/workspace/project', env: null })
Expand Down Expand Up @@ -215,6 +226,7 @@ const mockCodebase: Codebase = {
name: 'test-project',
repository_url: 'https://github.com/user/repo',
default_cwd: '/workspace/test-project',
default_branch: 'main',
ai_assistant_type: 'claude',
commands: {},
created_at: new Date(),
Expand Down Expand Up @@ -278,6 +290,9 @@ function clearAllMocks(): void {
mockExecuteWorkflow.mockClear();
mockFindWorkflow.mockClear();
mockSyncArchonToWorktree.mockClear();
mockSyncWorkspace.mockClear();
mockToRepoPath.mockClear();
mockToBranchName.mockClear();
mockValidateAndResolveIsolation.mockClear();
mockDispatchBackgroundWorkflow.mockClear();
mockBuildOrchestratorPrompt.mockClear();
Expand Down Expand Up @@ -650,6 +665,58 @@ describe('orchestrator-agent handleMessage', () => {
});
});

// ─── syncWorkspace branch forwarding ──────────────────────────────────

describe('syncWorkspace branch forwarding', () => {
test('passes configured default_branch to syncWorkspace when set', async () => {
const codbaseWithBranch: Codebase = {
...mockCodebase,
default_branch: 'develop',
default_cwd: '/home/test/.archon/workspaces/owner/repo/source',
};
mockGetOrCreateConversation.mockResolvedValue({
...mockConversationWithProject,
codebase_id: 'codebase-789',
});
mockGetCodebase.mockResolvedValue(codbaseWithBranch);
mockClient.sendQuery.mockImplementation(async function* () {
yield { type: 'result', sessionId: 'session-id' };
});

await handleMessage(platform, 'chat-456', 'help');

expect(mockSyncWorkspace).toHaveBeenCalledWith(
expect.any(String),
'develop',
expect.any(Object)
);
});

test('passes undefined branch to syncWorkspace when default_branch is null', async () => {
const codebaseNoBranch: Codebase = {
...mockCodebase,
default_branch: null,
default_cwd: '/home/test/.archon/workspaces/owner/repo/source',
};
mockGetOrCreateConversation.mockResolvedValue({
...mockConversationWithProject,
codebase_id: 'codebase-789',
});
mockGetCodebase.mockResolvedValue(codebaseNoBranch);
mockClient.sendQuery.mockImplementation(async function* () {
yield { type: 'result', sessionId: 'session-id' };
});

await handleMessage(platform, 'chat-456', 'help');

expect(mockSyncWorkspace).toHaveBeenCalledWith(
expect.any(String),
undefined,
expect.any(Object)
);
});
});

// ─── Session Management ────────────────────────────────────────────────

describe('session management', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface Codebase {
name: string;
repository_url: string | null;
default_cwd: string;
default_branch: string | null;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ai_assistant_type: string;
commands: Record<string, { path: string; description: string }>;
created_at: Date;
Expand Down
1 change: 1 addition & 0 deletions packages/server/src/routes/schemas/codebase.schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const codebaseSchema = z
name: z.string(),
repository_url: z.string().nullable(),
default_cwd: z.string(),
default_branch: z.string().nullable(),
ai_assistant_type: z.string(),
commands: z.record(codebaseCommandSchema),
created_at: z.string(),
Expand Down
Loading