fix(core): use loadConfig() for default assistant in new conversations

[coleam00]

Summary

• Problem: getOrCreateConversation() read process.env.DEFAULT_AI_ASSISTANT ?? 'claude' directly, completely bypassing the config system. The default assistant saved via the Settings page was never consulted when creating new conversations.
• Why it matters: Any user who configures a default assistant via the Settings UI and lacks a matching env var will see their preference silently ignored — a regression every time a new Web conversation is opened.
• What changed: getOrCreateConversation() now calls loadConfig(), which applies the full priority chain: defaults → global config file (Settings page) → env var override. The DEFAULT_AI_ASSISTANT env var still wins when set.
• What did not change: Priority order, caller signatures, Settings UI, any other conversation field.

UX Journey

Before

User (Settings page)        getOrCreateConversation()         Result
────────────────────        ─────────────────────────         ──────
saves defaultAssistant=claude ──▶  reads process.env.DEFAULT_AI_ASSISTANT
                                    (unset or 'codex')
                            ◀──── creates conversation with ai_assistant_type='codex'
User sees Codex (wrong) ◀──

After

User (Settings page)        getOrCreateConversation()         loadConfig()
────────────────────        ─────────────────────────         ────────────
saves defaultAssistant=claude ──▶  [calls loadConfig()]  ──▶  1. defaults (claude)
                                                               2. [global config: claude] ← Settings
                                                               3. env override (if set)
                            ◀──── creates conversation with ai_assistant_type='claude'
User sees Claude (correct) ◀──

Architecture Diagram

Before

getOrCreateConversation()
  └── process.env.DEFAULT_AI_ASSISTANT ?? 'claude'   (direct env read)

After

getOrCreateConversation()
  └── [~] loadConfig()    (config priority chain)
        ├── getDefaults()           → 'claude'
        ├── loadGlobalConfig()      → Settings page value
        └── applyEnvOverrides()     → DEFAULT_AI_ASSISTANT (highest priority)

Connection inventory:

From	To	Status	Notes
conversations.ts	process.env	removed	Direct env read replaced
conversations.ts	config-loader.ts	new	loadConfig() call added

Label Snapshot

• Risk: risk: low
• Size: size: XS
• Scope: core
• Module: core:db

Change Metadata

• Change type: bug
• Primary scope: core

Linked Issue

• Closes New conversations ignore saved default assistant and use DEFAULT_AI_ASSISTANT #1172

Validation Evidence (required)

bun run validate
# type-check: ✅ (all 10 packages)
# lint:       ✅ (0 errors, 0 warnings)
# format:     ✅
# tests:      ✅ (all packages, 17 passed in conversations.test.ts)

• All checks passed on first run with no modifications during validation.
• New test 'uses saved global config defaultAssistant when no env var' verifies the fix explicitly.

Security Impact (required)

• New permissions/capabilities? No
• New external network calls? No
• Secrets/tokens handling changed? No
• File system access scope changed? No

Compatibility / Migration

• Backward compatible? Yes — DEFAULT_AI_ASSISTANT env var still takes highest priority via applyEnvOverrides() inside loadConfig().
• Config/env changes? No
• Database migration needed? No

Human Verification (required)

• Verified scenarios: CI-equivalent validation suite passed (bun run validate)
• Edge cases checked: env var override still wins when set; no env var + saved config → saved config wins; no env var + no saved config → default 'claude'
• What was not verified: Manual end-to-end browser test against a live server

Side Effects / Blast Radius (required)

• Affected subsystems/workflows: getOrCreateConversation() is called from web routes, orchestrator, CLI, and platform adapters — all now read the correct assistant type on first creation.
• Potential unintended effects: loadConfig() reads ~/.archon/config.yaml on first call (subsequent calls hit in-memory cache); negligible overhead.
• Guardrails/monitoring: loadConfig() throws on invalid provider config, surfacing misconfiguration early.

Rollback Plan (required)

• Fast rollback command/path: git revert 8a4b8a75 — single-commit revert restores the direct env var read.
• Feature flags or config toggles: None needed; behavior controlled by env var / config file as before.
• Observable failure symptoms: New conversations created with wrong ai_assistant_type (visible in API response or DB).

Risks and Mitigations

• Risk: loadConfig() adds a (cached) file read on every new conversation creation.
  • Mitigation: loadGlobalConfig() is memoized; the overhead is a cache lookup after the first call. Acceptable given it only runs at conversation creation time, not on every message.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 697a087e-1fcb-4922-8f81-58df889cf589

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch archon/task-fix-1172-default-assistant

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coleam00]

🔍 Comprehensive PR Review

PR: #1258 — fix(core): use loadConfig() for default assistant in new conversations
Reviewed by: 3 specialized agents (code-review, error-handling, test-coverage)
Date: 2026-04-16

---

Summary

