Multi cli integration #279
Conversation
- Add CodexProvider with streaming JSON execution support - Add OpenCodeProvider with MCP server integration - Add CursorProvider with authentication and model support - Add CodexConfigManager for MCP server configuration - Support for multiple model tiers (basic, standard, premium) - CLI path resolution and authentication checking
- Add LlmJudge utilities for evaluating agent outputs - Support direct scoring, pairwise comparison, and rubric generation - Add SubprocessManager for JSONL streaming CLI execution - Implement quality assessment and improvement suggestions - Enable automated evaluation of agent responses and code quality
- Add Codex, OpenCode, and Cursor status routes - Add Cursor installation and authentication verification - Add default provider configuration route - Add provider testing capabilities - Enable CLI provider setup and monitoring via API endpoints
- Remove backlog-plan routes and common utilities - Remove agent queue routes (add, clear, list, remove) - Remove deprecated UI components (backlog-plan dialog, claude settings) - Remove unused settings-helpers and os-detection hook - Clean up codebase by removing outdated functionality
- Extend provider types to support Codex, OpenCode, and Cursor - Update ProviderFactory to instantiate new CLI providers - Add CLI-specific configuration options and model definitions - Support for MCP servers, authentication, and streaming execution - Enhance provider detection and installation status tracking
- Add CLI status components for Codex, OpenCode, and Cursor - Add setup step components for CLI provider installation - Add AI provider settings section with CLI provider support - Enable real-time CLI status monitoring in settings UI - Add provider authentication and configuration interfaces
- Integrate LLM judge for automated agent output evaluation - Add auto-critique system to agent context parser - Enhance auto-mode service with quality assessment - Improve agent execution with feedback loops and corrections - Add context-aware agent behavior with self-improvement capabilities
- Add comprehensive tests for Cursor provider functionality - Update provider factory tests to include new CLI providers - Update model resolver tests for new provider mappings - Update agent service tests for auto-critique integration - Remove tests for deprecated backlog-plan and queue features
- Add provider sync hook for real-time CLI provider status - Enhance board view with improved model selection and constants - Update settings view with CLI status monitoring and navigation - Improve agent view with enhanced context parsing capabilities - Update setup view with provider-specific installation steps - Enhance sidebar with trash operations and file browser improvements - Update themes and global styles for better CLI integration - Improve electron integration with provider synchronization - Add path input enhancements and keyboard shortcut improvements - Update app store and setup store for CLI provider support
|
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 three new CLI-backed providers (Cursor, OpenCode, Codex), provider routing/defaults, subprocess JSONL utilities, LLM-judge, UI setup/settings for provider/model selection and CLI status/install/verify flows, model/type expansions, frontend-backend default-provider sync, and multiple server and UI integrations and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend
participant Store as App Store
participant Backend as Server API
participant Factory as ProviderFactory
participant Provider as CLI Provider
participant CLI as Native CLI
UI->>Store: setDefaultProvider('cursor')
Store->>Backend: POST /api/setup/default-provider
Backend->>Factory: ProviderFactory.setDefaultProvider('cursor')
Factory-->>Backend: ack
Backend-->>Store: success
UI->>Backend: POST /api/app-spec/generate (model='auto')
Backend->>Factory: getProviderForModel('auto')
Factory-->>Backend: CursorProvider
Backend->>Provider: executeQuery(options)
Provider->>CLI: spawn cursor-agent (jsonl)
CLI-->>Provider: stream JSONL events
Provider-->>Backend: stream ProviderMessage events
Backend-->>UI: stream result to frontend
sequenceDiagram
actor User
participant UI as Frontend Setup
participant Backend as Server
participant CLI as cursor-agent
participant Auth as Auth parser
User->>UI: Open Cursor Setup
UI->>Backend: GET /api/setup/cursor-status
Backend->>CLI: which/where + common-path checks
CLI-->>Backend: {installed?, path, version}
Backend-->>UI: status
alt Not installed
UI->>Backend: POST /api/setup/install-cursor
Backend->>CLI: run installer shell
CLI-->>Backend: install output
Backend-->>UI: install result
end
UI->>Backend: POST /api/setup/verify-cursor-auth
Backend->>CLI: spawn cursor-agent status (with env/key)
CLI-->>Backend: status output
Backend->>Auth: parse auth indicators
Auth-->>Backend: {authenticated?, method}
Backend-->>UI: auth result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 @DealSegfault, 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 expands the application's AI capabilities by integrating multiple external CLI agents, including Cursor, OpenCode, and Codex. It introduces an advanced 'LLM-as-a-Judge' system to improve the quality and efficiency of autonomous agent operations, particularly in plan generation and context summarization. Concurrently, several legacy features like backlog planning and agent queuing have been removed, and internal context management has been refactored for a cleaner architecture. These changes aim to provide users with more flexible and powerful AI-driven development workflows. 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 significant changes to the server's AI provider architecture, deprecating the backlog-plan feature and CLAUDE.md auto-loading in favor of a more flexible, multi-provider system. New providers for Cursor, OpenCode, and Codex CLIs are added, along with LLM-as-a-Judge utilities for evaluating AI outputs. The server's index.ts is refactored to remove SettingsService dependencies from several routes and services, and a new subprocess-manager.ts handles JSONL streaming from external CLIs. The UI is updated to reflect these changes, including new setup steps for Cursor, Codex, and OpenCode, a unified AI provider selection in settings, and removal of the backlog-plan dialog and agent queue functionality. Review comments highlighted a ReferenceError in subprocess-manager.ts due to an undeclared variable, a potential issue with claude-provider.ts discarding final results, and the need to replace console.log statements with a centralized logger in cursor-provider.ts for better log management.
|
|
||
| child.stderr?.on('data', (chunk) => { | ||
| stderr += chunk.toString(); | ||
| lastOutput = Date.now(); |
There was a problem hiding this comment.
The variable lastOutput is used here but it's not declared within the scope of the spawnProcess function. This will cause a ReferenceError at runtime. It seems this line was mistakenly copied from spawnJSONLProcess, where lastOutput is used for timeout handling. Since spawnProcess does not implement a timeout, this line is unnecessary and should be removed.
apps/server/src/lib/llm-judge.ts
Outdated
| if (!responseText && typeof msg.result === 'string') { | ||
| responseText = msg.result; | ||
| } |
There was a problem hiding this comment.
The current logic for collecting provider text might discard the final, complete result message if any partial assistant text has already been received. The condition !responseText prevents the final result from overwriting the accumulated partial text. To ensure the definitive final result is always used when available, the check for !responseText should be removed.
| if (!responseText && typeof msg.result === 'string') { | |
| responseText = msg.result; | |
| } | |
| if (typeof msg.result === 'string') { | |
| responseText = msg.result; | |
| } |
| logger.debug(`[spawn] cursor-agent ${args.slice(0, 6).join(' ')}... [prompt truncated]`); | ||
|
|
||
| // Spawn the cursor-agent process (NOT 'cursor agent') | ||
| console.log('[CursorProvider] Spawning cursor-agent process...'); |
There was a problem hiding this comment.
This file contains numerous console.log statements for debugging. While helpful during development, they can create noise in production logs. It's recommended to replace them with the centralized logger utility, which allows for consistent formatting and control over log levels across different environments. For instance, this log could be changed to logger.info(...).
| console.log('[CursorProvider] Spawning cursor-agent process...'); | |
| logger.info('[CursorProvider] Spawning cursor-agent process...'); |
Shironex
added
Enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
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/lib/agent-context-parser.ts (1)
49-54: The fallback logic at line 53 has a critical bug for O-series models.Models from the Cursor and OpenCode providers are properly handled with explicit checks (lines 38, 48). However, Codex's O-series models (
o1,o3) contain no hyphens and will fail the fallback logic:'o1'.split('-').slice(1, 3).join(' ') // Returns empty string ''Add explicit handling for O-series models:
if (model.startsWith('o') && /^o\d/.test(model)) return `O${model.slice(1).toUpperCase()}`;Also note:
provider-factory.tsimportsCURSOR_MODEL_MAP,OPENCODE_MODEL_MAP, andCODEX_MODEL_MAPfrom@automaker/types, but these are not defined or exported there—onlyCLAUDE_MODEL_MAPexists. This will cause a compilation error.apps/ui/src/hooks/use-electron-agent.ts (1)
27-37: Busy-agent messages are dropped instead of queued; wireaddToQueueor remove queueRight now, if
isProcessingis true,sendMessagejust logs a warning and returns, whileuseMessageQueueandqueuedMessagesare never actually used to enqueue anything. That means user messages sent while the agent is busy are silently discarded instead of being queued for later processing.Given the comment and presence of queue state, it looks like the intent is to queue messages when the agent is busy. Consider either:
- Wiring
sendMessagetoaddToQueuewhenisProcessingis true (and usingsendMessageDirectlyonly from the queue), or- Removing the queue-related state entirely if you no longer want client-side queuing.
Example of the first option:
Proposed wiring to enqueue when busy
- const { queuedMessages, isProcessingQueue, addToQueue, clearQueue, processNext } = + const { queuedMessages, isProcessingQueue, addToQueue, clearQueue, processNext } = useMessageQueue({ onProcessNext: async (queuedMessage) => { await sendMessageDirectly( queuedMessage.content, queuedMessage.images, queuedMessage.textFiles ); }, }); const sendMessage = useCallback( async (content: string, images?: ImageAttachment[], textFiles?: TextFileAttachment[]) => { const api = getElectronAPI(); if (!api?.agent) { setError('API not available'); return; } if (isProcessing) { - console.warn('[useElectronAgent] Already processing a message'); - return; + // Queue the message instead of dropping it + addToQueue({ + id: crypto.randomUUID ? crypto.randomUUID() : `${Date.now()}`, + content, + images, + textFiles, + timestamp: new Date(), + }); + return; }Also applies to: 136-147, 316-333
apps/ui/src/lib/electron.ts (1)
484-553: SetupAPI interface is missinggetCodexStatus, causing type drift with implementations
ElectronAPI.setupandcreateMockSetupAPIboth define agetCodexStatusmethod, but theSetupAPIinterface does not. BecausecreateMockSetupAPI(): SetupAPIreturns an object literal withgetCodexStatus, this will either:
- Fail type-checking in strict TypeScript (excess property on object literal), or
- Leave
getCodexStatuseffectively untyped whereverSetupAPIis used.To keep the types and implementations aligned, add
getCodexStatustoSetupAPIwith the same shape used inElectronAPI.setupandcreateMockSetupAPI.Proposed `SetupAPI` interface update
interface SetupAPI { getClaudeStatus: () => Promise<{ success: boolean; status?: string; installed?: boolean; method?: string; version?: string; path?: string; auth?: { authenticated: boolean; method: string; hasCredentialsFile?: boolean; hasToken?: boolean; hasStoredOAuthToken?: boolean; hasStoredApiKey?: boolean; hasEnvApiKey?: boolean; hasEnvOAuthToken?: boolean; hasCliAuth?: boolean; hasRecentActivity?: boolean; }; error?: string; }>; + getCursorStatus?: () => Promise<{ + success: boolean; + status?: string; + installed?: boolean; + method?: string; + version?: string; + path?: string; + auth?: { + authenticated: boolean; + method: string; + hasApiKey?: boolean; + hasStoredApiKey?: boolean; + hasEnvApiKey?: boolean; + apiKeyValid?: boolean; + }; + error?: string; + }>; + getOpenCodeStatus?: () => Promise<{ + success: boolean; + status?: string; + installed?: boolean; + method?: string; + version?: string; + path?: string; + auth?: { + authenticated: boolean; + method: string; + hasApiKey?: boolean; + apiKeyValid?: boolean; + hasEnvApiKey?: boolean; + }; + error?: string; + }>; + getCodexStatus?: () => Promise<{ + success: boolean; + status?: string; + installed?: boolean; + method?: string; + version?: string; + path?: string; + auth?: { + authenticated: boolean; + method: string; + hasApiKey?: boolean; + apiKeyValid?: boolean; + hasEnvApiKey?: boolean; + }; + error?: string; + }>; - getCursorStatus?: () => Promise<...>; - getOpenCodeStatus?: () => Promise<...>;(You can reorder as you prefer; key point is to define
getCodexStatusinSetupAPI.)Also applies to: 1115-1170, 1234-1298
apps/server/src/services/auto-mode-service.ts (1)
1999-2018:auto_mode_progressis emitted twice for the same text chunks before spec generationIn
runAgent, each text block currently triggers two progress events whilespecDetectedis still false:
- New emit near the top of the text-block handling:
this.emitAutoModeEvent('auto_mode_progress', { featureId, content: block.text })- Existing emit later in the same branch guarded by
if (!specDetected) { ... }.This will cause duplicate progress messages in the UI for all planning/implementation output up until
[SPEC_GENERATED]is seen.You can fix this by keeping a single emit site (the earlier one is fine) and removing the later
if (!specDetected)block.Proposed fix: remove redundant emit
if (block.type === 'text') { // Add separator before new text if we already have content and it doesn't end with newlines if (responseText.length > 0 && !responseText.endsWith('\n\n')) { if (responseText.endsWith('\n')) { responseText += '\n'; } else { responseText += '\n\n'; } } responseText += block.text || ''; - // Emit progress event for real-time UI updates - this.emitAutoModeEvent('auto_mode_progress', { - featureId, - content: block.text, - }); + // Emit progress event for real-time UI updates + this.emitAutoModeEvent('auto_mode_progress', { + featureId, + content: block.text, + }); // Check for authentication errors ... ... - // Only emit progress for non-marker text (marker was already handled above) - if (!specDetected) { - this.emitAutoModeEvent('auto_mode_progress', { - featureId, - content: block.text, - }); - }Also applies to: 2470-2476
apps/server/src/index.ts (1)
115-125: GuardagentService.initialize()against startup failures
agentService.initialize()is now awaited in an async IIFE without any error handling. If initialization throws (e.g., corrupt data dir, schema mismatch), you’ll get an unhandled rejection and a server that may continue running in a partially initialized state. Wrapping this in a try/catch and either logging and exiting, or clearly marking the service as unavailable, would make startup behavior more predictable and easier to debug.
🧹 Nitpick comments (40)
apps/ui/src/components/ui/path-input.tsx (1)
235-244: Consider using a button element for breadcrumb navigation.Using
href="#"withpreventDefaultis a common but technically suboptimal pattern. It can cause unwanted scroll-to-top behavior if preventDefault fails, and is semantically incorrect for non-link actions.If
BreadcrumbLinkaccepts anasChildprop or similar composition pattern (common in Radix-based UI libraries), consider wrapping a<button>instead:<BreadcrumbLink asChild> <button type="button" onClick={() => handleBreadcrumbClick(crumb.path)} className="font-mono text-xs text-muted-foreground hover:text-foreground transition-colors truncate max-w-[150px]" > {crumb.name} </button> </BreadcrumbLink>This is a minor accessibility improvement and can be deferred if the component API doesn't support it easily.
apps/ui/src/lib/log-parser.ts (1)
267-304: LGTM with optional DRY improvement.The normalization integration and new
Patch/Listcases are well-implemented. The file path extraction logic is consistent across cases.Consider extracting the repeated file path resolution into a helper to reduce duplication:
🔎 Optional refactor to reduce repetition
// Helper at top of function or as separate utility const getFilePath = (parsed: Record<string, unknown>): string | undefined => (parsed.file_path as string | undefined) || (parsed.filePath as string | undefined) || (parsed.path as string | undefined);This would simplify each case to:
case 'Read': { const filePath = getFilePath(parsed); return `Reading ${filePath?.split('/').pop() || 'file'}`; }apps/ui/src/components/dialogs/file-browser-dialog.tsx (1)
369-374: Consider replacingnavigator.platformwith the modern User-Agent Client Hints API.
navigator.platformis deprecated. Usenavigator.userAgentData.platforminstead:navigator.userAgentData?.platform?.includes('Mac') ? '⌘' : 'Ctrl'This is only a UI hint, so the current code remains functional. The keyboard event handler (lines 213-215) correctly uses
e.metaKey || e.ctrlKey, which is the proper approach and not deprecated.apps/ui/src/styles/global.css (1)
434-451: Optional: Reduce redundancy in retro theme scrollbar styling.The retro theme is included in the
:is(.dark, .retro)selector at lines 434-441, but immediately overridden by retro-specific rules at lines 444-450. While this works correctly due to CSS cascade, it creates redundant declarations that could confuse maintainers.🔎 Suggested refactor
Option 1: Remove retro from the shared
:is()block:-:is(.dark, .retro) ::-webkit-scrollbar-thumb { +.dark ::-webkit-scrollbar-thumb { background: oklch(0.3 0 0); border-radius: 4px; } -:is(.dark, .retro) ::-webkit-scrollbar-thumb:hover { +.dark ::-webkit-scrollbar-thumb:hover { background: oklch(0.4 0 0); } /* Retro Scrollbar override */ .retro ::-webkit-scrollbar-thumb { background: var(--primary); border-radius: 0; }Option 2: Add a comment explaining the intentional override pattern if this structure is preferred for organizational reasons.
apps/ui/src/components/views/settings-view/config/navigation.ts (1)
25-26: The navigation item uses provider-specific id but generic label — confirm intentional design choice.The
id: 'cursor'maps to a multi-provider settings section, as confirmed by the rendered content:AIProviderSectionallows selecting Cursor, Codex, OpenCode, or Claude, with corresponding CLI status displays. While this naming mismatch doesn't break functionality, renaming the id to'ai-provider'would improve clarity and better reflect the actual multi-provider purpose of this settings section.Before making the change, verify whether 'cursor' as the primary entry point for AI provider selection is intentional (e.g., Cursor as the "hero" provider). If so, consider adding a brief comment to explain why a provider-specific id labels a generic multi-provider interface.
apps/ui/src/components/views/profiles-view/utils.ts (1)
10-21: Add explicit CLAUDE_MODELS validation for clarity and consistency.The function currently defaults to
'claude'without an explicit check. While the profile form restricts model selection to Claude models, adding an explicit check improves clarity and aligns with the pattern used inmodel-selector.tsx:export function getProviderFromModel(model: AgentModel): ModelProvider { if (CODEX_MODELS.some((option) => option.id === model)) { return 'codex'; } if (OPENCODE_MODELS.some((option) => option.id === model)) { return 'opencode'; } if (CURSOR_MODELS.some((option) => option.id === model)) { return 'cursor'; } + if (CLAUDE_MODELS.some((option) => option.id === model)) { + return 'claude'; + } - return 'claude'; + // Fallback for unknown models (should not happen with proper typing) + return 'claude'; }Model IDs are unique across all provider arrays, so no conflicts exist with the current order-dependent checks.
apps/ui/src/lib/agent-context-parser.ts (1)
39-47: Consider handling additional GPT variants.The GPT parsing logic correctly extracts version and recognizes 'codex', 'max', and 'mini' suffixes. However, other common GPT variants like 'turbo', 'preview', or 'instruct' will be silently ignored in the display label.
🔎 Proposed enhancement to handle additional GPT suffixes
if (model.startsWith('gpt-')) { const [, version, ...suffixes] = model.split('-'); if (!version) return model; let label = `GPT-${version}`; if (suffixes.includes('codex')) label += ' Codex'; + if (suffixes.includes('turbo')) label += ' Turbo'; + if (suffixes.includes('preview')) label += ' Preview'; + if (suffixes.includes('instruct')) label += ' Instruct'; if (suffixes.includes('max')) label += ' Max'; if (suffixes.includes('mini')) label += ' Mini'; return label; }apps/server/src/providers/codex-config-manager.ts (1)
20-68: Codex MCP config writer looks solid; only minor TOML robustness nitsThe overall flow (read existing, append non-duplicate server blocks, and TOML-escape values) is sound and should work well with Codex’s CLI config expectations. Two small, optional robustness points:
content.includes(header)can theoretically match inside comments/strings; if you ever hit false positives, consider anchoring on line starts (e.g., regex over lines) rather than raw substring search.- Env keys are written as bare identifiers (
KEY = "value"). That’s fine for typicalABC_123-style keys, but if you ever support keys with spaces or exotic chars, you may want to quote the key name as well.No blockers from this file.
Also applies to: 94-145
apps/server/src/services/auto-mode-service.ts (1)
59-69: Judge/summarization integration is powerful but adds several extra LLM calls per runThe new judge-based workflows (
maybeGenerateContextSummary,maybeRevisePlanWithJudge) give you nice capabilities:
- Automatic context summaries for follow-ups (
agent-summary.md+feature.summary).- Plan quality scoring and optional revisions before execution.
However, when
AUTOMAKER_JUDGE_ENABLEDandAUTOMAKER_CONTEXT_SUMMARY/AUTOMAKER_PLAN_JUDGEare left at their default “on” values, a single feature run can now invoke multiple extra model calls (e.g., rubric generation once per process, tworunJudgeTextcalls, apairwiseCompare, and adirectScore, plus optional plan revision streams).Given the cost/latency implications, it may be worth:
- Documenting these env flags prominently so operators can disable them in resource‑constrained environments.
- Optionally short‑circuiting judge/summary for obviously small runs (e.g., very short
responseText, or planningMode === 'skip' + no spec detected), beyond the currentSUMMARY_MIN_CHARSgate.No correctness issue here—just a consideration for operational cost and performance.
Also applies to: 2611-2765, 2767-2890
apps/ui/src/app.tsx (1)
6-7: Startup provider sync hook is a reasonable place for default-provider alignmentCalling
useProviderSync()once inAppkeeps the backend’s default provider in sync with the store on startup, without complicating router/layout components. Since the hook already guards onhasSyncedand logs errors internally, this is a low‑risk integration.Also applies to: 25-27
apps/server/src/routes/models/routes/providers.ts (1)
20-29: Consider API consistency for provider response structure.The new provider entries have inconsistent structures:
anthropicandcodexinclude bothavailableandhasApiKeycursorandopencodeonly includeavailableWhile this may align with each provider's authentication model (cursor/opencode supporting free tiers), the inconsistent API shape could complicate client-side handling. Consider either:
- Adding
hasApiKey: falseto cursor/opencode for consistency- Documenting the optional nature of
hasApiKeyin the API contractapps/ui/src/components/views/board-view/hooks/use-board-features.ts (1)
59-67: LGTM with a suggestion for validation.The change correctly replaces the hardcoded 'opus' fallback with the dynamic
defaultModelfrom the store, enabling multi-provider support. The backward compatibility approach is sound for features loaded from disk.Consider adding validation to ensure the resolved model is valid:
Optional: Add model validation
const featuresWithIds = result.features.map((f: any, index: number) => ({ ...f, id: f.id || `feature-${index}-${Date.now()}`, status: f.status || 'backlog', startedAt: f.startedAt, // Preserve startedAt timestamp // Ensure model and thinkingLevel are set for backward compatibility - model: f.model || defaultModel, + model: (f.model && isValidModel(f.model)) ? f.model : defaultModel, thinkingLevel: f.thinkingLevel || 'none', }));This would protect against corrupted or obsolete model values in persisted features.
apps/server/tests/unit/lib/model-resolver.test.ts (2)
54-60: Consider using CODEX_MODEL_MAP constants instead of hardcoded strings.The test hardcodes specific Codex model strings like
'gpt-5.2'and'gpt-5.1-codex'. For consistency with the Cursor tests (lines 73-79) that useObject.values(CURSOR_MODEL_MAP), consider using constants fromCODEX_MODEL_MAPif these models are defined there.Optional: Use map values for consistency
it('should pass through Codex model strings', () => { - const models = ['gpt-5.2', 'gpt-5.1-codex', 'o1']; + const models = Object.values(CODEX_MODEL_MAP); models.forEach((model) => { const result = resolveModelString(model); expect(result).toBe(model); }); });
73-87: Clarify distinction between "passthrough" and "resolve" tests.Lines 73-79 test "pass through Cursor model aliases" and lines 81-87 test "resolve Codex model aliases", but both verify that
resolveModelString(model)returnsmodelunchanged. The distinction between "passthrough" and "resolve" is unclear—are these testing different behaviors, or is this duplication?If both provider model maps should pass through unchanged, consider:
- Merging into a single parameterized test, or
- Clarifying the test names to explain the difference
apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx (1)
81-84: Authentication status icon inconsistency.The CheckCircle2 icon is displayed regardless of whether the user is authenticated or not. When not authenticated, this could be misleading as a green checkmark typically implies success.
🔎 Proposed fix to show appropriate icon based on auth state
- <div className="flex items-center gap-2 text-xs text-sky-400/70 ml-1"> - <CheckCircle2 className="w-3.5 h-3.5" /> - <span>{isAuthenticated ? 'Authenticated' : 'Run: codex login'}</span> - </div> + <div className={cn( + "flex items-center gap-2 text-xs ml-1", + isAuthenticated ? "text-sky-400/70" : "text-amber-400/70" + )}> + {isAuthenticated ? ( + <CheckCircle2 className="w-3.5 h-3.5" /> + ) : ( + <AlertCircle className="w-3.5 h-3.5" /> + )} + <span>{isAuthenticated ? 'Authenticated' : 'Run: codex login'}</span> + </div>apps/server/src/routes/models/routes/available.ts (1)
21-157: Consider extracting model definitions to a shared constant or configuration.The hardcoded model catalog will require code changes each time models are added/updated. For now this is acceptable, but as the model list grows, consider extracting to a configuration file or database-backed source for easier maintenance.
apps/ui/src/hooks/use-provider-sync.ts (2)
16-34: Dependency array includesdefaultProviderbut effect only runs once.The
useEffectdepends ondefaultProvider, but thehasSynced.currentguard ensures only a single execution. While functionally correct (store changes sync separately), the dependency could be removed or replaced with an empty array to better reflect the "once on mount" intent.🔎 Proposed clarification
useEffect(() => { // Only sync once on mount if (hasSynced.current) return; hasSynced.current = true; const syncProvider = async () => { try { const api = getElectronAPI(); if (api.setup && 'setDefaultProvider' in api.setup) { await (api.setup as any).setDefaultProvider(defaultProvider); console.log(`[ProviderSync] Synced default provider: ${defaultProvider}`); } } catch (error) { console.error('[ProviderSync] Failed to sync provider:', error); } }; syncProvider(); - }, [defaultProvider]); + // Note: defaultProvider is captured at mount time; subsequent changes sync via store + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []);
24-26: Type assertion bypasses type safety.The cast to
anyon line 25 avoids type checking. If the API shape is dynamic, consider using a more precise type or type guard.apps/server/tests/unit/providers/cursor-provider.test.ts (1)
130-170: Consider adding error scenario tests.The existing tests cover happy-path buffering well. Consider adding tests for:
- Error messages during buffering
- Empty stream (no messages)
- Process exit with non-zero code
apps/server/src/routes/setup/routes/default-provider.ts (1)
29-37: Consider adding type narrowing after validation.After the validation check,
provideris still typed asanyfromreq.body. Adding explicit type narrowing improves type safety.🔎 Proposed fix
+type ValidProvider = 'claude' | 'cursor' | 'opencode' | 'codex'; +const VALID_PROVIDERS: ValidProvider[] = ['claude', 'cursor', 'opencode', 'codex']; + export function createSetDefaultProviderHandler() { return async (req: Request, res: Response): Promise<void> => { try { const { provider } = req.body || {}; - if (!provider || !['claude', 'cursor', 'opencode', 'codex'].includes(provider)) { + if (!provider || !VALID_PROVIDERS.includes(provider as ValidProvider)) { res.status(400).json({ success: false, error: 'Invalid provider. Must be "claude", "cursor", "opencode", or "codex".', }); return; } - ProviderFactory.setDefaultProvider(provider); + ProviderFactory.setDefaultProvider(provider as ValidProvider);apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (1)
107-115: Consider adding a security awareness note for installation commands.The installation commands pipe remote scripts directly to shell interpreters (
curl ... | bashandiwr ... | iex), which is a common but potentially risky pattern. While these are official Cursor installation commands, consider adding a brief note advising users to review the script source or link to Cursor's official documentation for transparency.Optional: Add security context
<div className="space-y-3"> - <p className="text-xs font-medium text-foreground/80">Installation Commands:</p> + <p className="text-xs font-medium text-foreground/80"> + Installation Commands: + </p> + <p className="text-[10px] text-muted-foreground/70 -mt-1"> + These commands download and execute the official Cursor CLI installer. + </p> <div className="space-y-2">apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx (3)
34-41: Consider refactoring nested ternary for readability.The nested ternary operator for model selection works correctly but can be difficult to read and maintain. A switch statement or object lookup would be more maintainable.
Refactor to object lookup for better readability
- const models = - defaultProvider === 'cursor' - ? CURSOR_MODELS - : defaultProvider === 'codex' - ? CODEX_MODELS - : defaultProvider === 'opencode' - ? OPENCODE_MODELS - : CLAUDE_MODELS; + const MODEL_MAP = { + cursor: CURSOR_MODELS, + codex: CODEX_MODELS, + opencode: OPENCODE_MODELS, + claude: CLAUDE_MODELS, + } as const; + + const models = MODEL_MAP[defaultProvider];
48-56: Simplify default model selection logic.Similar to the model array selection, this nested ternary for default model selection could be more readable as an object lookup.
Refactor to object lookup
// Set a sensible default model for the new provider - if (provider === 'cursor') { - onModelChange('auto'); - } else if (provider === 'codex') { - onModelChange('gpt-5.2-codex'); - } else if (provider === 'opencode') { - onModelChange('glm4.7'); - } else { - onModelChange('sonnet'); - } + const DEFAULT_MODELS: Record<ModelProvider, AgentModel> = { + cursor: 'auto', + codex: 'gpt-5.2-codex', + opencode: 'glm4.7', + claude: 'sonnet', + }; + onModelChange(DEFAULT_MODELS[provider]);
319-341: Simplify info box conditional rendering.The nested ternary for provider-specific info text could be more maintainable as an object lookup, similar to the earlier suggestions.
Refactor to object lookup
+ const PROVIDER_INFO: Record<ModelProvider, React.ReactNode> = { + cursor: ( + <> + <strong className="text-purple-400">Cursor Auto Mode</strong> automatically selects + the best model for each task. You can also explicitly choose Claude Sonnet through + the Cursor platform. + </> + ), + codex: ( + <> + <strong className="text-sky-400">Codex CLI</strong> runs locally with JSONL + streaming, tool use, and MCP server support using GPT-5.x Codex models. + </> + ), + opencode: ( + <> + <strong className="text-emerald-400">OpenCode CLI</strong> uses your local + configuration to run the free GLM 4.7 model with tool support. + </> + ), + claude: ( + <> + <strong className="text-amber-400">Claude SDK</strong> provides direct access to + Anthropic's models with full control over thinking levels and extended context + windows. + </> + ), + }; + <div className="p-4 rounded-xl bg-accent/20 border border-border/30"> <p className="text-xs text-muted-foreground/90 leading-relaxed"> - {defaultProvider === 'cursor' ? ( - <>...</> - ) : defaultProvider === 'codex' ? ( - <>...</> - ) : defaultProvider === 'opencode' ? ( - <>...</> - ) : ( - <>...</> - )} + {PROVIDER_INFO[defaultProvider]} </p> </div>apps/server/src/routes/setup/routes/test-provider.ts (1)
44-51: Simplify nested ternaries with object lookups.The provider label and model selection use nested ternaries that reduce readability. Consider using object lookups for cleaner code.
Refactor to object lookups
+ const PROVIDER_LABELS = { + cursor: 'Cursor CLI', + opencode: 'OpenCode CLI', + codex: 'Codex CLI', + claude: 'Claude SDK', + } as const; + + const PROVIDER_MODELS = { + cursor: 'auto', + opencode: 'glm4.7', + codex: 'gpt-5.2-codex', + claude: 'claude-sonnet-4-20250514', + } as const; + - const providerLabel = - providerName === 'cursor' - ? 'Cursor CLI' - : providerName === 'opencode' - ? 'OpenCode CLI' - : providerName === 'codex' - ? 'Codex CLI' - : 'Claude SDK'; + const providerLabel = PROVIDER_LABELS[providerName as keyof typeof PROVIDER_LABELS] || 'Unknown Provider'; const testPrompt = `Create a file at /tmp/hello.md with the following content: ...`; - const model = - providerName === 'cursor' - ? 'auto' - : providerName === 'opencode' - ? 'glm4.7' - : providerName === 'codex' - ? 'gpt-5.2-codex' - : 'claude-sonnet-4-20250514'; + const model = PROVIDER_MODELS[providerName as keyof typeof PROVIDER_MODELS] || 'claude-sonnet-4-20250514';Also applies to: 69-76
apps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsx (1)
29-51: Consider handling thesuccess: falsecase explicitly.When
status.successis falsy (lines 35-44),cliStatusremains at its previous value (or null). The user sees no feedback in the UI beyond the toast on exception. Consider setting an error state or clearingcliStatuswhen the API returnssuccess: false.if (status.success) { setCliStatus({ installed: !!status.installed, version: status.version, path: status.path, auth: status.auth ? { authenticated: status.auth.authenticated, method: status.auth.method } : undefined, }); + } else { + setCliStatus(null); }apps/ui/src/components/views/setup-view/steps/codex-setup-step.tsx (1)
25-57: Significant code duplication withOpenCodeSetupStep.The
checkStatuscallback, state management, anduseEffectpattern are nearly identical toOpenCodeSetupStep. Consider extracting a shared hook likeuseCliStatusCheck(apiMethod)to reduce duplication and ensure consistent behavior across setup steps.Example shared hook
// useCliStatusCheck.ts export function useCliStatusCheck<T>( getStatus: () => Promise<{ success: boolean; installed?: boolean; version?: string; path?: string; auth?: { authenticated: boolean; method?: string } }> ) { const [cliStatus, setCliStatus] = useState<T | null>(null); const [isChecking, setIsChecking] = useState(false); const checkStatus = useCallback(async () => { setIsChecking(true); try { const status = await getStatus(); if (status.success) { setCliStatus(/* map status */); } } catch (error) { console.error('Failed to check CLI status:', error); toast.error('Failed to check CLI status'); } finally { setIsChecking(false); } }, [getStatus]); useEffect(() => { checkStatus(); }, [checkStatus]); return { cliStatus, isChecking, checkStatus }; }apps/server/src/routes/setup/routes/verify-cursor-auth.ts (1)
33-36: Unsafe type assertion onprocess.env.
process.envvalues can beundefined, but the assertionas Record<string, string>hides this. While this typically works, it's safer to filter out undefined values:- const env: Record<string, string> = { ...process.env } as Record<string, string>; + const env: Record<string, string | undefined> = { ...process.env }; if (apiKey) { env.CURSOR_API_KEY = apiKey; }Or, if you need strict
Record<string, string>, filter explicitly:const env = Object.fromEntries( Object.entries(process.env).filter(([, v]) => v !== undefined) ) as Record<string, string>;apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
129-135:ALL_MODELSordering places Cursor first.The order is: Cursor → Codex → OpenCode → Claude. If this affects UI presentation order (e.g., in dropdowns), ensure the ordering is intentional. Claude models were previously the only option, so users may expect them first.
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
1-127: Consider extracting a shared CLI status component.This component follows the same pattern as likely exists for Codex and Cursor CLI status components. A generic
CliStatusCardcomponent accepting provider-specific props (icon, color, name, install commands) would reduce duplication across the settings view.apps/server/src/lib/subprocess-manager.ts (2)
22-24:timeoutparameter is destructured but never used inspawnProcess.The
timeoutoption is extracted fromoptionsbut has no effect in this function. Either implement timeout logic (similar tospawnJSONLProcess) or remove it from the destructuring to avoid confusion.🔎 Proposed fix
export async function spawnProcess(options: SubprocessOptions): Promise<SubprocessResult> { - const { command, args, cwd, env, abortController } = options; + const { command, args, cwd, env, abortController, timeout } = options; const child = spawn(command, args, {Then implement timeout logic, or if not needed:
export async function spawnProcess(options: SubprocessOptions): Promise<SubprocessResult> { - const { command, args, cwd, env, abortController } = options; + const { command, args, cwd, env, abortController } = options; // timeout intentionally unused
54-64: Simplify duplicate return statements.Lines 59-61 and 63 return the same object. The conditional check for
abortedis unnecessary since both branches return identical values.🔎 Proposed fix
const exitCode = await new Promise<number | null>((resolve, reject) => { child.on('close', (code) => resolve(code ?? null)); child.on('error', (err) => reject(err)); }); - if (aborted) { - return { stdout, stderr, exitCode }; - } - return { stdout, stderr, exitCode }; }apps/ui/src/components/views/setup-view/steps/cursor-setup-step.tsx (2)
196-222: Consider verifying API key immediately after saving.Currently, users must save the API key and then separately click "Verify". Consider auto-verifying after successful save for better UX.
🔎 Proposed enhancement
if (result.success) { setApiKeys({ ...apiKeys, cursor: apiKey } as any); setCursorAuthStatus({ authenticated: true, method: 'api_key', hasApiKey: true, apiKeyValid: true, }); toast.success('API key saved successfully!'); + // Auto-verify after save + await verifyApiKeyAuth(); } else {
205-205: Type safety:as anycasts onapiKeys.The
as anycasts indicate a type mismatch between the component's expectations and the store'sapiKeystype. Consider updating the store type or using a proper interface.Also applies to: 272-272
apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
163-165: Consider expanding shortName prefix stripping.The
shortNamelogic removes 'Claude ' and 'Cursor ' prefixes but not others like 'GPT-', 'GLM '. For consistency, consider extending this or using a more generic approach.🔎 Proposed enhancement
- const shortName = option.label.replace('Claude ', '').replace('Cursor ', ''); + const shortName = option.label + .replace(/^(Claude|Cursor|GPT-|GLM)\s*/i, '');apps/ui/src/store/app-store.ts (1)
483-486: Default provider/model wiring is consistent but could validate model on provider changeAdding
defaultProvider/defaultModelintoAppState, initializing them to'cursor'/'auto', persisting them viapartialize, and syncingdefaultProviderto the backend throughsetDefaultProvideris coherent and matches the new multi-provider design. One improvement to consider is resetting or validatingdefaultModelwheneverdefaultProviderchanges so you don’t end up with incompatible pairs (e.g.,defaultProvider: 'claude'withdefaultModel: 'auto'), which would simplify downstream resolver logic.Also applies to: 760-763, 937-939, 1562-1578, 2725-2727
apps/server/src/providers/cursor-provider.ts (1)
41-43: Cursor CLI integration works but should honor provider config env and use logger consistentlyThe streaming/buffering logic and final
resultfallback look good. Two minor follow-ups to consider:
- In the spawn env, you currently spread only
process.env; ifBaseProvideris configured withconfig.envor an API key, you may want to merge those in (and setCURSOR_API_KEYwhen provided) to keep behavior aligned with other providers.- There’s a mix of
console.logandloggerusage; standardizing on the logger (or gating the console logs behind a debug flag) would reduce noise in production logs.Also applies to: 83-96, 133-179
apps/ui/src/lib/http-api-client.ts (1)
501-588: Setup API extensions look correct; consider reusing a shared provider typeThe new setup methods for Cursor/OpenCode/Codex status, install/auth, and default provider wiring map cleanly onto the expected
/api/setup/*routes and return shapes that match the setup-store usage. To avoid future drift, you might want to replace the repeated'claude' | 'cursor' | 'opencode' | 'codex'unions here with the sharedModelProvidertype from@automaker/types, so HTTP client, store, and backend stay in sync from a single source.apps/server/src/providers/provider-factory.ts (1)
17-26: Model-to-provider routing and default provider behavior look correctThe use of shared model maps plus prefix/regex checks gives solid coverage for routing Cursor/OpenCode/Claude/Codex models, and the
defaultProviderfallback ensures unknown IDs still resolve in a predictable way. Instantiating fresh providers ingetProviderForModel/getAllProvidersis fine for current usage; if this path ever becomes hot, you could cache instances and replace theconsole.logcalls with a logger to reduce noise, but the current behavior is functionally sound.Also applies to: 50-99, 105-111, 138-153, 163-173
apps/server/src/lib/llm-judge.ts (1)
258-381: Consider extracting scale parsing logic.The scale parsing at line 264 (
Number.parseInt(scale.split('-')[1], 10)) is safe given theRubricScaletype constraint, but this pattern repeats at line 594. Consider extracting to a helper function for reusability and to make the parsing logic more explicit:function parseRubricScale(scale: RubricScale): { min: number; max: number } { const [min, max] = scale.split('-').map(v => Number.parseInt(v, 10)); return { min, max }; }This would improve maintainability and make the intent clearer.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (108)
apps/server/src/index.tsapps/server/src/lib/llm-judge.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/secure-fs.tsapps/server/src/lib/settings-helpers.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/codex-config-manager.tsapps/server/src/providers/codex-provider.tsapps/server/src/providers/cursor-provider.tsapps/server/src/providers/opencode-provider.tsapps/server/src/providers/provider-factory.tsapps/server/src/providers/types.tsapps/server/src/routes/agent/index.tsapps/server/src/routes/agent/routes/queue-add.tsapps/server/src/routes/agent/routes/queue-clear.tsapps/server/src/routes/agent/routes/queue-list.tsapps/server/src/routes/agent/routes/queue-remove.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/routes/app-spec/index.tsapps/server/src/routes/app-spec/routes/generate-features.tsapps/server/src/routes/app-spec/routes/generate.tsapps/server/src/routes/backlog-plan/common.tsapps/server/src/routes/backlog-plan/generate-plan.tsapps/server/src/routes/backlog-plan/index.tsapps/server/src/routes/backlog-plan/routes/apply.tsapps/server/src/routes/backlog-plan/routes/generate.tsapps/server/src/routes/backlog-plan/routes/status.tsapps/server/src/routes/backlog-plan/routes/stop.tsapps/server/src/routes/context/index.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/github/index.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/models/routes/available.tsapps/server/src/routes/models/routes/providers.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/codex-status.tsapps/server/src/routes/setup/routes/cursor-status.tsapps/server/src/routes/setup/routes/default-provider.tsapps/server/src/routes/setup/routes/install-cursor.tsapps/server/src/routes/setup/routes/opencode-status.tsapps/server/src/routes/setup/routes/test-provider.tsapps/server/src/routes/setup/routes/verify-cursor-auth.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/suggestions/index.tsapps/server/src/routes/suggestions/routes/generate.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/server/tests/unit/lib/model-resolver.test.tsapps/server/tests/unit/providers/cursor-provider.test.tsapps/server/tests/unit/providers/provider-factory.test.tsapps/server/tests/unit/services/agent-service.test.tsapps/ui/src/app.tsxapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/ui/src/components/layout/sidebar.tsxapps/ui/src/components/layout/sidebar/dialogs/trash-dialog.tsxapps/ui/src/components/layout/sidebar/hooks/index.tsapps/ui/src/components/layout/sidebar/hooks/use-trash-dialog.tsapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/ui/src/components/ui/path-input.tsxapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/agent-output-modal.tsxapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/index.tsapps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/profiles-view/components/profile-form.tsxapps/ui/src/components/views/profiles-view/utils.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsxapps/ui/src/components/views/settings-view/ai-provider/index.tsapps/ui/src/components/views/settings-view/claude/claude-md-settings.tsxapps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsxapps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsxapps/ui/src/components/views/settings-view/cli-status/index.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/components/views/settings-view/shared/types.tsapps/ui/src/components/views/setup-view.tsxapps/ui/src/components/views/setup-view/steps/codex-setup-step.tsxapps/ui/src/components/views/setup-view/steps/cursor-setup-step.tsxapps/ui/src/components/views/setup-view/steps/index.tsapps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsxapps/ui/src/hooks/index.tsapps/ui/src/hooks/use-electron-agent.tsapps/ui/src/hooks/use-os-detection.tsapps/ui/src/hooks/use-provider-sync.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/agent-context-parser.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/lib/log-parser.tsapps/ui/src/store/app-store.tsapps/ui/src/store/setup-store.tsapps/ui/src/styles/global.cssapps/ui/src/styles/themes/red.cssapps/ui/src/styles/themes/retro.cssapps/ui/src/types/electron.d.ts
💤 Files with no reviewable changes (22)
- apps/ui/src/hooks/index.ts
- apps/ui/src/styles/themes/retro.css
- apps/ui/src/hooks/use-os-detection.ts
- apps/server/src/providers/claude-provider.ts
- apps/server/src/routes/backlog-plan/routes/generate.ts
- apps/server/src/routes/agent/index.ts
- apps/server/src/lib/secure-fs.ts
- apps/ui/src/styles/themes/red.css
- apps/server/src/routes/agent/routes/queue-remove.ts
- apps/server/src/routes/agent/routes/queue-list.ts
- apps/server/src/routes/agent/routes/queue-add.ts
- apps/server/src/routes/backlog-plan/routes/apply.ts
- apps/server/src/routes/backlog-plan/routes/status.ts
- apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx
- apps/server/src/routes/backlog-plan/routes/stop.ts
- apps/server/src/routes/backlog-plan/index.ts
- apps/server/src/routes/backlog-plan/generate-plan.ts
- apps/server/src/lib/settings-helpers.ts
- apps/ui/src/components/views/board-view/dialogs/index.ts
- apps/server/src/routes/backlog-plan/common.ts
- apps/ui/src/components/views/settings-view/claude/claude-md-settings.tsx
- apps/server/src/routes/agent/routes/queue-clear.ts
🧰 Additional context used
🧬 Code graph analysis (37)
apps/ui/src/hooks/use-provider-sync.ts (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(769-778)
apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx (2)
apps/ui/src/lib/utils.ts (1)
cn(5-7)apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/server/src/routes/setup/routes/verify-cursor-auth.ts (1)
apps/server/src/routes/setup/common.ts (1)
getApiKey(18-20)
apps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsx (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(769-778)
apps/server/src/routes/setup/routes/cursor-status.ts (1)
apps/server/src/routes/setup/get-cursor-status.ts (1)
getCursorStatus(33-143)
apps/ui/src/components/layout/sidebar/hooks/use-trash-dialog.ts (3)
apps/ui/src/lib/electron.ts (1)
TrashedProject(2972-2975)apps/ui/src/components/layout/sidebar/hooks/index.ts (2)
useTrashDialog(11-11)useTrashOperations(5-5)apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (1)
useTrashOperations(12-92)
apps/ui/src/app.tsx (1)
apps/ui/src/hooks/use-provider-sync.ts (1)
useProviderSync(12-35)
apps/server/src/routes/setup/routes/test-provider.ts (1)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)
apps/ui/src/components/views/board-view/board-header.tsx (1)
init.mjs (1)
isWindows(33-33)
apps/server/src/routes/setup/routes/default-provider.ts (1)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)
apps/server/src/routes/suggestions/routes/generate.ts (3)
apps/server/src/routes/app-spec/routes/generate.ts (1)
createGenerateHandler(19-92)apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/routes/suggestions/generate-suggestions.ts (1)
generateSuggestions(124-254)
apps/ui/src/components/views/settings-view.tsx (6)
apps/ui/src/components/views/settings-view/shared/types.ts (1)
Theme(3-3)apps/ui/src/store/app-store.ts (2)
ThemeMode(18-18)ThemeMode(34-51)libs/types/src/index.ts (1)
ThemeMode(50-50)libs/types/src/settings.ts (1)
ThemeMode(24-59)apps/server/src/providers/provider-factory.ts (1)
setDefaultProvider(32-35)apps/ui/src/components/views/settings-view/cli-status/index.ts (3)
CursorCliStatus(3-3)CodexCliStatus(2-2)OpenCodeCliStatus(4-4)
apps/server/src/routes/github/index.ts (2)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/routes/github/routes/validate-issue.ts (1)
createValidateIssueHandler(193-287)
apps/ui/src/components/views/agent-view.tsx (4)
apps/server/src/services/agent-service.ts (1)
sendMessage(113-380)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
CLAUDE_MODELS(12-34)libs/types/src/index.ts (1)
CLAUDE_MODELS(79-79)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/server/src/providers/cursor-provider.ts (1)
apps/ui/scripts/update-version.mjs (1)
version(15-15)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
apps/ui/src/lib/utils.ts (1)
cn(5-7)apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/src/components/views/setup-view/steps/cursor-setup-step.tsx (4)
apps/ui/src/store/app-store.ts (1)
useAppStore(968-2753)apps/ui/src/lib/electron.ts (1)
getElectronAPI(769-778)apps/ui/src/components/ui/card.tsx (5)
Card(84-84)CardHeader(84-84)CardTitle(84-84)CardDescription(84-84)CardContent(84-84)apps/ui/src/components/views/setup-view/components/terminal-output.tsx (1)
TerminalOutput(5-18)
apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (2)
apps/ui/src/lib/electron.ts (1)
TrashedProject(2972-2975)apps/ui/src/components/layout/sidebar/hooks/index.ts (1)
useTrashOperations(5-5)
apps/ui/src/components/views/board-view/shared/model-selector.tsx (3)
apps/ui/src/store/app-store.ts (2)
AgentModel(15-15)ModelProvider(15-15)apps/ui/src/components/views/board-view/shared/model-constants.ts (5)
CODEX_MODELS(63-127)OPENCODE_MODELS(53-61)CURSOR_MODELS(36-51)CLAUDE_MODELS(12-34)ModelOption(4-10)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/server/src/lib/llm-judge.ts (1)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)
apps/ui/src/store/app-store.ts (4)
libs/types/src/index.ts (2)
ModelProvider(54-54)AgentModel(36-36)libs/types/src/settings.ts (2)
ModelProvider(71-71)AgentModel(12-12)libs/types/src/model.ts (1)
AgentModel(23-23)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(1141-1146)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (2)
apps/ui/src/store/setup-store.ts (1)
useSetupStore(165-246)apps/ui/src/lib/electron.ts (1)
getElectronAPI(769-778)
apps/ui/src/components/views/profiles-view/utils.ts (2)
apps/ui/src/store/app-store.ts (2)
AgentModel(15-15)ModelProvider(15-15)apps/ui/src/components/views/board-view/shared/model-constants.ts (3)
CODEX_MODELS(63-127)OPENCODE_MODELS(53-61)CURSOR_MODELS(36-51)
apps/server/src/routes/app-spec/generate-spec.ts (1)
apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
generateFeaturesFromSpec(18-162)
apps/server/src/routes/app-spec/routes/generate-features.ts (2)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
generateFeaturesFromSpec(18-162)
apps/server/src/routes/setup/routes/install-cursor.ts (1)
init.mjs (1)
isWindows(33-33)
apps/server/src/services/agent-service.ts (2)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)
apps/server/src/routes/app-spec/index.ts (1)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)
apps/ui/src/store/setup-store.ts (2)
apps/ui/src/components/views/settings-view/shared/types.ts (1)
CliStatus(5-27)apps/ui/src/lib/http-api-client.ts (1)
get(174-178)
apps/server/src/providers/provider-factory.ts (4)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(29-647)apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProvider(26-862)apps/server/src/providers/claude-provider.ts (1)
ClaudeProvider(17-178)apps/server/src/providers/codex-provider.ts (1)
CodexProvider(28-787)
apps/server/src/services/auto-mode-service.ts (4)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)apps/server/src/lib/llm-judge.ts (3)
GenerateRubricOutput(114-139)generateRubric(588-700)runJudgeText(250-256)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)libs/types/src/provider.ts (1)
ExecuteOptions(34-46)
apps/server/src/index.ts (2)
apps/server/src/routes/github/index.ts (1)
createGitHubRoutes(20-52)apps/server/src/routes/context/index.ts (1)
createContextRoutes(17-24)
apps/ui/src/components/views/board-view.tsx (1)
apps/ui/src/store/app-store.ts (1)
useAppStore(968-2753)
apps/server/src/routes/suggestions/index.ts (3)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/middleware/validate-paths.ts (1)
validatePathParams(22-69)apps/server/src/routes/suggestions/routes/generate.ts (1)
createGenerateHandler(13-58)
apps/server/src/routes/setup/get-cursor-status.ts (2)
init.mjs (1)
isWindows(33-33)apps/server/src/routes/setup/common.ts (1)
getApiKey(18-20)
apps/server/src/routes/setup/routes/codex-status.ts (1)
apps/server/src/providers/codex-provider.ts (1)
CodexProvider(28-787)
apps/server/src/routes/app-spec/routes/generate.ts (2)
apps/server/src/routes/suggestions/routes/generate.ts (1)
createGenerateHandler(13-58)apps/server/src/lib/events.ts (1)
EventEmitter(10-13)
| auth: { | ||
| authenticated: status.authenticated ?? false, | ||
| method: status.hasApiKey ? 'api_key' : status.authenticated ? 'cli' : 'none', | ||
| hasApiKey: status.hasApiKey ?? false, | ||
| apiKeyValid: status.hasApiKey ?? false, | ||
| hasEnvApiKey: !!process.env.OPENAI_API_KEY, | ||
| }, |
There was a problem hiding this comment.
apiKeyValid incorrectly equated to hasApiKey.
Line 26 sets apiKeyValid: status.hasApiKey ?? false, which conflates the presence of an API key with its validity. The existence of a key doesn't guarantee it's valid or authorized. Based on the CodexProvider.detectInstallation implementation (apps/server/src/providers/codex-provider.ts:27-786), the provider only checks for key presence, not validity.
This could mislead downstream consumers (UI/clients) into thinking authentication is properly configured when the key might be invalid or expired.
🔎 Recommended approach
Either:
- Rename the field to accurately reflect what's being checked (e.g.,
hasApiKey), or - Implement actual validation logic that attempts to verify the key with the OpenAI API
auth: {
authenticated: status.authenticated ?? false,
method: status.hasApiKey ? 'api_key' : status.authenticated ? 'cli' : 'none',
hasApiKey: status.hasApiKey ?? false,
- apiKeyValid: status.hasApiKey ?? false,
+ apiKeyValid: false, // TODO: Implement actual key validation
hasEnvApiKey: !!process.env.OPENAI_API_KEY,
},📝 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.
| auth: { | |
| authenticated: status.authenticated ?? false, | |
| method: status.hasApiKey ? 'api_key' : status.authenticated ? 'cli' : 'none', | |
| hasApiKey: status.hasApiKey ?? false, | |
| apiKeyValid: status.hasApiKey ?? false, | |
| hasEnvApiKey: !!process.env.OPENAI_API_KEY, | |
| }, | |
| auth: { | |
| authenticated: status.authenticated ?? false, | |
| method: status.hasApiKey ? 'api_key' : status.authenticated ? 'cli' : 'none', | |
| hasApiKey: status.hasApiKey ?? false, | |
| apiKeyValid: false, // TODO: Implement actual key validation | |
| hasEnvApiKey: !!process.env.OPENAI_API_KEY, | |
| }, |
🤖 Prompt for AI Agents
In apps/server/src/routes/setup/routes/codex-status.ts around lines 22 to 28,
the apiKeyValid field is incorrectly set to status.hasApiKey (presence ≠
validity); either replace this assignment with real validation (call the
Codex/OpenAI validation endpoint or make a lightweight auth request in
detectInstallation and set apiKeyValid based on the success/failure of that
request) or, if you cannot perform validation here, rename the field to
apiKeyPresent/hasApiKeyPresent and return the boolean from status to avoid
implying the key is valid. Ensure downstream consumers get the new field name or
the actual validation result consistently.
apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/lib/agent-context-parser.ts (1)
63-63: Improve fallback to handle models without hyphens.The current fallback
model.split('-').slice(1, 3).join(' ')returns an empty string for model names that don't contain hyphens (e.g., a single-word model name like 'unknownmodel'). This could result in blank labels in the UI.🔎 Proposed fix to return original model string as fallback
- return model.split('-').slice(1, 3).join(' '); + const parts = model.split('-').slice(1, 3).join(' '); + return parts || model;This ensures that if the split-based fallback produces an empty string, the original model string is displayed instead.
♻️ Duplicate comments (3)
apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (2)
21-36: Error handling now properly implemented.The try/catch block with user-facing toast notifications addresses the previously flagged concern. Users will now receive appropriate feedback on both success and failure scenarios.
73-95: Well-implemented guards and error handling for empty trash.
- Early return when
trashedProjects.length === 0prevents unnecessary operations- Confirmation dialog clearly explains the action scope
- Error handling with catch block addresses the previously flagged concern
- Using
trashedProjects.lengthin the dependency array is an appropriate optimization to avoid re-creating the callback when only array contents changeapps/server/src/lib/subprocess-manager.ts (1)
37-39: Critical:lastOutputis not declared inspawnProcessscope.Line 39 references
lastOutput, but this variable is only declared inspawnJSONLProcess(line 77). This will cause aReferenceErrorat runtime when stderr receives data. This line appears to have been mistakenly copied fromspawnJSONLProcess.🔎 Proposed fix
Since
spawnProcessdoesn't implement timeout tracking, remove the erroneous assignment:child.stderr?.on('data', (chunk) => { stderr += chunk.toString(); - lastOutput = Date.now(); });If timeout support was intended for
spawnProcess, you would need to declare and initializelastOutput, implement the timeout checking logic, and add cleanup similar tospawnJSONLProcess.
🧹 Nitpick comments (15)
apps/server/src/lib/subprocess-manager.ts (3)
7-14: Note:timeoutoption only applies tospawnJSONLProcess.The
timeoutfield is declared inSubprocessOptionsbut is only implemented inspawnJSONLProcess(line 66). ThespawnProcessfunction ignores this option. Consider either:
- Documenting that timeout only applies to JSONL processes
- Implementing timeout support in
spawnProcessfor consistency- Moving timeout to a separate options interface for JSONL-specific options
58-62: Remove redundant conditional.Both branches return the identical result. The abort status is already captured in
exitCodeand the collected output, so this check serves no purpose.🔎 Proposed fix
- if (aborted) { - return { stdout, stderr, exitCode }; - } - return { stdout, stderr, exitCode };
26-26: Consider security implications of environment variable merging.Both functions merge
process.envwith custom environment variables using the spread operator ({ ...process.env, ...env }). While this is a common pattern, be aware that all parent process environment variables (including potentially sensitive ones like API keys, tokens, or credentials) are passed to spawned subprocesses unless explicitly filtered.If the spawned commands are user-controllable or from untrusted sources, consider allowlisting only necessary environment variables instead of merging all of
process.env.Also applies to: 69-69
apps/ui/src/components/dialogs/file-browser-dialog.tsx (1)
78-85: Avoid code duplication: use the existingisMachelper.The inline Mac detection logic duplicates functionality already available in
apps/ui/src/lib/utils.ts(lines 59-64). The existing helper is more robust as it also checksprocess.platformfor SSR scenarios.🔎 Proposed fix: import and use the existing helper
+import { isMac } from '@/lib/utils'; + export function FileBrowserDialog({ open, onOpenChange, onSelect, title = 'Select Project Directory', description = 'Navigate to your project folder or paste a path directly', initialPath, }: FileBrowserDialogProps) { const [currentPath, setCurrentPath] = useState<string>(''); const [parentPath, setParentPath] = useState<string | null>(null); const [directories, setDirectories] = useState<DirectoryEntry[]>([]); const [drives, setDrives] = useState<string[]>([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(''); const [warning, setWarning] = useState(''); const [recentFolders, setRecentFolders] = useState<string[]>([]); - const isMac = - typeof navigator !== 'undefined' && - ( - (navigator as Navigator & { userAgentData?: { platform?: string } }).userAgentData - ?.platform ?? - navigator.platform ?? - '' - ).includes('Mac');libs/types/src/model.ts (1)
13-24: Consider documenting the identity mapping pattern.Unlike
CLAUDE_MODEL_MAPwhich maps aliases to full model strings (e.g.,haiku→claude-haiku-4-5-20251001), these new provider maps use identity mappings. This works for pass-through but offers no aliasing convenience. Consider either:
- Adding a brief comment explaining this is intentional for non-Claude providers
- Adding short aliases if the CLI tools support them (e.g.,
glm→glm4.7)Also,
OPENCODE_MODEL_MAPhas two entries (glm4.7andglm-4.7) that appear to be alternate spellings of the same model. Verify this is intentional for flexibility.apps/server/src/providers/cursor-provider.ts (2)
41-42: Hardcoded model override may confuse users.
effectiveModelis always set to'auto'regardless of what the user requests. While line 77-79 logs this override, silently ignoring the user's model selection could be confusing. Consider either:
- Throwing an error if an unsupported model is requested
- Documenting this limitation in
getAvailableModels()(which only returnsauto)
94-97: Remove unnecessary stdin handling.When
stdio[0]is'ignore', the child process's stdin is set to/dev/nullandchildProcess.stdinwill benull. The optional chaining on line 96 safely handles this, but the call is a no-op. Consider removing line 96 along with its comment.stdio: ['ignore', 'pipe', 'pipe'], }); logger.info('[CursorProvider] Process spawned, PID:', childProcess.pid); - // Explicitly end stdin in case the process expects EOF to begin execution - childProcess.stdin?.end();apps/ui/src/components/views/board-view/hooks/use-board-features.ts (1)
60-69: Consider extracting validModelIds to module scope.The
validModelIdsSet is recreated on everyloadFeaturescall. SinceALL_MODELSis static, you could definevalidModelIdsonce at module level to avoid repeated Set construction.🔎 Proposed refactor
At the top of the file, after imports:
+const VALID_MODEL_IDS = new Set(ALL_MODELS.map((modelOption) => modelOption.id)); + interface UseBoardFeaturesProps { currentProject: { path: string; id: string } | null; }Then in the function:
- const { defaultModel } = useAppStore.getState(); - const validModelIds = new Set(ALL_MODELS.map((modelOption) => modelOption.id)); + const { defaultModel } = useAppStore.getState(); const featuresWithIds = result.features.map((f: any, index: number) => ({ ...f, id: f.id || `feature-${index}-${Date.now()}`, status: f.status || 'backlog', startedAt: f.startedAt, - model: typeof f.model === 'string' && validModelIds.has(f.model) ? f.model : defaultModel, + model: typeof f.model === 'string' && VALID_MODEL_IDS.has(f.model) ? f.model : defaultModel, thinkingLevel: f.thinkingLevel || 'none', }));libs/model-resolver/src/resolver.ts (1)
34-41: Consider optimizing pass-through lookups.The pass-through check uses
Object.values().includes()for each model map, which performs linear searches. For frequently called resolution paths, consider creating a combined Set of all valid non-Claude model IDs at module scope.🔎 Proposed optimization
+// Create a combined set of all valid non-Claude model IDs for O(1) lookups +const NON_CLAUDE_MODEL_IDS = new Set([ + ...Object.values(CURSOR_MODEL_MAP), + ...Object.values(OPENCODE_MODEL_MAP), + ...Object.values(CODEX_MODEL_MAP), +]); + export function resolveModelString( modelKey?: string, defaultModel: string = DEFAULT_MODELS.claude ): string { if (!modelKey) { return defaultModel; } // Pass through known non-Claude model identifiers - if ( - Object.values(CURSOR_MODEL_MAP).includes(modelKey) || - Object.values(OPENCODE_MODEL_MAP).includes(modelKey) || - Object.values(CODEX_MODEL_MAP).includes(modelKey) - ) { + if (NON_CLAUDE_MODEL_IDS.has(modelKey)) { return modelKey; }apps/ui/src/hooks/use-provider-sync.ts (1)
12-34: Potential stale value on initial sync.The hook captures
defaultProvidervalue at mount time but thehasSyncedflag prevents re-syncing if the value changes before the asyncsyncProvider()completes. IfdefaultProviderupdates during the initial render cycle, the synced value may be stale.Also note that
setDefaultProviderinapp-store.tsalready performs backend sync (per the relevant code snippets), so this hook may duplicate the sync on startup.🔎 Suggested fix using a ref to capture current value
export function useProviderSync() { const { defaultProvider } = useAppStore(); const hasSynced = useRef(false); + const providerRef = useRef(defaultProvider); + providerRef.current = defaultProvider; useEffect(() => { // Only sync once on mount if (hasSynced.current) return; hasSynced.current = true; const syncProvider = async () => { try { const api = getElectronAPI(); if (api.setup?.setDefaultProvider) { - await api.setup.setDefaultProvider(defaultProvider); - console.log(`[ProviderSync] Synced default provider: ${defaultProvider}`); + await api.setup.setDefaultProvider(providerRef.current); + console.log(`[ProviderSync] Synced default provider: ${providerRef.current}`); } } catch (error) { console.error('[ProviderSync] Failed to sync provider:', error); } }; syncProvider(); - }, [defaultProvider]); + }, []); // Empty deps since we only sync once on mount }apps/ui/src/components/views/settings-view.tsx (1)
87-97: Type casting may hide runtime issues.The direct cast
newTheme as ThemeModebypasses TypeScript's type checking. IfThemeincludes values not present inThemeMode, the cast will succeed at compile time but could cause unexpected behavior at runtime.Consider using a type guard or validation:
🔎 Suggested safer approach
+ const isValidThemeMode = (theme: Theme): theme is ThemeMode => { + const validModes: ThemeMode[] = ['light', 'dark', 'system']; // adjust as needed + return validModes.includes(theme as ThemeMode); + }; + const handleSetTheme = (newTheme: Theme) => { - // Convert Theme to ThemeMode (ThemeMode is a subset of Theme) - // If the theme is not in ThemeMode, we'll use it as-is and let the store handle it - const themeMode = newTheme as ThemeMode; + const themeMode = isValidThemeMode(newTheme) ? newTheme : 'system'; // Always update global theme so user's preference persists across all projects setTheme(themeMode);apps/ui/src/components/views/settings-view/ai-provider/provider-card.tsx (1)
56-85: Well-structured component with good type safety.The
ProviderCardcomponent is clean and follows React best practices:
- Type-safe color theming via
ProviderColorunion- Proper button semantics with
type="button"- Consistent styling pattern via
COLOR_CLASSESrecordConsider adding accessibility attributes for screen readers:
<button type="button" onClick={onClick} + aria-pressed={isSelected} + aria-label={`Select ${label} provider`} className={cn(apps/server/src/services/agent-service.ts (1)
213-224: Preliminary options created twice.The code creates
preliminaryOptionssolely to determine the provider, then creates the finalsdkOptionsagain with nearly identical parameters (plussystemPromptandautoLoadClaudeMd). Consider refactoring to avoid duplicate option construction.🔎 Suggested optimization
- // Resolve provider before building CLAUDE.md options - const preliminaryOptions = createChatOptions({ - cwd: effectiveWorkDir, - model: model, - sessionModel: session.model, - abortController: session.abortController!, - }); - const preliminaryModel = preliminaryOptions.model!; - const preliminaryProvider = ProviderFactory.getProviderForModel(preliminaryModel); + // Determine effective model for provider resolution + const effectiveModel = model || session.model; + const preliminaryProvider = ProviderFactory.getProviderForModel( + effectiveModel || 'claude-sonnet-4-20250514' // default fallback + ); const autoLoadClaudeMd = autoLoadClaudeMdSetting && preliminaryProvider.getName() === 'claude';apps/server/src/routes/models/routes/available.ts (1)
22-120: Model specifications are duplicated across available.ts and provider implementations.The hardcoded model definitions here duplicate specs in provider files:
- Cursor Auto (contextWindow: 200000, maxOutputTokens: 16000) defined in both cursor-provider.ts and available.ts
- GPT-5 variants (contextWindow: 256000, maxOutputTokens: 32000/16000) defined in both codex-provider.ts and available.ts
- GLM 4.7 (contextWindow: 128000, maxOutputTokens: 4096) defined in both opencode-provider.ts and available.ts
This creates a maintenance burden where spec changes must be synchronized across multiple files. However, since this pattern is consistent with existing Claude model definitions, consolidation is a low-priority refactor.
apps/server/src/services/auto-mode-service.ts (1)
6-6: Update outdated comment to reflect multi-provider support.The comment still mentions "Claude or Cursor" specifically, but line 2 correctly generalizes to "AI providers". Update this comment to match.
🔎 Suggested fix
- * - Feature execution with Claude or Cursor + * - Feature execution with AI providers (Claude, Cursor, OpenCode, Codex)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
apps/server/src/index.tsapps/server/src/lib/llm-judge.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/codex-config-manager.tsapps/server/src/providers/cursor-provider.tsapps/server/src/providers/provider-factory.tsapps/server/src/providers/types.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/routes/app-spec/index.tsapps/server/src/routes/app-spec/routes/create.tsapps/server/src/routes/context/index.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/github/index.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/models/routes/available.tsapps/server/src/routes/models/routes/providers.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/setup/get-opencode-status.tsapps/server/src/routes/setup/routes/codex-status.tsapps/server/src/routes/setup/routes/install-cursor.tsapps/server/src/routes/setup/routes/opencode-status.tsapps/server/src/routes/setup/routes/test-provider.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/ui/src/components/ui/path-input.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/components/views/profiles-view/utils.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsxapps/ui/src/components/views/settings-view/ai-provider/provider-card.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-electron-agent.tsapps/ui/src/hooks/use-provider-sync.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/agent-context-parser.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/lib/log-parser.tsapps/ui/src/store/app-store.tsapps/ui/src/styles/global.csslibs/model-resolver/src/index.tslibs/model-resolver/src/resolver.tslibs/types/src/index.tslibs/types/src/model.ts
💤 Files with no reviewable changes (3)
- apps/server/src/routes/context/routes/describe-file.ts
- apps/server/src/routes/context/index.ts
- apps/server/src/routes/context/routes/describe-image.ts
✅ Files skipped from review due to trivial changes (1)
- apps/ui/src/lib/log-parser.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/server/src/routes/app-spec/generate-features-from-spec.ts
- apps/ui/src/components/views/board-view/board-header.tsx
- apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx
- apps/server/src/routes/setup/routes/test-provider.ts
- apps/ui/src/components/views/board-view.tsx
- apps/server/src/routes/setup/routes/install-cursor.ts
- apps/server/src/routes/github/index.ts
- apps/server/src/index.ts
- apps/ui/src/components/views/settings-view/hooks/use-settings-view.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:
apps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/hooks/use-provider-sync.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/server/src/routes/setup/get-opencode-status.tsapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/server/src/routes/app-spec/routes/create.tsapps/ui/src/hooks/use-settings-migration.tsapps/server/src/routes/setup/routes/codex-status.tsapps/server/src/lib/subprocess-manager.tslibs/model-resolver/src/index.tsapps/server/src/routes/models/routes/providers.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/app-spec/generate-spec.tsapps/ui/src/components/ui/path-input.tsxapps/ui/src/components/views/settings-view.tsxapps/server/src/providers/claude-provider.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/providers/cursor-provider.tsapps/server/src/routes/app-spec/index.tsapps/ui/src/hooks/use-electron-agent.tsapps/server/src/providers/codex-config-manager.tsapps/ui/src/store/app-store.tsapps/server/src/routes/setup/routes/opencode-status.tsapps/ui/src/lib/electron.tsapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/server/src/lib/sdk-options.tslibs/types/src/model.tsapps/ui/src/components/views/settings-view/ai-provider/provider-card.tsxapps/server/src/lib/llm-judge.tsapps/ui/src/lib/http-api-client.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/agent-service.tsapps/server/src/providers/provider-factory.tsapps/server/src/providers/types.tslibs/types/src/index.tsapps/server/src/routes/models/routes/available.tsapps/ui/src/lib/agent-context-parser.tslibs/model-resolver/src/resolver.tsapps/ui/src/components/views/profiles-view/utils.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/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/hooks/use-provider-sync.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/server/src/routes/setup/get-opencode-status.tsapps/ui/src/components/layout/sidebar/hooks/use-trash-operations.tsapps/server/src/routes/app-spec/routes/create.tsapps/ui/src/hooks/use-settings-migration.tsapps/server/src/routes/setup/routes/codex-status.tsapps/server/src/lib/subprocess-manager.tslibs/model-resolver/src/index.tsapps/server/src/routes/models/routes/providers.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/app-spec/generate-spec.tsapps/ui/src/components/ui/path-input.tsxapps/ui/src/components/views/settings-view.tsxapps/server/src/providers/claude-provider.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/providers/cursor-provider.tsapps/server/src/routes/app-spec/index.tsapps/ui/src/hooks/use-electron-agent.tsapps/server/src/providers/codex-config-manager.tsapps/ui/src/store/app-store.tsapps/server/src/routes/setup/routes/opencode-status.tsapps/ui/src/lib/electron.tsapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/server/src/lib/sdk-options.tslibs/types/src/model.tsapps/ui/src/components/views/settings-view/ai-provider/provider-card.tsxapps/server/src/lib/llm-judge.tsapps/ui/src/lib/http-api-client.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/agent-service.tsapps/server/src/providers/provider-factory.tsapps/server/src/providers/types.tslibs/types/src/index.tsapps/server/src/routes/models/routes/available.tsapps/ui/src/lib/agent-context-parser.tslibs/model-resolver/src/resolver.tsapps/ui/src/components/views/profiles-view/utils.ts
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/get-opencode-status.tsapps/server/src/routes/app-spec/routes/create.tsapps/server/src/routes/setup/routes/codex-status.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/routes/models/routes/providers.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/providers/cursor-provider.tsapps/server/src/routes/app-spec/index.tsapps/server/src/providers/codex-config-manager.tsapps/server/src/routes/setup/routes/opencode-status.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/llm-judge.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/agent-service.tsapps/server/src/providers/provider-factory.tsapps/server/src/providers/types.tsapps/server/src/routes/models/routes/available.ts
🧠 Learnings (4)
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
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:
libs/model-resolver/src/index.tsapps/server/src/lib/sdk-options.tslibs/types/src/model.tsapps/server/src/lib/llm-judge.tsapps/server/src/services/auto-mode-service.tslibs/types/src/index.tsapps/ui/src/lib/agent-context-parser.tslibs/model-resolver/src/resolver.tsapps/ui/src/components/views/profiles-view/utils.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always import from shared packages (automaker/*), never from old relative paths
Applied to files:
libs/model-resolver/src/index.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Each feature executes in an isolated git worktree created via automaker/git-utils to protect the main branch during AI agent execution
Applied to files:
apps/server/src/services/auto-mode-service.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Store project-specific rules in `.automaker/context/` and load them into agent prompts via `loadContextFiles()` from automaker/utils
Applied to files:
apps/server/src/services/auto-mode-service.tsapps/server/src/services/agent-service.ts
🧬 Code graph analysis (17)
apps/ui/src/components/views/board-view/hooks/use-board-features.ts (2)
apps/ui/src/store/app-store.ts (1)
useAppStore(975-2768)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
ALL_MODELS(130-135)
apps/ui/src/hooks/use-provider-sync.ts (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(785-794)
apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (2)
apps/ui/src/lib/electron.ts (1)
TrashedProject(3017-3020)apps/ui/src/components/layout/sidebar/hooks/index.ts (1)
useTrashOperations(5-5)
apps/server/src/routes/app-spec/routes/create.ts (2)
apps/server/src/lib/events.ts (1)
EventEmitter(10-13)apps/server/src/services/settings-service.ts (1)
SettingsService(104-583)
apps/server/src/lib/subprocess-manager.ts (1)
init.mjs (3)
exitCode(294-301)rl(188-191)parsed(102-102)
apps/server/src/routes/setup/get-cursor-status.ts (3)
apps/ui/scripts/update-version.mjs (1)
version(15-15)init.mjs (1)
isWindows(33-33)apps/server/src/routes/setup/common.ts (1)
getApiKey(18-20)
apps/server/src/routes/app-spec/index.ts (1)
apps/server/src/routes/app-spec/routes/create.ts (1)
createCreateHandler(20-95)
apps/ui/src/hooks/use-electron-agent.ts (1)
apps/server/src/services/agent-service.ts (1)
addToQueue(632-659)
apps/ui/src/store/app-store.ts (5)
libs/types/src/index.ts (2)
ModelProvider(66-66)AgentModel(47-47)libs/types/src/settings.ts (2)
ModelProvider(71-71)AgentModel(12-12)apps/server/src/types/settings.ts (2)
ModelProvider(14-14)AgentModel(11-11)libs/types/src/model.ts (1)
AgentModel(65-65)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(1180-1185)
apps/server/src/routes/setup/routes/opencode-status.ts (1)
apps/server/src/routes/setup/get-opencode-status.ts (1)
getOpenCodeStatus(26-65)
apps/ui/src/lib/electron.ts (1)
libs/types/src/index.ts (2)
BacklogPlanResult(115-115)BacklogPlanEvent(116-116)
apps/ui/src/components/dialogs/file-browser-dialog.tsx (1)
apps/ui/src/lib/utils.ts (1)
isMac(60-65)
apps/server/src/services/auto-mode-service.ts (8)
apps/server/src/routes/worktree/common.ts (1)
execAsync(13-13)apps/server/src/routes/github/routes/common.ts (1)
execAsync(8-8)apps/server/src/lib/llm-judge.ts (5)
GenerateRubricOutput(114-139)generateRubric(588-700)runJudgeText(250-256)pairwiseCompare(459-586)directScore(258-381)apps/server/src/lib/settings-helpers.ts (2)
getAutoLoadClaudeMdSetting(17-46)filterClaudeMdFromContext(60-99)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)apps/server/src/lib/sdk-options.ts (1)
validateWorkingDirectory(37-49)apps/server/src/providers/types.ts (1)
ExecuteOptions(34-46)libs/types/src/provider.ts (1)
ExecuteOptions(34-46)
apps/server/src/services/agent-service.ts (5)
apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(17-46)apps/server/src/lib/sdk-options.ts (1)
createChatOptions(307-328)apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)libs/utils/src/index.ts (1)
loadContextFiles(50-50)apps/server/src/lib/secure-fs.ts (1)
secureFs(8-23)
apps/server/src/providers/provider-factory.ts (4)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(29-647)apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProvider(26-862)apps/server/src/providers/claude-provider.ts (1)
ClaudeProvider(17-181)apps/server/src/providers/codex-provider.ts (1)
CodexProvider(28-787)
libs/model-resolver/src/resolver.ts (2)
libs/types/src/index.ts (3)
CURSOR_MODEL_MAP(38-38)OPENCODE_MODEL_MAP(39-39)CODEX_MODEL_MAP(40-40)libs/types/src/model.ts (3)
CURSOR_MODEL_MAP(13-16)OPENCODE_MODEL_MAP(21-24)CODEX_MODEL_MAP(29-42)
apps/ui/src/components/views/profiles-view/utils.ts (2)
apps/ui/src/store/app-store.ts (2)
AgentModel(15-15)ModelProvider(15-15)apps/ui/src/components/views/board-view/shared/model-constants.ts (4)
CODEX_MODELS(63-127)OPENCODE_MODELS(53-61)CURSOR_MODELS(36-51)CLAUDE_MODELS(12-34)
🪛 ast-grep (0.40.3)
apps/server/src/providers/codex-config-manager.ts
[warning] 154-154: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^\\[mcp_servers\\.${escaped}\\]$, 'm')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (69)
apps/ui/src/components/layout/sidebar/hooks/use-trash-operations.ts (2)
5-10: LGTM! Clean interface extension for trash operations.The addition of
trashedProjectsprop is well-typed and correctly destructured in the hook. This enables the empty-state guard and aligns with theTrashedProjecttype defined in@/lib/electron.Also applies to: 12-17
38-71: Good addition of user confirmation before destructive disk operation.The confirmation dialog properly warns users before sending the project folder to system Trash. The async flow, error handling, and
activeTrashIdstate management are all correct.apps/ui/src/components/views/settings-view/config/navigation.ts (1)
12-12: Navigation configuration for multi-provider support is properly configured.The 'ai-provider' navigation item is correctly implemented:
SettingsViewIdtype includes the 'ai-provider' value- Corresponding component exists at
apps/ui/src/components/views/settings-view/ai-provider/- Icon choices are appropriate: Wand2 for AI provider selection, Terminal for Claude SDK settings
The changes properly separate general provider settings from Claude-specific SDK configuration, supporting the PR's multi-provider architecture goals.
apps/server/src/lib/subprocess-manager.ts (1)
65-166: LGTM! Solid JSONL streaming implementation.The async generator correctly:
- Spawns and manages the child process lifecycle
- Implements timeout tracking with
lastOutputupdates (lines 77, 88, 133, 95)- Parses JSONL line-by-line with proper error handling for malformed JSON (line 138)
- Cleans up resources (readline interface and timer) in the finally block (lines 142-145)
- Throws appropriate errors for spawn failures, aborts, timeouts, and non-zero exits (lines 150-165)
The timeout interval calculation on line 100 (
Math.min(timeout, 1000)) appropriately balances responsiveness with overhead.apps/ui/src/styles/global.css (1)
434-471: Excellent fix! Previous critical issue resolved.All 14 themes now have complete scrollbar thumb styling, addressing the critical gap flagged in the previous review. The implementation correctly uses a consolidated
:is()selector for 13 themes with shared styling, while theredtheme maintains its dedicated block with theme-specific colors.apps/ui/src/components/dialogs/file-browser-dialog.tsx (1)
243-243: Good accessibility improvement!Removing
focus-visible:outline-nonerestores the default focus indicator, which improves keyboard navigation accessibility.apps/server/src/routes/app-spec/index.ts (1)
20-20: LGTM! Consistency improvement.This change aligns the
/createroute with/generateand/generate-featuresroutes (lines 21-22) that already passsettingsService. Good catch fixing this omission.apps/server/src/routes/app-spec/routes/create.ts (1)
16-16: LGTM! Proper type import and signature update.The type-only import and optional parameter addition properly extend the handler to accept
SettingsService, enabling spec generation to utilize server-side settings.Also applies to: 20-20
apps/server/src/providers/codex-config-manager.ts (5)
5-7: LGTM: Standard library imports are appropriate.The use of Node.js standard library modules (fs/promises, os, path) is appropriate for file system operations and path resolution.
48-67: LGTM: Config merging logic is correct.The method properly validates entries, checks for existing blocks, and appends new configuration blocks without duplication.
69-126: LGTM: TOML block rendering is well-structured.The method properly validates required fields (command or url), handles optional fields, and supports both snake_case and camelCase naming conventions for compatibility.
146-151: LGTM: Key formatting logic is correct.The method appropriately quotes keys that contain special characters while allowing simple alphanumeric keys to remain unquoted.
153-157: LGTM: Static analysis ReDoS warning is a false positive.The regex is constructed from user input, but Line 154 properly escapes all regex special characters before interpolation. This converts the dynamic input into a literal pattern, effectively preventing Regular Expression Denial of Service (ReDoS) attacks. The static analysis tool cannot detect this sanitization.
apps/ui/src/lib/agent-context-parser.ts (4)
51-53: GLM parsing improvements look great—past concerns resolved!The new regex-based GLM parsing correctly extracts version numbers from model strings (e.g., 'glm-4.7' → 'GLM 4.7', 'glm-4.6' → 'GLM 4.6', 'opencode/glm-4.7' → 'GLM 4.7'). This addresses the concerns raised in previous review comments about hardcoding 'GLM 4.7' for all GLM models. The implementation now correctly displays different GLM versions and includes a sensible fallback to 'GLM' for generic references.
37-50: GPT model parsing logic is well-structured.The implementation correctly handles GPT model strings by:
- Extracting the version number
- Guarding against empty versions (line 41)
- Appending appropriate suffixes (Codex, Turbo, Preview, Instruct, Max, Mini)
This provides clear, user-friendly display labels for the various GPT model variants.
54-58: O-series model parsing handles expected patterns correctly.The regex
^o(\d+)(?:-(.+))?$properly matches O-series models like 'o2', 'o3-preview', etc., and formats them appropriately with or without suffixes.
27-31: No issues found. TheDEFAULT_MODEL = 'sonnet'constant is correctly used only for display purposes informatModelName()calls, which explicitly handles the 'sonnet' alias (line 61) and returns 'Sonnet 4.5'. The comment accurately describes it as a "display fallback only," and the actual default for model initialization comes from app-store.defaultModel as noted. The coding guideline aboutresolveModelString()applies to model initialization for API calls, not display formatting.apps/server/src/routes/app-spec/generate-spec.ts (1)
89-93: LGTM!The key change from
'[SpecRegeneration]'to'[SpecGeneration]'aligns the configuration namespace with the function name, improving clarity and consistency with other generation flows in the codebase.apps/server/src/lib/sdk-options.ts (3)
140-145: LGTM! Clarify the default behavior for undefined model.The helper correctly identifies Claude models. However, returning
truewhenmodelis undefined (line 142-143) assumes Claude is the default provider. This is reasonable given the current architecture, but consider adding a brief comment explaining this assumption for future maintainers.
158-184: LGTM!The updated logic correctly:
- Falls back to
sessionModelwhenmodelis not provided- Gates CLAUDE.md auto-loading on both the setting flag and Claude model detection
- Safely handles both string and object
systemPrompttypes with the type guard at line 178
200-213: LGTM!The type widening of
systemPrompttostring | SystemPromptConfigproperly supports both simple text prompts and structured preset configurations. The updated JSDoc comment clarifies the auto-loading behavior.apps/server/src/routes/github/routes/validate-issue.ts (1)
83-87: LGTM!The key change from
'[ValidateIssue]'to'[IssueValidation]'maintains consistency with the naming convention used across other Claude MD auto-loading call sites.libs/types/src/model.ts (2)
47-52: LGTM!The
DEFAULT_MODELSextension properly provides fallback model strings for each new provider, enabling consistent behavior when no explicit model is specified.
54-65: LGTM!The type aliases are correctly derived from the model maps, and the union type
ModelAliasproperly encompasses all provider aliases. TheAgentModelalias maintains backward compatibility.apps/server/src/providers/cursor-provider.ts (2)
316-336: LGTM!The readline-based stream parsing is robust—using
crlfDelay: Infinityhandles cross-platform line endings, and the dynamic import avoids top-level module issues.
536-617: LGTM with minor note on platform detection.The installation detection logic properly checks PATH first, then falls back to common installation locations. The
os.platform() === 'win32'check is consistent with Node.js conventions.Note: The external context snippet mentions
process.platformis used elsewhere—both approaches are equivalent, butos.platform()is slightly more idiomatic whenosis already imported.apps/server/src/lib/llm-judge.ts (3)
141-158: LGTM!The
getDefaultJudgeModelfunction correctly selects a model based on the configured default provider, with appropriate fallbacks using the newly defined model maps andDEFAULT_MODELS. Based on learnings,resolveModelString()is properly used to convert aliases to full model names.
169-189: LGTM! Result handling is correct.The current implementation at lines 179-182 unconditionally assigns
msg.resulttoresponseTextwhen a success result is received. This correctly prioritizes the final, authoritative result over partial assistant text, addressing the concern from the previous review.
228-248: LGTM!The
runJudgePromptfunction properly usesresolveModelStringwith a fallback togetDefaultJudgeModel(), integrates withProviderFactoryfor model-agnostic execution, and configures appropriate options including limited tool access (['Read']).apps/ui/src/hooks/use-settings-migration.ts (1)
225-226: LGTM!The addition of
defaultProvideranddefaultModelto the settings sync payload correctly persists the new multi-provider preferences to file-based storage. This aligns with the broader PR changes introducing Cursor, OpenCode, and Codex provider support.apps/server/src/providers/claude-provider.ts (2)
36-37: LGTM!Destructuring
settingSourcesfrom options improves code clarity.
59-61: LGTM!The conditional spread correctly forwards
settingSourcesto the Claude SDK only when provided, enabling CLAUDE.md auto-loading based on the configuration computed inbuildClaudeMdOptions.apps/ui/src/components/views/profiles-view/utils.ts (1)
10-24: LGTM!The provider resolution logic correctly checks model membership across all provider catalogs (Codex, OpenCode, Cursor, Claude) and returns the appropriate provider. The implementation is clear and maintains proper fallback behavior.
apps/server/src/routes/setup/routes/opencode-status.ts (1)
9-23: LGTM!The handler follows the established pattern for status endpoints: delegates to a dedicated status getter (
getOpenCodeStatus), returns the result with proper error handling, and uses consistent logging utilities.apps/server/src/providers/types.ts (2)
14-21: LGTM!The
SystemPromptConfiginterface provides good type safety with literal types fortypeandpreset, making the Claude SDK preset mode explicit and extensible. The optionalappendfield allows for customization while maintaining the preset structure.
38-38: LGTM!The union type
string | SystemPromptConfigforsystemPromptprovides flexibility while maintaining type safety, allowing both simple string prompts and structured preset configurations.libs/model-resolver/src/index.ts (1)
7-14: LGTM!The expanded re-exports appropriately surface the new provider model maps (Cursor, OpenCode, Codex) alongside existing Claude maps, providing a centralized public API for model resolution across all supported providers.
apps/server/src/routes/models/routes/providers.ts (1)
20-31: LGTM!The new provider entries (cursor, opencode, codex) follow the established pattern with consistent structure for
availableandhasApiKeyfields. The environment variable mappings are appropriate for each provider.apps/server/src/routes/setup/routes/codex-status.ts (1)
9-35: LGTM with acknowledged TODO.The handler follows the established pattern for CLI status endpoints. The previous concern about
apiKeyValidbeing incorrectly set tohasApiKeyhas been addressed by explicitly setting it tofalsewith a TODO comment (line 26), which is honest about the current limitation until proper key validation is implemented.apps/ui/src/components/views/settings-view.tsx (1)
124-149: New AI provider section looks well-structured.The
ai-providercase properly renders theAIProviderSectionwith provider/model state and change handlers, followed by CLI status components for each provider. The pattern is consistent with the existingclaudecase structure.apps/server/src/services/agent-service.ts (3)
293-295: Good addition of stream state tracking flags.The
sawErrorandemittedCompleteflags properly track stream state to ensure thecompleteevent is emitted exactly once, even when the stream ends without an explicit result message.
343-356: Robust handling of result messages.The code now correctly handles cases where
msg.resultis defined (not just truthy), and ensures an assistant message is created if one doesn't exist. This prevents silent failures when the stream ends with a result but no prior assistant message.
372-379: Good fallback for complete event emission.The fallback ensures a
completeevent is always emitted when the stream ends normally, preventing the UI from getting stuck in a "processing" state.apps/ui/src/lib/http-api-client.ts (1)
501-588: New provider status and configuration APIs are well-structured.The new setup methods follow established patterns:
- Status endpoints use GET with consistent response shapes
- Action endpoints (install, verify, setDefaultProvider) use POST
- Provider type is constrained to valid values via union literal
- Auth object structures are consistent across providers
libs/types/src/index.ts (1)
36-48: Exports correctly expanded for multi-provider support.The export structure properly distinguishes between value exports (model maps) and type exports (alias types). This enables consumers to use
resolveModelString()from@automaker/model-resolverwith the new provider-specific model maps. Based on learnings, this aligns with the pattern of using model resolver for alias conversion.apps/server/src/routes/setup/get-cursor-status.ts (3)
17-31: LGTM: Well-structured status interface.The CursorStatus interface provides comprehensive auth state tracking with multiple authentication sources, which aligns well with the priority-based auth detection logic implemented below.
33-88: LGTM: Robust installation detection with platform-aware fallbacks.The two-stage detection strategy (PATH lookup followed by common locations) provides good coverage across different installation methods and platforms. Version discovery gracefully degrades if unavailable.
116-135: Fix confirmed: Auth priority now correct.The guard at line 118 (
if (!auth.authenticated)) correctly prevents the config file check from overwriting higher-priority authentication methods (environment variable and stored API key). This resolves the issue flagged in the previous review.apps/ui/src/store/app-store.ts (4)
10-15: LGTM: Proper use of shared types.Importing from
@automaker/typesfollows the coding guideline to use shared packages, and re-exporting for convenience is a good pattern.As per coding guidelines, using @automaker/* packages.
483-485: LGTM: Clear state additions for provider configuration.The new fields integrate well with the existing state structure and use appropriate types from the shared package.
1569-1585: LGTM: Backend sync with appropriate error handling.The fire-and-forget pattern for
setDefaultProvideris appropriate here—UI updates immediately while backend sync happens asynchronously. Error logging provides visibility without blocking the user flow. The dynamic import prevents circular dependencies.
2739-2740: LGTM: Consistent persistence configuration.Adding these fields to the persisted state ensures user preferences survive app restarts, following the established pattern for other settings.
apps/server/src/providers/provider-factory.ts (3)
17-42: LGTM: Well-designed default provider mechanism.The module-level state with typed getter/setter provides a clean API for managing the default provider. Case-insensitive model ID sets will help avoid routing issues. The console logging aids debugging without being excessive.
50-100: LGTM: Comprehensive routing with sensible fallback.The routing logic covers multiple detection strategies (model ID sets, prefixes, literals, regex) and falls back to the configured default provider for unknown models. The regex
/^o\d/correctly matches OpenAI's o-series models (o1, o3, etc.).
105-158: LGTM: Provider discovery with alias support.
getAllProvidersprovides complete provider enumeration, andgetProviderByNamehandles common aliases gracefully. Returning null for unknown names is a safe default.apps/ui/src/lib/electron.ts (3)
475-488: LGTM: Consistent API design.The
backlogPlanAPI follows the established pattern for async operations with streaming (generate/stop/status/onEvent), maintaining consistency withsuggestionsandspecRegenerationAPIs.
520-568: LGTM: Consistent provider status API.The optional status methods provide a uniform interface for checking provider installation and authentication, with provider-specific auth fields appropriately tailored to each provider's authentication mechanism.
1291-1421: LGTM: Appropriate mock implementations.The mock implementations provide safe defaults (not installed, not authenticated) for development and testing, with helpful console logging. Error messages appropriately direct users to manual installation when mocking.
apps/server/src/services/auto-mode-service.ts (11)
61-88: LGTM! Environment-driven feature flags and thresholds are well-implemented.The judge and summary features are controlled by environment variables with sensible defaults. The helper functions include proper validation and fallback handling.
30-46: LGTM! Provider-aware default model selection is correctly implemented.The function correctly routes to provider-specific defaults using
ProviderFactory.getDefaultProvider()andDEFAULT_MODELSfrom@automaker/model-resolver, aligning with the coding guidelines.
1586-1631: LGTM! Summary and artifact helpers support context-engineering workflow.The new helper methods (
getFeatureSummaryPath,loadFeatureSummary,updateFeatureSummary,writeFeatureArtifact) provide a clean abstraction for managing compressed context summaries, which aligns with the PR's context-engineering integration objectives.
618-627: LGTM! Context loading follows guidelines and respects autoLoadClaudeMd setting.The code correctly uses
loadContextFiles()from@automaker/utilsand respects theautoLoadClaudeMdsetting viagetAutoLoadClaudeMdSetting()andfilterClaudeMdFromContext()helpers. Based on learnings, this aligns with the pattern of loading project-specific rules from.automaker/context/.Also applies to: 833-843
655-661: LGTM! Model resolution follows guidelines and provides clear provider context.The code correctly uses
resolveModelString()from@automaker/model-resolverto handle model aliases, and consistently logs both the model and provider for debugging. Based on learnings, this is the correct approach for model resolution.Also applies to: 888-894, 1192-1198
1164-1167: LGTM! Working directory validation is correctly applied.The
validateWorkingDirectory()call is a critical security boundary check. The comment clearly explains why this validation is necessary inanalyzeProject()since it buildsExecuteOptionsdirectly without using the factory function.
1773-1773: LGTM! Conciseness instructions improve agent output quality.The added instructions to keep outputs concise and avoid restating prompts will reduce verbosity and improve token efficiency. This aligns with best practices for agent prompting.
Also applies to: 1801-1801, 2982-2982
2515-2530: LGTM! Tool result handling and event emission improve observability.The new
tool_resultblock handling provides complete instrumentation of tool execution, emittingauto_mode_tool_resultevents with the result and tool use ID. This enhances real-time progress tracking.
2088-2106: LGTM! Judge integration for plan revision and context summaries is well-implemented.The judge integration follows a clean pattern:
maybeRevisePlanWithJudgescores plans and triggers revision if below thresholdmaybeGenerateContextSummarygenerates and scores summaries with pairwise comparison- Both are optional via environment flags and fail gracefully
- Proper abort signal handling and error recovery
The implementation aligns with the PR's goal of integrating "auto-critique/review pass to improve feedback quality" mentioned in the objectives.
Also applies to: 2632-2911
2551-2561: LGTM! Error handling ensures summary generation doesn't break core functionality.The try-catch wrapper with a warning log ensures that context summary generation failures don't impact the main feature execution flow, which is appropriate for this non-critical enhancement.
1966-1977: LGTM! System prompt handling includes helpful debug logging.The debug logging distinguishes between string and preset-based system prompts, providing useful trace information for debugging without being overly verbose.
| async configureMcpServers( | ||
| _cwd: string, | ||
| mcpServers: Record<string, unknown> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for interface definitions that might define configureMcpServers
echo "=== Searching for interfaces defining configureMcpServers ==="
rg -n "configureMcpServers" --type ts --type tsx -B5 -A5
echo -e "\n=== Searching for class implementing this method ==="
rg -n "class.*CodexConfigManager|async configureMcpServers" --type ts --type tsx -B3
echo -e "\n=== Searching for interface definitions with configureMcpServers ==="
ast-grep --pattern 'interface $_ {
$$$
configureMcpServers($$$)
$$$
}'Repository: AutoMaker-Org/automaker
Length of output: 325
🏁 Script executed:
#!/bin/bash
# View the target file
echo "=== Viewing codex-config-manager.ts ==="
cat -n apps/server/src/providers/codex-config-manager.ts
echo -e "\n=== Search for configureMcpServers references ==="
rg -n "configureMcpServers" --max-count=20
echo -e "\n=== Search for interface definitions ==="
rg -n "interface.*ConfigManager|export.*class.*ConfigManager" -A10Repository: AutoMaker-Org/automaker
Length of output: 6902
🏁 Script executed:
#!/bin/bash
# View the calling context
cat -n apps/server/src/providers/codex-provider.ts | sed -n '45,70p'
# Check if there are any other references to _cwd or cwd in the context
echo -e "\n=== Checking broader context of codex-provider.ts ==="
cat -n apps/server/src/providers/codex-provider.ts | sed -n '1,100p'Repository: AutoMaker-Org/automaker
Length of output: 4810
Remove the unused _cwd parameter or implement directory-scoped configuration.
The _cwd parameter is passed from the caller but never used in the method. The implementation ignores it and always uses process.env.CODEX_HOME with a fallback to the user's home directory, regardless of the cwd passed in. Either remove the parameter and update the call site, or use it to support directory-scoped configuration.
🤖 Prompt for AI Agents
In apps/server/src/providers/codex-config-manager.ts around lines 21–23, the
_cwd parameter is declared but never used; either remove it from the method
signature and update all call sites (and types/tests) to stop passing cwd, or
wire it into the configuration resolution so directory-scoped configuration
works: prefer the passed _cwd when computing CODEX_HOME (e.g. resolve/config
path based on _cwd) before falling back to process.env.CODEX_HOME or the user
home, update the function signature/type declarations accordingly, and adjust
any callers and tests to reflect the new behavior.
| private formatValue(value: unknown): string { | ||
| if (Array.isArray(value)) { | ||
| const items = value.map((item) => this.formatValue(item)); | ||
| return `[${items.join(', ')}]`; | ||
| } | ||
| if (typeof value === 'number' && Number.isFinite(value)) { | ||
| return String(value); | ||
| } | ||
| if (typeof value === 'boolean') { | ||
| return value ? 'true' : 'false'; | ||
| } | ||
| if (typeof value === 'string') { | ||
| const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); | ||
| return `"${escaped}"`; | ||
| } | ||
| return '""'; | ||
| } |
There was a problem hiding this comment.
Strengthen TOML string escaping and type handling.
Two concerns:
-
Line 140: String escaping only handles backslashes and double quotes. TOML strings may also contain newlines (
\n), tabs (\t), and other control characters that need proper escaping. -
Line 143: The default case returns
'""'for unhandled types (null, undefined, objects, etc.), which could silently produce invalid configurations.
🔎 Proposed fix to improve type handling and escaping
private formatValue(value: unknown): string {
if (Array.isArray(value)) {
const items = value.map((item) => this.formatValue(item));
return `[${items.join(', ')}]`;
}
if (typeof value === 'number' && Number.isFinite(value)) {
return String(value);
}
if (typeof value === 'boolean') {
return value ? 'true' : 'false';
}
if (typeof value === 'string') {
- const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
+ const escaped = value
+ .replace(/\\/g, '\\\\')
+ .replace(/"/g, '\\"')
+ .replace(/\n/g, '\\n')
+ .replace(/\r/g, '\\r')
+ .replace(/\t/g, '\\t');
return `"${escaped}"`;
}
+ // Warn about unhandled types
+ console.warn(`Unhandled value type in TOML formatting: ${typeof value}`);
return '""';
}Alternatively, consider using a dedicated TOML library (e.g., @iarna/toml or smol-toml) to ensure proper escaping and validation.
🤖 Prompt for AI Agents
In apps/server/src/providers/codex-config-manager.ts around lines 128 to 144,
the formatValue implementation only escapes backslashes and double quotes and
returns '""' for all other unhandled types; update it to fully escape TOML
string control characters (at minimum replace backslash, double quote, newline
(\n), carriage return (\r), tab (\t) and other common control chars with their
backslash-escaped equivalents) when handling strings, and replace the final
fallback behavior for null/undefined/objects by either (a) serializing
objects/arrays explicitly (or delegating to formatValue recursively for arrays)
and throwing a descriptive error for unsupported types, or (b) using a TOML
serializer library (e.g., @iarna/toml or smol-toml) to produce safe, validated
output; ensure any change preserves existing array/number/boolean handling and
surface a clear error instead of silently returning an empty string for
unsupported types.
| // Handle abort signal | ||
| if (abortController) { | ||
| abortController.signal.addEventListener('abort', () => { | ||
| abortRequested = true; | ||
| childProcess.kill('SIGTERM'); | ||
| }); | ||
| if (abortController.signal.aborted) { | ||
| abortRequested = true; | ||
| childProcess.kill('SIGTERM'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Abort listener is never removed, risking memory leaks.
The abort event listener added at line 104 is never removed. If the AbortController is reused or the generator is abandoned before completion, this listener remains attached. Consider using { once: true } option or removing the listener in a finally block.
if (abortController) {
- abortController.signal.addEventListener('abort', () => {
+ abortController.signal.addEventListener('abort', () => {
abortRequested = true;
childProcess.kill('SIGTERM');
- });
+ }, { once: true });📝 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.
| // Handle abort signal | |
| if (abortController) { | |
| abortController.signal.addEventListener('abort', () => { | |
| abortRequested = true; | |
| childProcess.kill('SIGTERM'); | |
| }); | |
| if (abortController.signal.aborted) { | |
| abortRequested = true; | |
| childProcess.kill('SIGTERM'); | |
| } | |
| } | |
| // Handle abort signal | |
| if (abortController) { | |
| abortController.signal.addEventListener('abort', () => { | |
| abortRequested = true; | |
| childProcess.kill('SIGTERM'); | |
| }, { once: true }); | |
| if (abortController.signal.aborted) { | |
| abortRequested = true; | |
| childProcess.kill('SIGTERM'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/providers/cursor-provider.ts around lines 102 to 112, the
abort event listener added to abortController.signal is never removed which can
leak memory if the controller is reused or the operation is abandoned; modify
the code to either register the listener with { once: true } or store the
listener function and explicitly remove it in a finally/cleanup block after the
child process exits, and keep the existing immediate aborted check (or call the
listener directly) so the abort path still terminates the child process as
intended.
| const auth = { | ||
| authenticated: false, | ||
| method: 'none', | ||
| hasApiKey: false, | ||
| hasStoredApiKey: !!getApiKey('opencode'), | ||
| hasEnvApiKey: !!process.env.OPENCODE_API_KEY, | ||
| apiKeyValid: false, | ||
| }; | ||
|
|
||
| if (auth.hasEnvApiKey) { | ||
| auth.authenticated = true; | ||
| auth.hasApiKey = true; | ||
| auth.method = 'api_key_env'; | ||
| } | ||
|
|
||
| if (!auth.authenticated && auth.hasStoredApiKey) { | ||
| auth.authenticated = true; | ||
| auth.hasApiKey = true; | ||
| auth.method = 'api_key'; | ||
| } | ||
|
|
||
| if (!auth.authenticated && installStatus.authenticated) { | ||
| auth.authenticated = true; | ||
| auth.hasApiKey = installStatus.hasApiKey ?? false; | ||
| auth.method = 'cli'; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check other status files for apiKeyValid handling
echo "=== get-cursor-status.ts ==="
rg -n 'apiKeyValid' apps/server/src/routes/setup/get-cursor-status.ts -A2 -B2
echo -e "\n=== get-codex-status.ts ==="
rg -n 'apiKeyValid' apps/server/src/routes/setup/get-codex-status.ts -A2 -B2
echo -e "\n=== get-opencode-status.ts ==="
rg -n 'apiKeyValid' apps/server/src/routes/setup/get-opencode-status.ts -A2 -B2Repository: AutoMaker-Org/automaker
Length of output: 1046
🏁 Script executed:
# Get full content of get-opencode-status.ts to understand the context
cat -n apps/server/src/routes/setup/get-opencode-status.tsRepository: AutoMaker-Org/automaker
Length of output: 2161
🏁 Script executed:
# Check if there are any other status files and their patterns
fd -e ts 'get-.*-status' apps/server/src/routes/setup/Repository: AutoMaker-Org/automaker
Length of output: 218
🏁 Script executed:
rg -n 'apiKeyValid' apps/server/src/routes/setup/get-claude-status.ts -A2 -B2Repository: AutoMaker-Org/automaker
Length of output: 901
Set apiKeyValid to true when API key authentication succeeds.
Currently, auth.apiKeyValid remains false even when authentication succeeds via API key (env or stored), whereas other provider status files (get-cursor-status.ts, get-claude-status.ts) correctly set this field to true. Additionally, the CLI authentication branch should set apiKeyValid based on installStatus.hasApiKey.
Suggested fix
if (auth.hasEnvApiKey) {
auth.authenticated = true;
auth.hasApiKey = true;
auth.method = 'api_key_env';
+ auth.apiKeyValid = true;
}
if (!auth.authenticated && auth.hasStoredApiKey) {
auth.authenticated = true;
auth.hasApiKey = true;
auth.method = 'api_key';
+ auth.apiKeyValid = true;
}
if (!auth.authenticated && installStatus.authenticated) {
auth.authenticated = true;
auth.hasApiKey = installStatus.hasApiKey ?? false;
auth.method = 'cli';
+ auth.apiKeyValid = installStatus.hasApiKey ?? false;
}🤖 Prompt for AI Agents
In apps/server/src/routes/setup/get-opencode-status.ts around lines 30 to 55,
auth.apiKeyValid is never set to true when authentication succeeds via env or
stored API key and the CLI branch ignores installStatus.hasApiKey; update the
three authentication branches so that when auth.hasEnvApiKey or
auth.hasStoredApiKey is used set auth.apiKeyValid = true, and in the CLI branch
set auth.apiKeyValid = !!installStatus.hasApiKey (or assign
installStatus.hasApiKey ?? false) so apiKeyValid correctly reflects whether a
usable API key is present.
| /* Custom scrollbar for dark themes */ | ||
| :is( | ||
| .dark, | ||
| .retro, | ||
| .dracula, | ||
| .nord, | ||
| .monokai, | ||
| .tokyonight, | ||
| .solarized, | ||
| .gruvbox, | ||
| .catppuccin, | ||
| .onedark, | ||
| .synthwave, | ||
| .red, | ||
| .sunset, | ||
| .gray | ||
| ) | ||
| ::-webkit-scrollbar { | ||
| width: 8px; | ||
| height: 8px; | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-track { | ||
| :is( | ||
| .dark, | ||
| .retro, | ||
| .dracula, | ||
| .nord, | ||
| .monokai, | ||
| .tokyonight, | ||
| .solarized, | ||
| .gruvbox, | ||
| .catppuccin, | ||
| .onedark, | ||
| .synthwave, | ||
| .red, | ||
| .sunset, | ||
| .gray | ||
| ) | ||
| ::-webkit-scrollbar-track { | ||
| background: var(--muted); | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-thumb { | ||
| background: var(--muted-foreground); | ||
| :is( | ||
| .dark, | ||
| .retro, | ||
| .dracula, | ||
| .nord, | ||
| .monokai, | ||
| .tokyonight, | ||
| .solarized, | ||
| .gruvbox, | ||
| .catppuccin, | ||
| .onedark, | ||
| .synthwave, | ||
| .sunset, | ||
| .gray | ||
| ) | ||
| ::-webkit-scrollbar-thumb { | ||
| background: oklch(0.3 0 0); | ||
| border-radius: 4px; | ||
| } | ||
|
|
||
| :is( | ||
| .dark, | ||
| .retro, | ||
| .dracula, | ||
| .nord, | ||
| .monokai, | ||
| .tokyonight, | ||
| .solarized, | ||
| .gruvbox, | ||
| .catppuccin, | ||
| .onedark, | ||
| .synthwave, | ||
| .sunset, | ||
| .gray | ||
| ) | ||
| ::-webkit-scrollbar-thumb:hover { | ||
| background: oklch(0.4 0 0); | ||
| } |
There was a problem hiding this comment.
Add scrollbar styling for .forest and .ocean themes.
The .forest and .ocean dark themes are defined as custom variants (lines 19-20) but are missing from all scrollbar selectors. Users selecting these themes will see default browser scrollbars instead of the styled versions provided to other dark themes.
🔎 Proposed fix: Include forest and ocean in scrollbar selectors
Add .forest and .ocean to each :is() selector:
:is(
.dark,
.retro,
.dracula,
.nord,
.monokai,
.tokyonight,
.solarized,
.gruvbox,
.catppuccin,
.onedark,
.synthwave,
.red,
.sunset,
- .gray
+ .gray,
+ .forest,
+ .ocean
)
::-webkit-scrollbar {
width: 8px;
height: 8px;
}
:is(
.dark,
.retro,
.dracula,
.nord,
.monokai,
.tokyonight,
.solarized,
.gruvbox,
.catppuccin,
.onedark,
.synthwave,
.red,
.sunset,
- .gray
+ .gray,
+ .forest,
+ .ocean
)
::-webkit-scrollbar-track {
background: var(--muted);
}
:is(
.dark,
.retro,
.dracula,
.nord,
.monokai,
.tokyonight,
.solarized,
.gruvbox,
.catppuccin,
.onedark,
.synthwave,
.sunset,
- .gray
+ .gray,
+ .forest,
+ .ocean
)
::-webkit-scrollbar-thumb {
background: oklch(0.3 0 0);
border-radius: 4px;
}
:is(
.dark,
.retro,
.dracula,
.nord,
.monokai,
.tokyonight,
.solarized,
.gruvbox,
.catppuccin,
.onedark,
.synthwave,
.sunset,
- .gray
+ .gray,
+ .forest,
+ .ocean
)
::-webkit-scrollbar-thumb:hover {
background: oklch(0.4 0 0);
}🤖 Prompt for AI Agents
In apps/ui/src/styles/global.css around lines 392 to 471, the custom scrollbar
selectors using :is(...) are missing the .forest and .ocean theme classes; add
.forest and .ocean to every :is(...) list (all four scrollbar selectors:
::-webkit-scrollbar, ::-webkit-scrollbar-track, ::-webkit-scrollbar-thumb, and
::-webkit-scrollbar-thumb:hover) so those themes receive the same dark scrollbar
styling as the other themes.
…egfault/automaker into multi_cli_integration
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/services/auto-mode-service.ts (1)
54-62: Duplicate imports detected.
getAutoLoadClaudeMdSettingandfilterClaudeMdFromContextare imported twice - once at line 54 and again at lines 59-62. This will cause a compilation error.🔎 Proposed fix
Remove the duplicate import block:
import { createAutoModeOptions, validateWorkingDirectory } from '../lib/sdk-options.js'; -import { filterClaudeMdFromContext, getAutoLoadClaudeMdSetting } from '../lib/settings-helpers.js'; import { FeatureLoader } from './feature-loader.js'; import type { SettingsService } from './settings-service.js'; import { pipelineService, PipelineService } from './pipeline-service.js'; import { getAutoLoadClaudeMdSetting, getEnableSandboxModeSetting, filterClaudeMdFromContext, } from '../lib/settings-helpers.js';
🧹 Nitpick comments (8)
apps/ui/src/components/views/agent-view.tsx (2)
52-52: Import CLAUDE_MODELS from the shared package.Per coding guidelines, import from shared packages (
@automaker/*) rather than internal paths. CLAUDE_MODELS is exported from@automaker/types.🔎 Proposed fix
-import { CLAUDE_MODELS } from '@/components/views/board-view/shared/model-constants'; +import { CLAUDE_MODELS } from '@automaker/types';As per coding guidelines, always import from shared packages.
66-66: UseresolveModelString()to convert model aliases.Per coding guidelines, model aliases ('haiku', 'sonnet', 'opus') should be resolved to full model names using
resolveModelString()from@automaker/model-resolverbefore passing to the agent hook.🔎 Proposed fix
Add import:
+import { resolveModelString } from '@automaker/model-resolver';Update the hook call to resolve the model:
const { messages, isProcessing, isConnected, sendMessage, clearHistory, stopExecution, error: agentError, } = useElectronAgent({ sessionId: currentSessionId || '', workingDirectory: currentProject?.path, - model: selectedModel, + model: resolveModelString(selectedModel), onToolUse: (toolName) => { setCurrentTool(toolName); setTimeout(() => setCurrentTool(null), 2000); }, });As per coding guidelines, use resolveModelString() to convert model aliases.
Also applies to: 93-93
apps/server/src/index.ts (1)
136-142: Consider clearing interval on shutdown.The stale validation cleanup interval is never cleared during graceful shutdown. While
process.exit()will terminate the interval anyway, for consistency with the cleanup pattern used forterminalService.cleanup(), consider storing the interval reference and clearing it.🔎 Proposed refactor
+const validationCleanupInterval = setInterval(() => { -setInterval(() => { const cleaned = cleanupStaleValidations(); if (cleaned > 0) { console.log(`[Server] Cleaned up ${cleaned} stale validation entries`); } }, VALIDATION_CLEANUP_INTERVAL_MS); // In shutdown handlers: process.on('SIGTERM', () => { console.log('SIGTERM received, shutting down...'); + clearInterval(validationCleanupInterval); terminalService.cleanup(); // ... });apps/server/src/services/auto-mode-service.ts (2)
29-46: Function defined between import statements.
getDefaultModelForProvider()is defined between import statements (lines 29-45), with more imports following at line 46. This violates typical TypeScript/ESLint conventions where all imports should be grouped at the top of the file.🔎 Proposed refactor
Move all imports to the top of the file before any function definitions:
import { directScore, pairwiseCompare, generateRubric, runJudgeText, type GenerateRubricOutput, } from '../lib/llm-judge.js'; - -/** - * Get the default model based on the current default provider setting - */ -function getDefaultModelForProvider(): string { - // ... function body -} import { resolveDependencies, areDependenciesSatisfied } from '@automaker/dependency-resolver'; // ... rest of imports ... +/** + * Get the default model based on the current default provider setting + */ +function getDefaultModelForProvider(): string { + // ... function body +}
2856-2857: Consider parallel summary generation for performance.The short and detailed summaries are generated sequentially using
runJudgeText. Since they're independent operations, running them in parallel withPromise.allcould reduce latency.🔎 Proposed refactor
- const shortSummary = (await runJudgeText(shortPrompt, systemPrompt, judgeConfig)).trim(); - const detailedSummary = (await runJudgeText(detailedPrompt, systemPrompt, judgeConfig)).trim(); + const [shortSummary, detailedSummary] = await Promise.all([ + runJudgeText(shortPrompt, systemPrompt, judgeConfig).then(s => s.trim()), + runJudgeText(detailedPrompt, systemPrompt, judgeConfig).then(s => s.trim()), + ]);apps/server/src/providers/claude-provider.ts (1)
36-37: Minor inconsistency in variable usage.
settingSourcesis destructured fromoptionsat line 36, but line 56 usesoptions.settingSourcesinstead of the destructured variable. While functionally equivalent, using the destructured variable would be more consistent.🔎 Proposed refactor
// Forward settingSources for CLAUDE.md file loading - ...(options.settingSources && { settingSources: options.settingSources }), + ...(settingSources && { settingSources }),apps/server/src/services/agent-service.ts (1)
225-235: Consider caching preliminary options to avoid duplicate work.The code creates
preliminaryOptionsviacreateChatOptionsto determine the provider, then creates another set of options at line 253. This duplicates some work. Consider extracting the model resolution logic or caching the preliminary result.However, the current approach is clear and the overhead is minimal. The separation makes the intent explicit: first determine provider eligibility, then build final options with the appropriate settings.
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
202-203: Consider removing unuseddefaultProviderfrom dependencies.
defaultProvideris included in the dependency array but is not referenced in the effect body (lines 164-206). Including unused dependencies can cause unnecessary re-renders whendefaultProviderchanges.Proposed fix to remove unused dependency
}, [ open, defaultSkipTests, defaultBranch, defaultPlanningMode, defaultRequirePlanApproval, defaultAIProfileId, aiProfiles, defaultModel, - defaultProvider, parentFeature, allFeatures, ]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/server/src/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/types.tsapps/server/src/routes/github/index.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/src/types/electron.d.tslibs/types/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/ui/src/store/app-store.ts
- apps/ui/src/components/views/board-view.tsx
- apps/ui/src/hooks/use-settings-migration.ts
- apps/ui/src/components/views/board-view/hooks/use-board-features.ts
- apps/server/src/routes/github/routes/validate-issue.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:
apps/ui/src/types/electron.d.tslibs/types/src/index.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/lib/http-api-client.tsapps/server/src/services/agent-service.tsapps/ui/src/components/views/settings-view.tsxapps/server/src/lib/sdk-options.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/server/src/providers/claude-provider.tsapps/server/src/providers/types.tsapps/ui/src/lib/electron.tsapps/server/src/index.tsapps/server/src/routes/github/index.tsapps/server/src/services/auto-mode-service.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/ui/src/types/electron.d.tslibs/types/src/index.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/lib/http-api-client.tsapps/server/src/services/agent-service.tsapps/ui/src/components/views/settings-view.tsxapps/server/src/lib/sdk-options.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/server/src/providers/claude-provider.tsapps/server/src/providers/types.tsapps/ui/src/lib/electron.tsapps/server/src/index.tsapps/server/src/routes/github/index.tsapps/server/src/services/auto-mode-service.ts
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/services/agent-service.tsapps/server/src/lib/sdk-options.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/types.tsapps/server/src/index.tsapps/server/src/routes/github/index.tsapps/server/src/services/auto-mode-service.ts
🧠 Learnings (4)
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
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:
libs/types/src/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/services/auto-mode-service.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Store project-specific rules in `.automaker/context/` and load them into agent prompts via `loadContextFiles()` from automaker/utils
Applied to files:
apps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Applies to apps/server/src/**/*.{ts,tsx} : Use `createEventEmitter()` from `lib/events.ts` for all server operations to emit events that stream to frontend via WebSocket
Applied to files:
apps/server/src/routes/github/index.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Each feature executes in an isolated git worktree created via automaker/git-utils to protect the main branch during AI agent execution
Applied to files:
apps/server/src/services/auto-mode-service.ts
🧬 Code graph analysis (7)
apps/ui/src/components/views/agent-view.tsx (4)
apps/server/src/services/agent-service.ts (1)
sendMessage(134-433)libs/types/src/index.ts (1)
CLAUDE_MODELS(91-91)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
CLAUDE_MODELS(12-34)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/server/src/services/agent-service.ts (4)
apps/server/src/lib/settings-helpers.ts (1)
getAutoLoadClaudeMdSetting(17-46)apps/server/src/lib/sdk-options.ts (1)
createChatOptions(311-334)apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)
apps/ui/src/components/views/settings-view.tsx (6)
apps/ui/src/store/app-store.ts (2)
ThemeMode(19-19)ThemeMode(35-52)libs/types/src/settings.ts (1)
ThemeMode(24-59)apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)
useCliStatus(32-234)apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx (1)
AIProviderSection(30-251)apps/server/src/providers/provider-factory.ts (1)
setDefaultProvider(32-35)apps/ui/src/components/views/settings-view/cli-status/index.ts (2)
CursorCliStatus(3-3)CodexCliStatus(2-2)
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
apps/ui/src/store/app-store.ts (1)
useAppStore(1004-2913)
apps/ui/src/lib/electron.ts (1)
libs/types/src/index.ts (2)
BacklogPlanResult(121-121)BacklogPlanEvent(122-122)
apps/server/src/index.ts (1)
apps/server/src/routes/backlog-plan/index.ts (1)
createBacklogPlanRoutes(14-30)
apps/server/src/services/auto-mode-service.ts (7)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)apps/server/src/lib/llm-judge.ts (5)
GenerateRubricOutput(114-139)generateRubric(588-700)runJudgeText(250-256)pairwiseCompare(459-586)directScore(258-381)apps/server/src/lib/settings-helpers.ts (2)
getAutoLoadClaudeMdSetting(17-46)filterClaudeMdFromContext(88-127)libs/utils/src/context-loader.ts (1)
loadContextFiles(148-202)libs/utils/src/index.ts (1)
loadContextFiles(50-50)apps/server/src/lib/sdk-options.ts (1)
validateWorkingDirectory(37-49)apps/server/src/providers/types.ts (1)
ExecuteOptions(34-47)
🔇 Additional comments (34)
libs/types/src/index.ts (1)
36-48: LGTM! All exports verified from model.ts.The expanded export statement follows the established naming pattern and is backward-compatible. The new model maps (CURSOR_MODEL_MAP, OPENCODE_MODEL_MAP, CODEX_MODEL_MAP) and their corresponding type aliases align well with the existing CLAUDE_MODEL_MAP pattern. All referenced constants and types are properly exported from
model.ts.apps/ui/src/components/views/agent-view.tsx (4)
138-154: LGTM: Simplified send logic with proper guards.The removal of queueing behavior in favor of immediate message sending is correctly implemented. The
isProcessingcheck at line 140 properly prevents duplicate sends, and the message content/attachments are cleared before sending.
551-584: LGTM: Clean model selector implementation.The model selector dropdown is well-implemented with:
- Proper disabled state during processing
- Clear visual feedback for the selected model
- Accessible dropdown menu structure
- Fallback display value
932-958: LGTM: Correct conditional rendering for Send/Stop button.The button logic properly toggles between Send and Stop based on processing state, with appropriate disabled conditions:
- Stop button shown during processing, disabled only when disconnected
- Send button shown otherwise, disabled when no content or disconnected
775-775: LGTM: Comprehensive disabled state handling.All file attachment and input controls are properly disabled during processing or when disconnected, providing consistent UX and preventing state mutations during agent execution.
Also applies to: 793-793, 829-829, 859-859, 890-890, 920-920
apps/ui/src/types/electron.d.ts (2)
185-192: LGTM! Consistent event variant structure.The new
auto_mode_tool_resultvariant follows the established pattern for AutoModeEvent types and appropriately supports the multi-provider tooling workflow.
983-983: LGTM! Provider support appropriately expanded.The ModelDefinition provider field correctly extends to support Cursor, OpenCode, and Codex providers as specified in the PR objectives.
apps/ui/src/lib/http-api-client.ts (3)
501-554: LGTM! Consistent status API implementation.The new status getter methods (getCursorStatus, getOpenCodeStatus, getCodexStatus) follow the established pattern from getClaudeStatus and correctly delegate to backend endpoints.
556-571: LGTM! Installation and auth methods properly structured.The installCursor and verifyCursorAuth methods correctly implement the backend API calls with appropriate parameter passing and return types.
573-587: LGTM! Default provider management correctly implemented.The getDefaultProvider and setDefaultProvider methods properly handle provider selection with the correct union types matching the expanded provider support.
apps/ui/src/lib/electron.ts (7)
535-583: LGTM! Multi-provider status methods properly defined.The getCursorStatus, getOpenCodeStatus, and getCodexStatus methods follow the established pattern and correctly define the provider status API surface for the Electron interface.
625-637: LGTM! Cursor-specific installation and auth methods correctly defined.The installCursor and verifyCursorAuth optional methods properly extend the setup API surface to support Cursor provider management.
1168-1216: LGTM! SetupAPI interface correctly extended.The status getter methods in the SetupAPI interface match the ElectronAPI definitions, maintaining consistency across the API surface.
1255-1273: LGTM! Installation, auth, and provider selection methods properly typed.The SetupAPI interface additions (installCursor, verifyCursorAuth, setDefaultProvider) are correctly defined with appropriate parameter and return types.
1306-1351: LGTM! Mock status implementations suitable for testing.The mock implementations for getCursorStatus, getOpenCodeStatus, and getCodexStatus provide appropriate test data with not_installed states and unauthenticated auth objects.
1363-1370: LGTM! Mock installCursor appropriately returns unavailable.The mock implementation correctly indicates that CLI installation is not supported in the mock environment with a clear error message.
1422-1436: LGTM! Mock auth and provider selection implementations functional.The mock implementations for verifyCursorAuth and setDefaultProvider provide appropriate testing behavior, with auth verification returning unauthenticated and provider selection succeeding.
apps/ui/src/components/views/settings-view.tsx (3)
2-2: LGTM: Imports follow established patterns.The new imports for ThemeMode and the CLI status components are consistent with existing import patterns in the file and appropriate for app-internal UI components.
Also applies to: 13-15, 18-18
53-56: LGTM: Store integration follows established patterns.The destructuring of provider and model state/setters from the app store is consistent with the existing approach for other settings.
102-115: LGTM: CLI status hook integration is correct.The expansion of
useCliStatus()to include Cursor, Codex, and OpenCode statuses follows the established pattern for Claude and correctly destructures the values documented in the hook's implementation.apps/server/src/routes/github/index.ts (1)
22-36: LGTM!The
SettingsServiceparameter is cleanly integrated as an optional dependency and properly forwarded tocreateValidateIssueHandler. This aligns with the pattern used in other route factories throughout the codebase.apps/server/src/index.ts (1)
126-132: LGTM!Wrapping the agent service initialization in a try/catch with
process.exit(1)on failure is a good practice. This ensures the server doesn't continue running in a broken state if a critical service fails to initialize.apps/server/src/services/auto-mode-service.ts (3)
77-93: LGTM with note on validation logic.The environment variable parsing helpers are well-implemented. Note that
readIntEnvrejects values ≤ 0 (line 83), which is appropriate for the current use cases (thresholds and counts). If zero becomes a valid value for any future config, a separate helper would be needed.
664-666: Good observability improvement.Logging the provider name alongside the model enhances debugging and operational visibility when multiple providers are in use.
2688-2703: LGTM!The
tool_resulthandling block properly emits events for real-time UI updates and persists tool results to the output file, maintaining consistency with the existingtool_usehandling pattern.apps/server/src/providers/types.ts (2)
14-21: LGTM!The
SystemPromptConfiginterface cleanly defines the structure for Claude SDK preset mode configuration. The type supports the CLAUDE.md auto-loading feature while maintaining type safety.
38-38: LGTM!The union type
string | SystemPromptConfigforsystemPromptprovides flexibility for both simple string prompts and the new preset-based configuration.apps/server/src/services/agent-service.ts (3)
303-304: LGTM!The
sawErrorandemittedCompleteflags provide robust tracking to ensure the completion event is emitted exactly once, even in edge cases where the stream ends without an explicit result message.
353-365: LGTM!The result handling now properly creates an assistant message if one doesn't exist, preventing cases where a successful result might not be captured in the conversation history.
381-388: LGTM!The fallback completion event emission ensures clients always receive a completion notification, even if the stream ends without an explicit result message and no error occurred.
apps/server/src/lib/sdk-options.ts (3)
140-145: LGTM!The
isClaudeModelKeyhelper correctly identifies Claude-compatible models by checking for theclaude-prefix or presence inCLAUDE_MODEL_MAP. Returningtruefor undefined models is the right default behavior.
154-184: LGTM!The
buildClaudeMdOptionsfunction now correctly:
- Checks if the model is Claude-compatible before enabling auto-loading
- Includes both 'user' and 'project' in
settingSourcesfor comprehensive CLAUDE.md discovery- Only appends string-based systemPrompts to the preset configuration
This provider-aware approach prevents attempting to use Claude-specific features with non-Claude providers.
200-200: LGTM!The type signature update to
string | SystemPromptConfigaligns with the imported type fromproviders/types.ts.apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
121-131: LGTM - Store-based default model initialization.The migration from hardcoded
'opus'to store-baseddefaultModelis well implemented. The initial value is set from the store on mount, and the effect at line 164 ensures proper synchronization when the dialog opens.
| app.use('/api/agent', createAgentRoutes(agentService, events)); | ||
| app.use('/api/sessions', createSessionsRoutes(agentService)); | ||
| app.use('/api/features', createFeaturesRoutes(featureLoader)); | ||
| app.use('/api/backlog-plan', createBacklogPlanRoutes(events, settingsService)); |
There was a problem hiding this comment.
Duplicate route mounting detected.
The /api/backlog-plan route is mounted twice - once at line 154 and again at line 171. This will cause the second mount to shadow the first, which could lead to unexpected behavior if the route handlers differ or cause routing conflicts.
🔎 Proposed fix
Remove one of the duplicate mounts. Keep only line 154 since it appears earlier in the logical grouping:
app.use('/api/context', createContextRoutes(settingsService));
-app.use('/api/backlog-plan', createBacklogPlanRoutes(events, settingsService));
app.use('/api/pipeline', createPipelineRoutes(pipelineService));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/src/index.ts around lines 154 to 171, the /api/backlog-plan route
is mounted twice (once at line 154 and again at line 171); remove the duplicate
mounting so the route is only registered once (keep the earlier mount at line
154 as suggested) and delete or comment out the later
app.use('/api/backlog-plan', ...) call to prevent routing conflicts.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/server/src/index.ts (1)
161-161: Duplicate route mounting detected.The
/api/backlog-planroute is mounted twice—once at Line 161 and again at Line 178. The second mount will shadow the first, potentially causing routing conflicts or unexpected behavior.🔎 Proposed fix
Remove the duplicate mount at Line 178:
app.use('/api/context', createContextRoutes(settingsService)); -app.use('/api/backlog-plan', createBacklogPlanRoutes(events, settingsService)); app.use('/api/mcp', createMCPRoutes(mcpTestService));Also applies to: 178-178
🧹 Nitpick comments (3)
apps/server/src/lib/sdk-options.ts (1)
224-228: Consider handlingSystemPromptConfigpassthrough.When
config.systemPromptis already aSystemPromptConfigobject (not a string), it's silently ignored in favor of the preset. If the caller explicitly provides aSystemPromptConfig, consider either merging it or passing it through.🔎 Proposed enhancement
if (config.systemPrompt) { if (typeof config.systemPrompt === 'string') { result.systemPrompt.append = config.systemPrompt; + } else if (config.systemPrompt.type === 'preset' && config.systemPrompt.append) { + // Preserve user's append if they passed a SystemPromptConfig + result.systemPrompt.append = config.systemPrompt.append; } }apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
145-150: Non-Claude providers silently dropSystemPromptConfigobjects.When
options.systemPromptis aSystemPromptConfigobject (frombuildClaudeMdOptions), non-Claude providers receive nothing. Consider extracting theappendtext or logging a warning.🔎 Proposed fix
if (providerName === 'claude') { streamOptions.systemPrompt = options.systemPrompt; streamOptions.settingSources = options.settingSources; } else if (typeof options.systemPrompt === 'string') { streamOptions.systemPrompt = options.systemPrompt; + } else if (options.systemPrompt && typeof options.systemPrompt === 'object' && 'append' in options.systemPrompt) { + // Extract append text for CLI-based providers + streamOptions.systemPrompt = options.systemPrompt.append; }apps/server/src/providers/codex-provider.ts (1)
793-795:WebSearchandWebFetchclassified as write tools.These are read-only network operations but are included in
writeTools. If this is intentional (e.g., treating external network access as elevated privilege), consider adding a comment explaining the rationale.🔎 Suggested clarification
private needsWriteAccess(allowedTools?: string[]): boolean { if (!allowedTools) { return true; } if (allowedTools.length === 0) { return false; } + // WebSearch/WebFetch included because external network access requires elevated sandbox permissions const writeTools = new Set(['Write', 'Edit', 'Bash', 'WebSearch', 'WebFetch']); return allowedTools.some((tool) => writeTools.has(tool)); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/server/src/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsdocs/server/providers.mddocs/server/utilities.mdlibs/types/src/index.tslibs/types/src/provider.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts
- apps/server/src/lib/subprocess-manager.ts
- apps/ui/src/lib/http-api-client.ts
- apps/server/src/providers/claude-provider.ts
- apps/ui/src/components/views/settings-view.tsx
- apps/ui/src/store/app-store.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.tsapps/server/src/lib/sdk-options.tsapps/ui/src/hooks/use-settings-migration.tsapps/server/src/index.tslibs/types/src/index.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.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:
libs/types/src/provider.tsapps/server/src/lib/sdk-options.tsapps/ui/src/hooks/use-settings-migration.tsapps/server/src/index.tslibs/types/src/index.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.ts
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/lib/sdk-options.tsapps/server/src/index.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
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/server/src/lib/sdk-options.tslibs/types/src/index.tsapps/server/src/routes/app-spec/generate-features-from-spec.ts
📚 Learning: 2025-12-28T05:07:48.138Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.138Z
Learning: Applies to apps/server/src/**/*.{ts,tsx} : Use `createEventEmitter()` from `lib/events.ts` for all server operations to emit events that stream to frontend via WebSocket
Applied to files:
apps/server/src/index.ts
🧬 Code graph analysis (3)
apps/server/src/lib/sdk-options.ts (1)
libs/types/src/index.ts (1)
CLAUDE_MODEL_MAP(41-41)
apps/server/src/index.ts (1)
apps/server/src/routes/backlog-plan/index.ts (1)
createBacklogPlanRoutes(14-30)
apps/server/src/providers/codex-provider.ts (3)
apps/server/src/providers/types.ts (6)
ExecuteOptions(11-11)ProviderMessage(34-45)InstallationStatus(50-58)ModelDefinition(72-84)ContentBlock(21-29)ConversationMessage(10-10)apps/server/src/providers/codex-config-manager.ts (1)
CodexConfigManager(20-158)apps/server/src/lib/subprocess-manager.ts (1)
spawnJSONLProcess(66-167)
🔇 Additional comments (13)
apps/server/src/index.ts (3)
34-34: LGTM!The import is correctly structured for an internal route module.
133-139: LGTM!The explicit error handling prevents the server from continuing in a broken state and provides clear failure signaling.
142-149: LGTM!The periodic cleanup mechanism is a good practice for preventing memory leaks in long-running processes.
docs/server/utilities.md (1)
584-594: LGTM!The updated example accurately documents the new sandboxed Codex CLI invocation pattern with
--sandbox workspace-writeand--ask-for-approval neverflags, which aligns with the CodexProvider implementation.apps/server/src/lib/sdk-options.ts (1)
140-145: LGTM!The
isClaudeModelKeyhelper correctly identifies Claude models by checking for the'claude-'prefix or presence inCLAUDE_MODEL_MAP. Returningtruefor undefined/empty models maintains backward compatibility with existing Claude-defaulting behavior.libs/types/src/provider.ts (1)
77-77: LGTM!The
timeoutMsoptional property cleanly extendsExecuteOptionsto support per-call timeout configuration, which integrates well with the new provider execution paths.docs/server/providers.md (1)
234-234: LGTM!The documentation accurately reflects the updated Codex CLI invocation pattern with sandboxed execution and automatic approval bypass.
libs/types/src/index.ts (1)
40-52: LGTM!The expanded exports cleanly extend the public API surface to support the new multi-provider model maps and alias types, following the existing export patterns in the file.
apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
167-211: Good error recovery pattern.The
streamErrorcapture with partial output continuation is a solid resilience pattern—allowing feature parsing to proceed with accumulated text even if the stream terminates early.apps/server/src/providers/codex-provider.ts (4)
50-59: LGTM!Clean CLI path resolution with proper error yielding before early return, preventing undefined behavior downstream.
95-102: LGTM!The bidirectional API key synchronization (
OPENAI_API_KEY↔CODEX_API_KEY) ensures compatibility with different authentication configurations.
369-561: LGTM!The
toProviderMessagesmethod provides comprehensive JSONL event translation with graceful fallback handling for unknown event types. The defensive programming approach with null checks throughout is appropriate for parsing external CLI output.
85-87: LGTM!Passing the prompt as a positional argument to
spawnJSONLProcess(which usesspawnwith an args array) is safe from shell injection. The subprocess manager handles this correctly.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
apps/server/src/index.ts (1)
161-161: Duplicate route mounting detected.The
/api/backlog-planroute is mounted twice—once at line 161 and again at line 178. The second mount will shadow the first, potentially causing unexpected routing behavior.🔎 Proposed fix
Remove the duplicate mount at line 178:
app.use('/api/context', createContextRoutes(settingsService)); -app.use('/api/backlog-plan', createBacklogPlanRoutes(events, settingsService)); app.use('/api/mcp', createMCPRoutes(mcpTestService));Also applies to: 178-178
apps/ui/src/lib/electron.ts (1)
4-18: Backlog plan type wiring is now consistent and import-safeImporting and re‑exporting
BacklogPlanResultandBacklogPlanEventfrom@automaker/typesaligns this file with the shared types and resolves the earlier missing‑import issue noted in past reviews.Also applies to: 21-35
apps/ui/src/components/views/settings-view.tsx (1)
127-152: [Unresolved from previous review] Import and resolve model aliases usingresolveModelString().This issue was flagged in a previous review but has not been addressed. Per the coding guidelines for TypeScript files, model aliases (haiku, sonnet, opus, auto, glm4.7) must be resolved to full model names using
resolveModelString()from@automaker/model-resolver.Currently, line 134 passes
setDefaultModeldirectly without resolution, meaning aliases flow into the store unresolved.Add the import and wrap the callback:
import { resolveModelString } from '@automaker/model-resolver';Then update line 134:
onModelChange={(model) => { const resolved = resolveModelString(model); setDefaultModel(resolved ?? model); }}Based on learnings, this is required for all TypeScript files handling model strings.
🧹 Nitpick comments (7)
apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
32-59: Consider memoizing the provider inference.
inferProvideris redefined and invoked on every render. For a simple UI component this is likely fine, but if the parent re-renders frequently, wrapping the derivation inuseMemowould prevent redundant computation.🔎 Proposed optimization
+ import { useMemo } from 'react'; // inside the component: - const inferProvider = (): ModelProvider => { - // ...inference logic - }; - const resolvedProvider = selectedProvider ?? inferProvider(); + const resolvedProvider = useMemo(() => { + if (selectedProvider) return selectedProvider; + if (CODEX_MODELS.some((model) => resolveModelString(model.id) === normalizedSelectedModel)) { + return 'codex'; + } + if (OPENCODE_MODELS.some((model) => resolveModelString(model.id) === normalizedSelectedModel)) { + return 'opencode'; + } + if (CURSOR_MODELS.some((model) => resolveModelString(model.id) === normalizedSelectedModel)) { + return 'cursor'; + } + if (CLAUDE_MODELS.some((model) => resolveModelString(model.id) === normalizedSelectedModel)) { + return 'claude'; + } + const lowerModel = normalizedSelectedModel.toLowerCase(); + if (lowerModel.startsWith('gpt-') || /^o\d/.test(lowerModel)) { + return 'codex'; + } + if (lowerModel.startsWith('glm') || lowerModel === 'opencode') { + return 'opencode'; + } + if (lowerModel === 'auto' || lowerModel.startsWith('cursor-')) { + return 'cursor'; + } + return 'claude'; + }, [selectedProvider, normalizedSelectedModel]);
83-151: Consider DRYing up the provider buttons.The four provider buttons share nearly identical structure. Extracting this into a loop over a config array would reduce duplication and make adding/removing providers easier.
🔎 Proposed refactor
const providerDefaults: Record<ModelProvider, string> = { cursor: 'auto', codex: 'gpt-5.2-codex', opencode: 'glm4.7', claude: 'opus', }; // Then in JSX: {(['cursor', 'codex', 'opencode', 'claude'] as ModelProvider[]).map((provider) => { const { icon: Icon, label } = providerMeta[provider]; const shortLabel = label.split(' ')[0]; // e.g., "Cursor" return ( <button key={provider} type="button" onClick={() => { onProviderSelect?.(provider); onModelSelect(resolveModelString(providerDefaults[provider]) as AgentModel); }} className={cn( 'flex-1 flex items-center justify-center gap-2 px-3 py-2 rounded-md border text-sm font-medium transition-colors', resolvedProvider === provider ? 'bg-primary text-primary-foreground border-primary' : 'bg-background hover:bg-accent border-input' )} data-testid={`${testIdPrefix}-provider-${provider}`} > <Icon className="w-4 h-4" /> {shortLabel} </button> ); })}docs/tuto.md (3)
10-16: Brief CLI setup section lacks motivation and detail.The instructions install four tools (Opencode, Codex, Claude, Cursor) without explaining why each is needed, which constraint each satisfies, or how they fit together in the workflow. Given the PR adds multi-provider support, this section should clarify the role of each provider (e.g., "Cursor for X, Codex for Y, OpenCode for Z") and any licensing/installation prerequisites.
Consider expanding this section or cross-referencing setup documentation with provider roles. For example:
- Opencode for GLM models (free)
- Codex CLI for OpenAI models
- Claude for critique/review (see context-engineering workflow below)
- Cursor for lightweight, fast tasks
421-421: Use more concise phrasing.Replace "A lot of things and exemple are not required" with a more direct statement, e.g., "Remove features and examples unrelated to your project spec."
23-410: Spec template is comprehensive but disconnected from tutorial flow.The template (lines 23–410) is detailed and well-structured; however, it is inserted mid-tutorial (between CLI setup and usage steps) without a transition sentence. Consider:
- Explicitly state that this is a reference template to be used in step 7 (spec generation).
- Add a brief preamble explaining when and how to apply the template.
- Consider moving the template to an appendix or separate file to improve readability and modularity.
apps/ui/src/store/app-store.ts (1)
1615-1641: Consider making backend sync await consistent.The
setDefaultProvideraction performs two async operations (backend API call and settings sync) in fire-and-forget style, whilesetDefaultModelonly syncs settings. This creates a potential race condition:
- User changes provider → backend API call fires
- User immediately changes model → settings sync fires
- If the provider API call completes after the model sync, the model might be overwritten with stale data
Consider one of these approaches:
Option 1: Await backend sync before updating state
- setDefaultProvider: (provider) => { - set({ defaultProvider: provider }); + setDefaultProvider: async (provider) => { + try { + const { getHttpApiClient } = await import('@/lib/http-api-client'); + await getHttpApiClient().setup.setDefaultProvider(provider); + set({ defaultProvider: provider }); + const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); + await syncSettingsToServer(); + } catch (err) { + console.error('[AppStore] Failed to sync provider:', err); + // Consider reverting state or showing error + } + },Option 2: Queue updates
Use a mutation queue to ensure operations complete in order.Based on learnings, fire-and-forget patterns are acceptable for UI responsiveness, but consider testing the race condition scenario where users rapidly switch provider then model.
apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx (1)
69-69: Consider displaying the normalized model for consistency.The UI displays the raw
profile.modelvalue, while the selection logic operates on the normalized model string. If these values differ significantly (e.g., "haiku" vs. "claude-3-haiku-20240307"), users might find it confusing.🔎 Optional: Display normalized model value
<p className="text-[10px] text-muted-foreground truncate"> - {profile.model} + {normalizedProfileModel} {profile.thinkingLevel !== 'none' && ` + ${profile.thinkingLevel}`} </p>Alternatively, if you prefer to preserve the raw model name for user reference, consider adding a tooltip or secondary text showing the resolved model.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/server/src/index.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/settings-service.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/board-view/shared/profile-quick-select.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/lib/electron.tsapps/ui/src/store/app-store.tsdocs/tuto.mdlibs/types/src/model.tslibs/types/src/settings.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
- apps/ui/src/components/views/agent-view.tsx
🧰 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:
apps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsxlibs/types/src/settings.tsapps/server/src/index.tsapps/ui/src/components/views/board-view/shared/profile-quick-select.tsxapps/server/src/services/settings-service.tsapps/ui/src/store/app-store.tsapps/server/src/providers/claude-provider.tslibs/types/src/model.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/lib/electron.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/ui/src/components/views/settings-view.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/services/auto-mode-service.tsapps/server/src/index.tsapps/server/src/services/settings-service.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.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/src/services/auto-mode-service.tsapps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsxlibs/types/src/settings.tsapps/server/src/index.tsapps/ui/src/components/views/board-view/shared/profile-quick-select.tsxapps/server/src/services/settings-service.tsapps/ui/src/store/app-store.tsapps/server/src/providers/claude-provider.tslibs/types/src/model.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/lib/electron.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/ui/src/components/views/settings-view.tsx
🧠 Learnings (4)
📚 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/server/src/services/auto-mode-service.tsapps/ui/src/components/views/board-view/shared/profile-quick-select.tsxlibs/types/src/model.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/settings-view.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: Each feature executes in an isolated git worktree created via automaker/git-utils to protect the main branch during AI agent execution
Applied to files:
apps/server/src/services/auto-mode-service.ts
📚 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/ai-provider/ai-provider-section.tsxapps/ui/src/components/views/board-view/shared/profile-quick-select.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/settings-view.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: Store project-specific rules in `.automaker/context/` and load them into agent prompts via `loadContextFiles()` from automaker/utils
Applied to files:
docs/tuto.md
🧬 Code graph analysis (8)
apps/server/src/services/auto-mode-service.ts (3)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)libs/types/src/model.ts (1)
DEFAULT_MODELS(47-52)libs/types/src/index.ts (1)
DEFAULT_MODELS(45-45)
apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx (6)
libs/types/src/settings.ts (2)
ModelProvider(71-71)AgentModel(12-12)libs/types/src/model.ts (1)
AgentModel(71-71)apps/ui/src/components/views/board-view/shared/model-constants.ts (3)
CURSOR_MODELS(36-51)CODEX_MODELS(63-127)OPENCODE_MODELS(53-61)apps/ui/src/lib/electron.ts (1)
getElectronAPI(804-813)apps/ui/src/lib/utils.ts (1)
cn(5-7)apps/ui/src/components/views/settings-view/ai-provider/provider-card.tsx (1)
ProviderCard(46-86)
apps/server/src/index.ts (1)
apps/server/src/routes/backlog-plan/index.ts (1)
createBacklogPlanRoutes(14-30)
apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx (1)
libs/types/src/model.ts (1)
AgentModel(71-71)
apps/server/src/services/settings-service.ts (2)
libs/types/src/settings.ts (1)
GlobalSettings(272-367)apps/server/src/types/settings.ts (1)
GlobalSettings(20-20)
apps/ui/src/store/app-store.ts (6)
libs/types/src/settings.ts (2)
ModelProvider(71-71)AgentModel(12-12)libs/types/src/index.ts (2)
ModelProvider(70-70)AgentModel(51-51)apps/server/src/types/settings.ts (2)
ModelProvider(14-14)AgentModel(11-11)libs/types/src/model.ts (1)
AgentModel(71-71)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(1328-1333)apps/ui/src/hooks/use-settings-migration.ts (1)
syncSettingsToServer(196-249)
apps/ui/src/lib/electron.ts (1)
libs/types/src/index.ts (2)
BacklogPlanResult(127-127)BacklogPlanEvent(128-128)
apps/server/src/routes/app-spec/generate-features-from-spec.ts (3)
apps/server/src/lib/sdk-options.ts (1)
getModelForUseCase(95-129)libs/types/src/model.ts (1)
DEFAULT_MODELS(47-52)libs/types/src/provider.ts (1)
ExecuteOptions(66-82)
🪛 LanguageTool
docs/tuto.md
[grammar] ~414-~414: Ensure spelling is correct
Context: ...ate in context tab copy response of the promps for specs in a md file called OBJECTIVE...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~421-~421: Consider using a synonym to be more concise.
Context: ...ECTIVES.md and @.automaker/app_spec.txt A lot of things and exemple are not required cle...
(A_LOT_OF)
[grammar] ~422-~422: Ensure spelling is correct
Context: ...omaker/app_spec.txt A lot of things and exemple are not required clean repo and update ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (18)
apps/server/src/index.ts (2)
133-139: LGTM! Good defensive practice.Wrapping the agent service initialization in a try-catch with an explicit exit ensures the server won't run in a broken state if initialization fails.
144-144: LGTM! Proper resource cleanup.The named interval variable enables proper cleanup during graceful shutdown, preventing resource leaks when the server terminates.
Also applies to: 504-504, 514-514
apps/server/src/providers/claude-provider.ts (1)
25-38: ForwardingsettingSourcesvia local destructuring looks correctUsing a local
settingSourcesfromExecuteOptionsand conditionally spreading it intosdkOptionspreserves the prior behavior while keeping the SDK options shape explicit. No further changes needed here.Also applies to: 55-78
apps/ui/src/lib/electron.ts (2)
494-507:ElectronAPI.backlogPlansurface looks coherent with shared typesThe new
backlogPlanAPI usesBacklogPlanResultforapplyandBacklogPlanEventforonEvent, with a simplegenerate/stop/statustrio matching other APIs in this file. Marking the wholebacklogPlanblock as optional keeps the web/mock case safe.
539-587: Setup API extensions for Cursor/OpenCode/Codex and default provider are consistentThe added setup methods (
getCursorStatus/getOpenCodeStatus/getCodexStatus,installCursor,verifyCursorAuth,setDefaultProvider) have matching shapes betweenElectronAPI.setup, theSetupAPIinterface, andcreateMockSetupAPI. Auth sub-objects are coherent and safely optional, and thesetDefaultProviderunion matches the provider identifiers used elsewhere.Also applies to: 629-641, 1172-1220, 1259-1277, 1310-1355, 1367-1374, 1426-1440
apps/ui/src/components/views/settings-view/config/navigation.ts (1)
3-14: AI Provider navigation entry is wired cleanlyImporting
Wand2and adding the'ai-provider'item intoNAV_ITEMSis consistent with existing patterns; assumingSettingsViewIdincludes'ai-provider', this should integrate the new settings section without issues.Also applies to: 24-36
apps/server/src/services/settings-service.ts (1)
410-445: Migration ofdefaultProvideranddefaultModellooks safe and backward‑compatibleAdding
defaultProvider(fallback'claude') anddefaultModel(fallback'sonnet') into the migratedglobalSettingsensures these new fields are populated even for legacy localStorage state, and they’ll still be merged withDEFAULT_GLOBAL_SETTINGSviaupdateGlobalSettings. No additional migration handling seems necessary.apps/server/src/services/auto-mode-service.ts (1)
614-616: Centralized default model resolution aligns feature runs with selected providerRouting all feature and pipeline executions through
resolveModelString(feature.model, this.getDefaultFeatureModel()), withgetDefaultFeatureModel()deriving its fallback fromProviderFactory.getDefaultProvider()andDEFAULT_MODELS, makes the default model/provider behavior consistent across:
- initial feature runs,
- post‑implementation pipeline steps, and
- follow‑up runs.
This keeps analysis flows explicitly on the Claude default while letting auto‑mode respect the globally selected provider. The helper is small, typed correctly against
DEFAULT_MODELS, and avoids scattering provider‑specific defaults.Also applies to: 754-755, 990-993, 1759-1765
apps/ui/src/components/views/settings-view/ai-provider/ai-provider-section.tsx (1)
1-253: LGTM! Well-structured provider selection UI.The component correctly:
- Uses
resolveModelString()from@automaker/model-resolverconsistently throughout (lines 37, 90, 92, 94, 96, 196, 203, 216) as required by coding guidelines- Imports from shared packages (
@automaker/*) rather than relative paths- Handles backend synchronization with appropriate error feedback via toast
- Implements proper type safety with
ModelProviderandAgentModeltypes- Uses the extracted
ProviderCardcomponent to avoid duplication (per previous review feedback)The provider selection logic (lines 89-97) correctly sets sensible defaults for each provider, and the backend sync (lines 100-119) now includes user-facing error toast as suggested in previous reviews.
libs/types/src/settings.ts (3)
71-71: LGTM! ModelProvider type correctly expanded.The
ModelProviderunion type now properly includes all four providers:'claude' | 'cursor' | 'opencode' | 'codex'. This aligns with the multi-provider architecture introduced in this PR.
311-314: LGTM! Clear documentation for new provider settings.The new
defaultProvideranddefaultModelfields are well-documented and have appropriate types. The comments clearly indicate their purpose for routing feature execution to the selected AI provider.
512-513: LGTM! Sensible defaults for backward compatibility.Defaulting to
'claude'provider and'sonnet'model maintains backward compatibility with existing installations while enabling the new multi-provider functionality.apps/server/src/routes/app-spec/generate-features-from-spec.ts (2)
180-214: LGTM! Excellent defensive coding and error handling.The stream handling demonstrates several best practices:
- Optional chaining on
msg.message?.content(line 180) andblock.text(line 182) prevents runtime errors- Captures stream errors separately without immediately throwing (lines 170, 196-200, 203-205)
- Allows partial output to continue after errors (lines 207-214): "if there's some output, use it; otherwise rethrow"
This graceful degradation is particularly valuable for long-running feature generation where partial results can still be useful.
105-114: The provider fallback logic is correctly typed and safe.The concern about
getDefaultProvider()returning an invalid provider key is unfounded. The return type ofgetDefaultProvider()is strictly'claude' | 'cursor' | 'opencode' | 'codex', which exactly matches the keys inDEFAULT_MODELS. TypeScript's type system prevents the alleged edge case.Additionally, the logic correctly handles the case when
defaultProvider === 'claude'—it intentionally does not updatefeatureModelin this scenario, leaving it asgetModelForUseCase('features'). This is the intended behavior.libs/types/src/model.ts (2)
13-42: LGTM! Model maps follow consistent pattern.The new model maps (
CURSOR_MODEL_MAP,OPENCODE_MODEL_MAP,CODEX_MODEL_MAP) follow the same structure as the existingCLAUDE_MODEL_MAP:
- Use
Record<string, string>withas constfor type inference- Provide meaningful alias-to-ID mappings
- Include relevant models for each provider
Note: Some maps like
OPENCODE_MODEL_MAPhave identity mappings (e.g.,'glm4.7': 'glm4.7'), which is appropriate when the model ID itself serves as the canonical identifier.
54-71: LGTM! Type definitions correctly unify all providers.The type hierarchy properly captures the multi-provider model system:
- Individual alias types per provider (
ClaudeModelAlias,CursorModelAlias, etc.)ModelAliasunion encompasses all provider aliasesModelIdunion includes all concrete model strings from the mapsAgentModel = ModelAlias | ModelIdallows both aliases and full IDsThis structure enables type-safe model selection across all providers while maintaining backward compatibility.
apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx (2)
6-6: LGTM! Correct usage of shared package import.The import correctly follows the coding guideline to use shared packages (@automaker/*) for model resolution utilities.
30-30: LGTM! Proper model normalization before comparison.The code correctly applies
resolveModelString()to normalize both the selected model and profile models before comparison, ensuring that model aliases are consistently resolved to full model names. This aligns with the coding guidelines and retrieved learnings.Also applies to: 46-49
| {models.map((option) => { | ||
| const resolvedOptionId = resolveModelString(option.id); | ||
| const isSelected = normalizedSelectedModel === resolvedOptionId; | ||
| const shortName = option.label.replace('Claude ', '').replace('Cursor ', ''); |
There was a problem hiding this comment.
Incomplete prefix stripping for short names.
Only 'Claude ' and 'Cursor ' are stripped from model labels. If OpenCode or Codex model labels include their provider name as a prefix, they won't be shortened consistently.
🔎 Proposed fix
- const shortName = option.label.replace('Claude ', '').replace('Cursor ', '');
+ const shortName = option.label
+ .replace('Claude ', '')
+ .replace('Cursor ', '')
+ .replace('OpenCode ', '')
+ .replace('Codex ', '');Alternatively, use a regex for a more generic approach:
const shortName = option.label.replace(/^(Claude|Cursor|OpenCode|Codex)\s+/i, '');📝 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 shortName = option.label.replace('Claude ', '').replace('Cursor ', ''); | |
| const shortName = option.label | |
| .replace('Claude ', '') | |
| .replace('Cursor ', '') | |
| .replace('OpenCode ', '') | |
| .replace('Codex ', ''); |
🤖 Prompt for AI Agents
In apps/ui/src/components/views/board-view/shared/model-selector.tsx around line
168, the shortName computation only strips 'Claude ' and 'Cursor ' prefixes so
other provider prefixes (e.g., OpenCode, Codex) remain; replace the current
chained replace calls with a single, case-insensitive regex that removes any of
the known provider prefixes (e.g., Claude, Cursor, OpenCode, Codex) followed by
whitespace so labels are shortened consistently.
| key={profile.id} | ||
| type="button" | ||
| onClick={() => onSelect(profile.model, profile.thinkingLevel)} | ||
| onClick={() => onSelect(normalizedProfileModel as AgentModel, profile.thinkingLevel)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find resolveModelString implementation in @automaker/model-resolver
fd model-resolver -t dRepository: AutoMaker-Org/automaker
Length of output: 87
🏁 Script executed:
# Search for resolveModelString definition across the codebase
rg -n "resolveModelString" --type=ts --type=tsx -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Search for AgentModel type definition
rg -n "AgentModel" --type=ts --type=tsx -B 2 -A 3Repository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Examine the specific file around line 54
cat -n apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx | sed -n '40,65p'Repository: AutoMaker-Org/automaker
Length of output: 1489
🏁 Script executed:
# Search for resolveModelString using rg without tsx type
rg -n "resolveModelString" --type=ts -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 50379
🏁 Script executed:
# Check the model-resolver library exports and index
ls -la libs/model-resolver/Repository: AutoMaker-Org/automaker
Length of output: 536
🏁 Script executed:
# Search for AgentModel without type filter
rg -n "type AgentModel\|interface AgentModel\|export.*AgentModel" -A 3Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Find AgentModel type definition in app-store
find . -name "app-store*" -type fRepository: AutoMaker-Org/automaker
Length of output: 99
🏁 Script executed:
# Search for AgentModel in store files
rg "type AgentModel|interface AgentModel|export.*AgentModel" --type=ts -B 2 -A 5Repository: AutoMaker-Org/automaker
Length of output: 5683
🏁 Script executed:
# Find ModelAlias and ModelId definitions
rg "type ModelAlias|type ModelId|export type ModelAlias|export type ModelId" --type=ts -B 2 -A 3Repository: AutoMaker-Org/automaker
Length of output: 1202
🏁 Script executed:
# Find the definitions of ClaudeModelAlias, CursorModelAlias, OpenCodeModelAlias
rg "type ClaudeModelAlias|type CursorModelAlias|type OpenCodeModelAlias" --type=ts -B 2 -A 3Repository: AutoMaker-Org/automaker
Length of output: 994
🏁 Script executed:
# Also check what CLAUDE_MODEL_MAP, CURSOR_MODEL_MAP, OPENCODE_MODEL_MAP, CODEX_MODEL_MAP contain
rg "CLAUDE_MODEL_MAP|CURSOR_MODEL_MAP|OPENCODE_MODEL_MAP|CODEX_MODEL_MAP" --type=ts -B 1 -A 5 | head -100Repository: AutoMaker-Org/automaker
Length of output: 5761
🏁 Script executed:
# Check the complete resolver.ts implementation to understand what it returns
cat -n libs/model-resolver/src/resolver.ts | sed -n '25,70p'Repository: AutoMaker-Org/automaker
Length of output: 1812
Add type-safe guarantees for resolveModelString() return value or remove the unsafe type assertion.
The type assertion as AgentModel bypasses TypeScript's type checking. While resolveModelString() is designed to return valid model strings, its signature returns string rather than AgentModel. The function can return any string containing 'claude-' (line 44 of resolver.ts), which is broader than the AgentModel union type and could include arbitrary model identifiers outside the known set.
Consider either:
- Updating
resolveModelString()to return a properly typedAgentModelwith type guards - Adding runtime validation before the assertion to ensure the result is a valid
AgentModel - Removing the assertion and handling the string result directly if type conversion is unnecessary
🤖 Prompt for AI Agents
In apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx
around line 54, the code unsafely asserts normalizedProfileModel as AgentModel
when calling onSelect; update this to be type-safe by either (a) changing
resolveModelString() signature to return AgentModel (with narrowing/type guards
in resolver.ts so only valid union members are returned), or (b) perform a
runtime validation here against the AgentModel union (e.g., a small
isAgentModel(value): value is AgentModel guard) and only call onSelect with the
asserted type when the guard passes, otherwise handle the invalid case (fallback
or error); remove the direct "as AgentModel" assertion if you implement
validation or change the resolver signature so the compiler guarantees
correctness.
|
|
||
| 5. Create new project in automaker use starter pack | ||
|
|
||
| 6. Create in context tab copy response of the promps for specs in a md file called OBJECTIVES.md |
There was a problem hiding this comment.
Fix spelling error: "promps" → "prompts".
🧰 Tools
🪛 LanguageTool
[grammar] ~414-~414: Ensure spelling is correct
Context: ...ate in context tab copy response of the promps for specs in a md file called OBJECTIVE...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In docs/tuto.md at line 414, fix the spelling error by replacing "promps" with
"prompts" so the line reads: "6. Create in context tab copy response of the
prompts for specs in a md file called OBJECTIVES.md".
| 8. Open cursor, use composer ask it following prompt. | ||
|
|
||
| Clean what's not required for the project: | ||
| Everything is explained in @.automaker/context/OBJECTIVES.md and @.automaker/app_spec.txt | ||
| A lot of things and exemple are not required clean repo and update tasks in to build the project properly that my autocode will do. | ||
| Remove uselss feature for the project specs and update the others one. @.automaker/features | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and get the relevant section with more context
wc -l docs/tuto.mdRepository: AutoMaker-Org/automaker
Length of output: 83
🏁 Script executed:
# Read lines around 418-424 to understand the context better
sed -n '400,430p' docs/tuto.mdRepository: AutoMaker-Org/automaker
Length of output: 1170
🏁 Script executed:
# Search for references to @.automaker pattern in the codebase to understand the notation
rg "@\.automaker" --type md -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 533
🏁 Script executed:
# Look for documentation about the @ notation or .automaker directory references
rg "@\." docs/ --type md -B 1 -A 1 | head -50Repository: AutoMaker-Org/automaker
Length of output: 489
🏁 Script executed:
# Check if there's a .automaker directory or documentation about it
fd -t d "\.automaker"Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Look for the entire steps 5-9 section to understand the workflow context
sed -n '380,440p' docs/tuto.mdRepository: AutoMaker-Org/automaker
Length of output: 1522
🏁 Script executed:
# Look for README or main documentation files that might explain notation
cat README.md 2>/dev/null | head -100Repository: AutoMaker-Org/automaker
Length of output: 5147
🏁 Script executed:
# Search for any references to Cursor or Composer in docs
rg -i "cursor|composer" docs/ --type mdRepository: AutoMaker-Org/automaker
Length of output: 1558
🏁 Script executed:
# Search for explanation of @ notation in documentation
rg "@\.|context/OBJECTIVES" --type mdRepository: AutoMaker-Org/automaker
Length of output: 274
🏁 Script executed:
# Check if there's a reference to how to use @. notation
rg -i "notation|file reference|path" docs/tuto.mdRepository: AutoMaker-Org/automaker
Length of output: 98
Clarify context file reference syntax and document the post-generation workflow steps.
The @.automaker/... syntax (lines 421, 423) is non-standard and unexplained—it appears throughout the prompt without definition. Clarify whether @. is a special notation for context-file paths within Cursor/the IDE, or if these should be standard relative paths (e.g., ./.automaker/context/OBJECTIVES.md).
Additionally, steps 5–9 form a post-generation cleanup workflow (using Cursor IDE and its Composer feature for AI-driven feature refinement). This should be explicitly labeled as a "post-generation refinement phase" to distinguish it from the core spec generation workflow (steps 1–4 in section 7). Cursor's role in step 8 is clear (AI-powered code/feature refinement), but adding a one-line note ("Cursor is used here for AI-driven refinement of generated features") would improve clarity.
🧰 Tools
🪛 LanguageTool
[style] ~421-~421: Consider using a synonym to be more concise.
Context: ...ECTIVES.md and @.automaker/app_spec.txt A lot of things and exemple are not required cle...
(A_LOT_OF)
[grammar] ~422-~422: Ensure spelling is correct
Context: ...omaker/app_spec.txt A lot of things and exemple are not required clean repo and update ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
|
|
||
| Clean what's not required for the project: | ||
| Everything is explained in @.automaker/context/OBJECTIVES.md and @.automaker/app_spec.txt | ||
| A lot of things and exemple are not required clean repo and update tasks in to build the project properly that my autocode will do. |
There was a problem hiding this comment.
Fix spelling error: "exemple" → "example".
🧰 Tools
🪛 LanguageTool
[grammar] ~422-~422: Ensure spelling is correct
Context: ...omaker/app_spec.txt A lot of things and exemple are not required clean repo and update ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In docs/tuto.md at line 422, the word "exemple" is misspelled; replace "exemple"
with "example" so the sentence reads correctly (ensure only that token is
changed and preserve surrounding punctuation and spacing).
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/server/providers.md (1)
239-239: Clarify relationship between "30s timeout" and timeout environment variables.Line 239 mentions "Timeout detection (30s no output)" as a feature, but the troubleshooting section (line 771) references
CODEX_CLI_TIMEOUT_MSwithout clarifying that this 30-second default corresponds to that variable. Make the connection explicit so operators understand which variable controls the "30s" behavior mentioned earlier.🔎 Suggested clarification
Revise line 239 to:
- ✅ Timeout detection (30s default idle timeout, configurable via `CODEX_CLI_TIMEOUT_MS`)Or add a note in the timeout environment variables section:
Default idle timeout is 30 seconds (`CODEX_CLI_TIMEOUT_MS`). This can be increased for complex operations.Also applies to: 771-771
♻️ Duplicate comments (1)
apps/server/src/providers/codex-provider.ts (1)
218-224: Command injection risk in version detection.This issue was previously flagged:
cliPathis passed toexecAsyncwith shell execution, creating command injection risk if the path comes from untrusted sources (config or environment variables). UseexecFileinstead of shell execution.
🧹 Nitpick comments (5)
apps/server/src/lib/subprocess-manager.ts (4)
23-24:timeoutandstartupTimeoutoptions are accepted but ignored.
spawnProcessdestructuresoptionsbut doesn't extract or usetimeout/startupTimeout. If timeout support isn't intended for this function, consider documenting this or using a narrower options type to avoid confusion.🔎 Option A: Extract a base options type
+export interface BaseSubprocessOptions { + command: string; + args: string[]; + cwd: string; + env?: Record<string, string>; + abortController?: AbortController; +} + +export interface SubprocessOptions extends BaseSubprocessOptions { + timeout?: number; + startupTimeout?: number; +} export async function spawnProcess(options: SubprocessOptions): Promise<SubprocessResult> { - const { command, args, cwd, env, abortController } = options; + const { command, args, cwd, env, abortController } = options; // timeout intentionally unused
42-52: Abort listener is never removed, potential minor memory leak.If the
AbortControlleris long-lived and reused, the listener registered here will persist after the process completes. Consider using{ once: true }or removing the listener in cleanup.🔎 Proposed fix
if (abortController) { - abortController.signal.addEventListener('abort', () => { + abortController.signal.addEventListener('abort', () => { aborted = true; child.kill('SIGTERM'); - }); + }, { once: true });
59-63: Redundant conditional return.Both branches return identical
{ stdout, stderr, exitCode }. Theif (aborted)check can be removed unless different handling is planned.🔎 Proposed simplification
- if (aborted) { - return { stdout, stderr, exitCode }; - } - return { stdout, stderr, exitCode };
136-140: Consider static import forreadline.The dynamic
await import('readline')is evaluated on every call. Sincereadlineis a Node.js built-in, a static import at the top of the file would be more efficient and conventional.🔎 Proposed change
import { spawn } from 'child_process'; +import { createInterface } from 'readline'; // ... later in spawnJSONLProcess ... - const readline = await import('readline'); - const rl = readline.createInterface({ + const rl = createInterface({ input: child.stdout as NodeJS.ReadableStream, crlfDelay: Infinity, });apps/server/src/providers/codex-provider.ts (1)
394-400: Consider expanding model alias mappings.The model mapper currently only handles
"codex"→"gpt-5.2-codex". Consider adding more user-friendly aliases (e.g.,"mini"→"gpt-5.1-codex-mini","max"→"gpt-5.1-codex-max") to improve developer experience.💡 Suggested expansion
private mapModelToCodexFormat(model: string): string { const modelMap: Record<string, string> = { codex: 'gpt-5.2-codex', + mini: 'gpt-5.1-codex-mini', + max: 'gpt-5.1-codex-max', + 'codex-mini': 'gpt-5.1-codex-mini', + 'codex-max': 'gpt-5.1-codex-max', }; return modelMap[model.toLowerCase()] || model; }
📜 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 (27)
apps/server/src/index.tsapps/server/src/lib/sdk-options.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/providers/claude-provider.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/server/src/services/settings-service.tsapps/server/tests/unit/lib/subprocess-manager.test.tsapps/server/tests/unit/services/agent-service.test.tsapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/dialogs/agent-output-modal.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/src/store/setup-store.tsapps/ui/src/types/electron.d.tsdocs/server/providers.mddocs/server/utilities.mdlibs/types/src/index.tslibs/types/src/settings.ts
💤 Files with no reviewable changes (1)
- apps/server/src/routes/context/routes/describe-image.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts
- apps/ui/src/components/views/board-view.tsx
- apps/ui/src/components/views/board-view/dialogs/agent-output-modal.tsx
- apps/server/src/index.ts
- docs/server/utilities.md
- apps/ui/src/store/setup-store.ts
- apps/server/src/services/settings-service.ts
- apps/ui/src/lib/http-api-client.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:
apps/ui/src/hooks/use-settings-migration.tsapps/server/src/providers/claude-provider.tsapps/server/tests/unit/lib/subprocess-manager.test.tsapps/server/tests/unit/services/agent-service.test.tsapps/ui/src/store/app-store.tsapps/ui/src/types/electron.d.tsapps/server/src/lib/subprocess-manager.tslibs/types/src/index.tsapps/server/src/services/agent-service.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/server/src/services/auto-mode-service.tslibs/types/src/settings.tsapps/ui/src/lib/electron.tsapps/server/src/lib/sdk-options.tsapps/server/src/providers/codex-provider.tsapps/ui/src/components/views/settings-view/config/navigation.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/ui/src/hooks/use-settings-migration.tsapps/server/src/providers/claude-provider.tsapps/server/tests/unit/lib/subprocess-manager.test.tsapps/server/tests/unit/services/agent-service.test.tsapps/ui/src/store/app-store.tsapps/ui/src/types/electron.d.tsapps/server/src/lib/subprocess-manager.tslibs/types/src/index.tsapps/server/src/services/agent-service.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/dialogs/file-browser-dialog.tsxapps/server/src/services/auto-mode-service.tslibs/types/src/settings.tsapps/ui/src/lib/electron.tsapps/server/src/lib/sdk-options.tsapps/server/src/providers/codex-provider.tsapps/ui/src/components/views/settings-view/config/navigation.ts
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/claude-provider.tsapps/server/src/lib/subprocess-manager.tsapps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.tsapps/server/src/lib/sdk-options.tsapps/server/src/providers/codex-provider.ts
🧠 Learnings (4)
📚 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:
libs/types/src/index.tsapps/server/src/services/agent-service.tsapps/ui/src/components/views/settings-view.tsxapps/server/src/services/auto-mode-service.tsapps/server/src/lib/sdk-options.ts
📚 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: Store project-specific rules in `.automaker/context/` and load them into agent prompts via `loadContextFiles()` from automaker/utils
Applied to files:
apps/server/src/services/agent-service.tsapps/server/src/services/auto-mode-service.ts
📚 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.tsxapps/ui/src/components/dialogs/file-browser-dialog.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: Each feature executes in an isolated git worktree created via automaker/git-utils to protect the main branch during AI agent execution
Applied to files:
apps/server/src/services/auto-mode-service.ts
🧬 Code graph analysis (7)
apps/ui/src/store/app-store.ts (4)
libs/types/src/settings.ts (2)
ModelProvider(72-72)AgentModel(13-13)libs/types/src/model.ts (1)
AgentModel(71-71)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(1826-1831)apps/ui/src/hooks/use-settings-migration.ts (1)
syncSettingsToServer(198-251)
apps/server/src/services/agent-service.ts (2)
apps/server/src/lib/sdk-options.ts (1)
getModelForUseCase(218-252)apps/server/src/lib/settings-helpers.ts (1)
filterClaudeMdFromContext(98-137)
apps/ui/src/components/dialogs/file-browser-dialog.tsx (1)
apps/ui/src/lib/utils.ts (1)
isMac(60-65)
apps/server/src/services/auto-mode-service.ts (4)
apps/server/src/lib/settings-helpers.ts (1)
filterClaudeMdFromContext(98-137)apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(28-174)libs/types/src/model.ts (1)
DEFAULT_MODELS(47-52)libs/types/src/index.ts (1)
DEFAULT_MODELS(45-45)
libs/types/src/settings.ts (2)
libs/types/src/index.ts (2)
ModelProvider(85-85)AgentModel(51-51)libs/types/src/model.ts (1)
AgentModel(71-71)
apps/ui/src/lib/electron.ts (1)
libs/types/src/index.ts (2)
BacklogPlanResult(142-142)BacklogPlanEvent(143-143)
apps/server/src/providers/codex-provider.ts (2)
apps/server/src/providers/types.ts (5)
ProviderMessage(34-45)InstallationStatus(50-58)ModelDefinition(72-84)ContentBlock(21-29)ConversationMessage(10-10)apps/server/src/lib/subprocess-manager.ts (1)
spawnJSONLProcess(66-183)
🔇 Additional comments (31)
docs/server/providers.md (1)
234-234: The--sandbox <mode>documentation is correct and already includes an example.The
docs/server/utilities.mdfile already demonstrates a concrete sandbox mode value (workspace-write). The valid modes supported by the Codex provider are determined by the code and are:read-only,workspace-write, anddanger-full-access. The suggested modes (auto,manual,disabled) are not valid for this implementation.apps/ui/src/components/views/settings-view/config/navigation.ts (2)
13-13: LGTM!The
Wand2icon import is consistent with the existing icon imports and is appropriately used for the AI Provider navigation item.
27-27: Navigation item structure is correct and properly integrated.The navigation item follows the correct pattern: the
SettingsViewIdtype includes'ai-provider'as a valid value, and the correspondingAIProviderSectionview component is properly exported fromapps/ui/src/components/views/settings-view/ai-provider/. The icon import and positioning are also correct.apps/server/tests/unit/lib/subprocess-manager.test.ts (1)
1-99: LGTM!The test suite provides good coverage for
spawnJSONLProcess:
- Basic JSONL parsing functionality
- stderr activity preventing false idle timeouts
- Startup timeout vs idle timeout distinction
- Timeout enforcement when no activity occurs
Test structure is clean, uses appropriate timing values, and assertions match expected behavior from the implementation.
apps/server/src/lib/subprocess-manager.ts (1)
179-182: Process killed by external signal (not abort/timeout) may not throw.If the process is killed externally (e.g.,
SIGKILLfrom system),codemay benullwith a non-nullsignal. The conditioncode && code !== 0won't catch this sincenull && ...is falsy. Consider whether silent completion is intended for this edge case.🔎 Optional: Handle signal-based termination
- if (code && code !== 0) { + if (code !== 0) { const suffix = signal ? ` (signal: ${signal})` : ''; - throw new Error(stderr || `Process exited with code ${code}${suffix}`); + throw new Error(stderr || `Process exited with code ${code ?? 'null'}${suffix}`); }apps/ui/src/components/dialogs/file-browser-dialog.tsx (2)
235-235: LGTM - Restoring default focus behavior.Removing
focus-visible:outline-nonerestores the default focus indicators, which improves keyboard navigation accessibility.
369-372: LGTM - Simplified keyboard hint implementation.The inline
kbdelement with conditional platform rendering is a clean, straightforward solution. The implementation correctly displays ⌘ on Mac and Ctrl on other platforms.apps/server/src/providers/claude-provider.ts (1)
63-63: LGTM!The refactor to extract
settingSourcesinto a local variable is clean and reduces redundant property access while preserving identical behavior.Also applies to: 102-102
apps/server/src/services/auto-mode-service.ts (4)
1753-1759: Good centralization of default model logic.The new
getDefaultFeatureModel()helper consolidates model resolution across the service, making it easier to maintain and align with the multi-provider architecture introduced in this PR.
505-519: Well-designed model-aware context handling.Computing the model early (lines 505-506) and using it to conditionally filter Claude.md (lines 518-519) ensures that Claude-specific context is only removed when using Claude models. This correctly prevents filtering for the newly added providers (Cursor, OpenCode, Codex).
As per coding guidelines: correctly uses
resolveModelString()from @automaker/model-resolver.
665-667: Consistent model resolution pattern across execution paths.The same model-aware context filtering logic is correctly applied in
executePipelineSteps()andfollowUpFeature(), ensuring consistent behavior throughout the service.Also applies to: 889-900
2023-2023: Correct systemPrompt propagation with fallback.The updated logic properly forwards systemPrompt through the execution chain, with a sensible fallback pattern at line 2067 that prioritizes
sdkOptions.systemPromptwhile preserving any explicitly providedoptions?.systemPrompt.Also applies to: 2067-2067
apps/server/src/lib/sdk-options.ts (2)
319-354: Conditional Claude.md loading supports multi-provider architecture.The updated
buildClaudeMdOptions()now correctly checks model compatibility before enabling Claude.md auto-loading (lines 329-330), preventing Claude-specific settings from being applied to Cursor, OpenCode, or Codex providers. The simplified append logic (lines 348-350) properly handles custom system prompts.
263-268: Verify the undefined model fallback behavior inisClaudeModelKey().The function returns
truewhenmodelis undefined (lines 264-266), meaning if bothconfig.modelandconfig.sessionModelare undefined, Claude.md settings will be applied. This is a defensive fallback that assumes Claude as the default provider.While this may be intentional, it creates implicit behavior where non-Claude providers receive Claude settings when no model is explicitly specified. Consider documenting this assumption or requiring explicit model configuration to prevent unexpected behavior in integrations with other providers.
apps/server/src/services/agent-service.ts (1)
253-255: Consistent model-aware filtering applied in agent service.The agent service correctly adopts the same pattern as auto-mode-service: computing the effective model via
getModelForUseCase()(line 253) and conditionally filtering Claude.md only for Claude models (lines 254-255). This ensures consistent multi-provider behavior across both services.As per coding guidelines: uses model resolution helpers from @automaker/model-resolver.
apps/server/tests/unit/services/agent-service.test.ts (1)
126-128: Test correctly updated for reduced file reads.The test assertion now expects 2 file reads (session and metadata) instead of 3, with a clear comment explaining the behavior. This aligns with the updated session initialization logic.
apps/ui/src/types/electron.d.ts (2)
185-192: LGTM! New event variant follows existing pattern.The
auto_mode_tool_resultvariant is well-structured and consistent with other AutoModeEvent types. It properly includes the necessary context fields (featureId, projectId, projectPath) and the result/toolUseId payload.
985-985: LGTM! Provider expansion aligns with multi-CLI integration.The expansion of the
providerfield to include 'cursor', 'opencode', and 'codex' alongside 'claude' is consistent with the PR's objective of supporting multiple agent CLIs.libs/types/src/index.ts (1)
40-52: LGTM! Public API expansion follows existing patterns.The new model map exports (CURSOR_MODEL_MAP, OPENCODE_MODEL_MAP, CODEX_MODEL_MAP) and their corresponding alias types follow the same pattern as the existing CLAUDE_MODEL_MAP export. This provides a consistent public API surface for consumers.
apps/ui/src/lib/electron.ts (4)
16-17: LGTM! Previously missing imports are now present.The BacklogPlanResult and BacklogPlanEvent imports and re-exports address the past review comment. The types are now properly available for use in the backlogPlan API definitions.
Also applies to: 33-34
498-511: LGTM! Backlog plan API follows existing patterns.The backlogPlan API interface is well-structured with generate, stop, status, apply, and onEvent methods. The method signatures are consistent with other async operation APIs in the codebase (e.g., autoMode, specRegeneration).
543-591: LGTM! Provider status APIs support multi-CLI integration.The getCursorStatus, getOpenCodeStatus, and getCodexStatus methods, along with installCursor and verifyCursorAuth, provide a consistent interface for managing multiple AI provider CLIs. The structure mirrors the existing Claude status/setup flow, which promotes maintainability.
Also applies to: 633-645, 1176-1224, 1263-1281
1314-1359: LGTM! Mock implementations provide consistent development experience.The mock implementations for the new provider status and installation methods follow the same patterns as existing mocks, returning appropriate not_installed/not_authenticated states. This enables UI development without requiring actual CLI installations.
Also applies to: 1371-1378, 1430-1444
libs/types/src/settings.ts (1)
72-72: LGTM! Multi-provider settings support properly implemented.The changes successfully address the past review comment about missing defaultProvider and defaultModel fields:
- ModelProvider expansion (line 72) - Now supports 'claude', 'cursor', 'opencode', and 'codex'
- AIProfile.provider documentation (line 159) - Updated to reflect generic provider usage (no longer Claude-specific)
- GlobalSettings fields (lines 312-315) - defaultProvider and defaultModel properly added with clear documentation
- Default values (lines 526-527) - Reasonable defaults (claude/sonnet) that maintain backward compatibility
The implementation is consistent with the multi-CLI integration objective and provides proper type safety for the settings sync in use-settings-migration.ts.
Also applies to: 159-160, 312-315, 526-527
apps/ui/src/store/app-store.ts (3)
17-18: LGTM!Re-exporting types from
@automaker/typesfor convenience is appropriate and follows the coding guidelines to import from shared packages.
487-490: LGTM!The new state fields
defaultProvideranddefaultModelare well-typed and appropriately placed in theAppStateinterface.
780-783: LGTM!Action signatures are correctly typed and follow the established pattern in the store.
apps/ui/src/components/views/settings-view.tsx (4)
1-29: LGTM!Import statements correctly use path aliases for internal components and shared packages. The distinction between internal app imports (
@/and./) and cross-package shared imports (@automaker/*) is appropriate.
32-70: LGTM!Store hook usage correctly destructures the new provider and model state fields and their setters.
103-113: LGTM!The theme handling logic is correct. The updated comment accurately explains that
Themeis a strict subset ofThemeModeand the cast is a safe widening operation. The past review concern has been properly addressed.
140-165: Model alias resolution is already correctly implemented in AIProviderSection.The concern raised in this review comment is incorrect.
AIProviderSectionalready importsresolveModelStringfrom@automaker/model-resolverand resolves model aliases before passing the value toonModelChange. This is verified in multiple locations within the component:
- The default model is normalized:
const normalizedDefaultModel = resolveModelString(defaultModel);- When the provider changes, the selected model is resolved:
onModelChange(resolveModelString('auto') as AgentModel);- When a user selects from the dropdown, the selection is resolved:
onValueChange={(v: string) => onModelChange(resolveModelString(v) as AgentModel)}The resolution occurs at the component level before calling
onModelChange, ensuring thatsetDefaultModelin the store receives fully resolved model names. This is a valid and consistent pattern used throughout the codebase (agent-view.tsx, add-feature-dialog.tsx, model-selector.tsx, etc.). No changes are needed.Likely an incorrect or invalid review comment.
| const isWindows = os.platform() === 'win32'; | ||
| try { | ||
| const findCommand = isWindows ? 'where codex' : 'which codex'; | ||
| const { stdout } = await execAsync(findCommand); | ||
| const cliPath = stdout.trim().split(/\r?\n/)[0]; | ||
| if (cliPath) { | ||
| return cliPath; | ||
| } | ||
| } catch { | ||
| // Fall back to common locations | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Prefer execFile over shell execution for CLI detection.
The which/where commands are executed via shell using execAsync. While the command string is hardcoded, using execFile is safer and more explicit about not needing shell features.
🔎 Proposed refactor using execFile
+import { execFile } from 'child_process';
+const execFileAsync = promisify(execFile);
const isWindows = os.platform() === 'win32';
try {
- const findCommand = isWindows ? 'where codex' : 'which codex';
- const { stdout } = await execAsync(findCommand);
+ const findCommand = isWindows ? 'where' : 'which';
+ const { stdout } = await execFileAsync(findCommand, ['codex']);
const cliPath = stdout.trim().split(/\r?\n/)[0];
if (cliPath) {
return cliPath;
}
} catch {
// Fall back to common locations
}Committable suggestion skipped: line range outside the PR's diff.
| const isMac = | ||
| typeof navigator !== 'undefined' && | ||
| ( | ||
| (navigator as Navigator & { userAgentData?: { platform?: string } }).userAgentData | ||
| ?.platform ?? | ||
| navigator.platform ?? | ||
| '' | ||
| ).includes('Mac'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Import isMac from the existing utility module.
This code duplicates the platform detection logic already available in @/lib/utils.ts (lines 59-64). The existing utility is more comprehensive (checks process.platform first) and is likely used elsewhere in the codebase.
🔎 Proposed fix
+import { isMac } from '@/lib/utils';
+
export function FileBrowserDialog({
open,
onOpenChange,
onSelect,
title = 'Select Project Directory',
description = 'Navigate to your project folder or paste a path directly',
initialPath,
}: FileBrowserDialogProps) {
const [currentPath, setCurrentPath] = useState<string>('');
const [parentPath, setParentPath] = useState<string | null>(null);
const [directories, setDirectories] = useState<DirectoryEntry[]>([]);
const [drives, setDrives] = useState<string[]>([]);
const [loading, setLoading] = useState(false);
const [error, setError] = useState('');
const [warning, setWarning] = useState('');
const [recentFolders, setRecentFolders] = useState<string[]>([]);
- const isMac =
- typeof navigator !== 'undefined' &&
- (
- (navigator as Navigator & { userAgentData?: { platform?: string } }).userAgentData
- ?.platform ??
- navigator.platform ??
- ''
- ).includes('Mac');Based on the relevant code snippet from apps/ui/src/lib/utils.ts.
📝 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 isMac = | |
| typeof navigator !== 'undefined' && | |
| ( | |
| (navigator as Navigator & { userAgentData?: { platform?: string } }).userAgentData | |
| ?.platform ?? | |
| navigator.platform ?? | |
| '' | |
| ).includes('Mac'); | |
| const [recentFolders, setRecentFolders] = useState<string[]>([]); |
🤖 Prompt for AI Agents
In apps/ui/src/components/dialogs/file-browser-dialog.tsx around lines 79-86,
the file contains duplicated platform detection logic for isMac; replace this
block by importing the existing isMac utility from '@/lib/utils' and use that
value instead (add an import at the top of the file: import { isMac } from
'@/lib/utils'), remove the local navigator/platform detection code, and ensure
any TypeScript types remain correct and no other references to the removed local
variable remain.
| defaultProvider: state.defaultProvider, | ||
| defaultModel: state.defaultModel, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check server-side settings update handling and resolveModelString usage
echo "=== Looking for updateGlobal or settings endpoints on server ==="
git ls-files | grep -E "server|service" | grep -E "\.ts$"
echo ""
echo "=== Check if resolveModelString is used in the codebase ==="
rg "resolveModelString" --type=ts --type=tsx
echo ""
echo "=== Check DEFAULT_GLOBAL_SETTINGS and defaultModel usage ==="
rg "DEFAULT_GLOBAL_SETTINGS|defaultModel" --type=ts --type=tsx -B 2 -A 2
echo ""
echo "=== Check if GlobalSettings interface expects resolved or aliased models ==="
rg "interface GlobalSettings" --type=ts -A 15
echo ""
echo "=== Check updateSettings or sync implementation ==="
rg "updateGlobal|syncSettings" --type=ts -A 10 | head -100Repository: AutoMaker-Org/automaker
Length of output: 23852
🏁 Script executed:
#!/bin/bash
echo "=== Full GlobalSettings interface ==="
cat libs/types/src/settings.ts | grep -A 100 "export interface GlobalSettings"
echo ""
echo "=== Check where DEFAULT_GLOBAL_SETTINGS is defined ==="
rg "DEFAULT_GLOBAL_SETTINGS\s*=" --type=ts -A 30
echo ""
echo "=== Check update-global route implementation ==="
cat apps/server/src/routes/settings/routes/update-global.ts
echo ""
echo "=== Check settings-service updateGlobalSettings method ==="
cat apps/server/src/services/settings-service.ts | head -200Repository: AutoMaker-Org/automaker
Length of output: 11495
🏁 Script executed:
#!/bin/bash
echo "=== Check if @automaker/model-resolver exists ==="
find . -name "model-resolver" -o -name "model-resolver.ts"
echo ""
echo "=== Check AgentModel type definition ==="
rg "type AgentModel|interface AgentModel" --type=ts -A 5
echo ""
echo "=== Check DEFAULT_GLOBAL_SETTINGS initialization ==="
rg "export const DEFAULT_GLOBAL_SETTINGS|DEFAULT_GLOBAL_SETTINGS\s*=" --type=ts -B 2 -A 40
echo ""
echo "=== Check resolveModelString in package.json or exports ==="
find . -name "package.json" -type f -path "*/model-resolver/*" -o -name "index.ts" -path "*/model-resolver/*"
echo ""
echo "=== Look for model resolution logic anywhere ==="
rg "haiku|sonnet|opus" --type=ts | head -20Repository: AutoMaker-Org/automaker
Length of output: 6319
🏁 Script executed:
#!/bin/bash
echo "=== Check model-resolver exports ==="
cat libs/model-resolver/src/index.ts
echo ""
echo "=== Check use-settings-migration.ts for any model resolution ==="
cat apps/ui/src/hooks/use-settings-migration.ts
echo ""
echo "=== Check if resolveModelString is used elsewhere in settings context ==="
rg "resolveModelString" --type=ts -B 3 -A 3Repository: AutoMaker-Org/automaker
Length of output: 50379
Resolve model aliases before syncing to server.
The syncSettingsToServer() function in lines 226-227 sends defaultModel to the server without resolving aliases, violating the coding guideline: "Use resolveModelString() from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names."
While the type system (AgentModel = ModelAlias | ModelId) allows both aliases and full IDs, and other UI components correctly use resolveModelString() (e.g., ai-provider-section.tsx), the sync path should be consistent.
Import resolveModelString from @automaker/model-resolver and resolve the model values before sending in the updates object:
export async function syncSettingsToServer(): Promise<boolean> {
try {
const api = getHttpApiClient();
+ const { resolveModelString } = await import('@automaker/model-resolver');
const automakerStorage = getItem('automaker-storage');
if (!automakerStorage) {
return false;
}
const parsed = JSON.parse(automakerStorage);
const state = parsed.state || parsed;
// Extract settings to sync
const updates = {
// ... other fields ...
- defaultModel: state.defaultModel,
+ defaultModel: resolveModelString(state.defaultModel) as AgentModel,
- enhancementModel: state.enhancementModel,
+ enhancementModel: resolveModelString(state.enhancementModel) as AgentModel,
- validationModel: state.validationModel,
+ validationModel: resolveModelString(state.validationModel) as AgentModel,
// ... rest of updates ...
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/ui/src/hooks/use-settings-migration.ts around lines 226-227, the
syncSettingsToServer call is sending state.defaultModel without resolving
possible model aliases; import resolveModelString from @automaker/model-resolver
and use it to convert state.defaultModel to a full model string before including
it in the updates object (e.g., const resolved =
resolveModelString(state.defaultModel) and send defaultModel: resolved),
ensuring you handle the import at top and preserve types.
| // Default Provider actions | ||
| setDefaultProvider: (provider) => { | ||
| set({ defaultProvider: provider }); | ||
| // Sync with backend (fire and forget) | ||
| import('@/lib/http-api-client').then(({ getHttpApiClient }) => { | ||
| getHttpApiClient() | ||
| .setup.setDefaultProvider(provider) | ||
| .catch((err: Error) => { | ||
| console.error('[AppStore] Failed to sync provider with backend:', err); | ||
| }); | ||
| }); | ||
| import('@/hooks/use-settings-migration').then(({ syncSettingsToServer }) => { | ||
| syncSettingsToServer().catch((err) => { | ||
| console.error('[AppStore] Failed to persist provider settings:', err); | ||
| }); | ||
| }); | ||
| }, | ||
| setDefaultModel: (model) => { | ||
| set({ defaultModel: model }); | ||
| // Log the change (backend uses defaultModel based on defaultProvider) | ||
| console.log('[AppStore] Default model updated:', model); | ||
| import('@/hooks/use-settings-migration').then(({ syncSettingsToServer }) => { | ||
| syncSettingsToServer().catch((err) => { | ||
| console.error('[AppStore] Failed to persist model settings:', err); | ||
| }); | ||
| }); | ||
| }, | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking current implementation of setDefaultModel in app-store.ts ==="
sed -n '1643,1652p' apps/ui/src/store/app-store.ts
echo -e "\n=== Checking imports at top of app-store.ts ==="
head -20 apps/ui/src/store/app-store.ts
echo -e "\n=== Checking if resolveModelString is imported ==="
rg "resolveModelString" apps/ui/src/store/app-store.ts
echo -e "\n=== Checking all callsites of setDefaultModel ==="
rg "setDefaultModel\(" apps/ui/src/ -B 2 -A 2
echo -e "\n=== Checking AIProviderSection for model resolution ==="
rg -l "AIProviderSection\|settings-view" apps/ui/src/components/ | head -5Repository: AutoMaker-Org/automaker
Length of output: 1325
🏁 Script executed:
#!/bin/bash
echo "=== Check if resolveModelString is used in AIProviderSection ==="
find apps/ui/src/components -name "*ai-provider*" -o -name "*AIProvider*" | head -10
echo -e "\n=== Look for settings-view structure ==="
find apps/ui/src/components -type f -name "*.tsx" | grep -i settings | head -10Repository: AutoMaker-Org/automaker
Length of output: 1064
🏁 Script executed:
#!/bin/bash
echo "=== Examine the AIProviderSection component ==="
rg -A 20 "const AIProviderSection|export.*AIProviderSection" apps/ui/src/components/views/settings-view/
echo -e "\n=== Check how model is handled in AIProviderSection ==="
rg "resolveModelString|setDefaultModel|onModelChange" apps/ui/src/components/views/settings-view/ -B 3 -A 3Repository: AutoMaker-Org/automaker
Length of output: 12042
🏁 Script executed:
#!/bin/bash
echo "=== Get the full setDefaultModel implementation ==="
rg -A 15 "setDefaultModel: \(model\)" apps/ui/src/store/app-store.ts
echo -e "\n=== Check resolveModelString usage patterns elsewhere ==="
rg "resolveModelString" apps/ui/src/ -B 2 -A 2 | head -40Repository: AutoMaker-Org/automaker
Length of output: 5217
Resolve model aliases in setters to ensure consistency across all call sites.
The setDefaultModel and setValidationModel setters store models directly without resolving aliases, violating the coding guideline requiring resolveModelString() from @automaker/model-resolver for TypeScript files. While AIProviderSection resolves before calling setDefaultModel, other components like feature-defaults-section call setValidationModel without resolution, creating inconsistent data handling.
🔎 Proposed fix
Import the resolver at the top of the file:
import { create } from 'zustand';
import { persist } from 'zustand/middleware';
+import { resolveModelString } from '@automaker/model-resolver';
import type { Project, TrashedProject } from '@/lib/electron';Then resolve models in both setters:
setDefaultModel: (model) => {
+ const resolved = resolveModelString(model);
- set({ defaultModel: model });
+ set({ defaultModel: resolved });
// Log the change (backend uses defaultModel based on defaultProvider)
- console.log('[AppStore] Default model updated:', model);
+ console.log('[AppStore] Default model updated:', model, '→', resolved);
import('@/hooks/use-settings-migration').then(({ syncSettingsToServer }) => {
syncSettingsToServer().catch((err) => {
console.error('[AppStore] Failed to persist model settings:', err);
});
});
},
// Validation Model actions
setValidationModel: (model) => {
+ const resolved = resolveModelString(model);
- set({ validationModel: model })
+ set({ validationModel: resolved })
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/ui/src/store/app-store.ts around lines 1626 to 1653, the setters store
model strings directly which can retain unresolved aliases; import
resolveModelString from '@automaker/model-resolver' at the top of the file and
update setDefaultModel (and the analogous setValidationModel setter) to call
const resolved = resolveModelString(model) and then set({ defaultModel: resolved
}) (or set validationModel to resolved), use the resolved value for logging and
any backend sync calls, and leave the rest of the fire-and-forget persistence
logic unchanged.
| CODEX_CLI_TIMEOUT_MS=300000 | ||
| CODEX_CLI_STARTUP_TIMEOUT_MS=600000 |
There was a problem hiding this comment.
Document timeout defaults and guidance for Codex CLI timeouts.
The new CODEX_CLI_TIMEOUT_MS and CODEX_CLI_STARTUP_TIMEOUT_MS environment variables lack:
- Default values (if any exist)
- Recommended ranges
- When to adjust each one (e.g., idle time vs. slow network startup)
Consider adding a brief table or explanatory text clarifying the purpose and typical values for each timeout.
🔎 Suggested documentation addition
Add to the "Codex Provider" section or the environment variables section:
| Variable | Purpose | Typical Value | Default |
| --- | --- | --- | --- |
| `CODEX_CLI_TIMEOUT_MS` | Idle timeout (no output) | 300000 (5 min) | 30000 (30s) |
| `CODEX_CLI_STARTUP_TIMEOUT_MS` | Time to first output | 600000 (10 min) | 60000 (60s) |Or include prose explanation:
CODEX_CLI_TIMEOUT_MS: Maximum time (in ms) to wait for any output from the CLI before timing out. Default is 30 seconds. Increase this if the CLI processes large files slowly.CODEX_CLI_STARTUP_TIMEOUT_MS: Maximum time (in ms) to wait for the CLI to produce its first output. Default is 60 seconds. Increase this if the CLI startup is slow on your system.
🤖 Prompt for AI Agents
In docs/server/providers.md around lines 731 to 732, the two new env vars
CODEX_CLI_TIMEOUT_MS and CODEX_CLI_STARTUP_TIMEOUT_MS are listed without
defaults, guidance or examples; update the documentation to state each
variable's purpose, provide a sensible default value and a typical recommended
range, and give brief guidance on when to increase them (e.g., idle processing
vs slow startup or network); include either a small table showing Variable |
Purpose | Typical Value | Default or a short prose block: CODEX_CLI_TIMEOUT_MS —
idle/output timeout (default 30000 ms, typical 30000–300000 ms; raise for
long-running operations), CODEX_CLI_STARTUP_TIMEOUT_MS — time to first
output/startup (default 60000 ms, typical 60000–600000 ms; raise for slow
environment or heavy initialization).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/server/src/providers/codex-provider.ts (2)
940-948: Refactor: PreferexecFileover shell execution for CLI detection.The
which/wherecommands are executed via shell usingexecAsync. While the command string is hardcoded and not a security risk, usingexecFileis safer and more explicit about not needing shell features.🔎 Proposed refactor using execFile
+import { execFile } from 'child_process'; +const execFileAsync = promisify(execFile); const isWindows = os.platform() === 'win32'; try { - const findCommand = isWindows ? 'where codex' : 'which codex'; - const { stdout } = await execAsync(findCommand); + const findCommand = isWindows ? 'where' : 'which'; + const { stdout } = await execFileAsync(findCommand, ['codex']); const cliPath = stdout.trim().split(/\r?\n/)[0]; if (cliPath) { return cliPath; } } catch { // Fall back to common locations }
219-225: Critical: Command injection risk remains despite past review.Although marked as addressed in previous commits, line 221 still executes
cliPathvia shell using string interpolation:`"${cliPath}" --version`. IfcliPathoriginates from user input (viaconfig.cliPathorCODEX_CLI_PATHenvironment variable), this creates a command injection vulnerability.🔎 Recommended fix using execFile
+import { execFile } from 'child_process'; +const execFileAsync = promisify(execFile); async detectInstallation(): Promise<InstallationStatus> { const cliPath = await this.resolveCliPath(); // ... let version = ''; try { - const { stdout } = await execAsync(`"${cliPath}" --version`); + const { stdout } = await execFileAsync(cliPath, ['--version']); version = stdout.trim().split('\n')[0]; } catch { // Version command might not be available }
🧹 Nitpick comments (2)
apps/server/src/providers/codex-provider.ts (2)
52-52: Consider usingresolveModelString()for model alias resolution.The coding guidelines specify using
resolveModelString()from@automaker/model-resolverto convert model aliases. Currently, the code uses a localmapModelToCodexFormat()method which may not align with the shared model resolution system.As per coding guidelines, providers should use the shared model resolver. Please verify whether the Codex models are registered in
@automaker/model-resolverand can be resolved throughresolveModelString().
395-401: Consider documenting the model mapping strategy.The
modelMapcontains only one alias (codex→gpt-5.2-codex). While this is functional, consider documenting whether additional common aliases (e.g.,opus,sonnet) should be supported here or handled by the shared@automaker/model-resolver.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/server/src/providers/codex-provider.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:
apps/server/src/providers/codex-provider.ts
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/codex-provider.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/src/providers/codex-provider.ts
🔇 Additional comments (9)
apps/server/src/providers/codex-provider.ts (9)
1-33: LGTM: Provider setup follows standard patterns.The imports and class initialization are appropriate for a CLI-based provider implementation.
120-149: LGTM: Thethisbinding issue is correctly resolved.The captured
providerreference (line 120) properly preserves the class context for method calls within therunWithArgsgenerator function, addressing the critical issue from previous reviews.
151-182: LGTM: Retry logic for--ask-for-approvalis well-implemented.The graceful fallback when the flag is unsupported, with proper retry prevention via
didRetry, demonstrates good defensive programming.
242-354: LGTM: Model definitions are comprehensive and well-structured.The catalog includes appropriate metadata for all Codex models with clear tier differentiation and capability flags.
424-826: LGTM: Event handling is robust and comprehensive.The
toProviderMessagesimplementation includes extensive fallback logic and type guards to handle the diverse JSONL event formats from the Codex CLI, demonstrating good defensive programming.
828-850: LGTM: Sandbox mode resolution is secure by default.The logic correctly defaults to requiring write access when
allowedToolsis undefined (line 842), which is a safe conservative choice. The three-tier sandbox model (read-only, workspace-write, danger-full-access) provides appropriate security boundaries.
880-904: LGTM: Authentication check is comprehensive.The implementation correctly checks multiple API key sources and the local
auth.jsonfile, providing complete authentication status information.
906-930: LGTM: Timeout configuration is properly validated.The environment variable parsing includes appropriate validation for edge cases (non-finite, negative values) with sensible defaults.
965-982: LGTM: Common installation paths are comprehensive.The platform-specific path lists cover standard installation locations for both Windows and Unix-like systems.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 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 (7)
apps/server/package.jsonapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/services/auto-mode-service.tsapps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.tsxlibs/prompts/src/defaults.tslibs/prompts/src/index.tslibs/types/src/pipeline.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/routes/app-spec/generate-spec.ts
- apps/server/src/services/auto-mode-service.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:
libs/prompts/src/index.tslibs/types/src/pipeline.tslibs/prompts/src/defaults.tsapps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.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/src/index.tslibs/types/src/pipeline.tslibs/prompts/src/defaults.tsapps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.tsx
🧠 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: Packages can only depend on packages above them in the dependency chain: types → (utils|prompts|platform|model-resolver|dependency-resolver) → git-utils → (server|ui)
Applied to files:
apps/server/package.json
📚 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/board-view/dialogs/pipeline-settings-dialog.tsx
🧬 Code graph analysis (1)
libs/prompts/src/defaults.ts (1)
libs/prompts/src/index.ts (1)
DEFAULT_SUBTASK_GENERATION_PROMPT(36-36)
🔇 Additional comments (8)
libs/types/src/pipeline.ts (1)
11-11: LGTM!The optional
enableSubtaskGenerationproperty is a clean addition to thePipelineStepinterface. The property name is descriptive and follows TypeScript conventions.libs/prompts/src/index.ts (1)
36-36: LGTM!The re-export of
DEFAULT_SUBTASK_GENERATION_PROMPTproperly exposes the new subtask generation prompt to consumers of the@automaker/promptspackage.apps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.tsx (5)
243-243: LGTM!The
enableSubtaskGenerationproperty is properly added to theEditingStepinterface and aligns with the type definition in@automaker/types.
285-286: LGTM!The initialization of
enableSubtaskGeneration: falsefor new steps provides a sensible default behavior.
296-297: LGTM!Properly copies the
enableSubtaskGenerationflag from the existing step, with a safe fallback tofalse.
718-737: LGTM!The checkbox UI for enabling subtask generation is well-implemented with:
- Clear labeling and help text
- Proper state binding to
editingStep.enableSubtaskGeneration- Accessible checkbox with appropriate styling
391-391: LGTM!The
enableSubtaskGenerationflag is correctly propagated through all save paths:
- Creating new steps (Line 391, 461)
- Updating existing steps (Line 430)
- Handling missing steps (Line 445)
This ensures the feature works consistently across all edit scenarios.
Also applies to: 430-430, 445-445, 461-461
libs/prompts/src/defaults.ts (1)
242-265: No duplicate declarations found.The search confirms only a single declaration of
DEFAULT_SUBTASK_GENERATION_PROMPTexists at line 242 in libs/prompts/src/defaults.ts. There is no duplicate, and the code will compile without redeclaration errors.Likely an incorrect or invalid review comment.
| "@automaker/model-resolver": "1.0.0", | ||
| "@automaker/platform": "1.0.0", | ||
| "@automaker/prompts": "1.0.0", | ||
| "@automaker/prompts": "^1.0.0", |
There was a problem hiding this comment.
Inconsistent dependency versioning for internal monorepo package.
All other @automaker/* dependencies use exact version pins ("1.0.0"), but @automaker/prompts now uses a caret range ("^1.0.0"). In a monorepo with internal packages, exact version pins (or workspace protocol) are recommended to:
- Keep all packages synchronized
- Prevent version drift between packages
- Avoid unexpected behavior from automatic minor/patch updates
🔎 Proposed fix to align with other internal dependencies
- "@automaker/prompts": "^1.0.0",
+ "@automaker/prompts": "1.0.0",📝 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.
| "@automaker/prompts": "^1.0.0", | |
| "@automaker/prompts": "1.0.0", |
🤖 Prompt for AI Agents
In apps/server/package.json around line 32, the @automaker/prompts dependency
uses a caret range "^1.0.0" which is inconsistent with other internal
@automaker/* packages that use exact pins; update the dependency to the exact
version "1.0.0" (or switch to the workspace: protocol if the repo uses workspace
protocol everywhere) so it matches the rest of the monorepo and prevents version
drift.
| name: editingStep.name, | ||
| instructions: editingStep.instructions, | ||
| colorClass: editingStep.colorClass, | ||
| colorClass: editingStep.colorClass, |
There was a problem hiding this comment.
Remove duplicate colorClass assignment.
Line 428 duplicates the colorClass property that already exists on Line 427. This creates a redundant assignment where the second value silently overwrites the first.
🔎 Proposed fix
colorClass: editingStep.colorClass,
- colorClass: editingStep.colorClass,
updatedAt: now,Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 427-428: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
In apps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.tsx
around line 428, there is a duplicate colorClass property assignment (it is
already set on the previous line); remove the redundant second colorClass entry
so the object only defines colorClass once, leaving the original intended value
intact.
| ) | ||
| } | ||
| rows={6} | ||
| rows={6} |
There was a problem hiding this comment.
Remove duplicate rows attribute.
The rows={6} attribute is specified twice on the same Textarea component. React will use the last value, but this redundancy should be removed for code clarity.
🔎 Proposed fix
}
rows={6}
- rows={6}
className="font-mono text-sm"📝 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.
| rows={6} | |
| } | |
| rows={6} | |
| className="font-mono text-sm" |
🧰 Tools
🪛 Biome (2.1.2)
[error] 712-713: This JSX property is assigned multiple times.
This attribute is assigned again here.
(lint/suspicious/noDuplicateJsxProps)
🤖 Prompt for AI Agents
In apps/ui/src/components/views/board-view/dialogs/pipeline-settings-dialog.tsx
around line 713 the Textarea component has a duplicate rows={6} attribute;
remove the redundant rows prop so only a single rows={6} remains on that
Textarea to eliminate the duplicate and keep behavior unchanged.
|
closing, too many conflicts, talk to core contributors on discord if you want to help |
3 participants
Integrated support for multiple agent CLIs.
What changed
The setup wizard now detects the new agent for Cursor as well (and others) by checking common install paths.
Cursor provider: constrained to
automode by default since it’s free. Additional models can be enabled later in the Cursor provider.cursor autoOpenCode provider: added because it supports many free models, including GLM 4.6 and GLM 4.7. I set GLM 4.7 as the default since it’s free and performs well (roughly comparable to Opus 4.5 per community feedback). Note: it can be a bit slow, but still a solid free option.
glm4.7Codex provider: added with full OpenAI model support. The implementation is largely aligned with the existing OpenAI path; the main motivation was enabling Codex 5.2.
Why this is useful
With Cursor/OpenCode/Codex coverage, this unlocks cheaper workflows. As a user, my priority was to run Automaker without relying on Claude Code, which is expensive.
Also included improved context management based on the examples from Agent-Skills-for-Context-Engineering (linked above).
What changed
Integrated the context-engineering workflow into the system to produce a compressed context representation.
Added a critique/review pass to improve feedback quality during development (stronger “critic” behavior when iterating).
Reference
https://github.com/muratcankoylan/Agent-Skills-for-Context-Engineering/tree/main/examples
Follow-up idea
Claude Code can still be valuable for small tasks. It should be straightforward to route lightweight operations (title renaming, summaries, spec generation, etc.) to a cheaper/faster CLI while keeping the main agent configurable.
Summary by CodeRabbit
New Features
Improvements
New Tools
✏️ Tip: You can customize this high-level summary in your review settings.