fix(core): resolve default assistant via config + folder detection on every codebase registration#1729
Merged
Merged
Conversation
… every codebase registration
## Summary
- Extract `resolveDefaultAssistant(repoPath)` helper into `packages/core/src/config/resolve-assistant.ts` with precedence: `.codex` / `.claude` folder → `loadConfig().assistant` → first built-in provider → `'claude'`.
- Call the helper from `clone.ts` (replacing the inline block) and from the three forge adapters (`github`, `gitlab`, `gitea`) which previously passed no `ai_assistant_type` and silently defaulted to `'claude'` regardless of the configured assistant.
- `createCodebase` stays a thin DB function with the simple `?? 'claude'` fallback. No dynamic-import config-loading inside the DB layer.
- Lazy-load `@archon/providers` inside the helper so the resolve module doesn't pull provider SDK chains at every adapter import site (which previously broke adapter tests that mock `@archon/paths` without `BUNDLED_IS_BINARY`).
- New `resolve-assistant.test.ts` uses `spyOn` (not `mock.module`) for `loadConfig` and `getRegisteredProviders` so the spies cleanly `mockRestore()` and do not pollute `config-loader.test.ts` running in the same batch.
- `config-loader.test.ts` switches its file I/O mocks from `mock.module('./config-loader', ...)` to `mock.module('fs/promises', ...)` for cross-Bun-version compatibility.
- New `clone.test.ts` cases verify the configured-provider and loadConfig-failure fallbacks via the integration path.
Fixes #1580.
## Test plan
- [x] `bun test src/db/adapters/sqlite.test.ts src/db/codebases.test.ts ... src/config/ src/state/` — exact CI batch, 366 pass / 0 fail
- [x] `bun --filter @archon/core --filter @archon/adapters --filter @archon/server test` — only pre-existing macOS-only telegram-markdown failures (unrelated, present on dev)
- [x] `bun run type-check`, `bun run lint`, `bun run format:check` all clean
📝 WalkthroughWalkthroughAdds resolveDefaultAssistant(repoPath) with precedence (.codex → .claude → repo config → first built-in provider → 'claude'); re-exports it; clone handler and GitHub/GitLab/Gitea adapters call it to set ai_assistant_type; tests and config-loader tests updated to mock fs/config IO. ChangesAssistant Resolution Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/config/resolve-assistant.test.ts (1)
64-70: ⚡ Quick winAdd an assertion that
loadConfigreceives the repository path.Current tests validate outcomes but not the call contract, so a regression to
loadConfig()(without path) can slip through undetected.Also applies to: 96-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/config/resolve-assistant.test.ts` around lines 64 - 70, The test currently asserts only the return value of resolveDefaultAssistant but not that configLoader.loadConfig was invoked with the repository path; update the test(s) (the one using spyLoadConfig and resolveDefaultAssistant and the similar test at lines 96-105) to assert that spyLoadConfig was called with the repo path string (e.g., '/repo')—use the existing spyLoadConfig mock to verify the call contract after calling resolveDefaultAssistant so regressions that omit passing the path to configLoader.loadConfig are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/config/resolve-assistant.ts`:
- Around line 26-40: The current detection logic swallows all errors when
probing codexFolder and claudeFolder using access, which hides non-ENOENT issues
(e.g., EACCES, EIO); update each catch to inspect the caught error (e) and only
continue when e.code === 'ENOENT', otherwise rethrow or wrap with a clear
message that probing failed for codexFolder/claudeFolder; reference the async
probes using access(...) and the logged contexts getLog().debug({ path:
codexFolder }, 'assistant_detected_codex') and getLog().debug({ path:
claudeFolder }, 'assistant_detected_claude') so the error handling change is
applied around those specific try/catch blocks.
- Around line 42-47: The config loader is called without the repository path so
per-repo config (.archon/config.yaml) is ignored; update the call in
resolve-assistant (where loadConfig() is invoked) to pass the repoPath argument
(i.e., use loadConfig(repoPath)) so repo-level assistant settings are
considered, and adjust any surrounding code/imports if needed to match the
loadConfig signature.
In `@packages/core/src/handlers/clone.test.ts`:
- Around line 804-815: The test is flaky because provider resolution depends on
the runtime provider ordering; update the test to stub the provider registry so
'claude' is deterministically chosen (for example mock the '`@archon/providers`'
export or the getProviders/getBuiltInProviders function used by cloneRepository
to return an array with a provider object for ai_assistant_type: 'claude' as the
first element), then run the same steps (mockLoadConfig rejection, spyFsAccess
rejection, mockCreateCodebase resolution) and assert createCodebase was called
with ai_assistant_type 'claude'; ensure the mock is restored/cleared after the
test to avoid affecting other tests.
---
Nitpick comments:
In `@packages/core/src/config/resolve-assistant.test.ts`:
- Around line 64-70: The test currently asserts only the return value of
resolveDefaultAssistant but not that configLoader.loadConfig was invoked with
the repository path; update the test(s) (the one using spyLoadConfig and
resolveDefaultAssistant and the similar test at lines 96-105) to assert that
spyLoadConfig was called with the repo path string (e.g., '/repo')—use the
existing spyLoadConfig mock to verify the call contract after calling
resolveDefaultAssistant so regressions that omit passing the path to
configLoader.loadConfig are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8dcb386-392c-4fe4-92a7-ef5c54e36b95
📒 Files selected for processing (9)
packages/adapters/src/community/forge/gitea/adapter.tspackages/adapters/src/community/forge/gitlab/adapter.tspackages/adapters/src/forge/github/adapter.tspackages/core/src/config/config-loader.test.tspackages/core/src/config/index.tspackages/core/src/config/resolve-assistant.test.tspackages/core/src/config/resolve-assistant.tspackages/core/src/handlers/clone.test.tspackages/core/src/handlers/clone.ts
Comment on lines
+26
to
+40
| 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 | ||
| } |
There was a problem hiding this comment.
Do not swallow non-ENOENT probe failures during SDK folder detection.
Lines 30 and 38 catch all errors and continue. Permission or I/O failures should not silently fall through to a different assistant; that masks unsafe states and makes behavior misleading.
Suggested fix
try {
await access(codexFolder);
getLog().debug({ path: codexFolder }, 'assistant_detected_codex');
return 'codex';
- } catch {
- // fall through
+ } catch (error) {
+ const err = error as NodeJS.ErrnoException;
+ if (err.code !== 'ENOENT') throw error;
}
try {
await access(claudeFolder);
getLog().debug({ path: claudeFolder }, 'assistant_detected_claude');
return 'claude';
- } catch {
- // fall through
+ } catch (error) {
+ const err = error as NodeJS.ErrnoException;
+ if (err.code !== 'ENOENT') throw error;
}As per coding guidelines, "Throw early with clear errors for unsupported or unsafe states; never silently swallow errors or broaden permissions".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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 | |
| } | |
| try { | |
| await access(codexFolder); | |
| getLog().debug({ path: codexFolder }, 'assistant_detected_codex'); | |
| return 'codex'; | |
| } catch (error) { | |
| const err = error as NodeJS.ErrnoException; | |
| if (err.code !== 'ENOENT') throw error; | |
| } | |
| try { | |
| await access(claudeFolder); | |
| getLog().debug({ path: claudeFolder }, 'assistant_detected_claude'); | |
| return 'claude'; | |
| } catch (error) { | |
| const err = error as NodeJS.ErrnoException; | |
| if (err.code !== 'ENOENT') throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/config/resolve-assistant.ts` around lines 26 - 40, The
current detection logic swallows all errors when probing codexFolder and
claudeFolder using access, which hides non-ENOENT issues (e.g., EACCES, EIO);
update each catch to inspect the caught error (e) and only continue when e.code
=== 'ENOENT', otherwise rethrow or wrap with a clear message that probing failed
for codexFolder/claudeFolder; reference the async probes using access(...) and
the logged contexts getLog().debug({ path: codexFolder },
'assistant_detected_codex') and getLog().debug({ path: claudeFolder },
'assistant_detected_claude') so the error handling change is applied around
those specific try/catch blocks.
Comment on lines
+804
to
+815
| 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<typeof makeCodebase> | ||
| ); | ||
|
|
||
| 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'); | ||
| }); |
There was a problem hiding this comment.
Make the fallback test deterministic by controlling provider registry.
Line 814 asserts 'claude', but resolver falls back to first built-in provider before hardcoded 'claude'. Without mocking @archon/providers, this test depends on runtime provider ordering.
As per coding guidelines, "keep tests deterministic with no flaky timing or network dependence without guardrails".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/handlers/clone.test.ts` around lines 804 - 815, The test is
flaky because provider resolution depends on the runtime provider ordering;
update the test to stub the provider registry so 'claude' is deterministically
chosen (for example mock the '`@archon/providers`' export or the
getProviders/getBuiltInProviders function used by cloneRepository to return an
array with a provider object for ai_assistant_type: 'claude' as the first
element), then run the same steps (mockLoadConfig rejection, spyFsAccess
rejection, mockCreateCodebase resolution) and assert createCodebase was called
with ai_assistant_type 'claude'; ensure the mock is restored/cleared after the
test to avoid affecting other tests.
config-loader.ts eagerly imports @archon/providers (for runtime validation of the configured assistant ID against the registry), which transitively loads claude/codex binary-resolvers and their BUNDLED_IS_BINARY dependency on @archon/paths. Static-importing loadConfig at module top therefore forces every caller of resolve-assistant — including the three forge adapters — to pull that chain too. Adapter tests that mock @archon/paths without BUNDLED_IS_BINARY then break on Linux. Move the loadConfig import to a dynamic import inside the function body alongside the getRegisteredProviders one. Nothing in this module is loaded eagerly anymore.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/config/resolve-assistant.ts (2)
25-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle non-ENOENT probe errors explicitly.
Line 29 and Line 37 currently swallow all
access(...)failures. If probe errors are from permissions or I/O (not missing folders), assistant resolution should fail instead of silently falling through.Proposed fix
try { await access(codexFolder); getLog().debug({ path: codexFolder }, 'assistant_detected_codex'); return 'codex'; - } catch { - // fall through + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'ENOENT') throw error; } try { await access(claudeFolder); getLog().debug({ path: claudeFolder }, 'assistant_detected_claude'); return 'claude'; - } catch { - // fall through + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'ENOENT') throw error; }Also applies to: 33-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/config/resolve-assistant.ts` around lines 25 - 31, The try/catch blocks that call access(codexFolder) and access(howtoFolder) currently swallow all errors; update both to only ignore errors with error.code === 'ENOENT' and rethrow (or propagate) others so permission or I/O errors fail fast. Concretely, inside the catch for the codexFolder and howtoFolder probes, inspect the caught error (e) and if e?.code !== 'ENOENT' throw e (or return a failing result) otherwise continue falling through; keep getLog().debug and the existing 'codex'/'howto' returns unchanged.
48-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
repoPathwhen loading config.Line 49 calls
loadConfig()without the target repository path, so repo-specific assistant config can be skipped during registration.Proposed fix
- const config = await loadConfig(); + const config = await loadConfig(repoPath);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/config/resolve-assistant.ts` around lines 48 - 50, The call to loadConfig() in resolve-assistant.ts ignores the repository path so repo-specific assistant config may be missed; update the loadConfig invocation (the loadConfig function import and its usage) to pass the repoPath argument (e.g., loadConfig(repoPath)) and ensure repoPath is available in the scope where resolve-assistant determines config.assistant so repository-specific settings are loaded during assistant registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/core/src/config/resolve-assistant.ts`:
- Around line 25-31: The try/catch blocks that call access(codexFolder) and
access(howtoFolder) currently swallow all errors; update both to only ignore
errors with error.code === 'ENOENT' and rethrow (or propagate) others so
permission or I/O errors fail fast. Concretely, inside the catch for the
codexFolder and howtoFolder probes, inspect the caught error (e) and if e?.code
!== 'ENOENT' throw e (or return a failing result) otherwise continue falling
through; keep getLog().debug and the existing 'codex'/'howto' returns unchanged.
- Around line 48-50: The call to loadConfig() in resolve-assistant.ts ignores
the repository path so repo-specific assistant config may be missed; update the
loadConfig invocation (the loadConfig function import and its usage) to pass the
repoPath argument (e.g., loadConfig(repoPath)) and ensure repoPath is available
in the scope where resolve-assistant determines config.assistant so
repository-specific settings are loaded during assistant registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 736dd089-98e5-4cec-b1db-6441c169c459
📒 Files selected for processing (1)
packages/core/src/config/resolve-assistant.ts
loadConfig() without a path only merges the global config; the repo's own .archon/config.yaml (which can set assistant: pi at the project level) is silently skipped. Pass repoPath so repo-level config is honored during registration. Add a call-contract assertion in resolve-assistant.test.ts so a future regression that drops the path is caught. Surfaced by CodeRabbit review on #1729.
…fig-loader chain @archon/core/config/index.ts does `export * from './config-loader'`, which forces eager loading of config-loader.ts (and its top-level @archon/providers import) at every site that imports from @archon/core/config. The three forge adapters were importing the helper through that barrel, which pulled in the binary-resolver chain and broke @archon/adapters tests on Linux (mocked @archon/paths without BUNDLED_IS_BINARY). Add a dedicated './config/resolve-assistant' subpath export and switch the three forge adapters to it. Only resolve-assistant.ts is loaded — no transitive @archon/providers at module-load time.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/adapters/src/community/forge/gitlab/adapter.ts (1)
596-596:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAssistant resolution runs too early for new registrations.
Line 596 calls
resolveDefaultAssistant(canonicalPath)before the repository is cloned. For new codebases, checked-in SDK folders and repo config cannot be detected yet, so fallback assistant selection is used unexpectedly. Move resolution after clone, or recalculate and persist it once clone completes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/src/community/forge/gitlab/adapter.ts` at line 596, The call to resolveDefaultAssistant(canonicalPath) occurs too early (before the repository is cloned) causing incorrect assistant selection for new repos; move the resolution so it happens after the clone completes (or re-run and persist it immediately after clone finishes). Specifically, locate the code that performs the repo clone (the clone/checkout routine used in this adapter) and either remove the pre-clone resolveDefaultAssistant(canonicalPath) call or add a follow-up call that recalculates and persists the selected assistant once cloning is done; ensure you use the same canonicalPath and any repo metadata produced by the clone to compute the correct assistant and persist that value back to whatever structure currently holds ai_assistant_type.packages/adapters/src/forge/github/adapter.ts (1)
624-624:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPre-clone assistant resolution misses repo-driven signals.
Line 624 resolves assistant type before
ensureRepoReady()has cloned the repository. On first-time codebase creation,.codex/.claudeand repo config aren’t present yet, so the resolver can’t honor intended precedence. Resolve after clone (or updateai_assistant_typeimmediately after clone for new codebases).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/src/forge/github/adapter.ts` at line 624, The ai_assistant_type is being computed too early with resolveDefaultAssistant(canonicalPath) before the repo is cloned by ensureRepoReady(), so repo signals like .codex/.claude and repo config are missed; move the call to resolveDefaultAssistant(canonicalPath) so it runs after ensureRepoReady() completes (or, if cloning occurs only for new codebases, set ai_assistant_type once more immediately after the clone/update step), updating any variable assignment that currently uses ai_assistant_type to use the post-clone value; target the resolveDefaultAssistant, ensureRepoReady, and ai_assistant_type sites in the function to implement this change.packages/adapters/src/community/forge/gitea/adapter.ts (1)
646-646:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftResolve assistant after repository materialization.
Line 646 resolves from
canonicalPathbeforeensureRepoReady()clones the repo. On first-time registrations,.codex/.claudeand repo config are not available yet, so assistant selection can still fall through to fallback behavior. Resolve after clone (or recompute and update for new codebases).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/src/community/forge/gitea/adapter.ts` at line 646, The code calls resolveDefaultAssistant(canonicalPath) too early — move the ai_assistant_type resolution to after the repository is materialized (after ensureRepoReady() / the clone completes) so that .codex/.claude and repo config are available; alternatively, if you must resolve earlier, recompute and update the stored ai_assistant_type once ensureRepoReady()/materializeRepo completes. Update the logic around resolveDefaultAssistant, ensureRepoReady, and the field ai_assistant_type to perform the resolution post-clone (or trigger a re-resolve and persist the new value for new codebases).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/adapters/src/community/forge/gitea/adapter.ts`:
- Line 646: The code calls resolveDefaultAssistant(canonicalPath) too early —
move the ai_assistant_type resolution to after the repository is materialized
(after ensureRepoReady() / the clone completes) so that .codex/.claude and repo
config are available; alternatively, if you must resolve earlier, recompute and
update the stored ai_assistant_type once ensureRepoReady()/materializeRepo
completes. Update the logic around resolveDefaultAssistant, ensureRepoReady, and
the field ai_assistant_type to perform the resolution post-clone (or trigger a
re-resolve and persist the new value for new codebases).
In `@packages/adapters/src/community/forge/gitlab/adapter.ts`:
- Line 596: The call to resolveDefaultAssistant(canonicalPath) occurs too early
(before the repository is cloned) causing incorrect assistant selection for new
repos; move the resolution so it happens after the clone completes (or re-run
and persist it immediately after clone finishes). Specifically, locate the code
that performs the repo clone (the clone/checkout routine used in this adapter)
and either remove the pre-clone resolveDefaultAssistant(canonicalPath) call or
add a follow-up call that recalculates and persists the selected assistant once
cloning is done; ensure you use the same canonicalPath and any repo metadata
produced by the clone to compute the correct assistant and persist that value
back to whatever structure currently holds ai_assistant_type.
In `@packages/adapters/src/forge/github/adapter.ts`:
- Line 624: The ai_assistant_type is being computed too early with
resolveDefaultAssistant(canonicalPath) before the repo is cloned by
ensureRepoReady(), so repo signals like .codex/.claude and repo config are
missed; move the call to resolveDefaultAssistant(canonicalPath) so it runs after
ensureRepoReady() completes (or, if cloning occurs only for new codebases, set
ai_assistant_type once more immediately after the clone/update step), updating
any variable assignment that currently uses ai_assistant_type to use the
post-clone value; target the resolveDefaultAssistant, ensureRepoReady, and
ai_assistant_type sites in the function to implement this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 672b772e-2dd0-4a2a-adef-229649e71744
📒 Files selected for processing (4)
packages/adapters/src/community/forge/gitea/adapter.tspackages/adapters/src/community/forge/gitlab/adapter.tspackages/adapters/src/forge/github/adapter.tspackages/core/package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ai_assistant_type='claude'regardless of the user's configured assistant.clone.tsalready did the right resolution (config + SDK folder detection), but the three forgecreateCodebasecall sites passed noai_assistant_typeat all. Issue Project registration falls back to Claude instead of configured Pi provider #1580.assistant: piin.archon/config.yamlwho first interacted with Archon by@archon-mentioning on a GitHub PR would get claude bound to that codebase forever, with no UI surface to discover or fix the mismatch.resolveDefaultAssistant(repoPath)helper intopackages/core/src/config/resolve-assistant.tswith the precedence used byclone.ts(.codex/.claudefolder →loadConfig().assistant→ first built-in provider →'claude').clone.tsand the three forge adapters now call the helper.createCodebasestays a thin DB function with the simple?? 'claude'fallback — no dynamic-import config-loading inside the DB layer. The orchestrator path (which already resolved vialoadConfigbefore callingcreateCodebase) is untouched.UX Journey
Before
```
User Forge adapter DB
──── ───────────── ──
@archon on PR ────────────▶ createCodebase({
name, repo_url,
default_cwd
}) ────────────────────▶ INSERT ai_assistant_type='claude' ← always
(silently ignores config.assistant)
```
After
```
User Forge adapter resolve-assistant DB
──── ───────────── ───────────────── ──
@archon on PR ────────────▶ resolveDefaultAssistant ─▶ .codex? .claude?
loadConfig().assistant?
first builtIn provider? ai_assistant_type=resolved ← respects user config
createCodebase({ 'claude'
ai_assistant_type: ─▶
resolved
}) ─────────────────────────────────────────────▶ INSERT
```
Architecture Diagram
Before
```
clone.ts ─── inline 27-line block ──▶ getRegisteredProviders + loadConfig + access(.codex/.claude)
github/adapter.ts ───────────────────▶ createCodebase (no ai_assistant_type)
gitlab/adapter.ts ───────────────────▶ createCodebase (no ai_assistant_type)
gitea/adapter.ts ───────────────────▶ createCodebase (no ai_assistant_type)
```
After
```
clone.ts ──────────────────[
]──▶ resolveDefaultAssistant() ─[+]──▶ createCodebase({ ai_assistant_type })]──▶ resolveDefaultAssistant() ─[+]──▶ createCodebase({ ai_assistant_type })github/adapter.ts ─────────[
gitlab/adapter.ts ─────────[
]──▶ resolveDefaultAssistant() ─[+]──▶ createCodebase({ ai_assistant_type })]──▶ resolveDefaultAssistant() ─[+]──▶ createCodebase({ ai_assistant_type })gitea/adapter.ts ─────────[
resolve-assistant.ts [+]
resolve-assistant.test.ts [+]
```
Connection inventory:
clone.tsresolveDefaultAssistantforge/github/adapter.tsresolveDefaultAssistantai_assistant_typecommunity/forge/gitlab/adapter.tsresolveDefaultAssistantcommunity/forge/gitea/adapter.tsresolveDefaultAssistantresolve-assistant.ts@archon/providersdb/codebases.ts@archon/providers?? 'claude'fallbackdb/codebases.tsconfig-loaderLabel Snapshot
risk: lowsize: Mcorecore:config,adapters:github,adapters:gitlab,adapters:giteaTest plan
```bash
bun run type-check # clean
bun run lint # clean (--max-warnings 0)
bun run format:check # clean
bun run test # only pre-existing macOS telegram-markdown failures (unrelated, present on dev)
```
resolve-assistant.test.tscover: SDK folder detection, config preference, registry fallback, hardcoded'claude'fallback, folder-vs-config precedenceSecurity Impact
access()calls on.codex/.claudethat clone.ts already did)Compatibility / Migration
Human Verification
BUNDLED_IS_BINARYmissing from mocked@archon/paths) and confirmed the dynamic-import fix preserves the originalclone.tslazy-loading pattern.'claude'; SDK folder always wins over configured assistant; community-only registry →'claude'.clone.test.tsintegration tests.Side Effects / Blast Radius
.codexfolder in their forge-cloned repo butassistant: claudein config will now get codex (the folder wins). This matches existingclone.tsbehavior; the forge paths just lacked the resolution entirely.Rollback Plan
'claude'(the pre-existing Project registration falls back to Claude instead of configured Pi provider #1580 behavior). No data migration needed.Risks and Mitigations
bun testbatch that mock@archon/pathscould be affected by the new@archon/providersimport chain.@archon/providersinside the helper function body (matching the originalclone.tspattern). Verified locally that@archon/adaptersand@archon/servertest packages run clean.Credit
This work supersedes #1700 (closed). Thanks to @kagura-agent for the initial investigation and the diagnosis that surfaced the underlying mock.module pollution on Linux. The
config-loader.test.tsswitch to mockingfs/promises(instead ofmock.module('./config-loader')) is preserved from their work — it's a real cross-platform improvement.Closes #1580.
Summary by CodeRabbit
New Features
Configuration
Tests