Clean, targeted single-line fix that replaces a direct process.env.DEFAULT_AI_ASSISTANT read in getOrCreateConversation() with a call to loadConfig(), ensuring the global config defaultAssistant (set via the Settings UI) is respected when new conversations are created. All three agents returned APPROVE verdicts. No bugs, no type safety issues, no blocking findings.

Verdict: ✅ APPROVE

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	1 (awareness only — no fix required)
🟢 LOW	6 (cosmetic / optional)

---

🟡 Medium Issues (No fix required before merge)

loadConfig() throws not logged at call site

📍 packages/core/src/db/conversations.ts:76-77

loadConfig() can throw on invalid provider config, propagating with no log breadcrumb in the db.conversations logger. The error message is descriptive and callers surface it in most paths. The error-handling agent explicitly recommends leaving as-is — consistent with CLAUDE.md fail-fast design and the scope document's explicit endorsement.

---

🟢 Low Issues (Optional cosmetic)

View 5 low-priority suggestions

Issue	Location	Suggestion
Redundant mockConfigAssistant reset in afterEach	conversations.test.ts:62	Remove dead assignment — beforeEach always overwrites it first
Misleading test name: "falls back to claude"	conversations.test.ts:192	Rename to 'uses config default assistant when codebase not found' — the value comes from loadConfig(), not a hardcoded fallback
Env var test comment could be expanded	conversations.test.ts:144-148	Add comment explaining why both process.env and mockConfigAssistant must be set in sync
Missing explicit type annotation on mockConfigAssistant	conversations.test.ts:15	Add : string for CLAUDE.md type-safety compliance
Redundant afterEach reset (corroboration)	conversations.test.ts:55-63	Same finding as first item above, independently flagged by test-coverage agent

---

✅ What's Good

• Correct fix scope: loadConfig() called without repoPath — the codebase repo-level assistant override is handled separately via DB codebase lookup, so no double-application.
• Caching is safe: loadConfig() uses cachedGlobalConfig; no filesystem hit on every conversation creation.
• Batch split is exactly right: conversations.test.ts now runs in its own bun test invocation, correctly preventing mock.module() pollution with config-loader.test.ts.
• New test is meaningful: 'uses saved global config defaultAssistant when no env var' directly exercises the bug — proves that loadConfig() returning 'codex' results in ai_assistant_type = 'codex' in the INSERT.
• Improved error detectability: Old code silently accepted any string (including invalid provider names). New code validates against the registry and throws with a descriptive message on invalid input.
• All 7 tests are outcome-based (asserting SQL parameters) — resilient to refactoring.

---

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/82bbb50cacf7edac3a9ae995771c017a/review/

[coleam00]

Self-Fix Report: Review Findings Addressed

All actionable findings from the three review agents have been addressed in commit 1b857fb1.

Changes Made (packages/core/src/db/conversations.test.ts)

Finding	Severity	Source	Action
mockConfigAssistant lacks explicit : string type annotation	LOW	error-handling	Fixed — added : string per CLAUDE.md type safety
Redundant mockConfigAssistant reset in afterEach	LOW	code-review + test-coverage	Fixed — removed dead assignment; replaced with clarifying comment (beforeEach owns the reset)
Env var test comment needs expansion	LOW	code-review	Fixed — expanded comment to explain why both process.env and mockConfigAssistant must be set in sync
Misleading test name: "falls back to claude when codebase not found"	LOW	code-review	Fixed — renamed to 'uses config default assistant when codebase not found'

Skipped

Finding	Severity	Reason
loadConfig() throws not logged at call site (conversations.ts:76)	MEDIUM	The review agent itself recommended leaving as-is (Option B). Fail-fast design is intentional per CLAUDE.md — loadConfig() throws produce descriptive messages; a catch-and-rethrow wrapper would violate YAGNI/KISS without meaningful benefit.

Validation

All checks pass after the fixes:

• bun run type-check: ✅ All 10 packages
• bun run lint: ✅ No warnings
• bun run test: ✅ All tests pass

[coleam00]

Archon PR Validation Report

Verdict: ✅ APPROVE

Summary

This PR correctly fixes a bug where getOrCreateConversation() read process.env.DEFAULT_AI_ASSISTANT directly, completely bypassing the loadConfig() priority chain. The Settings page default assistant preference was silently ignored for every new conversation. The fix is a minimal 2-line production change that routes through the existing, well-tested config system.

Bug Confirmation

Claim	Main	Feature
Direct env var read bypasses config system	Confirmed	Fixed
Settings page value is dead code for conversation creation	Confirmed	Fixed
Tests don't cover config system path	Confirmed	Fixed

Fix Quality: 5/5

• 1 import + 1 line swap in production code
• Leverages existing loadConfig() infrastructure
• Correct mock.module() batch isolation per CLAUDE.md
• Full CLAUDE.md compliance

Issues

No blocking issues found.

---

Validated by archon-validate-pr workflow