feat: Claude Compatible Providers System #629
Conversation
- Rename ClaudeApiProfile to ClaudeCompatibleProvider with models[] array - Each ProviderModel has mapsToClaudeModel field for Claude tier mapping - Add providerType field for provider-specific icons (glm, minimax, openrouter) - Add thinking level support for provider models in phase selectors - Show all mapped Claude models per provider model (e.g., "Maps to Haiku, Sonnet, Opus") - Add Bulk Replace feature to switch all phases to a provider at once - Hide Bulk Replace button when no providers are enabled - Fix project-level phaseModelOverrides not persisting after refresh - Fix deleting last provider not persisting (remove empty array guard) - Add getProviderByModelId() helper for all SDK routes - Update all routes to pass provider config for provider models - Update terminology from "profiles" to "providers" throughout UI - Update documentation to reflect new provider system
Resolved conflict in use-project-settings-loader.ts: - Keep React Query approach from upstream - Add phaseModelOverrides loading for provider model persistence - Update both currentProject and projects array to keep in sync
1. AtomicWriter Race Condition Fix (libs/utils/src/atomic-writer.ts):
- Changed temp file naming from Date.now() to Date.now() + random hex
- Uses crypto.randomBytes(4).toString('hex') for uniqueness
- Prevents ENOENT errors when multiple concurrent writes happen
within the same millisecond
2. Bulk Replace "Anthropic Direct" Reset (both dialogs):
- When selecting "Anthropic Direct", now uses DEFAULT_PHASE_MODELS
- Properly resets thinking levels and other settings to defaults
- Added thinkingLevel to the change detection comparison
- Affects both global and project-level bulk replace dialogs
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Claude-compatible provider system and provider-aware model resolution. Introduces settings migration (v5→v6), provider lookup helpers, provider-aware SDK/environment wiring, extensive UI for provider and phase-model management, and threads provider+credentials through server routes and services. Changes
Sequence Diagram(s)sequenceDiagram
participant Route as Route Handler
participant Helper as Settings Helpers
participant Service as Settings Service
participant SDK as Claude SDK
Route->>Helper: getPhaseModelWithOverrides(phase, settingsService?, projectPath?)
Helper->>Service: getGlobalSettings()
Helper->>Service: getProjectSettings(projectPath)
alt Project override exists
Helper->>Helper: use project override phaseModel
else
Helper->>Helper: use global phaseModel (DEFAULT_PHASE_MODELS fallback)
end
Helper->>Helper: if phaseModel.providerId -> resolve provider
alt provider found
Helper->>Service: getCredentials(provider.id)
Helper->>Route: return { phaseModel, provider, credentials, isProjectOverride }
else
Helper->>Route: return { phaseModel, provider: undefined, credentials: undefined, isProjectOverride }
end
Route->>SDK: executeQuery({ model: effectiveModel, claudeCompatibleProvider: provider, credentials })
SDK->>SDK: buildEnv(providerConfig, credentials)
alt providerConfig present
SDK->>SDK: use provider.baseUrl, apiKeySource, model mappings
else
SDK->>SDK: use legacy Anthropic endpoint / defaults
end
SDK->>SDK: make API call
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Summary of ChangesHello @stefandevo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's flexibility by introducing a robust system for integrating third-party Claude-compatible AI model providers. It allows users to leverage a wider array of models and endpoints, optimizing for cost, performance, or specific use cases across different application phases and projects. The changes span across type definitions, server-side logic for model resolution and API key handling, and a comprehensive overhaul of the user interface for managing these new providers and their models. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
1. model-resolver tests:
- Unknown models now pass through unchanged (provider model support)
- Removed expectations for warnings on unknown models
- Updated case sensitivity and edge case tests accordingly
- Added tests for provider-like model names (GLM-4.7, MiniMax-M2.1)
2. atomic-writer tests:
- Updated regex to match new temp file format with random suffix
- Format changed from .tmp.{timestamp} to .tmp.{timestamp}.{hex}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Claude Compatible Providers' system, replacing the deprecated 'Claude API Profiles' for managing AI model endpoints. The changes involve updating server-side logic to use new helper functions like getProviderById, getPhaseModelWithOverrides, and getProviderByModelId for resolving provider configurations and credentials based on model IDs and phase settings, including project-level overrides. The buildEnv function in claude-provider.ts is refactored to handle both legacy profiles and new providers, prioritizing the latter. A migration (v5 to v6) is added to settings-service.ts to automatically convert existing claudeApiProfiles to the new claudeCompatibleProviders format. On the UI side, the 'Claude' settings section is renamed to 'Models' and redesigned to manage these new providers, allowing users to configure custom providers with specific models and their Claude alias mappings. New UI components, including a 'Bulk Replace' dialog, enable users to apply provider models across all phases or override them per project. Review comments primarily suggest simplifying redundant ternary operators in several route files where settingsService is checked before calling getPhaseModelWithOverrides, as the function is designed to handle an undefined settingsService gracefully.
apps/server/src/routes/worktree/routes/generate-commit-message.ts
Outdated
Show resolved
Hide resolved
Address code review feedback on PR AutoMaker-Org#629: - Make settingsService parameter optional in getPhaseModelWithOverrides - Function now handles undefined settingsService gracefully by returning defaults - Remove redundant ternary checks in 4 call sites: - apps/server/src/routes/context/routes/describe-file.ts - apps/server/src/routes/context/routes/describe-image.ts - apps/server/src/routes/worktree/routes/generate-commit-message.ts - apps/server/src/services/auto-mode-service.ts - Remove unused DEFAULT_PHASE_MODELS imports where applicable
- Update model-resolver.test.ts to expect unknown models to pass through unchanged (supports ClaudeCompatibleProvider models like GLM-4.7) - Remove warning expectations for unknown models (valid for providers) - Add missing getCredentials and getGlobalSettings mocks to ideation-service.test.ts for settingsService
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/ui/src/components/views/project-settings-view/project-claude-section.tsx (1)
107-107: Update label to match provider terminology.The label still uses "Profile" terminology while the rest of the section header and description have been updated to "Provider" (lines 98, 101). This creates a minor inconsistency in the UI.
📝 Proposed fix
- <Label className="text-sm font-medium">Active Profile for This Project</Label> + <Label className="text-sm font-medium">Active Provider for This Project</Label>apps/server/src/routes/worktree/routes/generate-commit-message.ts (1)
160-210: Honor thinkingLevel for commit message generation.The resolved
thinkingLevelis currently ignored, so user-configured thinking for this phase won’t apply.💡 Suggested fix
- const { model } = resolvePhaseModel(phaseModelEntry); + const { model, thinkingLevel } = resolvePhaseModel(phaseModelEntry); … - const stream = aiProvider.executeQuery({ + const stream = aiProvider.executeQuery({ prompt: effectivePrompt, model: bareModel, cwd: worktreePath, systemPrompt: effectiveSystemPrompt, maxTurns: 1, allowedTools: [], readOnly: true, + thinkingLevel, claudeCompatibleProvider, // Pass provider for alternative endpoint configuration credentials, // Pass credentials for resolving 'credentials' apiKeySource });apps/server/src/services/ideation-service.ts (1)
226-239: PassclaudeCompatibleProvidertoexecuteOptionsin ideation calls.The service retrieves a provider via
ProviderFactory.getProviderForModel()on line 222 but does not pass it to theexecuteOptionsobject passed toprovider.executeQuery(). This prevents Claude-compatible provider routing for ideation sessions. Other routes likesync-spec.tsandgenerate-commit-message.tscorrectly passclaudeCompatibleProviderto their execute options. Add the retrieved provider toexecuteOptions:const executeOptions: ExecuteOptions = { prompt: message, model: bareModel, originalModel: modelId, cwd: projectPath, systemPrompt: sdkOptions.systemPrompt, maxTurns: 1, abortController: activeSession.abortController!, conversationHistory: conversationHistory.length > 0 ? conversationHistory : undefined, claudeCompatibleProvider: provider, credentials, };apps/server/src/routes/suggestions/generate-suggestions.ts (1)
174-205: Resolve provider configuration whenmodelOverridepoints to a provider model.Line 181-191 treats overrides as direct API and never resolves a provider by model ID. If callers pass a provider model (e.g., GLM/MiniMax/OpenRouter), this will hit the direct API with an unsupported model and fail. Please resolve provider + credentials for overrides, similar to the provider resolution in other routes.
🐛 Proposed fix (resolve provider for overrides)
import { getAutoLoadClaudeMdSetting, getPromptCustomization, getPhaseModelWithOverrides, + getProviderByModelId, } from '../../lib/settings-helpers.js'; if (modelOverride) { // Use explicit override - resolve the model string const resolved = resolvePhaseModel({ model: modelOverride, thinkingLevel: thinkingLevelOverride, }); model = resolved.model; thinkingLevel = resolved.thinkingLevel; - // For overrides, just get credentials without a specific provider - credentials = await settingsService?.getCredentials(); + if (settingsService) { + const providerResult = await getProviderByModelId( + modelOverride, + settingsService, + '[Suggestions]' + ); + provider = providerResult.provider; + model = providerResult.resolvedModel ?? model; + credentials = providerResult.credentials ?? (await settingsService.getCredentials()); + } else { + credentials = await settingsService?.getCredentials(); + } }
🤖 Fix all issues with AI agents
In `@apps/server/src/lib/settings-helpers.ts`:
- Around line 521-541: The fallback uses a hardcoded 'sonnet' for missing phase
models; instead, change the fallback in the phase model resolution (the block
that sets phaseModel when !phaseModel) to use DEFAULT_PHASE_MODELS[phase] (and
only fall back to 'sonnet' if that entry is also missing) so per-phase canonical
defaults are preserved; update the logger message in the same block to reflect
the actual default used. Reference: phaseModel, globalPhaseModels,
DEFAULT_PHASE_MODELS, projectSettings.phaseModelOverrides,
settingsService.getProjectSettings.
In `@apps/server/src/routes/app-spec/generate-features-from-spec.ts`:
- Around line 122-139: If settingsService returns a phaseModelEntry but provider
is undefined, guard before calling resolvePhaseModel by either switching
phaseModelEntry to DEFAULT_PHASE_MODELS.featureGenerationModel or clearing any
provider-specific identifier (e.g., providerId) so an invalid provider model ID
isn't propagated to the Anthropic/direct endpoint; update the logic around
getPhaseModelWithOverrides, phaseModelEntry, provider and resolvePhaseModel to
perform this fallback/clear and then call resolvePhaseModel with the safe phase
model.
In `@apps/server/src/routes/backlog-plan/generate-plan.ts`:
- Around line 124-156: When an explicit model override (variable model ->
effectiveModel) is passed you must also resolve its provider and credentials
instead of assuming a direct API call; update the first branch (where
effectiveModel is truthy) to lookup provider info the same way the settings
branch does (e.g., call the provider-resolution helper used elsewhere such as
getPhaseModelWithOverrides or the ProviderFactory resolver) and populate
claudeCompatibleProvider and credentials accordingly so ProviderFactory routes
to Claude-compatible endpoints when needed (ensure you update the variables
effectiveModel, thinkingLevel, claudeCompatibleProvider, and credentials).
In `@apps/server/src/routes/context/routes/describe-image.ts`:
- Around line 277-299: The current flow assumes the selected model can accept
image blocks; after resolving the phase model (phaseModelEntry via
getPhaseModelWithOverrides and resolvePhaseModel) add a capability check (e.g.,
phaseModelEntry.supportsVision or a supportsVision flag returned by
resolvePhaseModel) and gate any image-block prompt construction on that flag. If
supportsVision is false (or missing), switch to a text-only prompt path or
return a clear error indicating the chosen model does not support vision,
ensuring any later code that builds prompts or calls the model uses the
text-only branch; update logging (logger.info) to reflect the chosen
capability/provider to aid debugging.
In
`@apps/ui/src/components/views/project-settings-view/project-bulk-replace-dialog.tsx`:
- Around line 99-118: getClaudeModelAlias currently searches only
enabledProviders, causing overrides that reference disabled providers to
fallback to 'sonnet'; change it to look up providers from the full provider list
(the one containing both enabled and disabled providers used by the component)
when resolving provider.models?.find(...) inside getClaudeModelAlias, while
keeping the dropdown filtering logic separate; also add that full provider list
variable to the memo/useMemo dependency array(s) that compute previews so the
mapping updates when provider state changes.
In
`@apps/ui/src/components/views/project-settings-view/project-models-section.tsx`:
- Around line 107-125: The getModelDisplayName function currently maps only
short IDs (haiku/sonnet/opus) to friendly names and thus shows raw IDs for
canonical claude-* model IDs; update the mapping in getModelDisplayName (and/or
normalize entry.model) to include the canonical keys 'claude-haiku',
'claude-sonnet', and 'claude-opus' mapping to 'Claude Haiku', 'Claude Sonnet',
'Claude Opus' respectively so the UI displays friendly names for those IDs while
keeping the existing provider lookup via claudeCompatibleProviders and
PhaseModelEntry behavior intact.
In
`@apps/ui/src/components/views/settings-view/providers/claude-settings-tab/api-profiles-section.tsx`:
- Around line 222-248: The providerData object currently hardcodes enabled: true
which re-enables disabled providers on edit; change this to preserve the
existing provider's enabled state when editing by looking up the existing
provider from claudeCompatibleProviders (e.g. find where p.id ===
editingProviderId) and use its enabled value when editingProviderId is present,
otherwise default to true for new providers; keep the rest of the fields the
same and then call updateClaudeCompatibleProvider/addClaudeCompatibleProvider as
before.
In `@apps/ui/src/hooks/use-settings-migration.ts`:
- Around line 721-725: The migration currently drops claudeCompatibleProviders
because it isn't handled in parseLocalStorageSettings or mergeSettings; update
parseLocalStorageSettings to read/parse claudeCompatibleProviders from
localStorage (falling back to [] if missing) and update mergeSettings to merge
existing store.claudeCompatibleProviders with
parsedLocal.claudeCompatibleProviders (preserving existing provider entries and
deduplicating by provider id or unique key) instead of replacing it with an
empty array; reference the symbols parseLocalStorageSettings, mergeSettings, and
claudeCompatibleProviders when making the changes.
In `@docs/UNIFIED_API_KEY_PROFILES.md`:
- Around line 21-33: Docs and the TypeScript interface disagree: update the docs
in UNIFIED_API_KEY_PROFILES.md so the type snippets reflect the actual
interface: mark providerType, apiKeySource, and models as required (remove the
"optional" markers), include "ultrathink" in the thinkingLevel enum examples,
and show the model field as type ModelId (not a plain string); reference the
ClaudeCompatibleProvider interface and the fields providerType, apiKeySource,
models, thinkingLevel, and model when making these changes.
- Around line 211-214: The docs currently show Project.phaseModelOverrides as a
full PhaseModelConfig; update the documentation text and the example snippet to
reflect that phaseModelOverrides is Partial<PhaseModelConfig> (i.e., per-phase
entries may omit fields). Mention the exact type name PhaseModelConfig and the
property phaseModelOverrides on the Project interface so readers know it accepts
partial overrides rather than a complete config.
- Around line 152-163: The docs show an outdated return shape for
getPhaseModelWithOverrides; update the documented return type to match the
current contract which returns an object with keys phaseModel,
isProjectOverride, provider, and credentials (instead of
model/providerConfig/thinkingLevel/providerId); find the
getPhaseModelWithOverrides snippet in the docs and replace the old return
description with the new fields and their brief meanings (phaseModel,
isProjectOverride, provider, credentials) so the doc matches the actual function
signature and usage.
In `@libs/types/src/provider.ts`:
- Around line 5-10: The import of shared types (ThinkingLevel, ClaudeApiProfile,
ClaudeCompatibleProvider, Credentials) currently uses a relative path
('./settings.js'); update the import to use the `@automaker` namespace package per
guidelines (replace the relative import with the corresponding `@automaker/`*
settings module) so the same named symbols are imported from the shared package
instead of local file; ensure the import statement in provider.ts references the
`@automaker` settings module and retains the same type names.
In `@libs/types/src/settings.ts`:
- Around line 317-337: The defaultModels list contains duplicate ids (e.g.,
'GLM-4.7' and 'MiniMax-M2.1') with different mapsToClaudeModel values which
makes lookups using id ambiguous; update the provider model representation and
lookup logic: either collapse duplicates into a single entry with
mapsToClaudeModel as an array (e.g., mapsToClaudeModel: ['sonnet','opus']) in
the defaultModels arrays, or extend PhaseModelEntry to persist the resolved tier
(e.g., add a mappedTier or mapsToClaudeModels property) and change
getProviderByModelId and any code that uses mapsToClaudeModel/resolvedModel to
handle arrays or use the persisted mappedTier so lookups by id are
deterministic; search/adjust usages of mapsToClaudeModel, resolvedModel, and
getProviderByModelId to ensure UI keys and find() calls handle the new shape.
🧹 Nitpick comments (5)
apps/server/src/services/settings-service.ts (1)
34-37: Consider importing these types from@automaker/typesnamespace.The new types (
ClaudeApiProfile,ClaudeCompatibleProvider,ProviderModel) are imported from a relative path, but per coding guidelines, shared types should be imported from the@automaker/*namespace. These types appear to be defined inlibs/types/src/settings.ts.That said, this follows the existing import pattern in this file (lines 16-37), so it may be intentional if there's a local re-export structure. If the existing pattern is tech debt, consider consolidating all type imports to
@automaker/typesin a follow-up.apps/ui/src/components/views/settings-view/providers/claude-settings-tab/api-profiles-section.tsx (2)
54-56: Usesubstringinstead of deprecatedsubstr.
String.prototype.substr()is deprecated. Replace withsubstringorslicefor the same behavior.Suggested fix
function generateProviderId(): string { - return `provider-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + return `provider-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; }
417-439: Consider the UX when switching to a fixed provider type.When a user starts with "Custom Provider" and then selects "GLM" or "MiniMax" from the type dropdown, the form dynamically hides advanced options (API key source, auth token toggle, base URL). This is valid behavior, but could be surprising. Consider adding a brief tooltip or note explaining that some providers have fixed settings.
apps/ui/src/hooks/use-project-settings-loader.ts (1)
111-120: Consider a stable compare for phaseModelOverrides.
JSON.stringifycan be order-sensitive; a shallow key/value compare would avoid spurious updates.apps/server/src/routes/github/routes/validate-issue.ts (1)
170-205: Consider recomputing structured-output using the provider-resolved model.With provider models,
useStructuredOutputis currently decided before provider resolution and based on the original model. Claude-compatible providers likely support structured output, so treating them as non-structured forces fallback JSON parsing unnecessarily. Suggest moving the structured-output decision aftereffectiveModelis computed and usingisClaudeModel(effectiveModel)(orisCodexModel) instead.
| // Get model from phase settings with provider info | ||
| const { | ||
| phaseModel: phaseModelEntry, | ||
| provider, | ||
| credentials, | ||
| } = settingsService | ||
| ? await getPhaseModelWithOverrides( | ||
| 'featureGenerationModel', | ||
| settingsService, | ||
| projectPath, | ||
| '[FeatureGeneration]' | ||
| ) | ||
| : { | ||
| phaseModel: DEFAULT_PHASE_MODELS.featureGenerationModel, | ||
| provider: undefined, | ||
| credentials: undefined, | ||
| }; | ||
| const { model, thinkingLevel } = resolvePhaseModel(phaseModelEntry); |
There was a problem hiding this comment.
Guard against missing/disabled provider to avoid invalid model IDs
When provider is undefined, phaseModelEntry can still carry a provider model ID, which then gets sent to the direct Anthropic endpoint. For non‑Claude IDs this will fail. Consider falling back to the default phase model (or clearing providerId) when the provider cannot be resolved.
🛠️ Suggested fix
- const { model, thinkingLevel } = resolvePhaseModel(phaseModelEntry);
+ const effectivePhaseModel =
+ !provider && phaseModelEntry.providerId
+ ? DEFAULT_PHASE_MODELS.featureGenerationModel
+ : phaseModelEntry;
+ const { model, thinkingLevel } = resolvePhaseModel(effectivePhaseModel);🤖 Prompt for AI Agents
In `@apps/server/src/routes/app-spec/generate-features-from-spec.ts` around lines
122 - 139, If settingsService returns a phaseModelEntry but provider is
undefined, guard before calling resolvePhaseModel by either switching
phaseModelEntry to DEFAULT_PHASE_MODELS.featureGenerationModel or clearing any
provider-specific identifier (e.g., providerId) so an invalid provider model ID
isn't propagated to the Anthropic/direct endpoint; update the logic around
getPhaseModelWithOverrides, phaseModelEntry, provider and resolvePhaseModel to
perform this fallback/clear and then call resolvePhaseModel with the safe phase
model.
| // Get the model to use from settings or provided override with provider info | ||
| let effectiveModel = model; | ||
| let thinkingLevel: ThinkingLevel | undefined; | ||
| if (!effectiveModel) { | ||
| const settings = await settingsService?.getGlobalSettings(); | ||
| const phaseModelEntry = | ||
| settings?.phaseModels?.backlogPlanningModel || DEFAULT_PHASE_MODELS.backlogPlanningModel; | ||
| const resolved = resolvePhaseModel(phaseModelEntry); | ||
| let claudeCompatibleProvider: import('@automaker/types').ClaudeCompatibleProvider | undefined; | ||
| let credentials: import('@automaker/types').Credentials | undefined; | ||
|
|
||
| if (effectiveModel) { | ||
| // Use explicit override - just get credentials | ||
| credentials = await settingsService?.getCredentials(); | ||
| } else if (settingsService) { | ||
| // Use settings-based model with provider info | ||
| const phaseResult = await getPhaseModelWithOverrides( | ||
| 'backlogPlanningModel', | ||
| settingsService, | ||
| projectPath, | ||
| '[BacklogPlan]' | ||
| ); | ||
| const resolved = resolvePhaseModel(phaseResult.phaseModel); | ||
| effectiveModel = resolved.model; | ||
| thinkingLevel = resolved.thinkingLevel; | ||
| claudeCompatibleProvider = phaseResult.provider; | ||
| credentials = phaseResult.credentials; | ||
| } else { | ||
| // Fallback to defaults | ||
| const resolved = resolvePhaseModel(DEFAULT_PHASE_MODELS.backlogPlanningModel); | ||
| effectiveModel = resolved.model; | ||
| thinkingLevel = resolved.thinkingLevel; | ||
| } | ||
| logger.info('[BacklogPlan] Using model:', effectiveModel); | ||
| logger.info( | ||
| '[BacklogPlan] Using model:', | ||
| effectiveModel, | ||
| claudeCompatibleProvider ? `via provider: ${claudeCompatibleProvider.name}` : 'direct API' | ||
| ); |
There was a problem hiding this comment.
Resolve provider configuration for explicit model overrides.
Line 130-133 treats overrides as direct API and does not resolve provider/credentials. If a provider model ID is passed, ProviderFactory will route incorrectly and the Claude-compatible endpoint won’t be used. Please resolve provider info for overrides, similar to other routes.
🐛 Proposed fix (resolve provider for overrides)
import {
getAutoLoadClaudeMdSetting,
getPromptCustomization,
getPhaseModelWithOverrides,
+ getProviderByModelId,
} from '../../lib/settings-helpers.js';
if (effectiveModel) {
// Use explicit override - just get credentials
- credentials = await settingsService?.getCredentials();
+ if (settingsService) {
+ const providerResult = await getProviderByModelId(
+ effectiveModel,
+ settingsService,
+ '[BacklogPlan]'
+ );
+ claudeCompatibleProvider = providerResult.provider;
+ effectiveModel = providerResult.resolvedModel ?? effectiveModel;
+ credentials = providerResult.credentials ?? (await settingsService.getCredentials());
+ } else {
+ credentials = await settingsService?.getCredentials();
+ }
}🤖 Prompt for AI Agents
In `@apps/server/src/routes/backlog-plan/generate-plan.ts` around lines 124 - 156,
When an explicit model override (variable model -> effectiveModel) is passed you
must also resolve its provider and credentials instead of assuming a direct API
call; update the first branch (where effectiveModel is truthy) to lookup
provider info the same way the settings branch does (e.g., call the
provider-resolution helper used elsewhere such as getPhaseModelWithOverrides or
the ProviderFactory resolver) and populate claudeCompatibleProvider and
credentials accordingly so ProviderFactory routes to Claude-compatible endpoints
when needed (ensure you update the variables effectiveModel, thinkingLevel,
claudeCompatibleProvider, and credentials).
| // Get model from phase settings with provider info | ||
| const { | ||
| phaseModel: phaseModelEntry, | ||
| provider, | ||
| credentials, | ||
| } = settingsService | ||
| ? await getPhaseModelWithOverrides( | ||
| 'imageDescriptionModel', | ||
| settingsService, | ||
| cwd, | ||
| '[DescribeImage]' | ||
| ) | ||
| : { | ||
| phaseModel: DEFAULT_PHASE_MODELS.imageDescriptionModel, | ||
| provider: undefined, | ||
| credentials: undefined, | ||
| }; | ||
| const { model, thinkingLevel } = resolvePhaseModel(phaseModelEntry); | ||
|
|
||
| logger.info(`[${requestId}] Using model: ${model}`); | ||
| logger.info( | ||
| `[${requestId}] Using model: ${model}`, | ||
| provider ? `via provider: ${provider.name}` : 'direct API' | ||
| ); |
There was a problem hiding this comment.
Guard image-block prompts for non‑vision provider models.
Now that provider models can be selected, the current logic assumes all non-Cursor models accept image blocks. Provider models without vision support will fail. Please gate the image-block path using per-model capability flags (e.g., supportsVision) and fall back to a text-only prompt or return a clear error. Based on learnings, …
🤖 Prompt for AI Agents
In `@apps/server/src/routes/context/routes/describe-image.ts` around lines 277 -
299, The current flow assumes the selected model can accept image blocks; after
resolving the phase model (phaseModelEntry via getPhaseModelWithOverrides and
resolvePhaseModel) add a capability check (e.g., phaseModelEntry.supportsVision
or a supportsVision flag returned by resolvePhaseModel) and gate any image-block
prompt construction on that flag. If supportsVision is false (or missing),
switch to a text-only prompt path or return a clear error indicating the chosen
model does not support vision, ensuring any later code that builds prompts or
calls the model uses the text-only branch; update logging (logger.info) to
reflect the chosen capability/provider to aid debugging.
| // Get the Claude model alias from a PhaseModelEntry | ||
| const getClaudeModelAlias = (entry: PhaseModelEntry): ClaudeModelAlias => { | ||
| // Check if model string directly matches a Claude alias | ||
| if (entry.model === 'haiku' || entry.model === 'claude-haiku') return 'haiku'; | ||
| if (entry.model === 'sonnet' || entry.model === 'claude-sonnet') return 'sonnet'; | ||
| if (entry.model === 'opus' || entry.model === 'claude-opus') return 'opus'; | ||
|
|
||
| // If it's a provider model, look up the mapping | ||
| if (entry.providerId) { | ||
| const provider = enabledProviders.find((p) => p.id === entry.providerId); | ||
| if (provider) { | ||
| const model = provider.models?.find((m) => m.id === entry.model); | ||
| if (model?.mapsToClaudeModel) { | ||
| return model.mapsToClaudeModel; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Default to sonnet | ||
| return 'sonnet'; |
There was a problem hiding this comment.
Mapping ignores disabled providers used in overrides
getClaudeModelAlias only searches enabledProviders. If a project override references a disabled provider, the alias defaults to sonnet, and the preview/new mapping becomes inaccurate. Consider using the full provider list for lookups (while still filtering for the dropdown), and include it in the memo deps to avoid stale previews.
🛠️ Suggested fix
- const provider = enabledProviders.find((p) => p.id === entry.providerId);
+ const provider = (claudeCompatibleProviders || []).find((p) => p.id === entry.providerId);- }, [phaseModels, projectOverrides, selectedProviderConfig, enabledProviders]);
+ }, [phaseModels, projectOverrides, selectedProviderConfig, enabledProviders, claudeCompatibleProviders]);Also applies to: 193-193
🤖 Prompt for AI Agents
In
`@apps/ui/src/components/views/project-settings-view/project-bulk-replace-dialog.tsx`
around lines 99 - 118, getClaudeModelAlias currently searches only
enabledProviders, causing overrides that reference disabled providers to
fallback to 'sonnet'; change it to look up providers from the full provider list
(the one containing both enabled and disabled providers used by the component)
when resolving provider.models?.find(...) inside getClaudeModelAlias, while
keeping the dropdown filtering logic separate; also add that full provider list
variable to the memo/useMemo dependency array(s) that compute previews so the
mapping updates when provider state changes.
| export interface ClaudeCompatibleProvider { | ||
| id: string; // Unique identifier (UUID) | ||
| name: string; // Display name (e.g., "z.AI GLM") | ||
| baseUrl: string; // API endpoint URL | ||
| providerType?: string; // Provider type for icon/grouping (e.g., 'glm', 'minimax', 'openrouter') | ||
| apiKeySource?: ApiKeySource; // 'inline' | 'env' | 'credentials' | ||
| apiKey?: string; // API key (when apiKeySource = 'inline') | ||
| useAuthToken?: boolean; // Use ANTHROPIC_AUTH_TOKEN header | ||
| timeoutMs?: number; // Request timeout in milliseconds | ||
| disableNonessentialTraffic?: boolean; // Minimize non-essential API calls | ||
| enabled?: boolean; // Whether provider is active (default: true) | ||
| models?: ProviderModel[]; // Models exposed by this provider | ||
| } |
There was a problem hiding this comment.
Type snippets don’t match actual required fields
The docs mark providerType, apiKeySource, and models as optional, but the type defines them as required. Also, thinkingLevel includes ultrathink, and model is ModelId.
📝 Suggested doc fix
- providerType?: string; // Provider type for icon/grouping (e.g., 'glm', 'minimax', 'openrouter')
- apiKeySource?: ApiKeySource; // 'inline' | 'env' | 'credentials'
+ providerType: ClaudeCompatibleProviderType; // Provider type for icon/grouping
+ apiKeySource: ApiKeySource; // 'inline' | 'env' | 'credentials'
...
- models?: ProviderModel[]; // Models exposed by this provider
+ models: ProviderModel[]; // Models exposed by this provider- model: string; // Model ID or alias
- thinkingLevel?: ThinkingLevel; // 'none' | 'low' | 'medium' | 'high'
+ model: ModelId; // Model ID or alias
+ thinkingLevel?: ThinkingLevel; // 'none' | 'low' | 'medium' | 'high' | 'ultrathink'Also applies to: 56-60
🤖 Prompt for AI Agents
In `@docs/UNIFIED_API_KEY_PROFILES.md` around lines 21 - 33, Docs and the
TypeScript interface disagree: update the docs in UNIFIED_API_KEY_PROFILES.md so
the type snippets reflect the actual interface: mark providerType, apiKeySource,
and models as required (remove the "optional" markers), include "ultrathink" in
the thinkingLevel enum examples, and show the model field as type ModelId (not a
plain string); reference the ClaudeCompatibleProvider interface and the fields
providerType, apiKeySource, models, thinkingLevel, and model when making these
changes.
| export async function getPhaseModelWithOverrides( | ||
| phaseKey: PhaseModelKey, | ||
| settingsService: SettingsService, | ||
| projectPath?: string, | ||
| logPrefix?: string | ||
| ): Promise<{ | ||
| model: string; | ||
| thinkingLevel?: ThinkingLevel; | ||
| providerId?: string; | ||
| providerConfig?: ClaudeCompatibleProvider; | ||
| credentials?: Credentials; | ||
| }>; |
There was a problem hiding this comment.
getPhaseModelWithOverrides return shape is out of sync
The helper now returns { phaseModel, isProjectOverride, provider, credentials }, not the older model/providerConfig shape. The doc snippet should match the current contract.
📝 Suggested doc fix
-): Promise<{
- model: string;
- thinkingLevel?: ThinkingLevel;
- providerId?: string;
- providerConfig?: ClaudeCompatibleProvider;
- credentials?: Credentials;
-}>;
+): Promise<{
+ phaseModel: PhaseModelEntry;
+ isProjectOverride: boolean;
+ provider?: ClaudeCompatibleProvider;
+ credentials?: Credentials;
+}>;📝 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.
| export async function getPhaseModelWithOverrides( | |
| phaseKey: PhaseModelKey, | |
| settingsService: SettingsService, | |
| projectPath?: string, | |
| logPrefix?: string | |
| ): Promise<{ | |
| model: string; | |
| thinkingLevel?: ThinkingLevel; | |
| providerId?: string; | |
| providerConfig?: ClaudeCompatibleProvider; | |
| credentials?: Credentials; | |
| }>; | |
| export async function getPhaseModelWithOverrides( | |
| phaseKey: PhaseModelKey, | |
| settingsService: SettingsService, | |
| projectPath?: string, | |
| logPrefix?: string | |
| ): Promise<{ | |
| phaseModel: PhaseModelEntry; | |
| isProjectOverride: boolean; | |
| provider?: ClaudeCompatibleProvider; | |
| credentials?: Credentials; | |
| }>; |
🤖 Prompt for AI Agents
In `@docs/UNIFIED_API_KEY_PROFILES.md` around lines 152 - 163, The docs show an
outdated return shape for getPhaseModelWithOverrides; update the documented
return type to match the current contract which returns an object with keys
phaseModel, isProjectOverride, provider, and credentials (instead of
model/providerConfig/thinkingLevel/providerId); find the
getPhaseModelWithOverrides snippet in the docs and replace the old return
description with the new fields and their brief meanings (phaseModel,
isProjectOverride, provider, credentials) so the doc matches the actual function
signature and usage.
| interface Project { | ||
| // ... | ||
| phaseModelOverrides?: PhaseModelConfig; // Per-phase overrides | ||
| } |
There was a problem hiding this comment.
Project overrides should be documented as Partial
phaseModelOverrides is Partial<PhaseModelConfig> in types. The docs currently imply full configs.
📝 Suggested doc fix
- phaseModelOverrides?: PhaseModelConfig; // Per-phase overrides
+ phaseModelOverrides?: Partial<PhaseModelConfig>; // Per-phase overrides📝 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.
| interface Project { | |
| // ... | |
| phaseModelOverrides?: PhaseModelConfig; // Per-phase overrides | |
| } | |
| interface Project { | |
| // ... | |
| phaseModelOverrides?: Partial<PhaseModelConfig>; // Per-phase overrides | |
| } |
🤖 Prompt for AI Agents
In `@docs/UNIFIED_API_KEY_PROFILES.md` around lines 211 - 214, The docs currently
show Project.phaseModelOverrides as a full PhaseModelConfig; update the
documentation text and the example snippet to reflect that phaseModelOverrides
is Partial<PhaseModelConfig> (i.e., per-phase entries may omit fields). Mention
the exact type name PhaseModelConfig and the property phaseModelOverrides on the
Project interface so readers know it accepts partial overrides rather than a
complete config.
| import type { | ||
| ThinkingLevel, | ||
| ClaudeApiProfile, | ||
| ClaudeCompatibleProvider, | ||
| Credentials, | ||
| } from './settings.js'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use @automaker/* import for shared types.
Line 5-10 uses a relative import from ./settings.js, which conflicts with the repo guideline to import shared packages via @automaker/*. Please switch to the namespace import. As per coding guidelines, …
♻️ Proposed fix
-import type {
- ThinkingLevel,
- ClaudeApiProfile,
- ClaudeCompatibleProvider,
- Credentials,
-} from './settings.js';
+import type {
+ ThinkingLevel,
+ ClaudeApiProfile,
+ ClaudeCompatibleProvider,
+ Credentials,
+} from '@automaker/types';🤖 Prompt for AI Agents
In `@libs/types/src/provider.ts` around lines 5 - 10, The import of shared types
(ThinkingLevel, ClaudeApiProfile, ClaudeCompatibleProvider, Credentials)
currently uses a relative path ('./settings.js'); update the import to use the
`@automaker` namespace package per guidelines (replace the relative import with
the corresponding `@automaker/`* settings module) so the same named symbols are
imported from the shared package instead of local file; ensure the import
statement in provider.ts references the `@automaker` settings module and retains
the same type names.
| defaultModels: [ | ||
| { id: 'GLM-4.5-Air', displayName: 'GLM 4.5 Air', mapsToClaudeModel: 'haiku' }, | ||
| { id: 'GLM-4.7', displayName: 'GLM 4.7', mapsToClaudeModel: 'sonnet' }, | ||
| { id: 'GLM-4.7', displayName: 'GLM 4.7', mapsToClaudeModel: 'opus' }, | ||
| ], | ||
| }, | ||
| { | ||
| templateId: 'minimax', | ||
| name: 'MiniMax', | ||
| providerType: 'minimax', | ||
| baseUrl: 'https://api.minimax.io/anthropic', | ||
| defaultApiKeySource: 'inline', | ||
| useAuthToken: true, | ||
| timeoutMs: 3000000, | ||
| disableNonessentialTraffic: true, | ||
| description: 'MiniMax M2.1 coding model with extended context', | ||
| apiKeyUrl: 'https://platform.minimax.io/user-center/basic-information/interface-key', | ||
| defaultModels: [ | ||
| { id: 'MiniMax-M2.1', displayName: 'MiniMax M2.1', mapsToClaudeModel: 'haiku' }, | ||
| { id: 'MiniMax-M2.1', displayName: 'MiniMax M2.1', mapsToClaudeModel: 'sonnet' }, | ||
| { id: 'MiniMax-M2.1', displayName: 'MiniMax M2.1', mapsToClaudeModel: 'opus' }, |
There was a problem hiding this comment.
Duplicate ProviderModel IDs make mapsToClaudeModel ambiguous
defaultModels repeats GLM-4.7 and MiniMax-M2.1 with different mapsToClaudeModel values (Line 318–320, Line 335–337). Because lookups use find by id, only the first mapping is returned, and UI list keys can collide. Consider representing tier mappings in a single entry (e.g., mapsToClaudeModel: ['sonnet','opus']) or persisting the mapped tier in PhaseModelEntry so lookups can disambiguate.
Run this to confirm how mapsToClaudeModel / resolvedModel are used at runtime:
#!/bin/bash
rg -n "mapsToClaudeModel|resolvedModel|getProviderByModelId" -S🤖 Prompt for AI Agents
In `@libs/types/src/settings.ts` around lines 317 - 337, The defaultModels list
contains duplicate ids (e.g., 'GLM-4.7' and 'MiniMax-M2.1') with different
mapsToClaudeModel values which makes lookups using id ambiguous; update the
provider model representation and lookup logic: either collapse duplicates into
a single entry with mapsToClaudeModel as an array (e.g., mapsToClaudeModel:
['sonnet','opus']) in the defaultModels arrays, or extend PhaseModelEntry to
persist the resolved tier (e.g., add a mappedTier or mapsToClaudeModels
property) and change getProviderByModelId and any code that uses
mapsToClaudeModel/resolvedModel to handle arrays or use the persisted mappedTier
so lookups by id are deterministic; search/adjust usages of mapsToClaudeModel,
resolvedModel, and getProviderByModelId to ensure UI keys and find() calls
handle the new shape.
- Honor thinkingLevel in generate-commit-message.ts - Pass claudeCompatibleProvider in ideation-service.ts for provider models - Resolve provider configuration for model overrides in generate-suggestions.ts - Update "Active Profile" to "Active Provider" label in project-claude-section - Use substring instead of deprecated substr in api-profiles-section - Preserve provider enabled state when editing in api-profiles-section
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/ui/src/hooks/use-settings-migration.ts (2)
169-217: Migration dropsclaudeCompatibleProvidersfrom localStorage.
parseLocalStorageSettings()never reads this field andmergeSettings()doesn’t preserve it, so provider configs can be lost during first-time migration/merge.🛠️ Suggested fix
@@ promptCustomization: state.promptCustomization as GlobalSettings['promptCustomization'], eventHooks: state.eventHooks as GlobalSettings['eventHooks'], + claudeCompatibleProviders: + (state.claudeCompatibleProviders as GlobalSettings['claudeCompatibleProviders']) ?? [], projects: state.projects as GlobalSettings['projects'], @@ if ( (!serverSettings.claudeApiProfiles || serverSettings.claudeApiProfiles.length === 0) && localSettings.claudeApiProfiles && localSettings.claudeApiProfiles.length > 0 ) { merged.claudeApiProfiles = localSettings.claudeApiProfiles; } + + if ( + (!serverSettings.claudeCompatibleProviders || + serverSettings.claudeCompatibleProviders.length === 0) && + localSettings.claudeCompatibleProviders && + localSettings.claudeCompatibleProviders.length > 0 + ) { + merged.claudeCompatibleProviders = localSettings.claudeCompatibleProviders; + }Also applies to: 337-349
198-217: DuplicateeventHookskey in parseLocalStorageSettings.
The object literal defines it twice, which triggers the esbuild warning and silently overwrites the first value.🛠️ Suggested fix
- // Event hooks - eventHooks: state.eventHooks as GlobalSettings['eventHooks'],apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
122-155: Guard against provider-missing fallback using provider-specific model IDs.If
getPhaseModelWithOverridesreturnsproviderasundefinedwhilephaseModelEntry.providerIdis still set (provider disabled/removed),resolvePhaseModelpasses through the provider model ID andstreamingQueryhits the direct API with an unsupported model. Add a safe fallback to a Claude model when provider is missing.🐛 Suggested safeguard
- const { model, thinkingLevel } = resolvePhaseModel(phaseModelEntry); + const safePhaseModelEntry = + !provider && phaseModelEntry.providerId + ? { + ...DEFAULT_PHASE_MODELS.featureGenerationModel, + thinkingLevel: phaseModelEntry.thinkingLevel, + } + : phaseModelEntry; + const { model, thinkingLevel } = resolvePhaseModel(safePhaseModelEntry);apps/server/src/services/auto-mode-service.ts (1)
3406-3447: Use the resolved provider model consistently across all follow‑up streams.
The initial call useseffectiveBareModel(mapped viamapsToClaudeModel), but revision/task/continuation streams still usebareModel. That can switch models mid-run or send an unsupported ID to the provider-backed endpoint.🛠️ Proposed fix
- model: bareModel, + model: effectiveBareModel,- model: bareModel, + model: effectiveBareModel,- model: bareModel, + model: effectiveBareModel,Also applies to: 3744-3754, 3893-3903, 3993-4003
apps/server/src/providers/claude-provider.ts (1)
83-151: Auth-token env source reads wrong environment variable.When
apiKeySource === 'env'anduseAuthToken === true, the code reads onlyANTHROPIC_API_KEYinstead ofANTHROPIC_AUTH_TOKEN. If users set onlyANTHROPIC_AUTH_TOKEN, the auth-token configuration won't resolve. The environment variable should be selected based on theuseAuthTokenflag, with an optional fallback to the other for backward compatibility.Proposed fix
switch (source) { case 'inline': apiKey = providerConfig.apiKey; break; case 'env': - apiKey = process.env.ANTHROPIC_API_KEY; + apiKey = providerConfig.useAuthToken + ? process.env.ANTHROPIC_AUTH_TOKEN + : process.env.ANTHROPIC_API_KEY; + if (!apiKey && providerConfig.useAuthToken) { + // Optional fallback if users still set ANTHROPIC_API_KEY + apiKey = process.env.ANTHROPIC_API_KEY; + } break; case 'credentials': apiKey = credentials?.apiKeys?.anthropic; break; }
🤖 Fix all issues with AI agents
In `@apps/server/src/lib/settings-helpers.ts`:
- Around line 556-575: The code clears the local provider variable but leaves
provider-specific IDs on phaseModel, causing downstream direct-Anthropic calls
to use non‑Claude model IDs; when provider is unset (either not found or
disabled) update phaseModel to remove provider linkage by clearing
phaseModel.providerId and any provider-specific model identifier fields (e.g.,
phaseModel.modelId or phaseModel.providerModelId) or mapping them to a valid
direct-provider model before returning; make this change in the same block where
provider is set to undefined (referencing provider, phaseModel,
globalSettings.claudeCompatibleProviders, logPrefix, and phase) so downstream
callers no longer receive a stale provider model id.
- Around line 549-553: The current fallback in settings-helpers.ts sets
phaseModel to a hardcoded { model: 'sonnet' }, discarding per-phase defaults;
change the fallback to pull the default entry from DEFAULT_PHASE_MODELS for the
given phase (e.g., DEFAULT_PHASE_MODELS[phase] or a safe lookup) so thinking
levels and other phase-specific defaults are preserved, and keep the
logger.debug call but update its message if needed to reflect that the per‑phase
default was used rather than the literal "sonnet"; ensure you reference and
import DEFAULT_PHASE_MODELS where phaseModel is assigned.
In `@apps/server/src/routes/backlog-plan/generate-plan.ts`:
- Around line 124-156: When an explicit model override (effectiveModel/model) is
provided we currently only load credentials, so provider-backed IDs aren't
resolved and claudeCompatibleProvider remains unset; update the override branch
(the if (effectiveModel) block) to mirror the settings-based flow by invoking
getPhaseModelWithOverrides (or the equivalent provider-resolution helper) and
resolvePhaseModel to obtain resolved.model, resolved.thinkingLevel,
phaseResult.provider and phaseResult.credentials, then assign effectiveModel,
thinkingLevel, claudeCompatibleProvider and credentials accordingly so
provider-backed model IDs route via the provider instead of as direct API calls.
In `@apps/server/src/routes/github/routes/validate-issue.ts`:
- Around line 170-191: The handler currently validates the incoming model string
using isClaudeModel/isCodexModel/isCursorModel/isOpencodeModel before you call
getProviderByModelId, which causes provider-specific IDs (e.g., GLM-4.5-Air) to
be rejected prematurely; change the validation flow to first call
getProviderByModelId(model, settingsService, '[ValidateIssue]') when
settingsService exists (or otherwise detect provider IDs) and accept
provider-resolved models, falling back to the existing is* checks only if
getProviderByModelId returns no provider; update the logic around model
validation and the effectiveModel computation (references: getProviderByModelId,
settingsService, model, effectiveModel,
isClaudeModel/isCodexModel/isCursorModel/isOpencodeModel) so provider IDs are
allowed and resolved before performing model-type validation.
In `@apps/server/src/services/ideation-service.ts`:
- Around line 226-227: The call to this.settingsService?.getCredentials() can
still throw if getCredentials is undefined; update the code in ideation-service
(the spot that prepares credentials before executeQuery) to explicitly check
that this.settingsService and typeof this.settingsService.getCredentials ===
"function" before invoking it, and fall back to null/empty credentials when the
method is missing; apply the same guard to the other occurrence around lines
handling executeQuery (the other getCredentials call referenced) so tests won't
fail when mocks lack getCredentials.
In
`@apps/ui/src/components/views/project-settings-view/project-bulk-replace-dialog.tsx`:
- Around line 149-191: The preview generation drops per-phase thinkingLevel when
findModelForClaudeAlias returns a newEntry without it; update the preview logic
so after computing newEntry (in the useMemo block where currentEntry and
newEntry are defined) you carry over the existing thinkingLevel from
currentEntry when newEntry.thinkingLevel is undefined (i.e., set
newEntry.thinkingLevel = newEntry.thinkingLevel ?? currentEntry.thinkingLevel)
so provider model switches preserve any per-phase thinking overrides; keep
references to currentEntry, newEntry, and findModelForClaudeAlias to locate the
change.
In
`@apps/ui/src/components/views/project-settings-view/project-models-section.tsx`:
- Around line 107-125: The getModelDisplayName function currently maps only
'haiku', 'sonnet', 'opus' to friendly names and will show raw IDs for
'claude-haiku/claude-sonnet/claude-opus'; update getModelDisplayName (used in
PhaseModelEntry handling) to also recognize the 'claude-' prefixed IDs—either by
adding 'claude-haiku', 'claude-sonnet', 'claude-opus' to the modelMap with the
same friendly names or by normalizing entry.model (e.g., strip a leading
'claude-' before lookup) so built-in Claude variants render as "Claude
Haiku/Sonnet/Opus" instead of raw IDs.
In
`@apps/ui/src/components/views/settings-view/providers/claude-settings-tab/api-profiles-section.tsx`:
- Around line 209-249: The save path currently sets enabled: true
unconditionally, which re-enables disabled providers when editing; update
handleSave so providerData.enabled preserves the existing enabled state when
editing and only defaults to true for new providers — e.g., when building
providerData in handleSave, set enabled to formData.enabled if present (or fetch
the current provider by editingProviderId and use its enabled flag) otherwise
default to true; ensure updateClaudeCompatibleProvider receives the preserved
enabled value when editing and addClaudeCompatibleProvider keeps the default
true for new providers.
In `@docs/UNIFIED_API_KEY_PROFILES.md`:
- Around line 149-163: The doc signature for getPhaseModelWithOverrides is out
of sync with the implementation; update the documented return shape to match the
actual function return values (use phaseModel, isProjectOverride, provider,
credentials) and remove/replace the incorrect fields (model, thinkingLevel,
providerId, providerConfig); reference the getPhaseModelWithOverrides helper and
its actual returned properties so the docs accurately reflect the runtime shape.
- Around line 20-33: The docs snippet for the ClaudeCompatibleProvider interface
is incorrect: update the snippet so providerType and apiKeySource are shown as
required (remove the optional ?), and ensure their types match the canonical
types used in the codebase by declaring providerType: string and apiKeySource:
ApiKeySource (and keep the interface name ClaudeCompatibleProvider and field
names providerType and apiKeySource to make it easy to find and verify against
the actual types definition).
In `@libs/model-resolver/src/resolver.ts`:
- Around line 204-218: The providerId branch currently returns as long as
phaseModel.providerId is set, even if phaseModel.model is empty; update the
branch in resolver.ts to guard against empty model strings by checking
phaseModel.model (e.g., if (!phaseModel.model) { const resolved = await
resolveModelString(phaseModel.model); return { model: resolved.model,
thinkingLevel: phaseModel.thinkingLevel, providerId: phaseModel.providerId }; })
otherwise keep the existing fast-path return; reference phaseModel.providerId,
phaseModel.model and the resolveModelString function to locate and implement the
fallback.
♻️ Duplicate comments (2)
apps/server/src/routes/app-spec/generate-spec.ts (1)
99-118: Same provider-missing fallback risk as in generate-features-from-spec.Please apply the same safeguard here to avoid passing provider-specific model IDs to the direct API when the provider is missing/disabled.
apps/server/src/routes/context/routes/describe-image.ts (1)
277-293: Same provider-missing fallback risk as earlier.Guard against provider-less fallbacks so provider-specific model IDs aren’t sent to the direct API when the provider is disabled or missing.
🧹 Nitpick comments (5)
libs/utils/tests/atomic-writer.test.ts (1)
67-78: LGTM!The updated regex
/\.tmp\.\d+\.[a-f0-9]+$/correctly validates the new temp file format with timestamp and hex suffix.Optionally, you could make the regex more specific by enforcing exactly 8 hex characters (since
randomBytes(4)produces 8 hex chars):🔧 Optional: More specific regex pattern
- expect(writeCall[0]).toMatch(/\.tmp\.\d+\.[a-f0-9]+$/); + expect(writeCall[0]).toMatch(/\.tmp\.\d+\.[a-f0-9]{8}$/);This would catch potential implementation changes that alter the random suffix length, but the current flexible pattern is also acceptable.
apps/server/src/services/settings-service.ts (1)
415-418: Add null-guards for legacy profile stringsIf a legacy profile is missing
baseUrlorname,toLowerCase()will throw during migration. A small guard keeps migrations resilient to malformed settings.💡 Suggested fix
- const baseUrl = profile.baseUrl.toLowerCase(); - const name = profile.name.toLowerCase(); + const baseUrl = (profile.baseUrl ?? '').toLowerCase(); + const name = (profile.name ?? '').toLowerCase();apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
990-1223: Gate thinking levels by provider model capabilities.Provider models always show the full thinking‑level list. If
model.capabilities.supportsThinkingis false ormaxThinkingLevelis set, consider clamping the options to avoid invalid selections.💡 Suggested patch
- const currentThinking = isSelected ? selectedThinkingLevel : 'none'; + const currentThinking = isSelected ? selectedThinkingLevel : 'none'; + const maxThinkingIndex = model.capabilities?.maxThinkingLevel + ? THINKING_LEVELS.indexOf(model.capabilities.maxThinkingLevel) + : THINKING_LEVELS.length - 1; + const availableThinkingLevels = + model.capabilities?.supportsThinking === false + ? (['none'] as const) + : THINKING_LEVELS.filter((_, idx) => idx <= maxThinkingIndex);Then use
availableThinkingLevelsin both the mobile and desktop loops.apps/ui/src/store/app-store.ts (2)
2044-2134: Consider rollback on failed persistence of phase overrides.These actions optimistically update the store but only log on
updateProjectfailure. If the server write fails, the UI will show overrides that won’t survive reload. Consider reverting local state (or surfacing an error) on failure.♻️ Suggested rollback pattern (apply similarly to clearAllProjectPhaseModelOverrides)
setProjectPhaseModelOverride: (projectId, phase, entry) => { + const previousProjects = get().projects; + const previousCurrentProject = get().currentProject; // Find the project to get its path for server sync const project = get().projects.find((p) => p.id === projectId); if (!project) { console.error('Cannot set phase model override: project not found'); return; } @@ httpClient.settings .updateProject(project.path, { phaseModelOverrides: Object.keys(newOverrides).length > 0 ? newOverrides : '__CLEAR__', }) .catch((error) => { - console.error('Failed to persist phaseModelOverrides:', error); + logger.error('Failed to persist phaseModelOverrides:', error); + set({ projects: previousProjects, currentProject: previousCurrentProject }); }); },
2720-2764: Handle syncSettingsToServer failures to avoid silent desync.Provider changes are persisted immediately, but failures are ignored. Since
syncSettingsToServer()returns a success flag, consider reverting on failure or surfacing an error to the user.♻️ Example rollback for one action (apply to others)
toggleClaudeCompatibleProviderEnabled: async (id) => { - set({ - claudeCompatibleProviders: get().claudeCompatibleProviders.map((p) => - p.id === id ? { ...p, enabled: p.enabled === false ? true : false } : p - ), - }); + const previous = get().claudeCompatibleProviders; + const next = previous.map((p) => + p.id === id ? { ...p, enabled: p.enabled === false ? true : false } : p + ); + set({ claudeCompatibleProviders: next }); // Sync immediately to persist change const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); - await syncSettingsToServer(); + const ok = await syncSettingsToServer(); + if (!ok) { + logger.error('Failed to sync claudeCompatibleProviders - reverting'); + set({ claudeCompatibleProviders: previous }); + } },
| let provider: ClaudeCompatibleProvider | undefined; | ||
| if (phaseModel.providerId) { | ||
| const providers = globalSettings.claudeCompatibleProviders || []; | ||
| provider = providers.find((p) => p.id === phaseModel.providerId); | ||
|
|
||
| if (provider) { | ||
| if (provider.enabled === false) { | ||
| logger.warn( | ||
| `${logPrefix} Provider "${provider.name}" for ${phase} is disabled, falling back to direct API` | ||
| ); | ||
| provider = undefined; | ||
| } else { | ||
| logger.debug(`${logPrefix} Using provider "${provider.name}" for ${phase}`); | ||
| } | ||
| } else { | ||
| logger.warn( | ||
| `${logPrefix} Provider ${phaseModel.providerId} not found for ${phase}, falling back to direct API` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Disabled/missing provider still returns provider model IDs.
Line 556–575 clears provider but keeps the provider-specific phaseModel. Downstream calls will then attempt direct Anthropic with a non‑Claude model ID (e.g., GLM/MiniMax), which can fail.
💡 Suggested patch
if (provider) {
if (provider.enabled === false) {
logger.warn(
`${logPrefix} Provider "${provider.name}" for ${phase} is disabled, falling back to direct API`
);
- provider = undefined;
+ provider = undefined;
+ phaseModel = DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' };
} else {
logger.debug(`${logPrefix} Using provider "${provider.name}" for ${phase}`);
}
} else {
logger.warn(
`${logPrefix} Provider ${phaseModel.providerId} not found for ${phase}, falling back to direct API`
);
+ phaseModel = DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' };
}🤖 Prompt for AI Agents
In `@apps/server/src/lib/settings-helpers.ts` around lines 556 - 575, The code
clears the local provider variable but leaves provider-specific IDs on
phaseModel, causing downstream direct-Anthropic calls to use non‑Claude model
IDs; when provider is unset (either not found or disabled) update phaseModel to
remove provider linkage by clearing phaseModel.providerId and any
provider-specific model identifier fields (e.g., phaseModel.modelId or
phaseModel.providerModelId) or mapping them to a valid direct-provider model
before returning; make this change in the same block where provider is set to
undefined (referencing provider, phaseModel,
globalSettings.claudeCompatibleProviders, logPrefix, and phase) so downstream
callers no longer receive a stale provider model id.
| // Get the model to use from settings or provided override with provider info | ||
| let effectiveModel = model; | ||
| let thinkingLevel: ThinkingLevel | undefined; | ||
| if (!effectiveModel) { | ||
| const settings = await settingsService?.getGlobalSettings(); | ||
| const phaseModelEntry = | ||
| settings?.phaseModels?.backlogPlanningModel || DEFAULT_PHASE_MODELS.backlogPlanningModel; | ||
| const resolved = resolvePhaseModel(phaseModelEntry); | ||
| let claudeCompatibleProvider: import('@automaker/types').ClaudeCompatibleProvider | undefined; | ||
| let credentials: import('@automaker/types').Credentials | undefined; | ||
|
|
||
| if (effectiveModel) { | ||
| // Use explicit override - just get credentials | ||
| credentials = await settingsService?.getCredentials(); | ||
| } else if (settingsService) { | ||
| // Use settings-based model with provider info | ||
| const phaseResult = await getPhaseModelWithOverrides( | ||
| 'backlogPlanningModel', | ||
| settingsService, | ||
| projectPath, | ||
| '[BacklogPlan]' | ||
| ); | ||
| const resolved = resolvePhaseModel(phaseResult.phaseModel); | ||
| effectiveModel = resolved.model; | ||
| thinkingLevel = resolved.thinkingLevel; | ||
| claudeCompatibleProvider = phaseResult.provider; | ||
| credentials = phaseResult.credentials; | ||
| } else { | ||
| // Fallback to defaults | ||
| const resolved = resolvePhaseModel(DEFAULT_PHASE_MODELS.backlogPlanningModel); | ||
| effectiveModel = resolved.model; | ||
| thinkingLevel = resolved.thinkingLevel; | ||
| } | ||
| logger.info('[BacklogPlan] Using model:', effectiveModel); | ||
| logger.info( | ||
| '[BacklogPlan] Using model:', | ||
| effectiveModel, | ||
| claudeCompatibleProvider ? `via provider: ${claudeCompatibleProvider.name}` : 'direct API' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "generate-plan.ts" --type fRepository: AutoMaker-Org/automaker
Length of output: 119
🏁 Script executed:
rg "getPhaseModelWithOverrides|getProviderByModelId" --type ts -lRepository: AutoMaker-Org/automaker
Length of output: 796
🏁 Script executed:
cat -n apps/server/src/routes/backlog-plan/generate-plan.ts | sed -n '120,160p'Repository: AutoMaker-Org/automaker
Length of output: 2026
🏁 Script executed:
cat -n apps/server/src/lib/settings-helpers.js | head -50Repository: AutoMaker-Org/automaker
Length of output: 138
🏁 Script executed:
cat -n apps/server/src/lib/settings-helpers.ts | head -100Repository: AutoMaker-Org/automaker
Length of output: 4330
🏁 Script executed:
rg "export.*getProviderByModelId|export.*getPhaseModelWithOverrides" apps/server/src/lib/settings-helpers.ts -A 5Repository: AutoMaker-Org/automaker
Length of output: 466
🏁 Script executed:
rg "import.*settings-helpers" apps/server/src/routes/backlog-plan/generate-plan.ts -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
head -50 apps/server/src/routes/backlog-plan/generate-plan.tsRepository: AutoMaker-Org/automaker
Length of output: 1684
🏁 Script executed:
rg "from.*settings-helpers" apps/server/src/routes/backlog-plan/generate-plan.tsRepository: AutoMaker-Org/automaker
Length of output: 106
🏁 Script executed:
cat -n apps/server/src/lib/settings-helpers.ts | grep -A 20 "export async function getProviderByModelId"Repository: AutoMaker-Org/automaker
Length of output: 979
🏁 Script executed:
cat -n apps/server/src/lib/settings-helpers.ts | grep "export.*function"Repository: AutoMaker-Org/automaker
Length of output: 789
🏁 Script executed:
cat -n apps/server/src/lib/settings-helpers.ts | sed -n '620,680p'Repository: AutoMaker-Org/automaker
Length of output: 2585
🏁 Script executed:
rg "ProviderByModelIdResult" apps/server/src/lib/settings-helpers.ts -B 5Repository: AutoMaker-Org/automaker
Length of output: 340
🏁 Script executed:
rg "interface ProviderByModelIdResult" apps/server/src/lib/settings-helpers.ts -A 10Repository: AutoMaker-Org/automaker
Length of output: 552
🏁 Script executed:
cat -n apps/server/src/routes/backlog-plan/generate-plan.ts | sed -n '155,165p'Repository: AutoMaker-Org/automaker
Length of output: 586
🏁 Script executed:
rg "getProviderByModelId" apps/server/src/routes/backlog-plan/ -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
rg "getProviderByModelId" apps/server/src --type ts -l | head -5Repository: AutoMaker-Org/automaker
Length of output: 327
🏁 Script executed:
cat -n apps/server/src/routes/backlog-plan/generate-plan.ts | sed -n '158,180p'Repository: AutoMaker-Org/automaker
Length of output: 1177
🏁 Script executed:
rg "getProviderByModelId" apps/server/src/services/ideation-service.ts -B 5 -A 10Repository: AutoMaker-Org/automaker
Length of output: 1437
🏁 Script executed:
rg "if.*effectiveModel\|override.*branch" apps/server/src/services/ideation-service.ts -B 3 -A 8Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
cat -n apps/server/src/services/ideation-service.ts | sed -n '150,200p'Repository: AutoMaker-Org/automaker
Length of output: 1999
🏁 Script executed:
cat -n apps/server/src/services/ideation-service.ts | sed -n '200,240p'Repository: AutoMaker-Org/automaker
Length of output: 1904
🏁 Script executed:
rg "claudeCompatibleProvider.*provider" apps/server/src/routes/backlog-plan/generate-plan.tsRepository: AutoMaker-Org/automaker
Length of output: 306
Add provider resolution to model override branch
The override branch (lines 130–132) only loads credentials, so a provider-backed model ID won't populate claudeCompatibleProvider and will route as a direct API call. This differs from the settings-based branch which resolves the provider via getPhaseModelWithOverrides. Add provider lookup for consistency:
Suggested fix
import {
getAutoLoadClaudeMdSetting,
getPromptCustomization,
getPhaseModelWithOverrides,
+ getProviderByModelId,
} from '../../lib/settings-helpers.js';
if (effectiveModel) {
// Use explicit override - just get credentials
credentials = await settingsService?.getCredentials();
+ if (settingsService) {
+ const providerResult = await getProviderByModelId(
+ effectiveModel,
+ settingsService,
+ '[BacklogPlan]'
+ );
+ if (providerResult.provider) {
+ claudeCompatibleProvider = providerResult.provider;
+ credentials = providerResult.credentials ?? credentials;
+ }
+ }
} else if (settingsService) {🤖 Prompt for AI Agents
In `@apps/server/src/routes/backlog-plan/generate-plan.ts` around lines 124 - 156,
When an explicit model override (effectiveModel/model) is provided we currently
only load credentials, so provider-backed IDs aren't resolved and
claudeCompatibleProvider remains unset; update the override branch (the if
(effectiveModel) block) to mirror the settings-based flow by invoking
getPhaseModelWithOverrides (or the equivalent provider-resolution helper) and
resolvePhaseModel to obtain resolved.model, resolved.thinkingLevel,
phaseResult.provider and phaseResult.credentials, then assign effectiveModel,
thinkingLevel, claudeCompatibleProvider and credentials accordingly so
provider-backed model IDs route via the provider instead of as direct API calls.
| // Check if the model is a provider model (like "GLM-4.5-Air") | ||
| // If so, get the provider config and resolved Claude model | ||
| let claudeCompatibleProvider: import('@automaker/types').ClaudeCompatibleProvider | undefined; | ||
| let providerResolvedModel: string | undefined; | ||
| let credentials = await settingsService?.getCredentials(); | ||
|
|
||
| if (settingsService) { | ||
| const providerResult = await getProviderByModelId(model, settingsService, '[ValidateIssue]'); | ||
| if (providerResult.provider) { | ||
| claudeCompatibleProvider = providerResult.provider; | ||
| providerResolvedModel = providerResult.resolvedModel; | ||
| credentials = providerResult.credentials; | ||
| logger.info( | ||
| `Using provider "${providerResult.provider.name}" for model "${model}"` + | ||
| (providerResolvedModel ? ` -> resolved to "${providerResolvedModel}"` : '') | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Get active Claude API profile for alternative endpoint configuration | ||
| const { profile: claudeApiProfile, credentials } = await getActiveClaudeApiProfile( | ||
| settingsService, | ||
| '[IssueValidation]', | ||
| projectPath | ||
| ); | ||
| // Use provider resolved model if available, otherwise use original model | ||
| const effectiveModel = providerResolvedModel || (model as string); | ||
| logger.info(`Using model: ${effectiveModel}`); |
There was a problem hiding this comment.
Provider model requests can be rejected before provider lookup
Provider-aware resolution is added here, but the handler still validates model using only isClaudeModel/isCodexModel/isCursorModel/isOpencodeModel. Provider model IDs (e.g., GLM-4.5-Air) will be rejected before this lookup runs. Consider allowing provider IDs by checking getProviderByModelId during validation.
💡 Suggested fix
- const isValidModel =
- isClaudeModel(model) ||
- isCursorModel(model) ||
- isCodexModel(model) ||
- isOpencodeModel(model);
+ let isValidModel =
+ isClaudeModel(model) ||
+ isCursorModel(model) ||
+ isCodexModel(model) ||
+ isOpencodeModel(model);
+
+ if (!isValidModel && settingsService) {
+ const providerResult = await getProviderByModelId(model, settingsService, '[ValidateIssue]');
+ isValidModel = !!providerResult.provider;
+ }🤖 Prompt for AI Agents
In `@apps/server/src/routes/github/routes/validate-issue.ts` around lines 170 -
191, The handler currently validates the incoming model string using
isClaudeModel/isCodexModel/isCursorModel/isOpencodeModel before you call
getProviderByModelId, which causes provider-specific IDs (e.g., GLM-4.5-Air) to
be rejected prematurely; change the validation flow to first call
getProviderByModelId(model, settingsService, '[ValidateIssue]') when
settingsService exists (or otherwise detect provider IDs) and accept
provider-resolved models, falling back to the existing is* checks only if
getProviderByModelId returns no provider; update the logic around model
validation and the effectiveModel computation (references: getProviderByModelId,
settingsService, model, effectiveModel,
isClaudeModel/isCodexModel/isCursorModel/isOpencodeModel) so provider IDs are
allowed and resolved before performing model-type validation.
| // Get credentials for API calls (uses hardcoded model, no phase setting) | ||
| const credentials = await this.settingsService?.getCredentials(); |
There was a problem hiding this comment.
Fix TypeError when mocks lack getCredentials (tests already failing).
this.settingsService?.getCredentials() still throws if the method is missing, which is causing the test failure at Line 686 and prevents executeQuery from running. Guard the method call explicitly.
🐛 Safe optional chaining on the method
- const credentials = await this.settingsService?.getCredentials();
+ const credentials = await this.settingsService?.getCredentials?.();
...
- const credentials = await this.settingsService?.getCredentials();
+ const credentials = await this.settingsService?.getCredentials?.();Also applies to: 685-687
🤖 Prompt for AI Agents
In `@apps/server/src/services/ideation-service.ts` around lines 226 - 227, The
call to this.settingsService?.getCredentials() can still throw if getCredentials
is undefined; update the code in ideation-service (the spot that prepares
credentials before executeQuery) to explicitly check that this.settingsService
and typeof this.settingsService.getCredentials === "function" before invoking
it, and fall back to null/empty credentials when the method is missing; apply
the same guard to the other occurrence around lines handling executeQuery (the
other getCredentials call referenced) so tests won't fail when mocks lack
getCredentials.
apps/ui/src/components/views/project-settings-view/project-models-section.tsx
Show resolved
Hide resolved
| const handleSave = () => { | ||
| const profileData: ClaudeApiProfile = { | ||
| id: editingProfileId ?? generateProfileId(), | ||
| // For GLM/MiniMax, enforce fixed settings | ||
| const isFixedProvider = hasFixedSettings(formData.providerType); | ||
|
|
||
| // Convert form models to ProviderModel format | ||
| const models: ProviderModel[] = formData.models | ||
| .filter((m) => m.id.trim()) // Only include models with IDs | ||
| .map((m) => ({ | ||
| id: m.id.trim(), | ||
| displayName: m.displayName.trim() || m.id.trim(), | ||
| mapsToClaudeModel: m.mapsToClaudeModel, | ||
| })); | ||
|
|
||
| const providerData: ClaudeCompatibleProvider = { | ||
| id: editingProviderId ?? generateProviderId(), | ||
| name: formData.name.trim(), | ||
| providerType: formData.providerType, | ||
| enabled: true, | ||
| baseUrl: formData.baseUrl.trim(), | ||
| apiKeySource: formData.apiKeySource, | ||
| // For fixed providers, always use inline | ||
| apiKeySource: isFixedProvider ? 'inline' : formData.apiKeySource, | ||
| // Only include apiKey when source is 'inline' | ||
| apiKey: formData.apiKeySource === 'inline' ? formData.apiKey : undefined, | ||
| useAuthToken: formData.useAuthToken, | ||
| apiKey: isFixedProvider || formData.apiKeySource === 'inline' ? formData.apiKey : undefined, | ||
| // For fixed providers, always use auth token | ||
| useAuthToken: isFixedProvider ? true : formData.useAuthToken, | ||
| timeoutMs: (() => { | ||
| const parsed = Number(formData.timeoutMs); | ||
| return Number.isFinite(parsed) ? parsed : undefined; | ||
| })(), | ||
| modelMappings: | ||
| formData.modelMappings.haiku || formData.modelMappings.sonnet || formData.modelMappings.opus | ||
| ? { | ||
| ...(formData.modelMappings.haiku && { haiku: formData.modelMappings.haiku }), | ||
| ...(formData.modelMappings.sonnet && { sonnet: formData.modelMappings.sonnet }), | ||
| ...(formData.modelMappings.opus && { opus: formData.modelMappings.opus }), | ||
| } | ||
| : undefined, | ||
| disableNonessentialTraffic: formData.disableNonessentialTraffic || undefined, | ||
| models, | ||
| // For fixed providers, always disable non-essential | ||
| disableNonessentialTraffic: isFixedProvider | ||
| ? true | ||
| : formData.disableNonessentialTraffic || undefined, | ||
| }; | ||
|
|
||
| if (editingProfileId) { | ||
| updateClaudeApiProfile(editingProfileId, profileData); | ||
| if (editingProviderId) { | ||
| updateClaudeCompatibleProvider(editingProviderId, providerData); | ||
| } else { | ||
| addClaudeApiProfile(profileData); | ||
| addClaudeCompatibleProvider(providerData); | ||
| } |
There was a problem hiding this comment.
Editing a provider always re-enables it.
Line 223–226 sets enabled: true unconditionally. Editing a disabled provider will silently re-enable it, which is a behavior change.
💡 Suggested patch
+ const existingProvider = editingProviderId
+ ? claudeCompatibleProviders.find((p) => p.id === editingProviderId)
+ : undefined;
const providerData: ClaudeCompatibleProvider = {
id: editingProviderId ?? generateProviderId(),
name: formData.name.trim(),
providerType: formData.providerType,
- enabled: true,
+ enabled: existingProvider?.enabled ?? true,
baseUrl: formData.baseUrl.trim(),📝 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.
| const handleSave = () => { | |
| const profileData: ClaudeApiProfile = { | |
| id: editingProfileId ?? generateProfileId(), | |
| // For GLM/MiniMax, enforce fixed settings | |
| const isFixedProvider = hasFixedSettings(formData.providerType); | |
| // Convert form models to ProviderModel format | |
| const models: ProviderModel[] = formData.models | |
| .filter((m) => m.id.trim()) // Only include models with IDs | |
| .map((m) => ({ | |
| id: m.id.trim(), | |
| displayName: m.displayName.trim() || m.id.trim(), | |
| mapsToClaudeModel: m.mapsToClaudeModel, | |
| })); | |
| const providerData: ClaudeCompatibleProvider = { | |
| id: editingProviderId ?? generateProviderId(), | |
| name: formData.name.trim(), | |
| providerType: formData.providerType, | |
| enabled: true, | |
| baseUrl: formData.baseUrl.trim(), | |
| apiKeySource: formData.apiKeySource, | |
| // For fixed providers, always use inline | |
| apiKeySource: isFixedProvider ? 'inline' : formData.apiKeySource, | |
| // Only include apiKey when source is 'inline' | |
| apiKey: formData.apiKeySource === 'inline' ? formData.apiKey : undefined, | |
| useAuthToken: formData.useAuthToken, | |
| apiKey: isFixedProvider || formData.apiKeySource === 'inline' ? formData.apiKey : undefined, | |
| // For fixed providers, always use auth token | |
| useAuthToken: isFixedProvider ? true : formData.useAuthToken, | |
| timeoutMs: (() => { | |
| const parsed = Number(formData.timeoutMs); | |
| return Number.isFinite(parsed) ? parsed : undefined; | |
| })(), | |
| modelMappings: | |
| formData.modelMappings.haiku || formData.modelMappings.sonnet || formData.modelMappings.opus | |
| ? { | |
| ...(formData.modelMappings.haiku && { haiku: formData.modelMappings.haiku }), | |
| ...(formData.modelMappings.sonnet && { sonnet: formData.modelMappings.sonnet }), | |
| ...(formData.modelMappings.opus && { opus: formData.modelMappings.opus }), | |
| } | |
| : undefined, | |
| disableNonessentialTraffic: formData.disableNonessentialTraffic || undefined, | |
| models, | |
| // For fixed providers, always disable non-essential | |
| disableNonessentialTraffic: isFixedProvider | |
| ? true | |
| : formData.disableNonessentialTraffic || undefined, | |
| }; | |
| if (editingProfileId) { | |
| updateClaudeApiProfile(editingProfileId, profileData); | |
| if (editingProviderId) { | |
| updateClaudeCompatibleProvider(editingProviderId, providerData); | |
| } else { | |
| addClaudeApiProfile(profileData); | |
| addClaudeCompatibleProvider(providerData); | |
| } | |
| const handleSave = () => { | |
| // For GLM/MiniMax, enforce fixed settings | |
| const isFixedProvider = hasFixedSettings(formData.providerType); | |
| // Convert form models to ProviderModel format | |
| const models: ProviderModel[] = formData.models | |
| .filter((m) => m.id.trim()) // Only include models with IDs | |
| .map((m) => ({ | |
| id: m.id.trim(), | |
| displayName: m.displayName.trim() || m.id.trim(), | |
| mapsToClaudeModel: m.mapsToClaudeModel, | |
| })); | |
| const existingProvider = editingProviderId | |
| ? claudeCompatibleProviders.find((p) => p.id === editingProviderId) | |
| : undefined; | |
| const providerData: ClaudeCompatibleProvider = { | |
| id: editingProviderId ?? generateProviderId(), | |
| name: formData.name.trim(), | |
| providerType: formData.providerType, | |
| enabled: existingProvider?.enabled ?? true, | |
| baseUrl: formData.baseUrl.trim(), | |
| // For fixed providers, always use inline | |
| apiKeySource: isFixedProvider ? 'inline' : formData.apiKeySource, | |
| // Only include apiKey when source is 'inline' | |
| apiKey: isFixedProvider || formData.apiKeySource === 'inline' ? formData.apiKey : undefined, | |
| // For fixed providers, always use auth token | |
| useAuthToken: isFixedProvider ? true : formData.useAuthToken, | |
| timeoutMs: (() => { | |
| const parsed = Number(formData.timeoutMs); | |
| return Number.isFinite(parsed) ? parsed : undefined; | |
| })(), | |
| models, | |
| // For fixed providers, always disable non-essential | |
| disableNonessentialTraffic: isFixedProvider | |
| ? true | |
| : formData.disableNonessentialTraffic || undefined, | |
| }; | |
| if (editingProviderId) { | |
| updateClaudeCompatibleProvider(editingProviderId, providerData); | |
| } else { | |
| addClaudeCompatibleProvider(providerData); | |
| } |
🤖 Prompt for AI Agents
In
`@apps/ui/src/components/views/settings-view/providers/claude-settings-tab/api-profiles-section.tsx`
around lines 209 - 249, The save path currently sets enabled: true
unconditionally, which re-enables disabled providers when editing; update
handleSave so providerData.enabled preserves the existing enabled state when
editing and only defaults to true for new providers — e.g., when building
providerData in handleSave, set enabled to formData.enabled if present (or fetch
the current provider by editingProviderId and use its enabled flag) otherwise
default to true; ensure updateClaudeCompatibleProvider receives the preserved
enabled value when editing and addClaudeCompatibleProvider keeps the default
true for new providers.
| ```typescript | ||
| export interface ClaudeApiProfile { | ||
| id: string; | ||
| name: string; | ||
| baseUrl: string; | ||
|
|
||
| // NEW: API key sourcing strategy (default: 'inline' for backwards compat) | ||
| apiKeySource?: ApiKeySource; | ||
|
|
||
| // Now optional - only required when apiKeySource = 'inline' | ||
| apiKey?: string; | ||
|
|
||
| // Existing fields unchanged... | ||
| useAuthToken?: boolean; | ||
| timeoutMs?: number; | ||
| modelMappings?: { haiku?: string; sonnet?: string; opus?: string }; | ||
| disableNonessentialTraffic?: boolean; | ||
| export interface ClaudeCompatibleProvider { | ||
| id: string; // Unique identifier (UUID) | ||
| name: string; // Display name (e.g., "z.AI GLM") | ||
| baseUrl: string; // API endpoint URL | ||
| providerType?: string; // Provider type for icon/grouping (e.g., 'glm', 'minimax', 'openrouter') | ||
| apiKeySource?: ApiKeySource; // 'inline' | 'env' | 'credentials' | ||
| apiKey?: string; // API key (when apiKeySource = 'inline') | ||
| useAuthToken?: boolean; // Use ANTHROPIC_AUTH_TOKEN header | ||
| timeoutMs?: number; // Request timeout in milliseconds | ||
| disableNonessentialTraffic?: boolean; // Minimize non-essential API calls | ||
| enabled?: boolean; // Whether provider is active (default: true) | ||
| models?: ProviderModel[]; // Models exposed by this provider | ||
| } |
There was a problem hiding this comment.
Docs show providerType/apiKeySource as optional, but types require them.
In libs/types/src/settings.ts, both fields are required and typed, so the doc snippet should reflect that to avoid confusion.
📝 Suggested doc fix
- providerType?: string; // Provider type for icon/grouping (e.g., 'glm', 'minimax', 'openrouter')
- apiKeySource?: ApiKeySource; // 'inline' | 'env' | 'credentials'
+ providerType: ClaudeCompatibleProviderType; // Provider type for icon/grouping
+ apiKeySource: ApiKeySource; // 'inline' | 'env' | 'credentials'🤖 Prompt for AI Agents
In `@docs/UNIFIED_API_KEY_PROFILES.md` around lines 20 - 33, The docs snippet for
the ClaudeCompatibleProvider interface is incorrect: update the snippet so
providerType and apiKeySource are shown as required (remove the optional ?), and
ensure their types match the canonical types used in the codebase by declaring
providerType: string and apiKeySource: ApiKeySource (and keep the interface name
ClaudeCompatibleProvider and field names providerType and apiKeySource to make
it easy to find and verify against the actual types definition).
| The `getPhaseModelWithOverrides()` helper gets effective phase model config: | ||
|
|
||
| Added an informational note: | ||
| ```typescript | ||
| export async function getPhaseModelWithOverrides( | ||
| phaseKey: PhaseModelKey, | ||
| settingsService: SettingsService, | ||
| projectPath?: string, | ||
| logPrefix?: string | ||
| ): Promise<{ | ||
| model: string; | ||
| thinkingLevel?: ThinkingLevel; | ||
| providerId?: string; | ||
| providerConfig?: ClaudeCompatibleProvider; | ||
| credentials?: Credentials; | ||
| }>; |
There was a problem hiding this comment.
getPhaseModelWithOverrides signature is out of sync with code.
apps/server/src/lib/settings-helpers.ts returns { phaseModel, isProjectOverride, provider, credentials }, not { model, thinkingLevel, providerId, providerConfig }.
📝 Suggested doc fix
-export async function getPhaseModelWithOverrides(
- phaseKey: PhaseModelKey,
- settingsService: SettingsService,
- projectPath?: string,
- logPrefix?: string
-): Promise<{
- model: string;
- thinkingLevel?: ThinkingLevel;
- providerId?: string;
- providerConfig?: ClaudeCompatibleProvider;
- credentials?: Credentials;
-}>;
+export async function getPhaseModelWithOverrides(
+ phaseKey: PhaseModelKey,
+ settingsService?: SettingsService,
+ projectPath?: string,
+ logPrefix?: string
+): Promise<{
+ phaseModel: PhaseModelEntry;
+ isProjectOverride: boolean;
+ provider?: ClaudeCompatibleProvider;
+ credentials?: Credentials;
+}>;🤖 Prompt for AI Agents
In `@docs/UNIFIED_API_KEY_PROFILES.md` around lines 149 - 163, The doc signature
for getPhaseModelWithOverrides is out of sync with the implementation; update
the documented return shape to match the actual function return values (use
phaseModel, isProjectOverride, provider, credentials) and remove/replace the
incorrect fields (model, thinkingLevel, providerId, providerConfig); reference
the getPhaseModelWithOverrides helper and its actual returned properties so the
docs accurately reflect the runtime shape.
| `[ModelResolver] phaseModel is object format: model="${phaseModel.model}", thinkingLevel="${phaseModel.thinkingLevel}", providerId="${phaseModel.providerId}"` | ||
| ); | ||
|
|
||
| // If providerId is set, pass through the model string unchanged | ||
| // (it's a provider-specific model ID like "GLM-4.5-Air", not a Claude alias) | ||
| if (phaseModel.providerId) { | ||
| console.log( | ||
| `[ModelResolver] Using provider model: providerId="${phaseModel.providerId}", model="${phaseModel.model}"` | ||
| ); | ||
| return { | ||
| model: phaseModel.model, // Pass through unchanged | ||
| thinkingLevel: phaseModel.thinkingLevel, | ||
| providerId: phaseModel.providerId, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Guard providerId branch against empty model strings.
With providerId set, an empty phaseModel.model now bypasses resolveModelString and can return an invalid model. Add a fallback to prevent downstream failures.
🛠️ Suggested fix
- if (phaseModel.providerId) {
+ if (phaseModel.providerId && phaseModel.model) {
console.log(
`[ModelResolver] Using provider model: providerId="${phaseModel.providerId}", model="${phaseModel.model}"`
);
return {
model: phaseModel.model, // Pass through unchanged
thinkingLevel: phaseModel.thinkingLevel,
providerId: phaseModel.providerId,
};
}
+
+ if (phaseModel.providerId && !phaseModel.model) {
+ console.warn(
+ `[ModelResolver] providerId set but model is empty; falling back to default model`
+ );
+ return {
+ model: resolveModelString(undefined, defaultModel),
+ thinkingLevel: phaseModel.thinkingLevel,
+ };
+ }📝 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.
| `[ModelResolver] phaseModel is object format: model="${phaseModel.model}", thinkingLevel="${phaseModel.thinkingLevel}", providerId="${phaseModel.providerId}"` | |
| ); | |
| // If providerId is set, pass through the model string unchanged | |
| // (it's a provider-specific model ID like "GLM-4.5-Air", not a Claude alias) | |
| if (phaseModel.providerId) { | |
| console.log( | |
| `[ModelResolver] Using provider model: providerId="${phaseModel.providerId}", model="${phaseModel.model}"` | |
| ); | |
| return { | |
| model: phaseModel.model, // Pass through unchanged | |
| thinkingLevel: phaseModel.thinkingLevel, | |
| providerId: phaseModel.providerId, | |
| }; | |
| } | |
| `[ModelResolver] phaseModel is object format: model="${phaseModel.model}", thinkingLevel="${phaseModel.thinkingLevel}", providerId="${phaseModel.providerId}"` | |
| ); | |
| // If providerId is set, pass through the model string unchanged | |
| // (it's a provider-specific model ID like "GLM-4.5-Air", not a Claude alias) | |
| if (phaseModel.providerId && phaseModel.model) { | |
| console.log( | |
| `[ModelResolver] Using provider model: providerId="${phaseModel.providerId}", model="${phaseModel.model}"` | |
| ); | |
| return { | |
| model: phaseModel.model, // Pass through unchanged | |
| thinkingLevel: phaseModel.thinkingLevel, | |
| providerId: phaseModel.providerId, | |
| }; | |
| } | |
| if (phaseModel.providerId && !phaseModel.model) { | |
| console.warn( | |
| `[ModelResolver] providerId set but model is empty; falling back to default model` | |
| ); | |
| return { | |
| model: resolveModelString(undefined, defaultModel), | |
| thinkingLevel: phaseModel.thinkingLevel, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@libs/model-resolver/src/resolver.ts` around lines 204 - 218, The providerId
branch currently returns as long as phaseModel.providerId is set, even if
phaseModel.model is empty; update the branch in resolver.ts to guard against
empty model strings by checking phaseModel.model (e.g., if (!phaseModel.model) {
const resolved = await resolveModelString(phaseModel.model); return { model:
resolved.model, thinkingLevel: phaseModel.thinkingLevel, providerId:
phaseModel.providerId }; }) otherwise keep the existing fast-path return;
reference phaseModel.providerId, phaseModel.model and the resolveModelString
function to locate and implement the fallback.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/server/src/services/auto-mode-service.ts (1)
3744-3754: Keep mapped model consistent across follow‑up calls.
WhenmapsToClaudeModelis set, the initial call useseffectiveBareModel, but plan revisions/tasks/continuations still usebareModel. That can break provider mappings.🛠️ Suggested fix
- const revisionStream = provider.executeQuery({ + const revisionStream = provider.executeQuery({ prompt: revisionPrompt, - model: bareModel, + model: effectiveBareModel, maxTurns: maxTurns || 100, cwd: workDir, allowedTools: allowedTools, abortController, mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, credentials, // Pass credentials for resolving 'credentials' apiKeySource claudeCompatibleProvider, // Pass provider for alternative endpoint configuration }); ... - const taskStream = provider.executeQuery({ + const taskStream = provider.executeQuery({ prompt: taskPrompt, - model: bareModel, + model: effectiveBareModel, maxTurns: Math.min(maxTurns || 100, 50), // Limit turns per task cwd: workDir, allowedTools: allowedTools, abortController, mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, credentials, // Pass credentials for resolving 'credentials' apiKeySource claudeCompatibleProvider, // Pass provider for alternative endpoint configuration }); ... - const continuationStream = provider.executeQuery({ + const continuationStream = provider.executeQuery({ prompt: continuationPrompt, - model: bareModel, + model: effectiveBareModel, maxTurns: maxTurns, cwd: workDir, allowedTools: allowedTools, abortController, mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, credentials, // Pass credentials for resolving 'credentials' apiKeySource claudeCompatibleProvider, // Pass provider for alternative endpoint configuration });Also applies to: 3893-3903, 3993-4003
apps/server/src/routes/suggestions/generate-suggestions.ts (1)
182-206: Fix TypeScript error: unreachable code path.The pipeline is failing with
TS2339: Property 'getCredentials' does not exist on type 'never'at line 205. This occurs because inside theelsebranch ofif (settingsService), TypeScript narrowssettingsServicetoundefined/never, makingsettingsService?.getCredentials()a type error.The intent appears to be to get credentials when
settingsServiceexists but provider lookup fails, but the current nesting is incorrect.🔧 Proposed fix
if (modelOverride) { // Use explicit override - resolve the model string const resolved = resolvePhaseModel({ model: modelOverride, thinkingLevel: thinkingLevelOverride, }); model = resolved.model; thinkingLevel = resolved.thinkingLevel; // Try to find a provider for this model (e.g., GLM, MiniMax models) if (settingsService) { const providerResult = await getProviderByModelId( modelOverride, settingsService, '[Suggestions]' ); provider = providerResult.provider; // Use resolved model from provider if available (maps to Claude model) if (providerResult.resolvedModel) { model = providerResult.resolvedModel; } credentials = providerResult.credentials ?? (await settingsService.getCredentials()); - } else { - credentials = await settingsService?.getCredentials(); }
♻️ Duplicate comments (5)
apps/server/src/services/ideation-service.ts (1)
215-216: Guard getCredentials calls when the method is missing.
Line 215 and Line 703 can still throw ifsettingsServiceexists butgetCredentialsis undefined (common in mocks).🛠️ Suggested fix
- let credentials = await this.settingsService?.getCredentials(); + let credentials = await this.settingsService?.getCredentials?.(); ... - const credentials = await this.settingsService?.getCredentials(); + const credentials = await this.settingsService?.getCredentials?.();Also applies to: 703-705
apps/server/src/services/auto-mode-service.ts (1)
3406-3408: Guard getCredentials calls when the method is missing.
Line 3406 can still throw ifsettingsServiceexists butgetCredentialsis undefined.🛠️ Suggested fix
- const credentials = await this.settingsService?.getCredentials(); + const credentials = await this.settingsService?.getCredentials?.();apps/server/src/lib/settings-helpers.ts (2)
549-552: Use per‑phase defaults instead of hardcodingsonnet.
Line 550 and Line 587 drop the per‑phase defaults (and thinking levels).🛠️ Suggested fix
- if (!phaseModel) { - phaseModel = { model: 'sonnet' }; - logger.debug(`${logPrefix} No ${phase} configured, using default: sonnet`); - } + if (!phaseModel) { + phaseModel = DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' }; + logger.debug( + `${logPrefix} No ${phase} configured, using default: ${phaseModel.model ?? 'sonnet'}` + ); + } ... - return { - phaseModel: { model: 'sonnet' }, + return { + phaseModel: DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' }, isProjectOverride: false, provider: undefined, credentials: undefined, };Also applies to: 587-591
556-574: Reset phase model when provider is missing/disabled.
Line 562–573 clears the provider but keeps the provider-specific model; downstream direct calls can then receive non‑Claude IDs.🛠️ Suggested fix
if (provider) { if (provider.enabled === false) { logger.warn( `${logPrefix} Provider "${provider.name}" for ${phase} is disabled, falling back to direct API` ); - provider = undefined; + provider = undefined; + phaseModel = DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' }; } else { logger.debug(`${logPrefix} Using provider "${provider.name}" for ${phase}`); } } else { logger.warn( `${logPrefix} Provider ${phaseModel.providerId} not found for ${phase}, falling back to direct API` ); + phaseModel = DEFAULT_PHASE_MODELS[phase] ?? { model: 'sonnet' }; }apps/server/src/routes/context/routes/describe-image.ts (1)
305-324: Vision capability check still needed for provider models.The current branching logic assumes all non-Cursor models support image content blocks. With the new provider system, models like GLM or MiniMax may not support vision. Consider adding a
supportsVisioncapability flag to provider model definitions and gating the image-block path accordingly.
🧹 Nitpick comments (3)
apps/ui/src/components/views/settings-view/providers/claude-settings-tab/api-profiles-section.tsx (2)
605-666: Consider extracting duplicated model row UI into a reusable component.The model editing row (Model ID input, Display Name input, Claude mapping select, delete button) is duplicated between fixed and non-fixed provider sections. Extracting this to a
ModelRowcomponent would reduce ~60 lines of duplication.♻️ Suggested extraction
interface ModelRowProps { model: ModelFormEntry; index: number; onUpdate: (index: number, updates: Partial<ModelFormEntry>) => void; onRemove: (index: number) => void; } function ModelRow({ model, index, onUpdate, onRemove }: ModelRowProps) { return ( <div className="flex items-center gap-2 p-3 bg-card/50 rounded-lg border border-border/30"> <div className="flex-1 space-y-2"> <div className="grid grid-cols-2 gap-2"> <div className="space-y-1"> <Label className="text-xs text-muted-foreground">Model ID</Label> <Input value={model.id} onChange={(e) => onUpdate(index, { id: e.target.value })} placeholder="e.g., GLM-4.7" className="text-xs h-8" /> </div> {/* ... rest of the fields ... */} </div> </div> <Button type="button" variant="ghost" size="icon" className="shrink-0 text-muted-foreground hover:text-destructive" onClick={() => onRemove(index)} > <Trash2 className="w-4 h-4" /> </Button> </div> ); }Also applies to: 716-777
866-879: Consider using index in Badge key to guarantee uniqueness.The current key
${model.id}-${model.mapsToClaudeModel}could produce duplicates if the user configures two models with identical IDs mapping to the same Claude model (e.g., during editing). Using the array index as a fallback ensures uniqueness.♻️ Suggested fix
- key={`${model.id}-${model.mapsToClaudeModel}`} + key={`${index}-${model.id}-${model.mapsToClaudeModel}`}Or simply:
- key={`${model.id}-${model.mapsToClaudeModel}`} + key={index}apps/server/src/routes/context/routes/describe-file.ts (1)
171-174: Consider using a single template string for logger arguments.The
logger.info()call passes two separate arguments, which may not format as intended depending on the logger implementation. Using a single template string ensures consistent output.🔧 Suggested fix
- logger.info( - `Resolved model: ${model}, thinkingLevel: ${thinkingLevel}`, - provider ? `via provider: ${provider.name}` : 'direct API' - ); + logger.info( + `Resolved model: ${model}, thinkingLevel: ${thinkingLevel}, ${provider ? `via provider: ${provider.name}` : 'direct API'}` + );
- Fix TypeScript TS2339 error in generate-suggestions.ts where settingsService was narrowed to 'never' type in else branch - Use DEFAULT_PHASE_MODELS per-phase defaults instead of hardcoded 'sonnet' in settings-helpers.ts - Remove duplicate eventHooks key in use-settings-migration.ts - Add claudeCompatibleProviders to localStorage migration parsing and merging functions - Handle canonical claude-* model IDs (claude-haiku, claude-sonnet, claude-opus) in project-models-section display names This resolves the CI build failures and addresses code review feedback.
…label - Skip list-view-priority.spec.ts with TODO explaining the infrastructure issue: setupRealProject only sets localStorage but server settings take precedence with localStorageMigrated: true - Add 'Priority' label to list-header.tsx for the priority column (was empty string, now shows proper header text) - Increase column width to accommodate the label The E2E test issue is that tests create features in a temp directory, but the server loads from the E2E Test Project fixture path set in setup-e2e-fixtures.mjs. Needs infrastructure fix to properly switch projects or create features through UI instead of on disk.
1 participant
Summary
This PR introduces the Claude Compatible Providers system, a major enhancement that allows users to configure alternative API endpoints that expose Claude-compatible models throughout the application.
Key Features
phaseModelOverridesTechnical Changes
Types (
libs/types/src/settings.ts):ClaudeApiProfiletoClaudeCompatibleProviderProviderModeltype withmapsToClaudeModelfieldPhaseModelEntrywith optionalproviderIdfieldServer (
apps/server/src/):getProviderByModelId(),getPhaseModelWithOverrides()UI (
apps/ui/src/):providerTypefieldphaseModelOverridesBug Fixes in This PR
DEFAULT_PHASE_MODELSincluding thinking levelsTest Plan
Screenshots
Summary by CodeRabbit
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.