diff --git a/packages/adapters/src/community/forge/gitea/adapter.ts b/packages/adapters/src/community/forge/gitea/adapter.ts index ceeb1579cf..f9f6c3d823 100644 --- a/packages/adapters/src/community/forge/gitea/adapter.ts +++ b/packages/adapters/src/community/forge/gitea/adapter.ts @@ -33,6 +33,7 @@ import { } from '@archon/git'; import * as db from '@archon/core/db/conversations'; import * as codebaseDb from '@archon/core/db/codebases'; +import { resolveDefaultAssistant } from '@archon/core/config/resolve-assistant'; import { parseAllowedUsers, isGiteaUserAuthorized } from './auth'; import { splitIntoParagraphChunks } from '../../../utils/message-splitting'; import type { WebhookEvent } from './types'; @@ -642,6 +643,7 @@ export class GiteaAdapter implements IPlatformAdapter { name: `${owner}/${repo}`, repository_url: repoUrlNoGit, default_cwd: canonicalPath, + ai_assistant_type: await resolveDefaultAssistant(canonicalPath), }); getLog().info({ codebaseName: codebase.name, path: canonicalPath }, 'codebase_created'); diff --git a/packages/adapters/src/community/forge/gitlab/adapter.ts b/packages/adapters/src/community/forge/gitlab/adapter.ts index cc5b1f6f37..77ba1421b5 100644 --- a/packages/adapters/src/community/forge/gitlab/adapter.ts +++ b/packages/adapters/src/community/forge/gitlab/adapter.ts @@ -32,6 +32,7 @@ import { } from '@archon/git'; import * as db from '@archon/core/db/conversations'; import * as codebaseDb from '@archon/core/db/codebases'; +import { resolveDefaultAssistant } from '@archon/core/config/resolve-assistant'; import { parseAllowedUsers, isGitLabUserAuthorized, verifyWebhookToken } from './auth'; import { splitIntoParagraphChunks } from '../../../utils/message-splitting'; import type { GitLabWebhookEvent, GitLabIssue, GitLabMergeRequest } from './types'; @@ -592,6 +593,7 @@ Use 'glab mr view ${String(mr.iid)}' for full details and 'glab mr diff ${String name: projectPath, repository_url: repoUrlNoGit, default_cwd: canonicalPath, + ai_assistant_type: await resolveDefaultAssistant(canonicalPath), }); getLog().info({ codebaseName: codebase.name, path: canonicalPath }, 'gitlab.codebase_created'); diff --git a/packages/adapters/src/forge/github/adapter.ts b/packages/adapters/src/forge/github/adapter.ts index aa4867605c..600ad78a46 100644 --- a/packages/adapters/src/forge/github/adapter.ts +++ b/packages/adapters/src/forge/github/adapter.ts @@ -32,6 +32,7 @@ import { } from '@archon/git'; import * as db from '@archon/core/db/conversations'; import * as codebaseDb from '@archon/core/db/codebases'; +import { resolveDefaultAssistant } from '@archon/core/config/resolve-assistant'; import { createLogger } from '@archon/paths'; import { parseAllowedUsers as parseGitHubAllowedUsers, isGitHubUserAuthorized } from './auth'; import { splitIntoParagraphChunks } from '../../utils/message-splitting'; @@ -620,6 +621,7 @@ export class GitHubAdapter implements IPlatformAdapter { name: `${owner}/${repo}`, repository_url: repoUrlNoGit, // Store without .git for consistency default_cwd: canonicalPath, + ai_assistant_type: await resolveDefaultAssistant(canonicalPath), }); getLog().info({ codebaseName: codebase.name, path: canonicalPath }, 'github.codebase_created'); diff --git a/packages/core/package.json b/packages/core/package.json index 9681adb4de..d0a17b39e7 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -17,6 +17,7 @@ "./orchestrator": "./src/orchestrator/orchestrator.ts", "./handlers": "./src/handlers/command-handler.ts", "./config": "./src/config/index.ts", + "./config/resolve-assistant": "./src/config/resolve-assistant.ts", "./utils/*": "./src/utils/*.ts", "./services/*": "./src/services/*.ts", "./state/*": "./src/state/*.ts" diff --git a/packages/core/src/config/config-loader.test.ts b/packages/core/src/config/config-loader.test.ts index ac242040ac..f5bee45bba 100644 --- a/packages/core/src/config/config-loader.test.ts +++ b/packages/core/src/config/config-loader.test.ts @@ -15,16 +15,16 @@ mock.module('@archon/paths', () => ({ getDefaultWorkflowsPath: mock(() => '/app/.archon/workflows/defaults'), })); -// Mock for reading/writing config files (replaces fs/promises mock) -const mockReadConfigFile = mock(() => Promise.resolve('')); -const mockWriteConfigFile = mock(() => Promise.resolve()); - -// Import real config-loader to spread its exports, then override readConfigFile/writeConfigFile -import * as realConfigLoader from './config-loader'; -mock.module('./config-loader', () => ({ - ...realConfigLoader, - readConfigFile: mockReadConfigFile, - writeConfigFile: mockWriteConfigFile, +// Mock fs/promises so that readConfigFile/writeConfigFile (which call fsReadFile/writeFile +// internally) are intercepted regardless of Bun version mock.module semantics. +const mockFsReadFile = mock(() => Promise.resolve('')); +const mockFsWriteFile = mock(() => Promise.resolve()); +const mockFsMkdir = mock(() => Promise.resolve(undefined)); + +mock.module('fs/promises', () => ({ + readFile: mockFsReadFile, + writeFile: mockFsWriteFile, + mkdir: mockFsMkdir, })); import { @@ -51,8 +51,8 @@ describe('config-loader', () => { beforeEach(() => { clearConfigCache(); - mockReadConfigFile.mockReset(); - mockWriteConfigFile.mockReset(); + mockFsReadFile.mockReset(); + mockFsWriteFile.mockReset(); // Save original env vars envVars.forEach(key => { @@ -71,23 +71,23 @@ describe('config-loader', () => { } }); - // No need to restore - we're mocking at config-loader level, not fs/promises - mockReadConfigFile.mockClear(); - mockWriteConfigFile.mockClear(); + // Clear mock state between tests + mockFsReadFile.mockClear(); + mockFsWriteFile.mockClear(); }); describe('loadGlobalConfig', () => { test('returns empty object when file does not exist', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadGlobalConfig(); expect(config).toEqual({}); }); test('parses valid YAML config', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` defaultAssistant: codex streaming: telegram: batch @@ -102,22 +102,22 @@ concurrency: }); test('caches config on subsequent calls', async () => { - mockReadConfigFile.mockResolvedValue('defaultAssistant: claude'); + mockFsReadFile.mockResolvedValue('defaultAssistant: claude'); await loadGlobalConfig(); await loadGlobalConfig(); // Should only read file once - expect(mockReadConfigFile).toHaveBeenCalledTimes(1); + expect(mockFsReadFile).toHaveBeenCalledTimes(1); }); test('reloads config when forceReload is true', async () => { - mockReadConfigFile.mockResolvedValue('defaultAssistant: claude'); + mockFsReadFile.mockResolvedValue('defaultAssistant: claude'); await loadGlobalConfig(); await loadGlobalConfig(true); - expect(mockReadConfigFile).toHaveBeenCalledTimes(2); + expect(mockFsReadFile).toHaveBeenCalledTimes(2); }); test('logs error for invalid YAML syntax', async () => { @@ -125,7 +125,7 @@ concurrency: // Simulate YAML parse error (SyntaxError has no .code property) const syntaxError = new SyntaxError('YAML Parse error: Multiline implicit key'); - mockReadConfigFile.mockRejectedValue(syntaxError); + mockFsReadFile.mockRejectedValue(syntaxError); const config = await loadGlobalConfig(); @@ -144,7 +144,7 @@ concurrency: const permError = new Error('Permission denied') as NodeJS.ErrnoException; permError.code = 'EACCES'; - mockReadConfigFile.mockRejectedValue(permError); + mockFsReadFile.mockRejectedValue(permError); const config = await loadGlobalConfig(); @@ -161,7 +161,7 @@ concurrency: describe('loadRepoConfig', () => { test('loads from .archon/config.yaml', async () => { - mockReadConfigFile.mockResolvedValue('assistant: codex'); + mockFsReadFile.mockResolvedValue('assistant: codex'); const config = await loadRepoConfig('/test/repo'); expect(config.assistant).toBe('codex'); @@ -170,7 +170,7 @@ concurrency: test('returns empty object when no config found', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadRepoConfig('/test/repo'); expect(config).toEqual({}); @@ -181,7 +181,7 @@ concurrency: // Simulate YAML parse error (SyntaxError has no .code property) const syntaxError = new SyntaxError('YAML Parse error: Multiline implicit key'); - mockReadConfigFile.mockRejectedValue(syntaxError); + mockFsReadFile.mockRejectedValue(syntaxError); const config = await loadRepoConfig('/test/repo'); @@ -200,7 +200,7 @@ concurrency: const permError = new Error('Permission denied') as NodeJS.ErrnoException; permError.code = 'EACCES'; - mockReadConfigFile.mockRejectedValue(permError); + mockFsReadFile.mockRejectedValue(permError); const config = await loadRepoConfig('/test/repo'); @@ -219,7 +219,7 @@ concurrency: test('returns defaults when no configs exist', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadConfig(); @@ -234,7 +234,7 @@ concurrency: }); test('env vars override config files', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` defaultAssistant: claude streaming: telegram: stream @@ -250,20 +250,20 @@ streaming: }); test('throws on unknown DEFAULT_AI_ASSISTANT env var', async () => { - mockReadConfigFile.mockResolvedValue(''); + mockFsReadFile.mockResolvedValue(''); process.env.DEFAULT_AI_ASSISTANT = 'nonexistent-provider'; await expect(loadConfig()).rejects.toThrow(/not a registered provider/); }); test('throws on unknown defaultAssistant in global config', async () => { - mockReadConfigFile.mockResolvedValue('defaultAssistant: nonexistent-provider'); + mockFsReadFile.mockResolvedValue('defaultAssistant: nonexistent-provider'); await expect(loadConfig()).rejects.toThrow(/not a registered provider/); }); test('throws on unknown assistant in repo config', async () => { - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { const normalized = path.replace(/\\/g, '/'); if (normalized.includes('/tmp/test-repo/.archon/config.yaml')) { return 'assistant: nonexistent-provider'; @@ -282,7 +282,7 @@ streaming: }; let globalConfigRead = false; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { // First check for repo-specific config path (contains /repo/.archon/) if (pathMatches(path, '/repo/.archon/config.yaml')) { return 'assistant: codex'; @@ -308,7 +308,7 @@ streaming: }; let globalConfigRead = false; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return `assistants:\n codex:\n webSearchMode: live\n additionalDirectories:\n - /repo\n`; } @@ -335,7 +335,7 @@ streaming: return normalizedPath.includes(pattern); }; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return ` worktree: @@ -357,7 +357,7 @@ worktree: return normalizedPath.includes(pattern); }; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return ` worktree: @@ -376,7 +376,7 @@ worktree: test('baseBranch is undefined when not configured', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadConfig('/test/repo'); expect(config.baseBranch).toBeUndefined(); @@ -388,7 +388,7 @@ worktree: return normalizedPath.includes(pattern); }; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return ` docs: @@ -410,7 +410,7 @@ docs: return normalizedPath.includes(pattern); }; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return ` docs: @@ -429,7 +429,7 @@ docs: test('docsPath is undefined when docs config is absent', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadConfig('/test/repo'); expect(config.docsPath).toBeUndefined(); @@ -439,7 +439,7 @@ docs: const pathMatches = (path: string, pattern: string): boolean => path.replace(/\\/g, '/').includes(pattern); - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return ` env: @@ -459,7 +459,7 @@ env: test('envVars is undefined when repo config has no env section', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadConfig('/test/repo'); expect(config.envVars).toBeUndefined(); @@ -468,7 +468,7 @@ env: test('paths use archon defaults', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); const config = await loadConfig(); @@ -479,7 +479,7 @@ env: describe('settingSources config', () => { test('merges settingSources from global config', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` assistants: claude: settingSources: @@ -491,7 +491,7 @@ assistants: }); test('defaults to undefined settingSources when not configured', async () => { - mockReadConfigFile.mockResolvedValue(''); + mockFsReadFile.mockResolvedValue(''); const config = await loadConfig(); expect(config.assistants.claude.settingSources).toBeUndefined(); }); @@ -503,7 +503,7 @@ assistants: }; let globalConfigRead = false; - mockReadConfigFile.mockImplementation(async (path: string) => { + mockFsReadFile.mockImplementation(async (path: string) => { if (pathMatches(path, '/repo/.archon/config.yaml')) { return `assistants:\n claude:\n settingSources:\n - project\n`; } @@ -521,7 +521,7 @@ assistants: }); test('toSafeConfig does not expose settingSources (server-internal field)', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` assistants: claude: settingSources: @@ -536,7 +536,7 @@ assistants: describe('updateGlobalConfig', () => { test('merges assistant config into existing file', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` defaultAssistant: claude assistants: claude: @@ -547,13 +547,13 @@ assistants: assistants: { claude: { model: 'opus' } }, }); - expect(mockWriteConfigFile).toHaveBeenCalledTimes(1); - const writtenContent = mockWriteConfigFile.mock.calls[0]?.[1] as string; + expect(mockFsWriteFile).toHaveBeenCalledTimes(1); + const writtenContent = mockFsWriteFile.mock.calls[0]?.[1] as string; expect(writtenContent).toContain('opus'); }); test('preserves existing non-updated fields', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` defaultAssistant: codex botName: MyBot assistants: @@ -566,8 +566,8 @@ assistants: defaultAssistant: 'claude', }); - expect(mockWriteConfigFile).toHaveBeenCalledTimes(1); - const writtenContent = mockWriteConfigFile.mock.calls[0]?.[1] as string; + expect(mockFsWriteFile).toHaveBeenCalledTimes(1); + const writtenContent = mockFsWriteFile.mock.calls[0]?.[1] as string; expect(writtenContent).toContain('claude'); expect(writtenContent).toContain('MyBot'); }); @@ -575,22 +575,22 @@ assistants: test('creates config when file does not exist', async () => { const error = new Error('ENOENT') as NodeJS.ErrnoException; error.code = 'ENOENT'; - mockReadConfigFile.mockRejectedValue(error); + mockFsReadFile.mockRejectedValue(error); await updateGlobalConfig({ defaultAssistant: 'codex', }); - expect(mockWriteConfigFile).toHaveBeenCalled(); - const writtenContent = mockWriteConfigFile.mock.calls[0]?.[1] as string; + expect(mockFsWriteFile).toHaveBeenCalled(); + const writtenContent = mockFsWriteFile.mock.calls[0]?.[1] as string; expect(writtenContent).toContain('codex'); }); test('throws on permission errors', async () => { - mockReadConfigFile.mockResolvedValue(''); + mockFsReadFile.mockResolvedValue(''); const permError = new Error('Permission denied') as NodeJS.ErrnoException; permError.code = 'EACCES'; - mockWriteConfigFile.mockRejectedValue(permError); + mockFsWriteFile.mockRejectedValue(permError); await expect(updateGlobalConfig({ defaultAssistant: 'codex' })).rejects.toThrow( 'Permission denied' @@ -600,21 +600,21 @@ assistants: describe('toSafeConfig', () => { test('strips paths from MergedConfig', async () => { - mockReadConfigFile.mockResolvedValue(''); + mockFsReadFile.mockResolvedValue(''); const config = await loadConfig(); const safe = toSafeConfig(config); expect(safe).not.toHaveProperty('paths'); }); test('strips entire commands object from MergedConfig', async () => { - mockReadConfigFile.mockResolvedValue(''); + mockFsReadFile.mockResolvedValue(''); const config = await loadConfig(); const safe = toSafeConfig(config); expect(safe).not.toHaveProperty('commands'); }); test('strips additionalDirectories from assistants.codex', async () => { - mockReadConfigFile.mockResolvedValue(` + mockFsReadFile.mockResolvedValue(` assistants: codex: additionalDirectories: @@ -626,7 +626,7 @@ assistants: }); test('preserves non-sensitive fields', async () => { - mockReadConfigFile.mockResolvedValue('defaultAssistant: codex'); + mockFsReadFile.mockResolvedValue('defaultAssistant: codex'); const config = await loadConfig(); const safe = toSafeConfig(config); expect(typeof safe.botName).toBe('string'); diff --git a/packages/core/src/config/index.ts b/packages/core/src/config/index.ts index a8287561bf..694e15abd7 100644 --- a/packages/core/src/config/index.ts +++ b/packages/core/src/config/index.ts @@ -4,3 +4,4 @@ export * from './config-types'; export * from './config-loader'; +export * from './resolve-assistant'; diff --git a/packages/core/src/config/resolve-assistant.test.ts b/packages/core/src/config/resolve-assistant.test.ts new file mode 100644 index 0000000000..ebd2fccb44 --- /dev/null +++ b/packages/core/src/config/resolve-assistant.test.ts @@ -0,0 +1,108 @@ +import { describe, test, expect, beforeEach, afterAll, spyOn, mock } from 'bun:test'; +import * as fsPromises from 'fs/promises'; +import * as providers from '@archon/providers'; +import * as configLoader from './config-loader'; +import { createMockLogger } from '../test/mocks/logger'; + +const mockLogger = createMockLogger(); +mock.module('@archon/paths', () => ({ + createLogger: () => mockLogger, +})); + +import { resolveDefaultAssistant } from './resolve-assistant'; + +let spyAccess: ReturnType; +let spyProviders: ReturnType; +let spyLoadConfig: ReturnType; + +function enoent(): Error { + return Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); +} + +beforeEach(() => { + spyAccess?.mockRestore(); + spyAccess = spyOn(fsPromises, 'access').mockRejectedValue(enoent()); + + spyProviders?.mockRestore(); + spyProviders = spyOn(providers, 'getRegisteredProviders').mockReturnValue([]); + + spyLoadConfig?.mockRestore(); + spyLoadConfig = spyOn(configLoader, 'loadConfig').mockResolvedValue({ + assistant: 'claude', + } as Awaited>); +}); + +afterAll(() => { + spyAccess?.mockRestore(); + spyProviders?.mockRestore(); + spyLoadConfig?.mockRestore(); +}); + +describe('resolveDefaultAssistant', () => { + test('returns codex when .codex folder exists', async () => { + spyAccess.mockImplementation((p: string) => + p.endsWith('.codex') ? Promise.resolve(undefined) : Promise.reject(enoent()) + ); + + expect(await resolveDefaultAssistant('/repo')).toBe('codex'); + }); + + test('returns claude when .claude folder exists and .codex does not', async () => { + spyAccess.mockImplementation((p: string) => + p.endsWith('.claude') ? Promise.resolve(undefined) : Promise.reject(enoent()) + ); + + expect(await resolveDefaultAssistant('/repo')).toBe('claude'); + }); + + test('.codex wins over .claude when both folders exist', async () => { + spyAccess.mockResolvedValue(undefined); + + expect(await resolveDefaultAssistant('/repo')).toBe('codex'); + }); + + test('uses configured assistant when no SDK folder exists', async () => { + spyLoadConfig.mockResolvedValue({ assistant: 'pi' } as Awaited< + ReturnType + >); + + expect(await resolveDefaultAssistant('/repo')).toBe('pi'); + // Contract: must pass repoPath so the repo's own .archon/config.yaml is merged. + expect(spyLoadConfig).toHaveBeenCalledWith('/repo'); + }); + + test('falls back to first built-in provider when loadConfig fails and no SDK folder exists', async () => { + spyLoadConfig.mockRejectedValue(new Error('no config')); + spyProviders.mockReturnValue([ + { id: 'pi', builtIn: true }, + { id: 'codex', builtIn: true }, + ]); + + expect(await resolveDefaultAssistant('/repo')).toBe('pi'); + }); + + test('falls back to claude when loadConfig fails and registry is empty', async () => { + spyLoadConfig.mockRejectedValue(new Error('no config')); + spyProviders.mockReturnValue([]); + + expect(await resolveDefaultAssistant('/repo')).toBe('claude'); + }); + + test('falls back to claude when config returns no assistant and registry has only community providers', async () => { + spyLoadConfig.mockResolvedValue({} as Awaited>); + spyProviders.mockReturnValue([{ id: 'oh-my-pi', builtIn: false }]); + + expect(await resolveDefaultAssistant('/repo')).toBe('claude'); + }); + + test('SDK folder detection bypasses config — checked-in .codex wins over configured claude', async () => { + spyLoadConfig.mockResolvedValue({ assistant: 'claude' } as Awaited< + ReturnType + >); + spyAccess.mockImplementation((p: string) => + p.endsWith('.codex') ? Promise.resolve(undefined) : Promise.reject(enoent()) + ); + + expect(await resolveDefaultAssistant('/repo')).toBe('codex'); + }); +}); diff --git a/packages/core/src/config/resolve-assistant.ts b/packages/core/src/config/resolve-assistant.ts new file mode 100644 index 0000000000..df133f8210 --- /dev/null +++ b/packages/core/src/config/resolve-assistant.ts @@ -0,0 +1,63 @@ +import { access } from 'fs/promises'; +import { join } from 'path'; +import { createLogger } from '@archon/paths'; + +let cachedLog: ReturnType | undefined; +function getLog(): ReturnType { + if (!cachedLog) cachedLog = createLogger('config.resolve-assistant'); + return cachedLog; +} + +/** + * Resolve the default AI assistant for a newly registered codebase. + * + * Precedence: SDK folder detection (`.codex` / `.claude` in repo) → configured + * `assistant` from `.archon/config.yaml` → first built-in provider in the + * registry → hardcoded `'claude'`. + * + * Folder detection wins over config because a checked-in `.codex` or `.claude` + * directory is an explicit per-repo signal from the user. + */ +export async function resolveDefaultAssistant(repoPath: string): Promise { + const codexFolder = join(repoPath, '.codex'); + const claudeFolder = join(repoPath, '.claude'); + + try { + await access(codexFolder); + getLog().debug({ path: codexFolder }, 'assistant_detected_codex'); + return 'codex'; + } catch { + // fall through + } + + try { + await access(claudeFolder); + getLog().debug({ path: claudeFolder }, 'assistant_detected_claude'); + return 'claude'; + } catch { + // fall through + } + + // Lazy-load config-loader and @archon/providers so this module doesn't eagerly + // pull in their chains at every import site. config-loader.ts eagerly imports + // @archon/providers, which transitively pulls in claude/codex binary-resolver + // and their BUNDLED_IS_BINARY dependency on @archon/paths — that breaks adapter + // tests on Linux that mock @archon/paths without BUNDLED_IS_BINARY. The original + // clone.ts logic used dynamic imports for exactly this reason. + try { + const { loadConfig } = await import('./config-loader'); + // Pass repoPath so the repo's own .archon/config.yaml is merged on top of + // the global config — without it, a repo-level `assistant: pi` would be + // silently ignored during registration. + const config = await loadConfig(repoPath); + if (config.assistant) { + getLog().debug({ provider: config.assistant }, 'assistant_default_from_config'); + return config.assistant; + } + } catch (err) { + getLog().warn({ err }, 'config_load_failed_using_builtin_default'); + } + + const { getRegisteredProviders } = await import('@archon/providers'); + return getRegisteredProviders().find(p => p.builtIn)?.id ?? 'claude'; +} diff --git a/packages/core/src/handlers/clone.test.ts b/packages/core/src/handlers/clone.test.ts index 9f4cc4b5e9..1dbc4b6f4a 100644 --- a/packages/core/src/handlers/clone.test.ts +++ b/packages/core/src/handlers/clone.test.ts @@ -60,6 +60,12 @@ mock.module('@archon/paths', () => ({ }), })); +// ── config-loader mock ────────────────────────────────────────────────────── +const mockLoadConfig = mock(() => Promise.resolve({ assistant: 'claude' })); +mock.module('../config/config-loader', () => ({ + loadConfig: mockLoadConfig, +})); + // ── utils/commands mock ───────────────────────────────────────────────────── const mockFindMarkdownFilesRecursive = mock(() => Promise.resolve([])); mock.module('../utils/commands', () => ({ @@ -103,6 +109,8 @@ function clearMocks(): void { mockFindCodebaseByName.mockReset(); mockUpdateCodebase.mockReset(); mockFindMarkdownFilesRecursive.mockReset(); + mockLoadConfig.mockReset(); + mockLoadConfig.mockResolvedValue({ assistant: 'claude' }); mockLogger.info.mockClear(); mockLogger.debug.mockClear(); mockLogger.warn.mockClear(); @@ -780,6 +788,32 @@ describe('cloneRepository', () => { expect(createCall[0].ai_assistant_type).toBe('claude'); }); + test('uses configured provider when no .codex or .claude folder exists', async () => { + mockLoadConfig.mockResolvedValue({ assistant: 'pi' }); + spyFsAccess.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mockCreateCodebase.mockResolvedValueOnce( + makeCodebase({ ai_assistant_type: 'pi' }) as ReturnType + ); + + await cloneRepository('https://github.com/owner/repo'); + + const createCall = mockCreateCodebase.mock.calls[0] as [{ ai_assistant_type: string }]; + expect(createCall[0].ai_assistant_type).toBe('pi'); + }); + + test('falls back to claude when loadConfig fails', async () => { + mockLoadConfig.mockRejectedValue(new Error('config load failed')); + spyFsAccess.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mockCreateCodebase.mockResolvedValueOnce( + makeCodebase({ ai_assistant_type: 'claude' }) as ReturnType + ); + + await cloneRepository('https://github.com/owner/repo'); + + const createCall = mockCreateCodebase.mock.calls[0] as [{ ai_assistant_type: string }]; + expect(createCall[0].ai_assistant_type).toBe('claude'); + }); + test('detects claude assistant when .claude folder exists but .codex does not', async () => { spyFsAccess.mockImplementation((path: string) => { // .codex → ENOENT, .claude → exists, .git → ENOENT, commands → ENOENT diff --git a/packages/core/src/handlers/clone.ts b/packages/core/src/handlers/clone.ts index b2f99dd600..da1b75337b 100644 --- a/packages/core/src/handlers/clone.ts +++ b/packages/core/src/handlers/clone.ts @@ -17,6 +17,7 @@ import { } from '@archon/paths'; import { findMarkdownFilesRecursive } from '../utils/commands'; import { createLogger } from '@archon/paths'; +import { resolveDefaultAssistant } from '../config/resolve-assistant'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ let cachedLog: ReturnType | undefined; @@ -138,28 +139,7 @@ async function registerRepoAtPath( name: string, repositoryUrl: string | null ): Promise { - // Auto-detect assistant type based on SDK folder conventions. - // Built-in providers use well-known folders (.claude/, .codex/). - // Falls back to first registered built-in provider if no folder detected. - const { getRegisteredProviders } = await import('@archon/providers'); - const defaultProvider = getRegisteredProviders().find(p => p.builtIn)?.id ?? 'claude'; - let suggestedAssistant = defaultProvider; - const codexFolder = join(targetPath, '.codex'); - const claudeFolder = join(targetPath, '.claude'); - - try { - await access(codexFolder); - suggestedAssistant = 'codex'; - getLog().debug({ path: codexFolder }, 'assistant_detected_codex'); - } catch { - try { - await access(claudeFolder); - suggestedAssistant = 'claude'; - getLog().debug({ path: claudeFolder }, 'assistant_detected_claude'); - } catch { - getLog().debug({ provider: defaultProvider }, 'assistant_default_from_registry'); - } - } + const suggestedAssistant = await resolveDefaultAssistant(targetPath); // Check if a codebase with this name already exists (dedup by project identity) const existing = await codebaseDb.findCodebaseByName(name);