feat: add dynamic model discovery and routing for OpenCode provider #426
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds dynamic OpenCode model/provider discovery via the OpenCode CLI, with parsing, caching, refresh and provider-auth listing; server API routes to expose/refresh/clear caches; UI wiring for listing/toggling dynamic models and providers; provider-based execution path and related types, store migrations, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Settings UI
participant Store as App Store
participant API as HttpApiClient
participant Server as Server Routes
participant Provider as OpencodeProvider
participant CLI as OpenCode CLI
UI->>Store: mount -> request opencode models
Store->>API: getOpencodeModels(refresh=false)
API->>Server: GET /api/setup/opencode/models
Server->>Provider: getAvailableModels()
alt cache valid
Provider-->>Server: return cached models (cached=true)
else cache missing/expired or refresh
Provider->>CLI: spawn CLI fetch (stream/JSON)
CLI-->>Provider: stream output
Provider->>Provider: parseModelsOutput & modelInfoToDefinition
Provider->>Provider: update cache
Provider-->>Server: return fresh models (cached=false)
end
Server-->>API: { models, cached, count }
API-->>Store: setDynamicOpencodeModels(models)
Store-->>UI: render models grouped by provider
UI->>Store: toggleDynamicModel(id, enabled)
Store->>API: sync settings update / optional refresh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 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 OpenCode provider's capabilities by introducing dynamic model discovery and management. It allows the application to automatically fetch and display AI models and authenticated providers directly from the OpenCode CLI, providing users with a more flexible and up-to-date selection of tools. The user interface has been updated to intelligently group these dynamic models by provider and assign relevant icons, improving usability. Additionally, the underlying event processing for OpenCode has been made more resilient, and the project's development dependencies have undergone a substantial cleanup. 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
|
![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 dynamic model discovery for the OpenCode provider, which is a significant feature enhancement. The implementation is comprehensive, covering server-side logic for fetching and caching models, API routes, and extensive UI updates to display and manage these dynamic models. The code is well-structured, but there are a few key areas for improvement, particularly regarding server performance and code duplication. My main feedback focuses on replacing synchronous file operations with asynchronous alternatives on the server to prevent blocking the event loop, which is critical for a responsive application. I've also identified what appears to be an unused UI component that should be removed.
apps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsx
Outdated
Show resolved
Hide resolved
|
@coderabbitai can you review this PR |
|
@stefandevo: I'll review the changes in this PR for you. ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @apps/server/src/routes/setup/routes/opencode-models.ts:
- Around line 124-126: The catch blocks in the refresh handler and
createGetOpencodeProvidersHandler are logging the same error twice via
logger.error(...) and logError(...); remove the duplicate by keeping only the
preferred logging call (use logError(error, 'Refresh OpenCode models failed')
and logError(error, 'Get OpenCode providers failed') for consistent
structured/telemetry logging) and delete the corresponding logger.error(...)
calls so each catch logs once.
In @apps/ui/src/components/ui/provider-icon.tsx:
- Around line 445-497: The dynamic model parsing branch incorrectly handles
strings with multiple slashes (e.g., "a/b/c") because it uses
modelStr.includes('/') and modelStr.split('/')[1]; update the logic in the
dynamic branch to only treat strings with exactly one slash as OpenCode-style
provider/model pairs (e.g., use a check like indexOf('/') !== -1 && indexOf('/')
=== lastIndexOf('/') or a regex /^[^/]+\/[^/]+$/), then destructure safely once
(const [providerName, modelName] = modelStr.split('/', 2)) and keep the existing
pattern checks against modelName/providerName; if the string does not have
exactly one slash, skip this branch and fall back to the normal detection flow.
In
@apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx:
- Around line 240-254: The selected OpenCode model always uses the generic
OpenCodeIcon; instead resolve the icon dynamically using getProviderIconForModel
for both static and dynamic cases. Update the opencodeModel branch to set icon:
getProviderIconForModel(opencodeModel.id) instead of OpenCodeIcon, and update
the dynamicModel branch to set icon: getProviderIconForModel(dynamicModel.id)
instead of OpenCodeIcon so the trigger shows provider-specific icons for models
like google/gemini-*.
- Around line 298-318: The merge logic currently filters out dynamic models that
collide with static ones, making static take precedence; to fix, build
dynamicModelOptions from dynamicOpencodeModels, create a map keyed by id, then
produce the merged array by iterating OPENCODE_MODELS and replacing any static
entry with the dynamic map value when present (so dynamic overrides but keeps
static ordering), and finally append any dynamicModelOptions whose ids were not
present in OPENCODE_MODELS so their relative order is preserved; update the
allOpencodeModels useMemo to use this replace-then-append strategy (referencing
allOpencodeModels, OPENCODE_MODELS, dynamicOpencodeModels, and
dynamicModelOptions).
In @apps/ui/src/store/app-store.ts:
- Around line 1953-1965: The static OpenCode settings enabledOpencodeModels and
opencodeDefaultModel are missing from the SETTINGS_FIELDS_TO_SYNC list in
use-settings-sync.ts; add those two keys to SETTINGS_FIELDS_TO_SYNC so they are
persisted like enabledCursorModels/cursorDefaultModel. For dynamic models,
decide whether enabledDynamicModelIds should be session-only or persisted: if
you want persistence, add enabledDynamicModelIds to the GlobalSettings interface
and include it in SETTINGS_FIELDS_TO_SYNC; otherwise explicitly leave it out and
add a code comment in setDynamicOpencodeModels (and the GlobalSettings
definition) noting that enabledDynamicModelIds is intentionally session-scoped.
🧹 Nitpick comments (9)
libs/types/src/provider-utils.ts (1)
90-137: Tightenprovider/modeldetection to avoid over-matching arbitrary strings.Right now any non-URL string with a single
/will route to OpenCode (e.g., accidentalfoo/barvalues). If that’s intended, fine; otherwise consider requiring exactly one slash via index checks (and/or whitelisting provider ids elsewhere) to reduce false positives.Proposed tweak (exactly one slash, fewer allocations)
- if (model.includes('/') && !model.includes('://')) { - const parts = model.split('/'); - // Valid dynamic model format: provider/model-name (exactly 2 parts) - if (parts.length === 2 && parts[0].length > 0 && parts[1].length > 0) { - return true; - } - } + if (!model.includes('://')) { + const first = model.indexOf('/'); + if (first !== -1 && first === model.lastIndexOf('/') && first > 0 && first < model.length - 1) { + return true; + } + }apps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsx (2)
20-25: Avoid duplicatingOpenCodeProviderInfoshape in UI (type drift risk).
Consider exporting/importing a singleOpenCodeProviderInfotype (e.g., from@automaker/types) instead of re-declaring it here. AlsoPROVIDER_CONFIG.iconis unused.Possible direction
-interface OpenCodeProviderInfo { - id: string; - name: string; - authenticated: boolean; - authMethod?: 'oauth' | 'api_key'; -} +// Prefer a shared type (example): +// import type { OpenCodeProviderInfo } from '@automaker/types';-const PROVIDER_CONFIG: Record<string, { icon: string; displayName: string }> = { +const PROVIDER_CONFIG: Record<string, { displayName: string }> = { copilot: { icon: '', displayName: 'GitHub Copilot' }, anthropic: { icon: '', displayName: 'Anthropic' }, openai: { icon: '', displayName: 'OpenAI' }, google: { icon: '', displayName: 'Google' }, 'amazon-bedrock': { icon: '', displayName: 'AWS Bedrock' }, azure: { icon: '', displayName: 'Azure OpenAI' }, ollama: { icon: '', displayName: 'Ollama' }, lmstudio: { icon: '', displayName: 'LM Studio' }, opencode: { icon: '', displayName: 'OpenCode' }, };Also applies to: 27-51
62-85: Consider user feedback on initial model fetch failures (silent failure).
fetchModels()logs errors but doesn’t surface a toast; if the UI depends on these models, a lightweight toast (or inline “failed to load”) may reduce confusion.Also applies to: 117-145
apps/ui/src/lib/http-api-client.ts (1)
1439-1499: Prefer shared types for OpenCode model/provider payloads to prevent drift.These new methods inline
models[]andproviders[]shapes; this will eventually diverge from server responses and@automaker/types(ModelDefinition, plus whatever provider-info type you standardize on).Direction (sketch)
- getOpencodeModels: ( - refresh?: boolean - ): Promise<{ - success: boolean; - models?: Array<{ - id: string; - name: string; - modelString: string; - provider: string; - description: string; - supportsTools: boolean; - supportsVision: boolean; - tier: string; - default?: boolean; - }>; + getOpencodeModels: ( + refresh?: boolean + ): Promise<{ + success: boolean; + models?: ModelDefinition[]; count?: number; cached?: boolean; error?: string; }> => this.get(`/api/setup/opencode/models${refresh ? '?refresh=true' : ''}`),apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
372-412: Provider grouping looks good; consider typingrenderOpencodeModelItemasModelOption.You’re now passing dynamic
ModelOptions through grouped rendering;renderOpencodeModelItemis typed as(typeof OPENCODE_MODELS)[0], which can become brittle ifOPENCODE_MODELS’ element type ever diverges fromModelOption.Also applies to: 1077-1082
apps/ui/src/store/app-store.ts (1)
574-583: MakeenabledDynamicModelIdsupdates idempotent + consider reconciling on refresh.
toggleDynamicModel(..., true)appends without checking existence, so duplicates are possible. Also, when models refresh, old enabled IDs may reference removed models.Proposed idempotent toggle
- toggleDynamicModel: (modelId, enabled) => - set((state) => ({ - enabledDynamicModelIds: enabled - ? [...state.enabledDynamicModelIds, modelId] - : state.enabledDynamicModelIds.filter((id) => id !== modelId), - })), + toggleDynamicModel: (modelId, enabled) => + set((state) => { + const cur = state.enabledDynamicModelIds; + if (enabled) { + return cur.includes(modelId) ? state : { enabledDynamicModelIds: [...cur, modelId] }; + } + return { enabledDynamicModelIds: cur.filter((id) => id !== modelId) }; + }),Also applies to: 1953-1973
apps/server/src/providers/opencode-provider.ts (1)
725-776: SynchronousexecFileSyncblocks the event loop during CLI operations.While
execFileSyncis acceptable for quick CLI operations, consider using asyncexecFilefromchild_process/promisesto avoid blocking, especially since timeouts are set to 30 seconds which could freeze the server.This is a recommended improvement for future iterations - the current implementation will work but may impact server responsiveness during model discovery.
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (1)
235-235: Unnecessary type assertion on providers prop.The type assertion
as OpenCodeProviderInfo[]suggests a potential type mismatch between the store and component. IfcachedOpencodeProvidersis already typed asOpenCodeProviderInfo[]in the store, this cast is unnecessary.Consider ensuring the store's
cachedOpencodeProvidersis correctly typed to avoid the need for this assertion.apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
8-13: DuplicateOpenCodeProviderInfointerface should be shared.This interface is duplicated from
apps/server/src/providers/opencode-provider.ts. Consider moving it to@automaker/typesto maintain a single source of truth and ensure type consistency between server and client.// In @automaker/types export interface OpenCodeProviderInfo { id: string; name: string; authenticated: boolean; authMethod?: 'oauth' | 'api_key'; }Then import from the shared package in both locations.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
apps/server/src/providers/opencode-provider.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tslibs/model-resolver/src/resolver.tslibs/types/src/provider-utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/types/src/provider-utils.tsapps/ui/src/components/ui/provider-icon.tsxapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsxlibs/model-resolver/src/resolver.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/ui/src/lib/http-api-client.tsapps/server/src/providers/opencode-provider.tsapps/ui/src/store/app-store.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/routes/setup/index.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/types/src/provider-utils.tsapps/ui/src/components/ui/provider-icon.tsxapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsxlibs/model-resolver/src/resolver.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/ui/src/lib/http-api-client.tsapps/server/src/providers/opencode-provider.tsapps/ui/src/store/app-store.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/routes/setup/index.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/setup/routes/opencode-models.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/providers/opencode-provider.tsapps/server/src/routes/setup/index.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/ui/src/components/ui/provider-icon.tsxlibs/model-resolver/src/resolver.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx
🧬 Code graph analysis (8)
apps/server/src/routes/setup/routes/opencode-models.ts (2)
apps/server/src/providers/opencode-provider.ts (2)
OpencodeProvider(228-1201)OpenCodeProviderInfo(64-73)apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
OpenCodeProviderInfo(8-13)
apps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsx (4)
apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(64-73)apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
OpenCodeProviderInfo(8-13)apps/ui/src/lib/electron.ts (1)
getElectronAPI(908-917)apps/ui/src/lib/utils.ts (1)
cn(6-8)
apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (3)
libs/types/src/index.ts (1)
ModelDefinition(15-15)apps/server/src/providers/index.ts (1)
ModelDefinition(18-18)apps/ui/src/components/ui/provider-icon.tsx (4)
GeminiIcon(158-160)OpenAIIcon(150-152)AnthropicIcon(146-148)GrokIcon(162-164)
apps/server/src/routes/enhance-prompt/routes/enhance.ts (1)
libs/types/src/provider-utils.ts (1)
isOpencodeModel(103-138)
apps/ui/src/store/app-store.ts (2)
apps/ui/src/types/electron.d.ts (1)
ModelDefinition(1029-1037)libs/types/src/provider.ts (1)
ModelDefinition(222-235)
apps/server/src/routes/setup/index.ts (1)
apps/server/src/routes/setup/routes/opencode-models.ts (4)
createGetOpencodeModelsHandler(61-103)createRefreshOpencodeModelsHandler(110-133)createGetOpencodeProvidersHandler(141-166)createClearOpencodeCacheHandler(173-191)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(64-73)
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (3)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
OpencodeAuthStatus(44-51)OpenCodeProviderInfo(8-13)apps/ui/src/lib/electron.ts (1)
getElectronAPI(908-917)apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(64-73)
🪛 Biome (2.1.2)
apps/server/src/providers/opencode-provider.ts
[error] 795-795: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 1051-1051: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (18)
libs/model-resolver/src/resolver.ts (1)
72-81: No issues found. All four documented OpenCode model formats are correctly handled byisOpencodeModel()in provider-utils.ts:
opencode-*prefix (line 107)opencode/*prefix (line 119)amazon-bedrock/*prefix (line 119)provider/model-nameformat with proper validation (lines 129-135)The comment documentation in resolver.ts is accurate.
apps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsx (1)
87-96: File does not exist. The pathapps/ui/src/components/views/settings-view/providers/opencode-dynamic-providers.tsxdoes not exist in the repository. Additionally,eslint-plugin-react-hooksis neither installed nor configured in the ESLint setup, so exhaustive-deps enforcement is not active.If this review is intended for a different file (such as
opencode-settings-tab.tsx), please clarify the correct path and lines.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
1-19: The model resolution pipeline is correctly implemented. Settings storeModelAliasvalues (like'sonnet','opus','haiku') in the UI, which is the proper design. Server-side routes explicitly callresolvePhaseModel()before API usage—this function internally usesresolveModelString()to resolve aliases to canonical model IDs. Examples includeauto-mode-service.ts,generate-suggestions.ts, anddescribe-file.ts, all of which callresolvePhaseModel(phaseModelEntry)at the execution boundary. No additional resolution is needed inPhaseModelSelector.apps/server/src/providers/opencode-provider.ts (3)
793-796: Static analysis false positive - ANSI escape code regex is correct.The regex
/\x1b\[[0-9;]*m/gcorrectly matches ANSI escape sequences. The\x1b(escape character, ASCII 27) is intentional for stripping terminal color codes from CLI output. The static analysis warning about "control characters in regex" can be safely ignored here.
828-844: Model tier and default assignment logic looks correct.The
modelInfoToDefinitionfunction properly converts CLI output toModelDefinitionwith inferred tier, vision support, and a sensible default selection for Claude Sonnet 4.
1023-1089: Provider parsing implementation handles CLI output format well.The
parseProvidersOutputfunction correctly parses theopencode auth listCLI output format with proper handling of ANSI codes and provider name mapping. The fallback for unknown providers (line 1075) is a good defensive approach.apps/server/src/routes/enhance-prompt/routes/enhance.ts (2)
101-133: Provider-based execution properly handles errors and accumulates text.The
executeWithProviderfunction correctly:
- Throws on provider errors with the error message
- Accumulates text from assistant messages
- Handles final result messages appropriately
221-227: OpenCode routing integration is correct.The new branch correctly identifies OpenCode models (both static and dynamic with
provider/modelformat) and routes them through the provider-based execution path with a combined system/user prompt.apps/server/src/routes/setup/index.ts (1)
74-79: OpenCode model discovery routes properly registered.The new routes follow consistent naming patterns and are logically grouped with other OpenCode CLI routes. The API design is RESTful with appropriate HTTP methods (GET for retrieval, POST for mutations).
apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (3)
79-106: Dynamic provider configuration with sensible defaults.The
DYNAMIC_PROVIDER_CONFIGprovides good coverage for known providers with appropriate icons. The fallback ingetDynamicProviderConfigcorrectly formats unknown provider IDs into title case.
164-189: Provider sorting implementation is well-designed.The sorting logic correctly:
- Filters out 'opencode' to avoid duplication with static models
- Prioritizes known providers by their position in
dynamicProviderOrder- Falls back to alphabetical sorting for unknown providers
319-392: Dynamic models section renders correctly with loading state.The conditional rendering handles edge cases properly:
- Shows section when there are providers OR when loading (to show spinner)
- Displays "Discovering..." indicator during loading
- Each provider group has its own section with appropriate icon and badge
apps/server/src/routes/setup/routes/opencode-models.ts (2)
21-29: Singleton provider pattern is appropriate for caching.Using a module-level singleton ensures the model cache is shared across all route handlers, which is the intended behavior for performance optimization.
61-102: Get models handler correctly implements cache-aware fetching.The handler properly:
- Supports
?refresh=truequery param for force refresh- Automatically triggers refresh when only default models are cached
- Returns cache status in the response
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (2)
81-119: Two-phase loading strategy provides good UX.The implementation correctly separates fast CLI status detection (Phase 1) from slower dynamic model discovery (Phase 2). The eslint-disable comment is appropriate since the effect should only re-run when CLI status changes, not when store setters change.
205-217: Dynamic model toggle handler follows established patterns.The
handleDynamicModelTogglecallback correctly mirrors the static model toggle implementation with proper loading state management and error handling.apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
259-298: Authentication status display correctly handles multiple scenarios.The conditional rendering properly distinguishes between:
- Direct authentication with a specific method
- Authentication via connected providers
- Not authenticated state with helpful guidance
300-336: Dynamic providers connected section provides clear visibility.The new section effectively communicates:
- Number of connected providers
- Individual provider badges with auth method
- Guidance for adding more providers
The use of semantic color (emerald for success) is consistent with the rest of the component.
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx
Outdated
Show resolved
Hide resolved
| // Combine static and dynamic OpenCode models | ||
| const allOpencodeModels: ModelOption[] = useMemo(() => { | ||
| // Start with static models | ||
| const staticModels = [...OPENCODE_MODELS]; | ||
|
|
||
| // Add dynamic models (convert ModelDefinition to ModelOption) | ||
| const dynamicModelOptions: ModelOption[] = dynamicOpencodeModels.map((model) => ({ | ||
| id: model.id, | ||
| label: model.name, | ||
| description: model.description, | ||
| badge: model.tier === 'premium' ? 'Premium' : model.tier === 'basic' ? 'Free' : undefined, | ||
| provider: 'opencode' as const, | ||
| })); | ||
|
|
||
| // Merge, avoiding duplicates (dynamic takes precedence for same ID) | ||
| const staticIds = new Set(staticModels.map((m) => m.id)); | ||
| const uniqueDynamic = dynamicModelOptions.filter((m) => !staticIds.has(m.id)); | ||
|
|
||
| return [...staticModels, ...uniqueDynamic]; | ||
| }, [dynamicOpencodeModels]); | ||
|
|
There was a problem hiding this comment.
Bug: “dynamic takes precedence” merge doesn’t actually override static models.
You build staticIds and then filter out dynamic IDs already in static, which makes static take precedence (opposite of the comment/intent).
Proposed fix (replace static entries when IDs collide, keep stable ordering)
const allOpencodeModels: ModelOption[] = useMemo(() => {
// Start with static models
const staticModels = [...OPENCODE_MODELS];
@@
- // Merge, avoiding duplicates (dynamic takes precedence for same ID)
- const staticIds = new Set(staticModels.map((m) => m.id));
- const uniqueDynamic = dynamicModelOptions.filter((m) => !staticIds.has(m.id));
-
- return [...staticModels, ...uniqueDynamic];
+ // Merge with override (dynamic takes precedence for same ID), preserving static order
+ const byId = new Map<string, ModelOption>();
+ for (const m of staticModels) byId.set(m.id, m);
+ for (const m of dynamicModelOptions) byId.set(m.id, m);
+
+ const ordered: ModelOption[] = [];
+ const seen = new Set<string>();
+ for (const m of staticModels) {
+ ordered.push(byId.get(m.id)!);
+ seen.add(m.id);
+ }
+ for (const m of dynamicModelOptions) {
+ if (!seen.has(m.id)) ordered.push(byId.get(m.id)!);
+ }
+ return ordered;
}, [dynamicOpencodeModels]);🤖 Prompt for AI Agents
In
@apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx
around lines 298 - 318, The merge logic currently filters out dynamic models
that collide with static ones, making static take precedence; to fix, build
dynamicModelOptions from dynamicOpencodeModels, create a map keyed by id, then
produce the merged array by iterating OPENCODE_MODELS and replacing any static
entry with the dynamic map value when present (so dynamic overrides but keeps
static ordering), and finally append any dynamicModelOptions whose ids were not
present in OPENCODE_MODELS so their relative order is preserved; update the
allOpencodeModels useMemo to use this replace-then-append strategy (referencing
allOpencodeModels, OPENCODE_MODELS, dynamicOpencodeModels, and
dynamicModelOptions).
| setDynamicOpencodeModels: (models) => { | ||
| // When setting dynamic models, auto-enable all of them if enabledDynamicModelIds is empty | ||
| const currentEnabled = get().enabledDynamicModelIds; | ||
| const newModelIds = models.map((m) => m.id); | ||
|
|
||
| // If no models were previously enabled, enable all new ones | ||
| if (currentEnabled.length === 0) { | ||
| set({ dynamicOpencodeModels: models, enabledDynamicModelIds: newModelIds }); | ||
| } else { | ||
| // Keep existing enabled state, just update the models list | ||
| set({ dynamicOpencodeModels: models }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's search for syncSettingsToServer usage to understand the persistence pattern
rg "syncSettingsToServer" --type ts --type tsx -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Search for enabledDynamicModelIds to see where it's used and if there's persistence logic
rg "enabledDynamicModelIds" --type ts --type tsx -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Search for OpenCode settings persistence patterns
rg "opencode|OpenCode" --type ts --type tsx -l | head -20Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Look for persist or storage related patterns in app-store.ts
cat -n apps/ui/src/store/app-store.ts | grep -i "persist\|storage\|sync" | head -30Repository: AutoMaker-Org/automaker
Length of output: 2092
🏁 Script executed:
# Check the app-store.ts file structure around the setDynamicOpencodeModels function
sed -n '1940,1975p' apps/ui/src/store/app-store.tsRepository: AutoMaker-Org/automaker
Length of output: 1619
🏁 Script executed:
# Find and examine use-settings-sync.ts
find . -name "use-settings-sync.ts" -o -name "use-settings-migration.ts"Repository: AutoMaker-Org/automaker
Length of output: 153
🏁 Script executed:
# Search for references to enabledDynamicModelIds across the codebase
rg "enabledDynamicModelIds" -i --max-count 20Repository: AutoMaker-Org/automaker
Length of output: 1743
🏁 Script executed:
# Check what settings are being synced - look at the settings sync implementation
fd "settings" -i | grep -E "\.(ts|tsx)$" | head -20Repository: AutoMaker-Org/automaker
Length of output: 1426
🏁 Script executed:
# Look for OpenCode-related persistence or sync patterns
rg "opencode|OpenCode" -i -A 3 -B 3 --max-count 30Repository: AutoMaker-Org/automaker
Length of output: 50380
🏁 Script executed:
# Examine use-settings-sync.ts to understand persistence strategy
cat -n apps/ui/src/hooks/use-settings-sync.ts | head -200Repository: AutoMaker-Org/automaker
Length of output: 8362
🏁 Script executed:
# Search for what fields are included in settings sync
rg "enabledDynamic|dynamicOpencode" --max-count 20Repository: AutoMaker-Org/automaker
Length of output: 3007
🏁 Script executed:
# Check if enabledDynamicModelIds is in the Settings type definition
cat -n libs/types/src/settings.ts | grep -A 5 -B 5 "enabledDynamic\|dynamicOpencode"Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check if there are any comments explaining why OpenCode settings aren't synced
rg "enabledOpencodeModels|opencodeDefaultModel" -B 5 -A 5 --type tsRepository: AutoMaker-Org/automaker
Length of output: 14749
🏁 Script executed:
# Search for OpenCode settings in use-settings-sync.ts or related files
rg "opencode" apps/ui/src/hooks/ -i -A 3 -B 3Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check if enabledDynamicModelIds is used anywhere with persistence concerns
rg "enabledDynamicModelIds" -B 3 -A 3 --type tsRepository: AutoMaker-Org/automaker
Length of output: 7462
Add static OpenCode settings (enabledOpencodeModels, opencodeDefaultModel) to the sync list, and clarify whether dynamic model selections should persist.
Both enabledDynamicModelIds and the static OpenCode settings are currently not persisted:
enabledDynamicModelIdsis session-only (not inGlobalSettingsinterface), which appears intentional for dynamically discovered models.- However,
enabledOpencodeModelsandopencodeDefaultModelare defined inGlobalSettingsbut missing fromSETTINGS_FIELDS_TO_SYNCinuse-settings-sync.ts—compare toenabledCursorModelsandcursorDefaultModelwhich are synced. Add static OpenCode settings to the sync list if persistence is intended. For dynamic models, decide whetherenabledDynamicModelIdsshould also be synced or remain session-only.
🤖 Prompt for AI Agents
In @apps/ui/src/store/app-store.ts around lines 1953 - 1965, The static OpenCode
settings enabledOpencodeModels and opencodeDefaultModel are missing from the
SETTINGS_FIELDS_TO_SYNC list in use-settings-sync.ts; add those two keys to
SETTINGS_FIELDS_TO_SYNC so they are persisted like
enabledCursorModels/cursorDefaultModel. For dynamic models, decide whether
enabledDynamicModelIds should be session-only or persisted: if you want
persistence, add enabledDynamicModelIds to the GlobalSettings interface and
include it in SETTINGS_FIELDS_TO_SYNC; otherwise explicitly leave it out and add
a code comment in setDynamicOpencodeModels (and the GlobalSettings definition)
noting that enabledDynamicModelIds is intentionally session-scoped.
|
The other test failures (common.test.ts and execFile mock issues) are pre-existing issues and not related to this PR. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
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 (1)
135-173: Persisted OpenCode fields will likely be dropped during migration sync (buildSettingsUpdateFromStore omission).
You addedenabledOpencodeModels/opencodeDefaultModelto localStorage parsing (Line 155-156), butbuildSettingsUpdateFromStore()(Line 578+) is used to build the payload forapi.settings.updateGlobal(...)during migration (needsSyncpath, Line 451+) and forsyncSettingsToServer(); it currently does not include these two fields, so they may be lost on first migration and on any code path callingsyncSettingsToServer().Proposed fix (add fields to buildSettingsUpdateFromStore)
function buildSettingsUpdateFromStore(): Record<string, unknown> { const state = useAppStore.getState(); const setupState = useSetupStore.getState(); return { @@ enabledCursorModels: state.enabledCursorModels, cursorDefaultModel: state.cursorDefaultModel, + enabledOpencodeModels: state.enabledOpencodeModels, + opencodeDefaultModel: state.opencodeDefaultModel, autoLoadClaudeMd: state.autoLoadClaudeMd, skipSandboxWarning: state.skipSandboxWarning, @@ }; }apps/ui/src/hooks/use-settings-sync.ts (1)
29-64: Good: new OpenCode settings are now included in the synced field set. (Line 47-48)
One gap:refreshSettingsFromServer()doesn’t setenabledOpencodeModels/opencodeDefaultModel, so a manual refresh won’t pick up those server-side changes (it’ll keep stale store values due to partialsetState).Proposed fix (extend refreshSettingsFromServer setState)
useAppStore.setState({ @@ enabledCursorModels: serverSettings.enabledCursorModels, cursorDefaultModel: serverSettings.cursorDefaultModel, + enabledOpencodeModels: serverSettings.enabledOpencodeModels, + opencodeDefaultModel: serverSettings.opencodeDefaultModel, autoLoadClaudeMd: serverSettings.autoLoadClaudeMd ?? false, @@ });apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (3)
298-412: Dynamic models are merged but not filtered by “enabled” state.
allOpencodeModelsincludes alldynamicOpencodeModels(Line 298-318) andgroupedOpencodeModelsgroups whatever is inopencode(Line 373-412), but this component does not consultenabledDynamicModelIds(store) to hide disabled dynamic models. That likely makes the “enable/disable” UI ineffective for the selector.Proposed fix (filter dynamic models using enabledDynamicModelIds)
const { @@ dynamicOpencodeModels, + enabledDynamicModelIds, } = useAppStore(); @@ const allOpencodeModels: ModelOption[] = useMemo(() => { @@ - const dynamicModelOptions: ModelOption[] = dynamicOpencodeModels.map((model) => ({ + const enabledSet = new Set(enabledDynamicModelIds); + const dynamicModelOptions: ModelOption[] = dynamicOpencodeModels + .filter((model) => enabledSet.has(model.id)) + .map((model) => ({ id: model.id, label: model.name, description: model.description, badge: model.tier === 'premium' ? 'Premium' : model.tier === 'basic' ? 'Free' : undefined, provider: 'opencode' as const, - })); + })); @@ - }, [dynamicOpencodeModels]); + }, [dynamicOpencodeModels, enabledDynamicModelIds]);
595-654: Selecting a dynamic OpenCode model may be incorrectly cast to OpencodeModelId.
renderOpencodeModelItemcallsonChange({ model: model.id as OpencodeModelId })(Line 608-609). For dynamic IDs likegithub-copilot/gpt-4o, that cast is suspicious and could either (a) break type safety or (b) hide a runtime routing bug if downstream assumesopencode/...only.If
PhaseModelEntry['model']is intended to accept arbitrary dynamic strings, preferas PhaseModelEntry['model'](or justmodel.idif typed) and avoid narrowing toOpencodeModelId.Also applies to: 1078-1083
92-263: PhaseModelEntry type does not support OpenCode models — add OpencodeModelId to the model union.Line 608 casts dynamic model IDs as
OpencodeModelId, butPhaseModelEntry.model(libs/types/src/settings.ts:118) is typed asModelAlias | CursorModelId | CodexModelIdand does not includeOpencodeModelId. This type incompatibility will cause issues at runtime and should be resolved by updating thePhaseModelEntry.modelunion to includeOpencodeModelId | stringto support both static and dynamic OpenCode model IDs.
🤖 Fix all issues with AI agents
In @apps/server/src/providers/opencode-provider.ts:
- Around line 832-855: The modelIdRegex currently rejects model names with colon
version specifiers; update the regex in modelIdRegex (used to validate lines) to
include the colon character in the model-name character class (e.g. change the
right-hand class to include ':' so it matches patterns like
provider/name:version), then keep the existing split('/') logic and
modelInfoToDefinition usage unchanged; do not remove or alter the intentional
ANSI escape stripping using /\x1b\[[0-9;]*m/g as that is correct.
In @apps/server/src/routes/setup/routes/opencode-models.ts:
- Around line 140-164: The handler createGetOpencodeProvidersHandler builds and
returns an unnecessary authenticated field; remove the authenticated variable
and the authenticated property from the response object so the JSON only
contains success and providers, and delete the filtering call
providers.filter((p) => p.authenticated); also update the ProvidersResponse
type/interface (if defined) to remove the authenticated field so the response
shape matches what the UI expects.
🧹 Nitpick comments (10)
libs/prompts/tests/enhancement.test.ts (1)
402-412: Test coverage for 'ux-reviewer' mode is incomplete.The assertions correctly verify that 'ux-reviewer' exists as a mode, but the test file lacks dedicated tests for this new mode:
- No import or test for
UX_REVIEWER_SYSTEM_PROMPTandUX_REVIEWER_EXAMPLESconstants (lines 21-47 and 51-111)- No
getEnhancementPrompt('ux-reviewer')test (lines 113-175)- No
getSystemPrompt('ux-reviewer')test (lines 177-197)- No
getExamples('ux-reviewer')test (lines 199-223)- Line 334's
modesarray excludes'ux-reviewer'frombuildUserPromptiterationThe integration test at lines 444-463 does provide indirect coverage since it iterates over all modes from
getAvailableEnhancementModes(). However, adding explicit tests would improve coverage consistency.Suggested additions for complete test coverage
Add
'ux-reviewer'to the modes array at line 334:describe('all modes', () => { it('should work for all valid enhancement modes', () => { - const modes: Array<'improve' | 'technical' | 'simplify' | 'acceptance'> = [ + const modes: Array<'improve' | 'technical' | 'simplify' | 'acceptance' | 'ux-reviewer'> = [ 'improve', 'technical', 'simplify', 'acceptance', + 'ux-reviewer', ];Consider adding dedicated tests for
'ux-reviewer'in thegetEnhancementPrompt,getSystemPrompt, andgetExamplesdescribe blocks to match the pattern used for other modes.apps/ui/src/components/ui/provider-icon.tsx (1)
393-530: Dynamicprovider/modeldetection: solid guardrails; consider centralizing parsing + alias normalization.
The “exactly one slash and not a URL” check (Line 448-505) is a good safety filter.Two follow-ups:
- This file now embeds model-family detection logic (glm/claude/gpt/gemini/…) that will likely be duplicated elsewhere (selectors, provider display). Consider extracting a shared helper (e.g.,
@automaker/model-resolveror a small@automaker/typesutility) so UI/server stay consistent.- Per repo guidance, consider normalizing aliases via
resolveModelString()before string matching (e.g.,sonnet/opusinputs) to avoid mismatches if upstream starts passing aliases here. Based on learnings, useresolveModelString()from@automaker/model-resolver.apps/server/src/routes/setup/routes/opencode-models.ts (2)
61-103: GET models handler is reasonable; minor simplification + avoid unused imports.
cachedModelslocal is currently unused beyond assignment (Line 75-84). AlsocreateLogger/loggerare unused in this module (Line 17-20).
1-189: Server guideline: consider emitting operation events via createEventEmitter().
This file is underapps/server/src/**and currently doesn’t usecreateEventEmitter()for these operations. If your server convention expects WebSocket-streamed ops/telemetry for setup actions, align these endpoints with that pattern. As per coding guidelines, usecreateEventEmitter()fromlib/events.ts.apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
1-29: Repo guideline: consider resolveModelString() usage where aliases are handled.
This component works with aliases (e.g.,sonnet,opus) and full model strings; per guidance, consider normalizing viaresolveModelString()from@automaker/model-resolverat boundaries where you compare/serialize models. Based on learnings, useresolveModelString()from@automaker/model-resolver.apps/ui/src/store/app-store.ts (2)
1956-1979: enabledDynamicModelIds maintenance: dedupe + prune stale IDs.
toggleDynamicModelcan introduce duplicates when called repeatedly withenabled=true.setDynamicOpencodeModelskeepsenabledDynamicModelIdsas-is when non-empty, even if refreshed models removed some IDs; you can end up with stale enabled IDs.Proposed fix (dedupe + prune)
setDynamicOpencodeModels: (models) => { @@ const currentEnabled = get().enabledDynamicModelIds; const newModelIds = models.map((m) => m.id); + const newModelIdSet = new Set(newModelIds); // If no models were previously enabled, enable all new ones if (currentEnabled.length === 0) { set({ dynamicOpencodeModels: models, enabledDynamicModelIds: newModelIds }); } else { - // Keep existing enabled state, just update the models list - set({ dynamicOpencodeModels: models }); + // Prune enabled IDs that no longer exist in refreshed model list + const prunedEnabled = currentEnabled.filter((id) => newModelIdSet.has(id)); + set({ dynamicOpencodeModels: models, enabledDynamicModelIds: prunedEnabled }); } }, @@ - toggleDynamicModel: (modelId, enabled) => - set((state) => ({ - enabledDynamicModelIds: enabled - ? [...state.enabledDynamicModelIds, modelId] - : state.enabledDynamicModelIds.filter((id) => id !== modelId), - })), + toggleDynamicModel: (modelId, enabled) => + set((state) => { + const current = state.enabledDynamicModelIds; + if (enabled) { + return current.includes(modelId) + ? { enabledDynamicModelIds: current } + : { enabledDynamicModelIds: [...current, modelId] }; + } + return { enabledDynamicModelIds: current.filter((id) => id !== modelId) }; + }),
973-978: Type hygiene: prefer the shared OpenCodeProviderInfo authMethod union.
cachedOpencodeProvidersusesauthMethod?: string(Line 585-586) while the server type isauthMethod?: 'oauth' | 'api_key'. Keeping this as the narrower union will prevent UI drift.Also applies to: 581-586
apps/server/src/providers/opencode-provider.ts (3)
326-331: Consider usingresolveModelString()for model alias support.Per coding guidelines,
resolveModelString()from@automaker/model-resolvershould be used to convert model aliases (e.g., "haiku", "sonnet", "opus") to full model names. Currently, only the provider prefix is stripped, but aliases won't be resolved.♻️ Proposed fix
+import { resolveModelString } from '@automaker/model-resolver'; // ... in buildCliArgs if (options.model) { - const model = stripProviderPrefix(options.model); + const strippedModel = stripProviderPrefix(options.model); + const model = resolveModelString(strippedModel); args.push('--model', model); }Based on learnings, this ensures aliases like "sonnet" are properly resolved to full model identifiers.
464-471: Type assertion for undeclaredresultproperty.Line 469 uses a type assertion to access
resultproperty which isn't defined inOpenCodePart. If the OpenCode CLI returns this field instep_finishevents, consider adding it to the interface for type safety.♻️ Add result to OpenCodePart interface
interface OpenCodePart { id?: string; sessionID?: string; messageID?: string; type: string; text?: string; reason?: string; error?: string; name?: string; args?: unknown; call_id?: string; output?: string; + result?: string; tokens?: { input?: number; output?: number; reasoning?: number; }; }
885-937: Consider consolidating provider name mappings.Both
formatProviderName(lines 886-897) andformatModelDisplayName(lines 922-933) maintain separate provider-to-display-name mappings with overlapping entries. Consider extracting a shared constant to reduce duplication and ensure consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/server/src/providers/opencode-provider.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/store/app-store.tslibs/prompts/tests/enhancement.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/prompts/tests/enhancement.test.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/store/app-store.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/ui/provider-icon.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/prompts/tests/enhancement.test.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/ui/src/store/app-store.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/ui/provider-icon.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/setup/routes/opencode-models.tsapps/server/src/providers/opencode-provider.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/ui/provider-icon.tsx
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/ui/provider-icon.tsx
🧬 Code graph analysis (2)
apps/server/src/routes/setup/routes/opencode-models.ts (1)
apps/server/src/providers/opencode-provider.ts (2)
OpencodeProvider(240-1231)OpenCodeProviderInfo(67-76)
apps/ui/src/store/app-store.ts (2)
apps/ui/src/types/electron.d.ts (1)
ModelDefinition(1029-1037)libs/types/src/provider.ts (1)
ModelDefinition(222-235)
🪛 Biome (2.1.2)
apps/server/src/providers/opencode-provider.ts
[error] 839-839: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 1081-1081: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (9)
apps/ui/src/hooks/use-settings-migration.ts (1)
498-565: Hydration of OpenCode settings looks correct.
Using?? current.enabledOpencodeModels/?? current.opencodeDefaultModel(Line 544-545) is consistent with the rest of the hydration defaults.apps/ui/src/store/app-store.ts (1)
573-587: State additions look coherent; comment about “session-only” needs alignment with persisted fields.
The new dynamic fields (Line 579-586) are explicitly session-only, while static OpenCode fields are persisted elsewhere—this is coherent, but ensure UI components consistently treat onlyenabledOpencodeModels/opencodeDefaultModelas persisted andenabledDynamicModelIdsas session-only.Also applies to: 1182-1187
apps/server/src/routes/setup/routes/opencode-models.ts (1)
21-29: Singleton provider caching is safe for Electron (local/single-user) deployments, but requires architectural clarification for web deployments.The module-level singleton (lines 21–29) shares cache across all requests in a single server process. This is fine for Electron mode (inherently single-user), but for web deployments clarify whether multiple users/tenants share one instance. If they do, the shared provider and cache could leak models or authentication state across users. The
OpencodeProviderhas internal cache expiry logic (modelsCacheExpiry), which provides some safety, but explicit per-user/per-session isolation would be more robust.apps/server/src/providers/opencode-provider.ts (6)
13-33: LGTM!Imports correctly use shared packages from
@automaker/*as per coding guidelines. Good use ofpromisifyto convert callback-basedexecFileto async.
722-739: LGTM!Good use of Promise-based deduplication pattern to avoid multiple concurrent refresh operations. The
finallyblock ensures proper cleanup ofrefreshPromiseandisRefreshingstate.
770-815: LGTM!Good use of async
execFileAsyncwith appropriate timeout (30s for model fetch). The three execution strategies (NPX, WSL, direct CLI) are handled correctly with proper logging.
1075-1115: LGTM!The provider parsing logic correctly handles the OpenCode CLI output format. The static analysis warning about
\x1bon line 1081 is a false positive—this is the standard approach for stripping ANSI escape sequences from terminal output.
532-557: LGTM!Comprehensive error extraction handling multiple formats (string, object with nested
data.message,message, ornameproperties). Good defensive coding with fallback to "Unknown error".
975-982: Heuristic-based vision detection is acceptable but may need updates.The vision model detection relies on substring matching against known model families. This is a reasonable approach for dynamic discovery, but be aware it may need periodic updates as new model families are added or if text-only variants exist (e.g., some Claude models may not support vision in certain configurations).
| export function createGetOpencodeProvidersHandler() { | ||
| return async (_req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const provider = getProvider(); | ||
| const providers = await provider.fetchAuthenticatedProviders(); | ||
|
|
||
| // Filter to only authenticated providers | ||
| const authenticated = providers.filter((p) => p.authenticated); | ||
|
|
||
| const response: ProvidersResponse = { | ||
| success: true, | ||
| providers, | ||
| authenticated, | ||
| }; | ||
|
|
||
| res.json(response); | ||
| } catch (error) { | ||
| logError(error, 'Get OpenCode providers failed'); | ||
| res.status(500).json({ | ||
| success: false, | ||
| error: getErrorMessage(error), | ||
| } as ProvidersResponse); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find UI client/consumers of /api/setup/opencode/providers and confirm expected response shape.
rg -n "/api/setup/opencode/providers|getOpencodeProviders" -S apps/ui libs || trueRepository: AutoMaker-Org/automaker
Length of output: 833
🏁 Script executed:
#!/bin/bash
# Check the expected response type in the API client
sed -n '1470,1500p' apps/ui/src/lib/http-api-client.tsRepository: AutoMaker-Org/automaker
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Check how the providersResult is used in the component
sed -n '90,160p' apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxRepository: AutoMaker-Org/automaker
Length of output: 2856
Remove unused authenticated field from response.
The UI only consumes the providers field (lines 99, 150 of opencode-settings-tab.tsx) and never accesses authenticated. This redundant field should be removed from the response to avoid sending unnecessary data.
🤖 Prompt for AI Agents
In @apps/server/src/routes/setup/routes/opencode-models.ts around lines 140 -
164, The handler createGetOpencodeProvidersHandler builds and returns an
unnecessary authenticated field; remove the authenticated variable and the
authenticated property from the response object so the JSON only contains
success and providers, and delete the filtering call providers.filter((p) =>
p.authenticated); also update the ProvidersResponse type/interface (if defined)
to remove the authenticated field so the response shape matches what the UI
expects.
|
@coderabbitai review this pr now |
|
@DhanushSantosh: I'll review the changes in this PR for you now. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/server/src/providers/opencode-provider.ts:
- Around line 786-819: The test extracts parseModelsOutput and calls it unbound,
causing this to be undefined when the method invokes this.modelInfoToDefinition;
update the test to either call parseModelsOutput on the provider instance (e.g.,
provider.parseModelsOutput(output)) or bind the method before calling (e.g.,
provider.parseModelsOutput.bind(provider)), so that parseModelsOutput has the
correct this context when calling modelInfoToDefinition.
In @apps/server/tests/unit/providers/opencode-provider.test.ts:
- Around line 106-122: The test pulls out parseModelsOutput unbound which loses
the provider instance context so calls to this.modelInfoToDefinition() fail; fix
by binding the method to the provider before invoking (e.g., use
provider.parseModelsOutput.bind(provider) or call
provider.parseModelsOutput(...) directly) so parseModelsOutput retains its this
and modelInfoToDefinition resolves correctly; locate the test where
parseModelsOutput is extracted and replace the unbound reference with a bound
function or direct method call.
In @apps/ui/src/hooks/use-settings-sync.ts:
- Around line 47-49: refreshSettingsFromServer is not restoring the new OpenCode
fields added to SETTINGS_FIELDS_TO_SYNC; update refreshSettingsFromServer to
read and apply enabledOpencodeModels, opencodeDefaultModel, and
enabledDynamicModelIds from the server response into the local settings
(matching the GlobalSettings types) alongside the other synced fields so these
values are not lost during sync.
🧹 Nitpick comments (6)
apps/server/src/providers/opencode-provider.ts (1)
750-764: Consider adding timeout handling for CLI execution.The
execFileAsynccall has a 30-second timeout, which is good. However, the error handling could be more specific about timeout vs. other failures for better debugging.♻️ Optional: More specific error handling
} catch (error) { - opencodeLogger.error(`Failed to fetch models from CLI: ${error}`); + const errorMessage = error instanceof Error ? error.message : String(error); + const isTimeout = errorMessage.includes('ETIMEDOUT') || errorMessage.includes('timed out'); + opencodeLogger.error(`Failed to fetch models from CLI: ${errorMessage}${isTimeout ? ' (timeout)' : ''}`); return []; }apps/ui/src/components/ui/provider-icon.tsx (1)
476-482: Consider adding 'haiku' to the Claude model detection patterns.The pattern matching for Anthropic models checks for
claude,sonnet, andopus, but omitshaiku. Dynamic models likeprovider/claude-3-haikuwould not match this pattern and would fall through to a potentially incorrect icon.♻️ Suggested fix
if ( modelName.includes('claude') || modelName.includes('sonnet') || - modelName.includes('opus') + modelName.includes('opus') || + modelName.includes('haiku') ) { return 'anthropic'; }apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (1)
614-631: Consider: "Select all" toggles all provider models, not just filtered ones.When a search filter is active, clicking "Select all" for a provider toggles all models for that provider (
models.map), not just the filtered models (filteredModels). This may be unexpected behavior when users have an active search filter.If the intent is to toggle only visible (filtered) models:
♻️ Suggested change
{models.length > 0 && ( <div className={OPENCODE_SELECT_ALL_CONTAINER_CLASS}> <Checkbox checked={getSelectionState( - models.map((model) => model.id), + filteredModels.map((model) => model.id), enabledDynamicModelIds )} onCheckedChange={(checked) => toggleProviderDynamicModels( - models.map((model) => model.id), + filteredModels.map((model) => model.id), checked ) } disabled={isSaving} />apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (1)
85-123: Consider adding cleanup/abort handling for the async effect.The effect runs an async operation but doesn't handle component unmount scenarios. If the component unmounts while
loadDynamicContentis running, it could cause state updates on an unmounted component.♻️ Suggested fix with AbortController or mounted flag
useEffect(() => { + let isMounted = true; const loadDynamicContent = async () => { const api = getElectronAPI(); const isInstalled = cliStatus?.success && cliStatus?.status === 'installed'; if (!isInstalled || !api?.setup) return; // Skip if already have cached data const needsProviders = cachedOpencodeProviders.length === 0; const needsModels = dynamicOpencodeModels.length === 0; if (!needsProviders && !needsModels) return; setIsLoadingDynamicModels(true); try { // Load providers if needed if (needsProviders && api.setup.getOpencodeProviders) { const providersResult = await api.setup.getOpencodeProviders(); - if (providersResult.success && providersResult.providers) { + if (isMounted && providersResult.success && providersResult.providers) { setCachedOpencodeProviders(providersResult.providers); } } // Load models if needed if (needsModels && api.setup.getOpencodeModels) { const modelsResult = await api.setup.getOpencodeModels(); - if (modelsResult.success && modelsResult.models) { + if (isMounted && modelsResult.success && modelsResult.models) { setDynamicOpencodeModels(modelsResult.models); } } } catch (error) { logger.error('Failed to load dynamic content:', error); } finally { - setIsLoadingDynamicModels(false); + if (isMounted) setIsLoadingDynamicModels(false); } }; loadDynamicContent(); + return () => { isMounted = false; }; }, [cliStatus?.success, cliStatus?.status]); // eslint-disable-line react-hooks/exhaustive-depsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
8-13: MoveOpenCodeProviderInfoto@automaker/typesand import it from both the UI and server.This interface is duplicated in
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxandapps/server/src/providers/opencode-provider.ts. Both define identical shapes. Add it tolibs/types/src/opencode-models.tsand update both locations to import from@automaker/typesto ensure type consistency and prevent drift across the codebase.apps/ui/src/store/app-store.ts (1)
1958-1972: Redundant ternary expression.The ternary on line 1970 is unnecessary. If
currentEnabledis empty, thenfilteredEnabled(which filters fromcurrentEnabled) will also be empty. The expression can be simplified:- const nextEnabled = currentEnabled.length === 0 ? [] : filteredEnabled; + const nextEnabled = filteredEnabled;The filtering logic is otherwise correct—it preserves user-enabled model IDs while removing stale references to models that no longer exist.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/server/src/providers/opencode-provider.tsapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsxapps/ui/src/components/views/setup-view/steps/providers-setup-step.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/store/app-store.tslibs/types/src/opencode-models.tslibs/types/src/settings.ts
✅ Files skipped from review due to trivial changes (2)
- apps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsx
- apps/ui/src/components/views/setup-view/steps/providers-setup-step.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/hooks/use-settings-migration.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/types/src/settings.tsapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/store/app-store.tslibs/types/src/opencode-models.tsapps/ui/src/components/ui/provider-icon.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/types/src/settings.tsapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/store/app-store.tslibs/types/src/opencode-models.tsapps/ui/src/components/ui/provider-icon.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/providers/opencode-provider.ts
🧠 Learnings (2)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/ui/provider-icon.tsx
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/ui/provider-icon.tsx
🧬 Code graph analysis (4)
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (3)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
OpencodeAuthStatus(45-52)OpenCodeProviderInfo(8-13)apps/ui/src/lib/electron.ts (1)
getElectronAPI(908-917)apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(67-76)
apps/server/tests/unit/providers/opencode-provider.test.ts (2)
apps/server/src/providers/opencode-provider.ts (1)
parseModelsOutput(776-819)libs/types/src/provider.ts (1)
ModelDefinition(222-235)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(67-76)
apps/ui/src/store/app-store.ts (3)
libs/types/src/opencode-models.ts (2)
OpencodeModelId(5-11)DEFAULT_OPENCODE_MODEL(107-107)apps/ui/src/types/electron.d.ts (1)
ModelDefinition(1029-1037)libs/types/src/provider.ts (1)
ModelDefinition(222-235)
🪛 Biome (2.1.2)
apps/server/src/providers/opencode-provider.ts
[error] 788-788: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 1044-1044: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🪛 GitHub Actions: Test Suite
apps/server/src/providers/opencode-provider.ts
[error] 808-809: TypeError: Cannot read properties of undefined (reading 'modelInfoToDefinition') while parsing provider output. Ensure modelInfoToDefinition is defined before use in parseModelsOutput.
🪛 GitHub Check: test
apps/server/src/providers/opencode-provider.ts
[failure] 808-808: [server] tests/unit/providers/opencode-provider.test.ts > opencode-provider.ts > parseModelsOutput > should parse nested provider model IDs
TypeError: Cannot read properties of undefined (reading 'modelInfoToDefinition')
❯ parseModelsOutput src/providers/opencode-provider.ts:808:16
❯ tests/unit/providers/opencode-provider.test.ts:113:22
🔇 Additional comments (33)
libs/types/src/opencode-models.ts (2)
5-16: Clean type simplification for dynamic model support.The reduction of
OpencodeModelIdto free-tier models andOpencodeProviderto a single'opencode'value aligns well with the PR's dynamic model discovery approach. Static types now represent the fallback/free tier, while dynamic models are handled at runtime.
105-107: Default model updated appropriately.The default model change to
'opencode/big-pickle'is consistent with the simplified free-tier model set.apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (6)
49-97: Well-organized provider configuration constants.The provider label maps, ordering arrays, and section configuration provide a clean foundation for the dynamic model grouping UI. The separation between static and dynamic provider labels is clear.
99-121: Helper functions for provider formatting.The helper functions handle provider key formatting and section determination cleanly. One minor observation:
formatProviderLabelcorrectly handles hyphen-separated words but could produce unexpected results for edge cases like empty strings or providers with consecutive hyphens.
308-322: Dynamic model resolution with appropriate icon handling.The current model resolution now correctly checks both static and dynamic OpenCode models, using
getProviderIconForModelfor provider-specific icons. This ensures consistent icon display across all OpenCode model sources.
366-386: Proper static/dynamic model merging with duplicate prevention.The merging logic correctly prioritizes static models over dynamic ones when IDs overlap, using a Set for efficient lookup. The comment noting that IDs don't overlap in practice is helpful context.
441-527: Comprehensive provider-based grouping implementation.The
opencodeSectionsmemo implements a robust grouping system with:
- Provider resolution from multiple sources (static, dynamic, ID parsing)
- Priority-based sorting for dynamic providers
- Section-aware group labeling
The logic is well-structured and handles the complexity of multi-source model aggregation.
1193-1219: Provider-grouped rendering with proper section hierarchy.The rendering correctly uses
Fragmentfor section iteration and applies conditional styling for dynamic sections. The nested group structure provides clear visual hierarchy in the UI.apps/server/src/providers/opencode-provider.ts (5)
788-788: Biome false positive: ANSI escape code regex is intentional.The regex
/\x1b\[[0-9;]*m/gcorrectly strips ANSI escape codes from CLI output. The\x1b(ESC character, hex 1B) is the standard ANSI escape sequence prefix. This is intentional and necessary for parsing colorized terminal output.
671-712: Robust refresh workflow with deduplication.The
refreshModelsanddoRefreshModelsimplementation correctly handles:
- Concurrent refresh deduplication via
isRefreshingflag and shared promise- Graceful fallback to cached/default models on failure
- Proper cleanup of
refreshPromisein finally block
582-599: Background refresh with stale-while-revalidate pattern.The
getAvailableModelsimplementation uses a sensible caching strategy: return cached models immediately (even if stale) while triggering a background refresh. This provides good UX by avoiding blocking on network calls.
904-932: Model tier inference logic is reasonable.The tier inference based on model ID substrings covers the major model families appropriately. The fallback to 'standard' for unrecognized models is sensible.
1015-1082: Provider parsing handles formatted CLI output well.The parsing of
opencode auth listoutput correctly:
- Identifies authenticated providers via the
●symbol- Strips ANSI codes for clean parsing
- Maps provider names to canonical IDs
- Extracts auth method from the last word
The provider ID mapping is comprehensive for common providers.
apps/server/tests/unit/providers/opencode-provider.test.ts (2)
54-86: Test expectations updated correctly for new model set.The updated tests correctly reflect:
- 5 models instead of 10
- Big Pickle as the new default
- GLM 4.7 Free and MiniMax M2.1 Free as free-tier models
1239-1239: Default model assertion updated correctly.The assertion now expects
'opencode/big-pickle'as the single default model, consistent with the provider implementation.libs/types/src/settings.ts (1)
404-406: LGTM!The new
enabledDynamicModelIdsfield follows the established pattern of other enabled model arrays (e.g.,enabledCursorModels,enabledOpencodeModels). The empty array default aligns with the documented semantic that "empty means all discovered".apps/ui/src/components/ui/provider-icon.tsx (2)
409-412: LGTM!The OpenRouter check is correctly placed before Bedrock checks since
openrouter/is a distinct prefix that should be handled separately.
161-163: LGTM!The
OpenRouterIconcomponent follows the established pattern used by other provider icon exports.apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (4)
39-51: LGTM!The extended props interface cleanly adds dynamic model support while maintaining backward compatibility with optional props.
156-165: LGTM!The
getSelectionStatefunction correctly implements the three-state checkbox logic (all/none/indeterminate) with proper empty array handling.
270-288: LGTM!The effect correctly handles both initial filter setup and stale filter recovery when the provider list changes, using a ref to prevent unwanted re-initialization after user interaction.
317-349: LGTM!The toggle functions efficiently filter out already-toggled models before calling the callbacks, preventing redundant state updates.
apps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsx (2)
125-180: LGTM with note on dependency design.The signature-based deduplication (
providerRefreshSignatureRef) correctly prevents redundant refresh calls even thoughisLoadingDynamicModelsis in the dependency array. The early return at line 131 combined with the signature check at line 156 ensures no infinite loops.
266-278: LGTM!The
handleDynamicModelTogglecallback follows the same pattern ashandleModelTogglewith appropriate error handling and saving state management.apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (3)
18-36: LGTM!The display configuration handles both
copilotandgithub-copilotvariants, and the fallback chain (PROVIDER_DISPLAY[id] || name || id) ensures unknown providers display gracefully.
260-299: LGTM!The authentication status logic correctly prioritizes:
- Direct authentication with a specific method
- Provider-based authentication (shows count)
- Free tier messaging with helpful guidance
The conditional rendering is clear and handles all cases appropriately.
301-337: LGTM!The Dynamic Providers Connected section provides useful visibility into authenticated providers with:
- Provider count in the header
- Individual badges with display names and auth method indicators
- Helpful guidance for adding more providers
apps/ui/src/store/app-store.ts (6)
26-27: LGTM!Import correctly uses the shared
@automaker/typespackage per coding guidelines. TheModelDefinitiontype fromlibs/types/src/provider.tswithprovider: stringis appropriate for dynamic multi-provider models.
37-38: LGTM!Constants are well-defined and consistently reused for bedrock filtering logic in both
setDynamicOpencodeModelsandsetCachedOpencodeProviders.
576-588: LGTM!State definitions appropriately capture session-only dynamic data. The inline comments clearly document that these fields are intentionally not persisted since they depend on CLI authentication state.
975-980: LGTM!Action signatures are consistent with state definitions and follow the established patterns in the store.
1185-1188: LGTM!Initial state values correctly default to empty arrays, appropriate for session-only data that will be populated from CLI discovery.
1974-1985: LGTM!
toggleDynamicModelfollows the established toggle pattern used bytoggleCursorModelandtoggleCodexModel.setCachedOpencodeProvidersconsistently filters out bedrock, matching the filtering insetDynamicOpencodeModels.
|
@coderabbitai recheck pr. |
|
@DhanushSantosh: I'll recheck the changes in this PR for you. ✅ Actions performedReview triggered.
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/src/hooks/use-settings-sync.ts (1)
434-438: Consider extracting the repeated expression.The expression
serverSettings.opencodeDefaultModel ?? currentAppState.opencodeDefaultModelis evaluated twice. Extracting it to a variable improves readability.♻️ Suggested refactor
const validOpencodeModelIds = new Set(getAllOpencodeModelIds()); const incomingEnabledOpencodeModels = serverSettings.enabledOpencodeModels ?? currentAppState.enabledOpencodeModels; -const sanitizedOpencodeDefaultModel = validOpencodeModelIds.has( - serverSettings.opencodeDefaultModel ?? currentAppState.opencodeDefaultModel -) - ? (serverSettings.opencodeDefaultModel ?? currentAppState.opencodeDefaultModel) - : DEFAULT_OPENCODE_MODEL; +const incomingOpencodeDefaultModel = + serverSettings.opencodeDefaultModel ?? currentAppState.opencodeDefaultModel; +const sanitizedOpencodeDefaultModel = validOpencodeModelIds.has(incomingOpencodeDefaultModel) + ? incomingOpencodeDefaultModel + : DEFAULT_OPENCODE_MODEL;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/hooks/use-settings-sync.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/hooks/use-settings-sync.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/hooks/use-settings-sync.ts
🧬 Code graph analysis (2)
apps/server/tests/unit/providers/opencode-provider.test.ts (2)
apps/server/src/providers/opencode-provider.ts (1)
parseModelsOutput(776-819)libs/types/src/provider.ts (1)
ModelDefinition(222-235)
apps/ui/src/hooks/use-settings-sync.ts (1)
libs/types/src/opencode-models.ts (2)
getAllOpencodeModelIds(119-121)DEFAULT_OPENCODE_MODEL(107-107)
🔇 Additional comments (10)
apps/server/tests/unit/providers/opencode-provider.test.ts (7)
6-6: LGTM!The import correctly uses the shared package
@automaker/typesas per coding guidelines, and theModelDefinitiontype is properly used to type theparseModelsOutputresult in the new test.
54-56: LGTM!Test expectation correctly updated to reflect the new model count of 5.
59-67: LGTM!Comprehensive test for the new default model with appropriate assertions for all key properties.
70-77: LGTM!New test correctly validates the GLM 4.7 Free model properties including the
basictier.
79-86: LGTM!Test correctly validates the MiniMax M2.1 Free model properties.
106-122: LGTM!Good test coverage for the
parseModelsOutputmethod, validating that nested provider model IDs (likeopenrouter/anthropic/claude-3.5-sonnet) are correctly parsed with the provider extracted from the first path segment and the full ID preserved inmodelString.
1239-1239: LGTM!Default model ID assertion correctly updated to match the new
opencode/big-pickledefault, consistent with other test updates in this file.apps/ui/src/hooks/use-settings-sync.ts (3)
22-26: LGTM!Imports correctly use the shared
@automaker/typespackage as per coding guidelines.
51-53: LGTM!New OpenCode settings fields are correctly added to the sync list, consistent with the PR's dynamic model discovery feature.
475-477: LGTM!Sanitized values are correctly applied to the store, ensuring only valid model IDs are persisted.
- Update isOpencodeModel() to detect dynamic models with provider/model format (e.g., github-copilot/gpt-4o, google/gemini-2.5-pro, zai-coding-plan/glm-4.7) - Update resolveModelString() to recognize and pass through OpenCode models - Update enhance route to route OpenCode models to OpenCode provider - Fix OpenCode CLI command format: use --format json (not stream-json) - Remove unsupported -q and - flags from CLI arguments - Update normalizeEvent() to handle actual OpenCode JSON event format - Add dynamic model configuration UI with provider grouping - Cache providers and models in app store for snappier navigation - Show authenticated providers in OpenCode CLI status card Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update error event interface to handle nested error objects with name/data/message structure from OpenCode CLI - Extract meaningful error messages from provider errors in normalizeEvent - Add error type handling in executeWithProvider to throw errors with actual provider messages instead of returning empty response Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add icon detection for dynamic OpenCode provider models (provider/model format) - Support zai-coding-plan, github-copilot, google, xai, and other providers - Detect model type from name (glm, claude, gpt, gemini, grok, etc.) - Fix React.useMemo → useMemo to resolve "React is not defined" error Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Convert execFileSync to async execFile in fetchModelsFromCli and fetchAuthenticatedProviders to avoid blocking the event loop - Remove unused opencode-dynamic-providers.tsx component - Use regex for more robust model ID validation in parseModelsOutput Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test expected 4 enhancement modes but there are now 5 after adding the ux-reviewer mode. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace busy-wait loop in refreshModels with Promise-based approach - Remove duplicate error logging in opencode-models.ts handlers - Fix multi-slash parsing in provider-icon.tsx (only handle exactly one slash) - Use dynamic icon resolution for selected OpenCode model in trigger - Fix misleading comment about merge precedence (static takes precedence) - Add enabledOpencodeModels and opencodeDefaultModel to settings sync - Add clarifying comments about session-only dynamic model settings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add enabledOpencodeModels and opencodeDefaultModel to the settings migration to ensure they are properly persisted like Cursor settings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
DhanushSantosh
force-pushed
the
opencode-dynamic-providers
branch
from
e19e1f0 to
6020219
Compare
2 participants
Summary
opencode modelsCLI command and caches them in the Zustand storeopencode auth listCLI command in the settings UIgithub-copilot/gpt-4o,google/gemini-2.5-pro) to the OpenCode providerChanges
opencode-provider.ts: Enhanced with model discovery and improved event normalizationopencode-models.ts: New route for fetching dynamic models and providersprovider-icon.tsx: Added dynamic model icon detectionphase-model-selector.tsx: Added provider-based grouping in dropdownopencode-model-configuration.tsx: Added dynamic model configuration UIopencode-cli-status.tsx: Shows connected dynamic providersresolver.ts: Recognizes dynamicprovider/modelformatprovider-utils.ts: EnhancedisOpencodeModelto detect dynamic modelsTest plan
opencode auth login(e.g., GitHub Copilot, Google AI)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.