Codex CLI Integration#356
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 full Codex (OpenAI) support across server, libs, and UI: new Codex provider (CLI+SDK), SDK client, tool-mapping, config manager, usage service, routes and setup handlers, extensive UI components and store fields, type library expansions, tests, and docs. Also forwards OpenAI SDK dependency and minor Prettier/ESLint tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant ProviderFactory
participant CodexProvider
participant CodexCLI
participant CodexSDK
participant OpenAI
Client->>ProviderFactory: executeQuery(model, prompt, options)
ProviderFactory->>CodexProvider: select provider for model (gpt-/o-series)
alt Tools requested OR no API key
CodexProvider->>CodexCLI: spawn subprocess, send prompt
CodexCLI-->>CodexProvider: stream JSONL events (tool_use, tool_result, completed)
else No tools + API key available
CodexProvider->>CodexSDK: executeCodexSdkQuery(prompt, options)
CodexSDK->>OpenAI: HTTP request
OpenAI-->>CodexSDK: response (text, optional schema)
end
CodexProvider->>Client: yield ProviderMessage stream (assistant/tool events/result)
sequenceDiagram
autonumber
participant User
participant SetupUI
participant CliSetupStep
participant ElectronAPI
participant CodexCLI
participant VerifyHandler
User->>SetupUI: open Codex setup
SetupUI->>CliSetupStep: render config & state
User->>CliSetupStep: click Install
CliSetupStep->>ElectronAPI: installCodex()
ElectronAPI->>CodexCLI: perform install (npm)
CodexCLI-->>ElectronAPI: progress events
ElectronAPI-->>CliSetupStep: progress updates
User->>CliSetupStep: verify auth
CliSetupStep->>VerifyHandler: verifyCodexAuth('cli'/'api_key')
VerifyHandler->>CodexCLI: handshake / rateLimits query
CodexCLI-->>VerifyHandler: usage/rateLimits
VerifyHandler-->>CliSetupStep: authenticated: true/false (with details)
CliSetupStep->>SetupUI: onNext() when ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
DhanushSantosh
requested review from
SuperComboGamer and
webdevcody
and removed request for
SuperComboGamer
Summary of ChangesHello @DhanushSantosh, 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 introduces OpenAI's Codex (GPT-5 series) as a new, first-class AI provider within Automaker. The integration provides full feature parity with the existing Claude provider, while also leveraging Codex's unique capabilities through a hybrid execution model that intelligently uses either the Codex CLI for complex, tool-driven tasks or the OpenAI SDK for faster, simpler queries. The changes encompass comprehensive authentication, configurable agent behavior, robust sandbox policies, and detailed usage tracking, ensuring a unified and powerful experience for users. 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 is an impressive and extensive pull request that successfully integrates the Codex provider, achieving feature parity with the existing Claude provider. The introduction of a hybrid CLI/SDK execution model is a smart optimization. The refactoring of many components to be provider-agnostic is a significant improvement for future maintainability. The new setup and settings UI for Codex are comprehensive and well-designed. I have two main points of feedback. One is a potentially high-risk change in a shared subprocess utility that could have unintended side effects on other CLI integrations. The other is a minor discrepancy in a dependency version that needs clarification. Overall, this is a very strong contribution.
| // Handle stderr output even when exit code is 0 | ||
| // Some CLIs (like Codex) exit with 0 but write auth errors to stderr | ||
| if (exitCode === 0 && stderrOutput) { | ||
| console.error(`[SubprocessManager] Process exited with code 0 but has stderr: ${stderrOutput}`); | ||
| yield { | ||
| type: 'error', | ||
| error: stderrOutput, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This change treats any stderr output as an error if the exit code is 0. While this might be necessary for the Codex CLI, spawnJSONLProcess is a generic utility. This change could unintentionally break other CLI integrations (like the Claude CLI) if they write warnings or other non-error information to stderr while still exiting successfully. This could lead to valid operations being reported as errors.
I'd suggest making this behavior specific to the Codex provider call site, or adding a parameter to spawnJSONLProcess to enable this strict stderr checking, rather than applying it globally.
| "express": "5.2.1", | ||
| "morgan": "1.10.1", | ||
| "node-pty": "1.1.0-beta41", | ||
| "openai": "6.15.0", |
There was a problem hiding this comment.
There's a discrepancy in the openai package version. The PR description states version ^4.77.3 is used, but this change introduces version 6.15.0. Could you please clarify which version is correct? Using an unexpected major version could introduce breaking changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
libs/platform/src/subprocess.ts (1)
132-155: Consider the impact of treating all stderr output as errors.This change treats any stderr output as an error condition, even when the exit code is 0. While this addresses Codex CLI auth errors, it may cause false positives for legitimate CLI tools that write warnings, progress info, or diagnostics to stderr without indicating failure.
Consider one of these alternatives:
- Make this behavior opt-in via a flag in
SubprocessOptions(e.g.,treatStderrAsError?: boolean)- Check for specific error patterns in stderr rather than any content
- Document this behavior clearly so callers understand the contract
🔎 Proposed fix: Add opt-in flag for stderr-as-error behavior
export interface SubprocessOptions { command: string; args: string[]; cwd: string; env?: Record<string, string>; abortController?: AbortController; timeout?: number; // Milliseconds of no output before timeout + treatStderrAsError?: boolean; // If true, non-empty stderr with exit code 0 yields an error }Then in the handler:
- // Handle stderr output even when exit code is 0 - // Some CLIs (like Codex) exit with 0 but write auth errors to stderr - if (exitCode === 0 && stderrOutput) { + // Handle stderr output even when exit code is 0 (opt-in behavior) + // Some CLIs (like Codex) exit with 0 but write auth errors to stderr + if (options.treatStderrAsError && exitCode === 0 && stderrOutput) { console.error(`[SubprocessManager] Process exited with code 0 but has stderr: ${stderrOutput}`); yield { type: 'error', error: stderrOutput, }; }apps/server/src/routes/context/routes/describe-file.ts (1)
180-188: UseresolveModelString()for model alias resolution.The code directly uses
CLAUDE_MODEL_MAP.haikuinstead ofresolveModelString('haiku'). Per the coding guidelines, model aliases should be resolved through the centralized resolver to ensure consistent model resolution across all providers.🔎 Proposed fix
-import { CLAUDE_MODEL_MAP } from '@automaker/types'; +import { resolveModelString } from '@automaker/model-resolver'; ... const sdkOptions = createCustomOptions({ cwd, - model: CLAUDE_MODEL_MAP.haiku, + model: resolveModelString('haiku'), maxTurns: 1, allowedTools: [], autoLoadClaudeMd,Based on coding guidelines: Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names.apps/server/src/routes/context/routes/describe-image.ts (2)
142-159: Error messages still reference Claude despite provider-agnostic refactor.The error mapping function retains Claude-specific error checks and messages (e.g., "Claude Code process exited", "Claude CLI could not be launched"). For provider parity, these should be generalized or include Codex equivalents.
🔎 Proposed fix to generalize error messages
if (rawMessage.includes('Claude Code process exited')) { return { statusCode: 503, userMessage: - 'Claude exited unexpectedly while describing the image. Try again. If it keeps happening, re-run `claude login` or update your API key in Setup so Claude can restart cleanly.', + 'The AI provider exited unexpectedly while describing the image. Try again. If it keeps happening, verify your authentication or update your API key in Setup.', }; } if ( rawMessage.includes('Failed to spawn Claude Code process') || - rawMessage.includes('Claude Code executable not found') || - rawMessage.includes('Claude Code native binary not found') + rawMessage.includes('Claude Code executable not found') || + rawMessage.includes('Claude Code native binary not found') || + rawMessage.includes('codex') // Add Codex CLI error patterns ) { return { statusCode: 503, userMessage: - 'Claude CLI could not be launched. Make sure the Claude CLI is installed and available in PATH, then try again.', + 'The AI CLI could not be launched. Make sure the required CLI is installed and available in PATH, then try again.', }; }
342-365: UseresolveModelString()from @automaker/model-resolver instead of directCLAUDE_MODEL_MAPaccess.Line 344 directly uses
CLAUDE_MODEL_MAP.haiku, but per coding guidelines, should callresolveModelString('haiku')to convert model aliases. This pattern is used consistently across the codebase (e.g.,enhance.ts,auto-mode-service.ts). While hardcoding Claude is intentional for image vision (Codex has limited vision support), the model should still be resolved through the dedicated utility.apps/ui/src/types/electron.d.ts (1)
1000-1008: Use consistent provider value for Codex models: change'openai'to'codex'in codex-provider.ts.The
getAvailableModels()method incodex-provider.tsreturns models withprovider: 'openai', but the same models are defined inmodel-display.tswithprovider: 'codex'. Since theModelDefinition.providertype includes both values, this inconsistency can cause type confusion and runtime mismatches. Updatecodex-provider.tsto useprovider: 'codex'to match the UI layer and clarify that these models are accessed through the Codex platform.
🧹 Nitpick comments (35)
apps/server/tests/unit/lib/validation-storage.test.ts (1)
181-187: Test boundary logic is correct.The updated test accurately validates the just-under-24-hour boundary condition. The date calculation using
setHourswith negative offsets works correctly because JavaScript Date handles negative values by rolling back days/hours appropriately.Optional: Consider millisecond arithmetic for clearer intent
The current approach is functionally correct but may be non-obvious to readers unfamiliar with JavaScript's Date handling of negative parameters. For improved clarity:
- const almostStaleDate = new Date(); - // 23.99 hours ago to avoid timing issues with test execution - almostStaleDate.setHours(almostStaleDate.getHours() - 23, almostStaleDate.getMinutes() - 59); + // 23 hours 59 minutes ago to avoid timing issues with test execution + const almostStaleDate = new Date(Date.now() - (23 * 60 * 60 * 1000 + 59 * 60 * 1000));This makes the time offset calculation more explicit and easier to verify at a glance.
libs/platform/tests/subprocess.test.ts (1)
12-18: Consider extracting the shared helper to avoid duplication.The
collectAsyncGeneratorhelper function is duplicated here and inapps/server/tests/utils/helpers.ts. Consider importing from a shared test utilities location or moving to a common test helper package.-/** - * Helper to collect all items from an async generator - */ -async function collectAsyncGenerator<T>(generator: AsyncGenerator<T>): Promise<T[]> { - const results: T[] = []; - for await (const item of generator) { - results.push(item); - } - return results; -} +// Consider importing from a shared test utilities package +// import { collectAsyncGenerator } from '@automaker/test-utils';apps/server/src/routes/setup/routes/auth-codex.ts (2)
13-18: Consider the semantics ofsuccess: truefor manual auth prompts.The handler returns
success: trueeven though it's only providing instructions for manual authentication, not actually authenticating. While this design may be intentional to indicate "successfully provided instructions," it could be semantically clearer.If this pattern is consistent with the existing Claude authentication flow (e.g.,
auth-claude.ts), then keep it for parity. Otherwise, consider whether a different response structure might be clearer (e.g.,instructionsProvided: trueinstead ofsuccess: true).
8-8: ExtractCODEX_LOGIN_COMMANDto a shared constants location.The constant is duplicated in at least two files:
apps/server/src/routes/setup/routes/auth-codex.tsandapps/ui/src/components/views/settings-view/codex/codex-usage-section.tsx. Additionally, the hardcoded string'codex login'appears in 10+ other files across the codebase. Following the UI pattern (which uses constants.ts files per feature), create a centralized constants file inapps/server/src/routes/setup/to consolidate Codex-related command constants.libs/platform/tests/system-paths.test.ts (1)
39-57: Consider adding edge cases for auth indicator tests.The current tests cover the happy paths for OAuth tokens and API keys. Consider adding test cases for:
- Empty or malformed auth.json content (e.g., invalid JSON, missing expected fields)
- Non-existent auth.json file (verify graceful handling)
- auth.json with both OAuth token and API key (verify precedence/multi-indicator behavior)
- Permissions errors when reading auth.json
📋 Suggested additional test cases
it('handles missing auth.json gracefully', async () => { const indicators = await getCodexAuthIndicators(); expect(indicators.hasAuthFile).toBe(false); expect(indicators.hasOAuthToken).toBe(false); expect(indicators.hasApiKey).toBe(false); }); it('handles malformed auth.json gracefully', async () => { const authPath = path.join(getCodexConfigDir(), AUTH_FILENAME); await fs.writeFile(authPath, '{ invalid json', 'utf-8'); const indicators = await getCodexAuthIndicators(); expect(indicators.hasAuthFile).toBe(true); expect(indicators.hasOAuthToken).toBe(false); expect(indicators.hasApiKey).toBe(false); }); it('detects both OAuth token and API key when present', async () => { const authPath = path.join(getCodexConfigDir(), AUTH_FILENAME); await fs.writeFile( authPath, JSON.stringify({ tokens: { access_token: 'token' }, [OPENAI_KEY_NAME]: 'sk-test', }), 'utf-8' ); const indicators = await getCodexAuthIndicators(); expect(indicators.hasAuthFile).toBe(true); expect(indicators.hasOAuthToken).toBe(true); expect(indicators.hasApiKey).toBe(true); });apps/server/tests/unit/providers/provider-factory.test.ts (1)
241-246: Consider testing specific Codex model IDs from CODEX_MODEL_MAP.While the current test verifies that gpt- prefixed models exist, it doesn't validate specific expected model IDs (e.g., gpt-5.2, gpt-5.1-codex, gpt-5.1-codex-max). Consider adding assertions for specific model IDs to catch accidental model list changes.
🔎 Suggested enhancement
it('should include Codex models', () => { const models = ProviderFactory.getAllAvailableModels(); const hasCodexModels = models.some((m) => m.id.toLowerCase().startsWith('gpt-')); expect(hasCodexModels).toBe(true); + + // Verify specific expected Codex models + const codexModelIds = models.map((m) => m.id.toLowerCase()).filter((id) => id.startsWith('gpt-')); + expect(codexModelIds).toContain('gpt-5.2'); + expect(codexModelIds).toContain('gpt-5.1-codex'); });docs/codex-parity-todo.md (1)
1-10: Documentation checklist looks complete.All parity items are marked as complete, which aligns with the comprehensive Codex integration observed across the codebase. Consider:
- Adding references to specific implementation files or tests that verify each checklist item
- Including a "Verified by" column with test file references
- Moving this to a permanent doc location or archiving if it's a temporary tracking doc
Example enhancement:
## Codex Parity Checklist This tracks parity between Codex and Claude provider behavior. | Item | Status | Verified By | |------|--------|-------------| | Tool stream parity | ✅ | `apps/server/tests/unit/providers/codex-provider.test.ts` | | Per-request options parity | ✅ | `apps/server/tests/unit/providers/codex-provider.test.ts` | | Instruction loading + sandbox safety | ✅ | `apps/server/src/providers/codex-provider.ts:L150-200` | | Auth verification parity | ✅ | `apps/server/tests/unit/routes/setup/verify-codex-auth.test.ts` | | MCP permission logic parity | ✅ | `apps/server/src/providers/codex-provider.ts:L300-350` | | Hybrid execution | ✅ | `apps/server/src/providers/codex-provider.ts:L400-450` |apps/server/tests/unit/providers/codex-tool-mapping.test.ts (1)
73-86: Consider expanding todo parsing test coverage.The current test covers the basic happy path with markdown-style todos. Consider adding tests for:
- Todo items from
todosarray field (mentioned in implementation)- Todo items from
itemsarray field (mentioned in implementation)- Edge cases: empty content, malformed markdown, missing status indicators
- Mixed status types in a single input
📋 Suggested additional test cases
it('parses todos from todos array field', () => { const todoInput = { todos: [ { content: 'First task', status: 'pending' }, { content: 'Second task', status: 'completed' }, ], }; const todos = extractCodexTodoItems(todoInput); expect(todos).toHaveLength(2); expect(todos[0]).toMatchObject({ content: 'First task', status: 'pending' }); }); it('parses todos from items array field', () => { const todoInput = { items: [ { content: 'Task from items', status: 'in_progress' }, ], }; const todos = extractCodexTodoItems(todoInput); expect(todos).toHaveLength(1); expect(todos[0]).toMatchObject({ content: 'Task from items', status: 'in_progress' }); }); it('returns null for empty or missing todo content', () => { expect(extractCodexTodoItems({})).toBeNull(); expect(extractCodexTodoItems({ content: '' })).toBeNull(); expect(extractCodexTodoItems({ text: '' })).toBeNull(); });apps/server/src/providers/codex-config-manager.ts (1)
29-31: Manual quote escaping may miss edge cases.The
formatTomlKeyfunction escapes double quotes but doesn't handle backslashes that could appear in server names. If a key contains a backslash, the TOML output may be malformed.Proposed fix for backslash handling
function formatTomlKey(key: string): string { - return `"${key.replace(/"/g, '\\"')}"`; + return `"${key.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`; }apps/ui/src/components/usage-popover.tsx (3)
120-126: Stale checkuseMemohas no reactive dependency on time.The
claudeIsStaleandcodexIsStalememos depend onDate.now()but only re-evaluate whenclaudeUsageLastUpdatedorcodexUsageLastUpdatedchange. This means staleness won't update automatically as time passes while the popover is open, only when the timestamps change.Consider adding a periodic re-evaluation or relying on the interval-based refresh logic alone rather than the
useMemostaleness checks, since the intervals already handle periodic updates.
270-277: Inner componentsProgressBarandUsageCardare recreated on every render.Defining components inside the parent function causes them to be recreated on each render, which can hurt performance and break memoization. Extract these to module-level or use
useMemo/useCallbackif they need closure over props.Extract inner components to module level
+const ProgressBar = ({ percentage, colorClass }: { percentage: number; colorClass: string }) => ( + <div className="h-2 w-full bg-secondary/50 rounded-full overflow-hidden"> + <div + className={cn('h-full transition-all duration-500', colorClass)} + style={{ width: `${Math.min(percentage, 100)}%` }} + /> + </div> +); + +interface UsageCardProps { + title: string; + subtitle: string; + percentage: number; + resetText?: string; + isPrimary?: boolean; + stale?: boolean; + getStatusInfo: (percentage: number) => { color: string; icon: React.ComponentType<any>; bg: string }; +} + +const UsageCard = ({ title, subtitle, percentage, resetText, isPrimary = false, stale = false, getStatusInfo }: UsageCardProps) => { + // ... component body +}; export function UsagePopover({ showClaude, showCodex }: UsagePopoverProps) { // ... remove inner component definitionsAlso applies to: 279-345
157-190: Async fetch functions don't handle component unmount.If the component unmounts while
fetchClaudeUsageorfetchCodexUsageis in progress, thesetClaudeError/setCodexError/setClaudeUsage/setCodexUsagecalls will attempt to update unmounted component state. Consider using an abort signal or a mounted ref.Add unmount guard pattern
export function UsagePopover({ showClaude, showCodex }: UsagePopoverProps) { + const mountedRef = useRef(true); + + useEffect(() => { + return () => { + mountedRef.current = false; + }; + }, []); + const fetchClaudeUsage = useCallback( async (isAutoRefresh = false) => { if (!showClaude) return; if (!isAutoRefresh) setClaudeLoading(true); setClaudeError(null); try { // ... existing code ... + if (!mountedRef.current) return; setClaudeUsage(data); } catch (err) { + if (!mountedRef.current) return; setClaudeError({ // ... }); } finally { + if (!mountedRef.current) return; if (!isAutoRefresh) setClaudeLoading(false); } }, [setClaudeUsage, showClaude] );Also applies to: 192-225
apps/server/src/services/codex-usage-service.ts (3)
42-49:isAvailable()conflates "command exists" with "spawn succeeded".The
which/wherespawn can fail for reasons other than the command not existing (e.g., insufficient permissions, PATH issues). Consider handling specific error cases or using thefindCodexCliPathutility that's already imported.Simpler approach using findCodexCliPath
async isAvailable(): Promise<boolean> { - return new Promise((resolve) => { - const checkCommand = process.platform === 'win32' ? 'where' : 'which'; - const checkProcess = spawn(checkCommand, [CODEX_COMMAND]); - checkProcess.on('close', (code) => resolve(code === EXIT_SUCCESS_CODE)); - checkProcess.on('error', () => resolve(false)); - }); + const path = await findCodexCliPath(); + return path !== null; }
18-19: HardcodedCLIENT_VERSIONmay drift from actual package version.Consider importing the version from
package.jsonor a shared constant to keep it synchronized with the actual application version.
113-120: Consider differentiating successful exit with no response from error exit.When the process exits with code 0 but no response was received (line 119), it's treated the same as receiving no response. This might mask scenarios where the server started but didn't respond to the rate limits request. A more specific error message could help debugging.
More specific error for exit-without-response
serverProcess.on('exit', (code) => { if (settled) return; if (code !== null && code !== EXIT_SUCCESS_CODE) { finish(new Error(stderrOutput || ERROR_COMMAND_FAILED)); return; } - finish(new Error(ERROR_NO_RESPONSE)); + finish(new Error(`Codex server exited (code ${code}) without providing rate limits`)); });apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (2)
34-67: Consider using typed parameters instead ofany.Both
mapClaudeAuthStatusandmapCodexAuthStatusacceptauth: any, which loses type safety. Consider defining explicit input types or using a more constrained type.🔎 Suggested approach
+interface ClaudeAuthInput { + authenticated: boolean; + method?: string; + hasCredentialsFile?: boolean; + oauthTokenValid?: boolean; + hasStoredOAuthToken?: boolean; + hasEnvOAuthToken?: boolean; + apiKeyValid?: boolean; + hasStoredApiKey?: boolean; + hasEnvApiKey?: boolean; +} -function mapClaudeAuthStatus(auth: any): ClaudeAuthStatus { +function mapClaudeAuthStatus(auth: ClaudeAuthInput): ClaudeAuthStatus {Similar typing can be applied to
mapCodexAuthStatus.
145-179: Redundant try/catch in refresh handlers.
checkCliStatusalready catches and logs errors internally (lines 113-114), so the outer try/catch inhandleRefreshClaudeCliandhandleRefreshCodexCliwill never trigger. Consider removing the redundant wrapping.🔎 Simplified handlers
const handleRefreshClaudeCli = useCallback(async () => { - try { - const api = getElectronAPI(); - await checkCliStatus({ - label: 'Claude CLI', - checkCli: api?.checkClaudeCli, - getStatus: api?.setup?.getClaudeStatus, - setCliStatus: setClaudeCliStatus, - setAuthStatus: setClaudeAuthStatus, - setIsChecking: setIsCheckingClaudeCli, - mapAuthStatus: mapClaudeAuthStatus, - }); - } catch (error) { - console.error('Failed to refresh Claude CLI status:', error); - } + const api = getElectronAPI(); + await checkCliStatus({ + label: 'Claude CLI', + checkCli: api?.checkClaudeCli, + getStatus: api?.setup?.getClaudeStatus, + setCliStatus: setClaudeCliStatus, + setAuthStatus: setClaudeAuthStatus, + setIsChecking: setIsCheckingClaudeCli, + mapAuthStatus: mapClaudeAuthStatus, + }); }, [checkCliStatus, setClaudeAuthStatus]);apps/server/src/routes/enhance-prompt/routes/enhance.ts (1)
64-80: Handle potential edge cases in stream extraction.The
extractTextFromStreamfunction handlesassistantandresultmessage types, but doesn't explicitly handle error types or other message types that the provider might emit. Consider adding explicit handling for error messages to provide better diagnostics.🔎 Add error handling
async function extractTextFromStream(stream: AsyncIterable<ProviderMessage>): Promise<string> { let responseText = ''; for await (const msg of stream) { if (msg.type === 'assistant' && msg.message?.content) { for (const block of msg.message.content) { if (block.type === 'text' && block.text) { responseText += block.text; } } } else if (msg.type === 'result' && msg.subtype === 'success') { responseText = msg.result || responseText; + } else if (msg.type === 'result' && msg.subtype === 'error') { + const errorMsg = msg as { error?: string }; + throw new Error(errorMsg.error || 'Provider returned an error'); } } return responseText; }apps/ui/src/store/app-store.ts (1)
602-638: Type definitions duplicated across packages.
CodexPlanType,CodexCreditsSnapshot,CodexRateLimitWindow,CodexRateLimits, andCodexUsageare defined here and also inapps/server/src/routes/codex/types.ts. Consider consolidating these into@automaker/typesto maintain a single source of truth and ensure type consistency between server and UI.apps/server/src/routes/codex/types.ts (1)
1-33: Consolidate Codex usage types in @automaker/types per coding guidelines.These Codex types (
CodexPlanType,CodexCreditsSnapshot,CodexRateLimitWindow,CodexRateLimits,CodexUsage) are currently duplicated acrossapps/server/src/routes/codex/types.ts,apps/ui/src/store/app-store.ts, andapps/ui/src/types/electron.d.ts. Per the coding guideline to "Always import from shared packages (@automaker/*)", these should be defined inlibs/types/src/codex.tsand exported from@automaker/types, then imported from the shared package in both server and UI code.Note: There's currently an inconsistency where server types omit
'unknown'fromCodexPlanTypewhileapp-store.tsincludes it—consolidation will resolve this.apps/server/src/providers/codex-tool-mapping.ts (1)
113-116: Escaped backslash (\\) becomes empty instead of a literal backslash.When the input contains
\\, the first backslash setsisEscaped = trueand is discarded, then the second backslash is added tocurrent. This works for\\→\. However, the logic drops standalone trailing backslashes. Consider if this edge case matters for your use cases.libs/model-resolver/tests/resolver.test.ts (1)
137-140: Add console.log assertion for consistency.The
o1test should verify the log message like thegpt-5.2test does, since both code paths execute the same logging statement. This ensures complete test coverage of the passthrough behavior.🔎 Proposed fix
it('should pass through o-series models unchanged', () => { const result = resolveModelString('o1'); expect(result).toBe('o1'); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('Using OpenAI/Codex model') + ); });apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
22-22: Consider defensive handling for missing provider icons.The dynamic icon lookup assumes all providers in MODEL_GROUPS have a corresponding entry in PROVIDER_ICON_COMPONENTS. While TypeScript's Record<ModelProvider, ...> type should enforce this at compile time, consider adding a fallback or explicit type guard for runtime safety.
🔎 Proposed defensive handling
- const Icon = PROVIDER_ICON_COMPONENTS[group.provider]; + const Icon = PROVIDER_ICON_COMPONENTS[group.provider]; + if (!Icon) { + console.warn(`No icon component found for provider: ${group.provider}`); + return null; + }apps/server/src/routes/app-spec/generate-features-from-spec.ts (2)
113-115: Consider emitting an error event for consistency.The model validation throws an error but doesn't emit a
spec-regeneration:eventwith typespec_regeneration_errorto the frontend. For consistency with other error paths in this function (e.g., lines 47-52), consider emitting an event before throwing.🔎 Proposed event emission for validation error
if (!options.model) { + events.emit('spec-regeneration:event', { + type: 'spec_regeneration_error', + error: ERROR_FEATURE_MODEL_REQUIRED, + projectPath: projectPath, + }); throw new Error(ERROR_FEATURE_MODEL_REQUIRED); }
126-130: Consider emitting error events in catch blocks.The error handlers log detailed information but don't emit
spec-regeneration:eventerrors to the frontend before re-throwing. For consistency with other error paths (lines 47-52) and better user experience, consider emitting events in both catch blocks.🔎 Proposed error event emissions
} catch (queryError) { logger.error('❌ executeQuery() threw an exception:'); logger.error('Error:', queryError); + events.emit('spec-regeneration:event', { + type: 'spec_regeneration_error', + error: 'Failed to execute query for feature generation', + projectPath: projectPath, + }); throw queryError; }And similarly for the stream iteration error:
} catch (streamError) { logger.error('❌ Error while iterating feature stream:'); logger.error('Stream error:', streamError); + events.emit('spec-regeneration:event', { + type: 'spec_regeneration_error', + error: 'Error processing feature generation stream', + projectPath: projectPath, + }); throw streamError; }Also applies to: 165-169
apps/server/src/providers/provider-factory.ts (1)
14-23: Consider consolidating isCodexModel logic into a shared package.Both the backend (here) and frontend (apps/ui/src/lib/utils.ts) implement similar isCodexModel detection logic with slightly different approaches. The backend uses CODEX_MODEL_PREFIXES and OPENAI_O_SERIES_PATTERN, while the frontend uses CODEX_MODEL_IDS, CODEX_MODEL_PREFIXES, and CODEX_MODEL_REGEX.
For consistency and maintainability, consider moving this model detection logic to a shared package (e.g., @automaker/model-resolver or @automaker/utils) that both frontend and backend can import.
apps/ui/src/lib/utils.ts (1)
16-31: Consider documenting the regex pattern's purpose.The
CODEX_MODEL_REGEXpattern/^o\d/appears to match OpenAI's reasoning models (e.g.,o1,o3). Adding a brief comment would clarify this intent for future maintainers.🔎 Suggested documentation
const CODEX_MODEL_PREFIXES = ['gpt-']; -const CODEX_MODEL_REGEX = /^o\d/; +// Matches OpenAI reasoning models (o1, o3, etc.) +const CODEX_MODEL_REGEX = /^o\d/;libs/types/src/model-display.ts (1)
146-157: Consider deriving display names from CODEX_MODELS to avoid duplication.The display name mappings for Codex models duplicate the
labelvalues already defined inCODEX_MODELS. This creates a maintenance burden if labels change.🔎 Suggested refactor to derive from CODEX_MODELS
export function getModelDisplayName(model: AgentModel | string): string { + // Check Codex models first + const codexModel = CODEX_MODELS.find(m => m.id === model); + if (codexModel) return codexModel.label; + + // Check Claude models + const claudeModel = CLAUDE_MODELS.find(m => m.id === model); + if (claudeModel) return claudeModel.label; + - const displayNames: Record<string, string> = { - haiku: 'Claude Haiku', - sonnet: 'Claude Sonnet', - opus: 'Claude Opus', - [CODEX_MODEL_MAP.gpt52]: 'Codex GPT-5.2', - [CODEX_MODEL_MAP.gpt51CodexMax]: 'Codex GPT-5.1 Max', - [CODEX_MODEL_MAP.gpt51Codex]: 'Codex GPT-5.1', - [CODEX_MODEL_MAP.gpt51CodexMini]: 'Codex GPT-5.1 Mini', - [CODEX_MODEL_MAP.gpt51]: 'GPT-5.1', - }; - return displayNames[model] || model; + // Fallback to raw model string + return model; }apps/ui/src/components/views/setup-view/steps/codex-setup-step.tsx (1)
22-36: Consider providing rejection reasons for better debugging.The
Promise.reject()calls without arguments will produce undefined rejection values, making debugging harder when these APIs are unavailable.🔎 Proposed fix
const statusApi = useCallback( - () => getElectronAPI().setup?.getCodexStatus() || Promise.reject(), + () => getElectronAPI().setup?.getCodexStatus() || Promise.reject(new Error('Codex status API unavailable')), [] ); const installApi = useCallback( - () => getElectronAPI().setup?.installCodex() || Promise.reject(), + () => getElectronAPI().setup?.installCodex() || Promise.reject(new Error('Codex install API unavailable')), [] ); const verifyAuthApi = useCallback( (method: 'cli' | 'api_key') => - getElectronAPI().setup?.verifyCodexAuth(method) || Promise.reject(), + getElectronAPI().setup?.verifyCodexAuth(method) || Promise.reject(new Error('Codex auth verification API unavailable')), [] );apps/ui/src/components/views/settings-view/api-keys/api-keys-section.tsx (1)
33-61: Missing early return after error toast may cause unexpected behavior.On line 55, after
toast.error('Failed to delete API key'), the function continues tofinallyblock without returning. While this works, a more defensive pattern would be to return early in the catch block to make the flow explicit.🔎 Proposed fix
} catch (error) { toast.error('Failed to delete API key'); + return; } finally { setLoading(false); }apps/ui/src/components/views/settings-view/codex/codex-usage-section.tsx (1)
110-145: Consider extractingRateLimitCardoutside the component to avoid recreation on each render.
RateLimitCardis defined insideCodexUsageSection, causing it to be recreated on every render. While this works, it's a mild performance concern and deviates from best practices.🔎 Proposed refactor
Extract
RateLimitCardas a separate component outsideCodexUsageSection:interface RateLimitCardProps { title: string; subtitle: string; window: CodexRateLimitWindow; getUsageColor: (percentage: number) => string; } function RateLimitCard({ title, subtitle, window: limitWindow, getUsageColor }: RateLimitCardProps) { const safePercentage = Math.min(Math.max(limitWindow.usedPercent, 0), MAX_PERCENTAGE); const resetLabel = formatCodexResetTime(limitWindow.resetsAt); // ... rest of implementation }Then pass
getUsageColoras a prop, or define it outside both components since it only depends on constants.apps/server/src/providers/codex-sdk-client.ts (1)
48-53: Type cast bypasses validation.The cast
prompt as PromptBlock[]assumes the array elements conform toPromptBlockshape without validation. If the caller passes malformed data, this could cause runtime errors downstream.Suggested validation
function normalizePromptBlocks(prompt: ExecuteOptions['prompt']): PromptBlock[] { if (Array.isArray(prompt)) { - return prompt as PromptBlock[]; + return prompt.map((block) => { + if (typeof block === 'object' && block !== null && 'type' in block) { + return block as PromptBlock; + } + throw new Error('Invalid prompt block format'); + }); } return [{ type: 'text', text: prompt }]; }apps/server/src/routes/app-spec/generate-spec.ts (1)
104-111: String-based provider check is fragile.The comparison
provider.getName() === 'claude'is prone to typos and doesn't leverage type safety. Consider using a typed constant or provider capability check.Suggested improvement
+ // Check if provider supports native structured output + const usesStructuredOutput = provider.supportsStructuredOutput?.() ?? false; - const usesStructuredOutput = provider.getName() === 'claude';Alternatively, define provider name constants:
import { PROVIDER_NAMES } from '../providers/constants.js'; // ... const usesStructuredOutput = provider.getName() === PROVIDER_NAMES.CLAUDE;apps/ui/src/components/views/setup-view/steps/cli-setup-step.tsx (1)
269-272: Consider error handling for clipboard API.The
copyCommandfunction doesn't handle clipboard API failures, which can occur in non-secure contexts or when permissions are denied.Suggested improvement
const copyCommand = (command: string) => { - navigator.clipboard.writeText(command); - toast.success('Command copied to clipboard'); + navigator.clipboard.writeText(command) + .then(() => toast.success('Command copied to clipboard')) + .catch(() => toast.error('Failed to copy command')); };apps/server/src/providers/codex-provider.ts (1)
462-496: Consider caching SettingsService instance.
loadCodexCliSettingsinstantiates a newSettingsServiceon every call. For frequently invoked operations, consider caching the service instance or using a shared singleton to avoid repeated filesystem access for the same settings.
| const CODEX_INSTRUCTIONS_USER_SOURCE = 'User instructions'; | ||
| const CODEX_INSTRUCTIONS_PROJECT_SOURCE = 'Project instructions'; | ||
| const CODEX_USER_INSTRUCTIONS_FILE = 'AGENTS.md'; | ||
| const CODEX_PROJECT_INSTRUCTIONS_FILES = ['AGENTS.nd', 'AGENTS.md'] as const; |
There was a problem hiding this comment.
Typo in project instructions filename.
'AGENTS.nd' appears to be a typo and should likely be 'AGENTS.md' to match the user instructions file pattern.
🔎 Proposed fix
-const CODEX_PROJECT_INSTRUCTIONS_FILES = ['AGENTS.nd', 'AGENTS.md'] as const;
+const CODEX_PROJECT_INSTRUCTIONS_FILES = ['AGENTS.md'] as const;Or if multiple filenames are intentional (e.g., supporting legacy naming), please clarify with a comment.
📝 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 CODEX_PROJECT_INSTRUCTIONS_FILES = ['AGENTS.nd', 'AGENTS.md'] as const; | |
| const CODEX_PROJECT_INSTRUCTIONS_FILES = ['AGENTS.md'] as const; |
🤖 Prompt for AI Agents
In apps/server/src/providers/codex-provider.ts around line 86, the constant
CODEX_PROJECT_INSTRUCTIONS_FILES includes a typo 'AGENTS.nd' that should be
'AGENTS.md'; update the array to use 'AGENTS.md' so filenames match the expected
project instructions pattern, or if you intentionally support multiple names
(legacy fallback), keep both values and add a clarifying inline comment
explaining why both variants are required.
| getAvailableModels(): ModelDefinition[] { | ||
| return [ | ||
| { | ||
| id: CODEX_MODEL_MAP.gpt52, | ||
| name: 'Codex GPT-5.2', | ||
| modelString: CODEX_MODEL_MAP.gpt52, | ||
| provider: 'openai', | ||
| description: 'Latest Codex model with strong reasoning and tools.', | ||
| contextWindow: CONTEXT_WINDOW_256K, | ||
| maxOutputTokens: MAX_OUTPUT_32K, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| tier: 'premium' as const, | ||
| default: true, | ||
| }, | ||
| { | ||
| id: CODEX_MODEL_MAP.gpt51CodexMax, | ||
| name: 'Codex GPT-5.1 Max', | ||
| modelString: CODEX_MODEL_MAP.gpt51CodexMax, | ||
| provider: 'openai', | ||
| description: 'Maximum capability Codex model.', | ||
| contextWindow: CONTEXT_WINDOW_256K, | ||
| maxOutputTokens: MAX_OUTPUT_32K, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| tier: 'premium' as const, | ||
| }, | ||
| { | ||
| id: CODEX_MODEL_MAP.gpt51Codex, | ||
| name: 'Codex GPT-5.1', | ||
| modelString: CODEX_MODEL_MAP.gpt51Codex, | ||
| provider: 'openai', | ||
| description: 'Balanced Codex model for general coding tasks.', | ||
| contextWindow: CONTEXT_WINDOW_256K, | ||
| maxOutputTokens: MAX_OUTPUT_32K, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| tier: 'standard' as const, | ||
| }, | ||
| { | ||
| id: CODEX_MODEL_MAP.gpt51CodexMini, | ||
| name: 'Codex GPT-5.1 Mini', | ||
| modelString: CODEX_MODEL_MAP.gpt51CodexMini, | ||
| provider: 'openai', | ||
| description: 'Lightweight Codex model for fast edits.', | ||
| contextWindow: CONTEXT_WINDOW_256K, | ||
| maxOutputTokens: MAX_OUTPUT_16K, | ||
| supportsVision: false, | ||
| supportsTools: true, | ||
| tier: 'basic' as const, | ||
| }, | ||
| { | ||
| id: CODEX_MODEL_MAP.gpt51, | ||
| name: 'GPT-5.1', | ||
| modelString: CODEX_MODEL_MAP.gpt51, | ||
| provider: 'openai', | ||
| description: 'General-purpose GPT model with strong tools support.', | ||
| contextWindow: CONTEXT_WINDOW_256K, | ||
| maxOutputTokens: MAX_OUTPUT_32K, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| tier: 'standard' as const, | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
current OpenAI GPT-5 model names identifiers 2025
💡 Result:
As of Jan 4, 2026 the primary OpenAI GPT‑5 model identifiers exposed in docs/releases are:
- gpt-5 (alias / snapshot: gpt-5-2025-08-07). (platform.openai.com)
- gpt-5-mini. (openai.com)
- gpt-5-nano. (openai.com)
Notes: OpenAI also documents usage modes (e.g., “GPT‑5 thinking” / reasoning router) and later variant announcements (e.g., GPT‑5–codex) may appear in release notes; check the OpenAI model/docs and release notes pages for the latest snapshots and additional variant IDs. (openai.com)
Citations:
- 1: https://platform.openai.com/docs/models/gpt-5?utm_source=openai
- 2: https://openai.com/index/introducing-gpt-5-for-developers?utm_source=openai
- 3: https://openai.com/index/introducing-gpt-5-for-developers?utm_source=openai
- 4: https://openai.com/index/introducing-gpt-5/?utm_source=openai
🏁 Script executed:
# Find the CODEX_MODEL_MAP definition to see actual model strings
fd -e ts -e tsx | xargs grep -l "CODEX_MODEL_MAP" | head -20Repository: AutoMaker-Org/automaker
Length of output: 247
🏁 Script executed:
# Read the codex-provider.ts file to see the model definitions and CODEX_MODEL_MAP
cat -n apps/server/src/providers/codex-provider.ts | head -100Repository: AutoMaker-Org/automaker
Length of output: 4482
🏁 Script executed:
# Find CODEX_MODEL_MAP definition in types file
grep -n "CODEX_MODEL_MAP" libs/types/src/index.ts libs/types/src/model.tsRepository: AutoMaker-Org/automaker
Length of output: 431
🏁 Script executed:
# Read the specific lines 827-891 from the codex-provider.ts file
sed -n '827,891p' apps/server/src/providers/codex-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 2214
🏁 Script executed:
# Also check if there's context around these lines to understand the full method
sed -n '820,895p' apps/server/src/providers/codex-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 2395
🏁 Script executed:
# Read the CODEX_MODEL_MAP definition with context
cat -n libs/types/src/model.ts | head -40Repository: AutoMaker-Org/automaker
Length of output: 1326
Model identifiers do not match OpenAI's current API documentation.
The CODEX_MODEL_MAP values (gpt-5.2, gpt-5.1-codex-max, gpt-5.1-codex, gpt-5.1-codex-mini, gpt-5.1) are not among OpenAI's documented model identifiers as of January 2026. OpenAI's current GPT-5 models are: gpt-5, gpt-5-mini, and gpt-5-nano. Update the model identifiers to match OpenAI's actual available models or clarify if these are custom internal model names.
| const hasAnthropicKey = !!process.env[ANTHROPIC_API_KEY_ENV]; | ||
| const hasOpenAIKey = !!process.env[OPENAI_API_KEY_ENV]; | ||
|
|
||
| logger.info(`${context} - Auth Status:`); | ||
| logger.info( | ||
| ` ANTHROPIC_API_KEY: ${ | ||
| hasApiKey ? 'SET (' + process.env.ANTHROPIC_API_KEY?.substring(0, 20) + '...)' : 'NOT SET' | ||
| hasAnthropicKey | ||
| ? 'SET (' + process.env[ANTHROPIC_API_KEY_ENV]?.substring(0, 20) + '...)' | ||
| : 'NOT SET' | ||
| }` | ||
| ); | ||
| logger.info( | ||
| ` OPENAI_API_KEY: ${ | ||
| hasOpenAIKey | ||
| ? 'SET (' + process.env[OPENAI_API_KEY_ENV]?.substring(0, 20) + '...)' | ||
| : 'NOT SET' | ||
| }` | ||
| ); | ||
|
|
||
| if (!hasApiKey) { | ||
| logger.warn('⚠️ WARNING: No authentication configured! SDK will fail.'); | ||
| if (!hasAnthropicKey && !hasOpenAIKey) { | ||
| logger.warn('WARNING: No authentication configured! Provider calls will fail.'); | ||
| } |
There was a problem hiding this comment.
Security risk: Partial API key logging.
Lines 44 and 51 log the first 20 characters of API keys. Even partial key exposure in logs can pose a security risk, especially in centralized logging systems or when logs are shared. Consider logging only the key presence status without revealing any part of the actual key value.
🔎 Proposed fix to remove partial key exposure
logger.info(`${context} - Auth Status:`);
logger.info(
- ` ANTHROPIC_API_KEY: ${
- hasAnthropicKey
- ? 'SET (' + process.env[ANTHROPIC_API_KEY_ENV]?.substring(0, 20) + '...)'
- : 'NOT SET'
- }`
+ ` ANTHROPIC_API_KEY: ${hasAnthropicKey ? 'SET' : 'NOT SET'}`
);
logger.info(
- ` OPENAI_API_KEY: ${
- hasOpenAIKey
- ? 'SET (' + process.env[OPENAI_API_KEY_ENV]?.substring(0, 20) + '...)'
- : 'NOT SET'
- }`
+ ` OPENAI_API_KEY: ${hasOpenAIKey ? 'SET' : 'NOT SET'}`
);📝 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 hasAnthropicKey = !!process.env[ANTHROPIC_API_KEY_ENV]; | |
| const hasOpenAIKey = !!process.env[OPENAI_API_KEY_ENV]; | |
| logger.info(`${context} - Auth Status:`); | |
| logger.info( | |
| ` ANTHROPIC_API_KEY: ${ | |
| hasApiKey ? 'SET (' + process.env.ANTHROPIC_API_KEY?.substring(0, 20) + '...)' : 'NOT SET' | |
| hasAnthropicKey | |
| ? 'SET (' + process.env[ANTHROPIC_API_KEY_ENV]?.substring(0, 20) + '...)' | |
| : 'NOT SET' | |
| }` | |
| ); | |
| logger.info( | |
| ` OPENAI_API_KEY: ${ | |
| hasOpenAIKey | |
| ? 'SET (' + process.env[OPENAI_API_KEY_ENV]?.substring(0, 20) + '...)' | |
| : 'NOT SET' | |
| }` | |
| ); | |
| if (!hasApiKey) { | |
| logger.warn('⚠️ WARNING: No authentication configured! SDK will fail.'); | |
| if (!hasAnthropicKey && !hasOpenAIKey) { | |
| logger.warn('WARNING: No authentication configured! Provider calls will fail.'); | |
| } | |
| const hasAnthropicKey = !!process.env[ANTHROPIC_API_KEY_ENV]; | |
| const hasOpenAIKey = !!process.env[OPENAI_API_KEY_ENV]; | |
| logger.info(`${context} - Auth Status:`); | |
| logger.info( | |
| ` ANTHROPIC_API_KEY: ${hasAnthropicKey ? 'SET' : 'NOT SET'}` | |
| ); | |
| logger.info( | |
| ` OPENAI_API_KEY: ${hasOpenAIKey ? 'SET' : 'NOT SET'}` | |
| ); | |
| if (!hasAnthropicKey && !hasOpenAIKey) { | |
| logger.warn('WARNING: No authentication configured! Provider calls will fail.'); | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/routes/app-spec/common.ts around lines 37 to 58, the code
currently logs the first 20 characters of the ANTHROPIC and OPENAI API keys
which risks exposing sensitive data; change the logging to only indicate
presence/absence (e.g., "SET" or "NOT SET") without printing any portion of the
keys, and keep the existing warning when neither key is set.
| import { ProviderFactory } from '../../../providers/provider-factory.js'; | ||
| import { getErrorMessage, logError } from '../common.js'; | ||
|
|
||
| interface ModelDefinition { | ||
| id: string; | ||
| name: string; | ||
| provider: string; | ||
| contextWindow: number; | ||
| maxOutputTokens: number; | ||
| supportsVision: boolean; | ||
| supportsTools: boolean; | ||
| } | ||
|
|
||
| export function createAvailableHandler() { | ||
| return async (_req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const models: ModelDefinition[] = [ | ||
| { | ||
| id: 'claude-opus-4-5-20251101', | ||
| name: 'Claude Opus 4.5', | ||
| provider: 'anthropic', | ||
| contextWindow: 200000, | ||
| maxOutputTokens: 16384, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| }, | ||
| { | ||
| id: 'claude-sonnet-4-20250514', | ||
| name: 'Claude Sonnet 4', | ||
| provider: 'anthropic', | ||
| contextWindow: 200000, | ||
| maxOutputTokens: 16384, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| }, | ||
| { | ||
| id: 'claude-3-5-sonnet-20241022', | ||
| name: 'Claude 3.5 Sonnet', | ||
| provider: 'anthropic', | ||
| contextWindow: 200000, | ||
| maxOutputTokens: 8192, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| }, | ||
| { | ||
| id: 'claude-3-5-haiku-20241022', | ||
| name: 'Claude 3.5 Haiku', | ||
| provider: 'anthropic', | ||
| contextWindow: 200000, | ||
| maxOutputTokens: 8192, | ||
| supportsVision: true, | ||
| supportsTools: true, | ||
| }, | ||
| ]; | ||
|
|
||
| const models = ProviderFactory.getAllAvailableModels(); | ||
| res.json({ success: true, models }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Excellent refactor to dynamic model retrieval!
Replacing hard-coded model definitions with ProviderFactory.getAllAvailableModels() is a significant improvement:
- Extensibility: New providers automatically surface their models without route changes
- Maintainability: Model definitions live with their providers, not scattered across the codebase
- Consistency: Guarantees UI and provider logic stay in sync
The API surface remains unchanged (same JSON response shape), ensuring backward compatibility.
🤖 Prompt for AI Agents
In apps/server/src/routes/models/routes/available.ts around lines 6 to 13, the
handler currently returns models via ProviderFactory.getAllAvailableModels() but
the try block is left open and there is no error handling or final response
path; close the try/catch and add an async catch that logs the error with
logError(getErrorMessage(err)) and returns res.status(500).json({ success:
false, error: getErrorMessage(err) }) so failures are logged and the client
receives a consistent JSON error response.
| try { | ||
| const findCommand = isWindows ? `where ${CODEX_COMMAND}` : `which ${CODEX_COMMAND}`; | ||
| const { stdout } = await execAsync(findCommand); | ||
| cliPath = stdout.trim().split(/\r?\n/)[0]; | ||
| installed = true; | ||
| method = 'path'; | ||
|
|
||
| try { | ||
| const { stdout: versionOut } = await execAsync(`${CODEX_COMMAND} --version`); | ||
| version = versionOut.trim(); | ||
| } catch { | ||
| version = ''; | ||
| } |
There was a problem hiding this comment.
Add timeout to prevent indefinite blocking on CLI commands.
The execAsync calls for which/where and --version have no timeout. If the Codex CLI hangs or is slow to respond, this could block the status endpoint indefinitely.
🔎 Proposed fix with timeout
+const EXEC_TIMEOUT_MS = 5000;
+
export async function getCodexStatus() {
let installed = false;
let version = '';
let cliPath = '';
let method = 'none';
const isWindows = process.platform === 'win32';
try {
const findCommand = isWindows ? `where ${CODEX_COMMAND}` : `which ${CODEX_COMMAND}`;
- const { stdout } = await execAsync(findCommand);
+ const { stdout } = await execAsync(findCommand, { timeout: EXEC_TIMEOUT_MS });
cliPath = stdout.trim().split(/\r?\n/)[0];
installed = true;
method = 'path';
try {
- const { stdout: versionOut } = await execAsync(`${CODEX_COMMAND} --version`);
+ const { stdout: versionOut } = await execAsync(`${CODEX_COMMAND} --version`, { timeout: EXEC_TIMEOUT_MS });
version = versionOut.trim();
} catch {
version = '';
}📝 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.
| try { | |
| const findCommand = isWindows ? `where ${CODEX_COMMAND}` : `which ${CODEX_COMMAND}`; | |
| const { stdout } = await execAsync(findCommand); | |
| cliPath = stdout.trim().split(/\r?\n/)[0]; | |
| installed = true; | |
| method = 'path'; | |
| try { | |
| const { stdout: versionOut } = await execAsync(`${CODEX_COMMAND} --version`); | |
| version = versionOut.trim(); | |
| } catch { | |
| version = ''; | |
| } | |
| const EXEC_TIMEOUT_MS = 5000; | |
| export async function getCodexStatus() { | |
| let installed = false; | |
| let version = ''; | |
| let cliPath = ''; | |
| let method = 'none'; | |
| const isWindows = process.platform === 'win32'; | |
| try { | |
| const findCommand = isWindows ? `where ${CODEX_COMMAND}` : `which ${CODEX_COMMAND}`; | |
| const { stdout } = await execAsync(findCommand, { timeout: EXEC_TIMEOUT_MS }); | |
| cliPath = stdout.trim().split(/\r?\n/)[0]; | |
| installed = true; | |
| method = 'path'; | |
| try { | |
| const { stdout: versionOut } = await execAsync(`${CODEX_COMMAND} --version`, { timeout: EXEC_TIMEOUT_MS }); | |
| version = versionOut.trim(); | |
| } catch { | |
| version = ''; | |
| } |
| it('injects user and project instructions when auto-load is enabled', async () => { | ||
| vi.mocked(spawnJSONLProcess).mockReturnValue((async function* () {})()); | ||
|
|
||
| const userPath = path.join('/home/test/.codex', 'AGENTS.md'); | ||
| const projectPath = path.join('/tmp/project', '.codex', 'AGENTS.nd'); | ||
| vi.mocked(secureFs.readFile).mockImplementation(async (filePath: string) => { | ||
| if (filePath === userPath) { | ||
| return 'User rules'; | ||
| } | ||
| if (filePath === projectPath) { | ||
| return 'Project rules'; | ||
| } | ||
| throw new Error('missing'); | ||
| }); | ||
|
|
||
| await collectAsyncGenerator( | ||
| provider.executeQuery({ | ||
| prompt: 'Hello', | ||
| model: 'gpt-5.2', | ||
| cwd: '/tmp/project', | ||
| codexSettings: { autoLoadAgents: true }, | ||
| }) | ||
| ); | ||
|
|
||
| const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; | ||
| const promptText = call.args[call.args.length - 1]; | ||
| expect(promptText).toContain('User rules'); | ||
| expect(promptText).toContain('Project rules'); | ||
| }); |
There was a problem hiding this comment.
Fix file extension typo.
Line 189 uses .nd extension instead of .md for the project AGENTS file path. This typo could cause the test to behave incorrectly if the implementation expects .md files.
🔎 Proposed fix
- const projectPath = path.join('/tmp/project', '.codex', 'AGENTS.nd');
+ const projectPath = path.join('/tmp/project', '.codex', 'AGENTS.md');🤖 Prompt for AI Agents
In apps/server/tests/unit/providers/codex-provider.test.ts around lines 185 to
213 the project AGENTS file path uses the wrong extension (.nd) causing the
readFile mock to miss the file; change the projectPath to use '.codex/AGENTS.md'
and update the secureFs.readFile conditional to check for that corrected path so
the mock returns 'Project rules' as intended.
| HTMLAudioElement: 'readonly', | ||
| Element: 'readonly', | ||
| // Event Types | ||
| Event: 'readonly', | ||
| KeyboardEvent: 'readonly', | ||
| MouseEvent: 'readonly', | ||
| DragEvent: 'readonly', | ||
| PointerEvent: 'readonly', | ||
| CustomEvent: 'readonly', | ||
| ClipboardEvent: 'readonly', | ||
| WheelEvent: 'readonly', | ||
| MediaQueryListEvent: 'readonly', | ||
| DataTransfer: 'readonly', | ||
| // Web APIs | ||
| ResizeObserver: 'readonly', | ||
| AbortSignal: 'readonly', | ||
| Audio: 'readonly', | ||
| ScrollBehavior: 'readonly', | ||
| Response: 'readonly', | ||
| RequestInit: 'readonly', | ||
| URLSearchParams: 'readonly', | ||
| Headers: 'readonly', | ||
| SVGSVGElement: 'readonly', | ||
| SVGElement: 'readonly', | ||
| URL: 'readonly', | ||
| XMLHttpRequest: 'readonly', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify usage of newly added ESLint globals in the TypeScript/TSX codebase
echo "=== Checking HTMLAudioElement usage ==="
rg -n --type=ts --type=tsx '\bHTMLAudioElement\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking MouseEvent usage ==="
rg -n --type=ts --type=tsx '\bMouseEvent\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking MediaQueryListEvent usage ==="
rg -n --type=ts --type=tsx '\bMediaQueryListEvent\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking Response usage ==="
rg -n --type=ts --type=tsx '\bResponse\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking RequestInit usage ==="
rg -n --type=ts --type=tsx '\bRequestInit\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking URLSearchParams usage ==="
rg -n --type=ts --type=tsx '\bURLSearchParams\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking Headers usage ==="
rg -n --type=ts --type=tsx '\bHeaders\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking SVGSVGElement usage ==="
rg -n --type=ts --type=tsx '\bSVGSVGElement\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking SVGElement usage ==="
rg -n --type=ts --type=tsx '\bSVGElement\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking XMLHttpRequest usage ==="
rg -n --type=ts --type=tsx '\bXMLHttpRequest\b' apps/ui/ 2>/dev/null | head -20
echo -e "\n=== Checking URL usage ==="
rg -n --type=ts --type=tsx '\bURL\b' apps/ui/ 2>/dev/null | head -20Repository: AutoMaker-Org/automaker
Length of output: 466
Remove unused ESLint globals or provide justification for their addition.
The newly added globals (HTMLAudioElement, MouseEvent, MediaQueryListEvent, Response, RequestInit, URLSearchParams, Headers, SVGSVGElement, SVGElement, URL, XMLHttpRequest) are not currently used anywhere in the apps/ui codebase. Adding unused globals to the ESLint configuration unnecessarily clutters it and can mask real missing type declarations. Either remove these globals until they're actually needed, or provide context explaining why they're being added in anticipation of future use.
🤖 Prompt for AI Agents
In apps/ui/eslint.config.mjs around lines 62 to 87, a set of globals
(HTMLAudioElement, MouseEvent, MediaQueryListEvent, Response, RequestInit,
URLSearchParams, Headers, SVGSVGElement, SVGElement, URL, XMLHttpRequest) were
added but are unused; remove each unused global from the config to avoid clutter
and masking missing types, or if you intend to keep them proactively, add a
one‑line justification comment (or TODO with an issue/ticket reference) directly
above the globals block explaining why they are needed and where/when they will
be used, and ensure any kept globals are limited to the appropriate
overrides/files rather than being globally enabled for the whole app.
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| type="button" | ||
| role="tab" | ||
| aria-pressed={activeProvider === PROVIDER_CLAUDE} | ||
| onClick={() => setActiveProvider(PROVIDER_CLAUDE)} | ||
| className={cn( | ||
| PROVIDER_BUTTON_BASE, | ||
| activeProvider === PROVIDER_CLAUDE | ||
| ? PROVIDER_BUTTON_ACTIVE | ||
| : PROVIDER_BUTTON_INACTIVE | ||
| )} | ||
| > | ||
| {CLAUDE_SECTION_TITLE} | ||
| </Button> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| type="button" | ||
| role="tab" | ||
| aria-pressed={activeProvider === PROVIDER_CODEX} | ||
| onClick={() => setActiveProvider(PROVIDER_CODEX)} | ||
| className={cn( | ||
| PROVIDER_BUTTON_BASE, | ||
| activeProvider === PROVIDER_CODEX | ||
| ? PROVIDER_BUTTON_ACTIVE | ||
| : PROVIDER_BUTTON_INACTIVE | ||
| )} | ||
| > | ||
| {CODEX_SECTION_TITLE} | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Provider toggle uses aria-pressed but should use aria-selected for tabs.
The buttons have role="tab" but use aria-pressed (which is for toggle buttons). Tabs should use aria-selected for accessibility compliance.
Proposed fix for ARIA attributes
<Button
variant="ghost"
size="sm"
type="button"
role="tab"
- aria-pressed={activeProvider === PROVIDER_CLAUDE}
+ aria-selected={activeProvider === PROVIDER_CLAUDE}
onClick={() => setActiveProvider(PROVIDER_CLAUDE)}
// ...
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/ui/src/components/usage-popover.tsx around lines 443 to 475, the tab
buttons currently use aria-pressed while having role="tab"; replace aria-pressed
with aria-selected on each Button so the tabs expose selection state correctly
(keep the same boolean expression activeProvider === PROVIDER_X to supply
true/false), and ensure the tab container uses role="tablist" elsewhere if not
already present.
| import { cn } from '@/lib/utils'; | ||
| import { useAppStore } from '@/store/app-store'; | ||
| import { CLAUDE_MODELS } from '@/components/views/board-view/shared/model-constants'; | ||
| import { MODEL_GROUPS } from '@/components/views/board-view/shared/model-constants'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure and locate MODEL_GROUPS definition
fd "model-constants" apps/ui/src/Repository: AutoMaker-Org/automaker
Length of output: 132
🏁 Script executed:
# Find where MODEL_GROUPS is defined
rg -n "export.*MODEL_GROUPS" apps/ui/src/ --type=ts --type=tsxRepository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Find all imports of MODEL_GROUPS
rg -n "import.*MODEL_GROUPS" apps/ui/src/ --type=ts --type=tsxRepository: AutoMaker-Org/automaker
Length of output: 94
🏁 Script executed:
# Check if @automaker shared packages exist
fd -t d "@automaker" . --max-depth 3 | head -20Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Look at the package structure to understand available shared packages
ls -la packages/ 2>/dev/null || echo "No packages directory at root"Repository: AutoMaker-Org/automaker
Length of output: 96
🏁 Script executed:
# Check the project structure to understand monorepo layout
git ls-files | grep -E "(package\.json|tsconfig|workspace)" | head -20Repository: AutoMaker-Org/automaker
Length of output: 757
🏁 Script executed:
# Search for MODEL_GROUPS imports without type filter
rg -n "import.*MODEL_GROUPS" apps/ui/src/Repository: AutoMaker-Org/automaker
Length of output: 553
🏁 Script executed:
# Check the tsconfig to understand path aliases
cat apps/ui/tsconfig.jsonRepository: AutoMaker-Org/automaker
Length of output: 589
🏁 Script executed:
# Check what's in model-constants to see MODEL_GROUPS definition
cat apps/ui/src/components/views/board-view/shared/model-constants.tsRepository: AutoMaker-Org/automaker
Length of output: 1782
🏁 Script executed:
# Check the libs packages to understand naming convention
ls -la libs/Repository: AutoMaker-Org/automaker
Length of output: 662
🏁 Script executed:
# Check libs/model-resolver package.json to understand @automaker naming
cat libs/model-resolver/package.json | head -20Repository: AutoMaker-Org/automaker
Length of output: 523
🏁 Script executed:
# Check what's in libs/types
cat libs/types/package.json | head -30Repository: AutoMaker-Org/automaker
Length of output: 577
🏁 Script executed:
# Look at libs/types exports
ls -la libs/types/src/ 2>/dev/null || ls -la libs/types/ | head -20Repository: AutoMaker-Org/automaker
Length of output: 1289
🏁 Script executed:
# Check if MODEL_GROUPS is defined in shared libs/types
rg -n "MODEL_GROUPS" libs/Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check apps/ui/package.json to see if @automaker packages are dependencies
cat apps/ui/package.json | grep -A 20 "dependencies"Repository: AutoMaker-Org/automaker
Length of output: 867
🏁 Script executed:
# Check if other ui components import from @automaker/*
rg -n "from ['\"]@automaker/" apps/ui/src/Repository: AutoMaker-Org/automaker
Length of output: 4047
🏁 Script executed:
# Check if MODEL_GROUPS should be in types or if there's a similar construct
cat libs/types/src/model-display.tsRepository: AutoMaker-Org/automaker
Length of output: 4197
🏁 Script executed:
# Check how model-constants is structured - maybe we need a shared ui constants
ls -la apps/ui/src/components/shared/ 2>/dev/null || echo "No shared constants directory"Repository: AutoMaker-Org/automaker
Length of output: 96
🏁 Script executed:
# Verify what model-constants actually exports to understand scope
cat -n apps/ui/src/components/views/board-view/shared/model-constants.ts | head -50Repository: AutoMaker-Org/automaker
Length of output: 1869
🏁 Script executed:
# Check if there's an @automaker package specifically for ui constants or if one should be created
ls -la libs/ | grep -i uiRepository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Verify the purpose of model-constants and understand full scope
wc -l apps/ui/src/components/views/board-view/shared/model-constants.tsRepository: AutoMaker-Org/automaker
Length of output: 135
🏁 Script executed:
# Check if feature-defaults-section uses MODEL_GROUPS same way as ai-enhancement
rg -A 5 "MODEL_GROUPS" apps/ui/src/components/views/settings-view/feature-defaults/Repository: AutoMaker-Org/automaker
Length of output: 1577
Move MODEL_GROUPS to a shared location within the UI layer.
This import violates architectural boundaries by importing from board-view into settings-view. Since MODEL_GROUPS is UI-specific presentation logic (not domain types), it should be moved to apps/ui/src/components/shared/ or another appropriate shared UI location, and imported from there.
🤖 Prompt for AI Agents
In
apps/ui/src/components/views/settings-view/ai-enhancement/ai-enhancement-section.tsx
around line 5, the file imports MODEL_GROUPS from board-view which breaks UI
layer boundaries; move the MODEL_GROUPS constant into a shared UI location (for
example apps/ui/src/components/shared/model-constants.ts or similar), update its
export there, and change this file to import MODEL_GROUPS from that new shared
path; ensure any other files that used the old path are updated to the new
shared import and run a quick TypeScript compile to catch broken imports.
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={() => copyCommand(suggestion.command)} | ||
| > | ||
| <Copy className="w-4 h-4" /> | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
TypeScript error: copyCommand expects string but suggestion.command may be undefined.
The copyCommand call receives suggestion.command which TypeScript would flag as potentially undefined, even though it's guarded by the outer {suggestion.command && ...} condition. The runtime is safe, but this could cause type errors.
Suggested fix
<Button
variant="ghost"
size="icon"
- onClick={() => copyCommand(suggestion.command)}
+ onClick={() => suggestion.command && copyCommand(suggestion.command)}
>Or use non-null assertion if you're confident in the guard: copyCommand(suggestion.command!)
📝 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.
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={() => copyCommand(suggestion.command)} | |
| > | |
| <Copy className="w-4 h-4" /> | |
| </Button> | |
| </div> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={() => suggestion.command && copyCommand(suggestion.command)} | |
| > | |
| <Copy className="w-4 h-4" /> | |
| </Button> | |
| </div> |
🤖 Prompt for AI Agents
In apps/ui/src/components/views/setup-view/steps/cli-setup-step.tsx around lines
570 to 577, TypeScript reports that copyCommand expects a string but
suggestion.command may be undefined; update the call site to satisfy the type
system by either asserting non-null (use the non-null assertion on
suggestion.command) or coalescing to a safe string (pass suggestion.command ??
'' or String(suggestion.command)), or adjust copyCommand's parameter type to
accept string | undefined; prefer the non-null assertion if the surrounding
guard guarantees presence.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdRELEASE_NOTES_v0.8.0.mdapps/server/package.jsonapps/ui/package.json
✅ Files skipped from review due to trivial changes (2)
- apps/ui/package.json
- RELEASE_NOTES_v0.8.0.md
🔇 Additional comments (4)
CHANGELOG.md (2)
31-31: Verify file extension for.codex/AGENTS.nd.The documentation lists both
.codex/AGENTS.ndand~/.codex/AGENTS.mdwith inconsistent extensions. Clarify whether.ndis intentional (custom format) or should be.md(markdown). If.ndis a valid format, document the expected structure and parsing logic.
1-99: Overall CHANGELOG structure and content look solid.The file follows Keep a Changelog conventions well, with clear sections and good alignment to the PR objectives. The comprehensive feature breakdown, backward compatibility note, and statistics summary make this a professional release entry.
Minor observations:
- Consider expanding the "Changed" section (lines 63–68) with a few concrete examples of which files/modules were affected (e.g., "provider-factory.ts, codex-provider.ts, type definitions").
- The PR objectives mention "User guide and tutorial remain TODO"—consider adding a note in the "Developer Experience" section or a new "Known Limitations" section to set expectations.
apps/server/package.json (2)
3-3: LGTM! Appropriate version bump for feature addition.The minor version increment from 0.7.3 to 0.8.0 correctly reflects the addition of the new Codex provider functionality while maintaining backward compatibility.
42-42: openai@6.15.0 is a legitimate npm package with no known vulnerabilities.Version 6.15.0 is available on npm (335 versions published) and maintains a clean security record. No direct or indirect vulnerabilities reported by Snyk or other vulnerability databases.
CHANGELOG.md
Outdated
| - **Hybrid Execution Modes**: | ||
| - **CLI Mode**: For tool-enabled requests with MCP servers and file operations | ||
| - **SDK Mode**: Optimized for simple queries using OpenAI SDK directly (faster, lower latency) | ||
| - **Model Support**: Added support for GPT-5.2, GPT-5.1 (Max, Codex, Mini) models with tier classifications (premium, standard, basic) |
There was a problem hiding this comment.
Clarify model naming and tier classification.
The phrasing "GPT-5.2, GPT-5.1 (Max, Codex, Mini)" is ambiguous—it's unclear whether Max, Codex, and Mini are variants of GPT-5.1 only, or distinct model families. Restructure to explicitly show the model hierarchy and tier mappings.
For example: GPT-5.2 (premium tier), GPT-5.1 (Max/Codex/Mini variants with standard/basic tiers) or similar.
🤖 Prompt for AI Agents
In CHANGELOG.md around line 19, the model listing "GPT-5.2, GPT-5.1 (Max, Codex,
Mini)" is ambiguous about which variants belong to which model and their tiers;
update the line to explicitly map each model and variant to its tier(s), e.g.,
state "GPT-5.2 (premium tier); GPT-5.1 — variants: Max (standard), Codex
(standard), Mini (basic)" or similar so the hierarchy and tier classifications
are unambiguous.
CHANGELOG.md
Outdated
|
|
||
| ### Changed | ||
|
|
||
| - **Model Resolver**: Extended to recognize OpenAI/Codex models (o1, o3, gpt-\* series) |
There was a problem hiding this comment.
Escape or clarify the model pattern notation.
Line 65 uses gpt-\* which may not render consistently across markdown viewers. Use backticks (`gpt-*`) or spell out gpt-4, gpt-3.5-turbo, etc., for clarity.
🤖 Prompt for AI Agents
In CHANGELOG.md around line 65, the model pattern `gpt-\*` may render
inconsistently; replace it with a clear, markdown-safe notation such as
enclosing the pattern in backticks (`gpt-*`) or explicitly listing common models
(e.g., `gpt-4`, `gpt-3.5-turbo`) to ensure correct rendering and clarity in all
viewers.
b684885 to
5b564e8
Compare
DhanushSantosh
force-pushed
the
feature/codex-cli
branch
2 times, most recently
from
5b564e8 to
4f0aa3c
Compare
…ne test validation logic.
…usage data fetching
DhanushSantosh
force-pushed
the
feature/codex-cli
branch
from
4cbb5d7 to
a5e1a97
Compare
1 participant
Codex CLI Provider Integration
Overview
Codex (OpenAI's GPT-5 series) as a new AI provider in Automaker, enabling users to choose between Claude and Codex for feature development. The implementation provides feature parity with the existing Claude provider while introducing Codex-specific capabilities like hybrid execution modes and configurable sandbox policies.
Key Features
1. Dual Provider Architecture
CodexProviderclass implementing the same interface asClaudeProviderclaude|codex)2. Hybrid Execution Modes
The Codex provider intelligently switches between two execution modes:
CLI Mode (Default for tool-enabled requests)
SDK Mode (Optimized for simple queries)
openainpm packageMode Selection Logic:
3. Authentication Methods
Two authentication pathways are supported:
CLI Authentication
codex loginto authenticate via OAuth~/.codex/auth.jsonAPI Key Authentication
OPENAI_API_KEYenvironment variable4. Codex-Specific Configuration
Three new global settings for Codex behavior:
Auto-load AGENTS.nd Instructions (
codexAutoLoadAgents)false.codex/AGENTS.ndand~/.codex/AGENTS.mdCLAUDE.mdauto-loadingSandbox Mode (
codexSandboxMode)danger-full-accessif workspace is in cloud storage (Dropbox, OneDrive, etc.)Approval Policy (
codexApprovalPolicy)5. Tool Stream Parity
Codex CLI output is transformed into the same message format as Claude:
tool_useblockstool_resultblocksthinkingblockstextblocksExample Transformation:
6. Per-Request Options Support
All execution options work with Codex:
maxTurns: Limit agent turns via--config max_turns=NallowedTools: Restrict available tools (Read, Write, Edit, Glob, Grep, Bash, WebSearch, WebFetch)outputFormat: JSON schema validation via--output-schemamcpServers: MCP server configuration written to.codex/mcp_config.jsonmcpAutoApproveTools: Controls approval policy for MCP toolsmcpUnrestrictedTools: Allows unrestricted tools when MCP is enabledconversationHistory: Multi-turn conversations with contextsdkSessionId: Resume SDK sessions usingprevious_response_id7. Setup Wizard Integration
New setup step for Codex configuration:
Installation Detection: Auto-detects
codexCLI in common paths~/.local/bin/codex/opt/homebrew/bin/codex/usr/local/bin/codex%APPDATA%\npm\codex.cmdInstallation Helper: One-click install via
npm install -g @openai/codexAuthentication Verification: Tests auth with a live API call
Skip Option: Users can skip Codex setup if only using Claude
8. Settings UI
API Keys Section
Codex Settings Section (
/settings/codex)CLI Status Section
9. Usage Tracking
New
CodexUsageServicefor tracking API consumption:data/codex-usage.jsonTechnical Implementation Details
Core Files Added
Server (Backend)
UI (Frontend)
Shared Libraries
Configuration Files
.codex/mcp_config.json(Auto-generated)MCP server configuration written when
mcpServersoption is provided:{ "mcpServers": { "server-name": { "command": "node", "args": ["server.js"], "env": { "KEY": "value" } } } }.codex/output-schema.json(Auto-generated)JSON schema written when
outputFormatoption is provided:{ "type": "object", "properties": { "result": { "type": "string" } }, "required": ["result"] }.codex/AGENTS.nd(User-created, optional)Project-specific instructions loaded when
codexAutoLoadAgentsis enabled:Provider Factory Changes
provider-factory.tsnow supports provider selection:All routes updated to accept
providerparameter:/agent/execute- Feature execution/enhance-prompt- Prompt enhancement/features/generate-title- Title generation/app-spec/generate-spec- Spec generation/app-spec/generate-features-from-spec- Feature extraction/context/describe-file- File description/context/describe-image- Image analysis/suggestions/generate- Suggestion generation/github/validate-issue- Issue validationModel Resolver Updates
New model aliases for Codex:
Model definitions with tiers:
Settings Migration
New fields in
GlobalSettings:New
ModelProvidertype:Updated
AIProfileinterface:Test Coverage
New test files:
codex-provider.test.ts- Provider initialization, model listing, installation detection (290 lines)codex-tool-mapping.test.ts- Tool name resolution, todo parsing (87 lines)verify-codex-auth.test.ts- Auth verification, error handling (156 lines)system-paths.test.ts- Codex path detection (58 lines)Updated test files:
provider-factory.test.ts- Codex provider creation testsBreaking Changes
None
This is a purely additive feature. All existing functionality with Claude provider remains unchanged.
Migration Notes
Dependencies Added
openai(npm package) - For SDK mode execution^4.77.3(already in package.json)Usage Examples
1. Basic Feature Execution with Codex
2. Codex with MCP Servers
3. SDK Mode (No Tools)
4. Output Schema Validation
Configuration Examples
User Instructions (
~/.codex/AGENTS.md)Project Instructions (
.codex/AGENTS.nd)Settings JSON
{ "codexAutoLoadAgents": true, "codexSandboxMode": "workspace-write", "codexApprovalPolicy": "on-request", "defaultProvider": "codex", "profiles": [ { "name": "Codex Fast", "model": "gpt-5.1-codex-mini", "provider": "codex", "thinkingLevel": "low", "isBuiltIn": false } ] }Error Handling
Authentication Errors
The implementation handles various auth failure scenarios:
Billing Issues
Rate Limits
Missing Auth
Invalid API Key
Execution Errors
Sandbox Violations
workspace-writemodedanger-full-accessif neededMCP Server Failures
Tool Execution Failures
tool_resultblocksPerformance Considerations
CLI Mode
SDK Mode
Mode Selection Impact
Security Considerations
Sandbox Safety
Sandbox mode disabled due to cloud storage locationAPI Key Storage
data/credentials.json(encrypted at rest)Command Execution
File Access
.codex/directory onlyTesting Strategy
Unit Tests
Integration Tests
Manual Testing
Future Enhancements
Potential Improvements
Known Limitations
Documentation Updates
Files Modified
docs/server/providers.md- Added Codex provider documentationdocs/codex-parity-todo.md- Feature parity checklist (all items complete)New Documentation Needed
Migration Path for Users
1. Existing Claude Users
2. New Users
3. Switching Providers
Rollback Plan
If issues arise:
defaultProvider: 'claude'in settingsChecklist
Implementation
CodexProvider)CodexSdkClient)UI
Testing
Documentation
Deployment
Statistics
Code Changes
New Files
Modified Files
Conclusion
This PR successfully integrates Codex as a first-class AI provider in Automaker, maintaining full feature parity with Claude while introducing Codex-specific optimizations. The hybrid execution mode provides performance benefits, and the comprehensive settings UI gives users fine-grained control over Codex behavior.
All items from
docs/codex-parity-todo.mdare complete:Summary by CodeRabbit
New Features
Changes
✏️ Tip: You can customize this high-level summary in your review settings.