V0.10.0rc #405
V0.10.0rc #405
Conversation
…SON-RPC API - Implemented a new method to retrieve usage data from the Codex app-server, providing real-time data and improving reliability. - Updated the fetchUsageData method to prioritize app-server data over fallback methods. - Added detailed logging for better traceability and debugging. - Removed unused methods related to OpenAI API usage and Codex CLI requests, streamlining the service. These changes enhance the functionality and robustness of the CodexUsageService, ensuring accurate usage statistics retrieval.
- Deleted the AI profile management feature, including all associated views, hooks, and types. - Updated settings and navigation components to remove references to AI profiles. - Adjusted local storage and settings synchronization logic to reflect the removal of AI profiles. - Cleaned up tests and utility functions that were dependent on the AI profile feature. These changes streamline the application by eliminating unused functionality, improving maintainability and reducing complexity.
refactor: remove AI profile functionality and related components
…gement - Bumped version numbers for @automaker/server and @automaker/ui to 0.9.0 in package-lock.json. - Introduced CodexAppServerService and CodexModelCacheService to manage communication with the Codex CLI's app-server and cache model data. - Updated CodexUsageService to utilize app-server for fetching usage data. - Enhanced Codex routes to support fetching available models and integrated model caching. - Improved UI components to dynamically load and display Codex models, including error handling and loading states. - Added new API methods for fetching Codex models and integrated them into the app store for state management. These changes improve the overall functionality and user experience of the Codex integration, ensuring efficient model management and data retrieval.
- Eliminated CodexCreditsSnapshot interface and related logic from CodexUsageService and UI components. - Updated CodexUsageSection to display only plan type, removing credits information for a cleaner interface. - Streamlined Codex usage formatting functions by removing unused credit formatting logic. These changes simplify the Codex usage management by focusing on plan types, enhancing clarity and maintainability.
Move .codex/config.toml to .gitignore to prevent accidental commits of API keys. The file will remain local to each user's setup. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add error logging to CodexProvider auth check instead of silent failure - Fix cachedAt timestamp to return actual cache time instead of request time - Replace misleading hardcoded rate limit values (100) with sentinel value (-1) - Fix unused parameter warning in codex routes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
feat: improve codex plan and usage detection
…-in-dep fix: security vulnerability in server
webdevcody
added
the
Do Not Merge
📝 WalkthroughWalkthroughAdds Codex app‑server JSON‑RPC client and TTL model cache; removes AI Profiles and associated UI/tests; adds OpenCode provider updates; implements worktree init‑script service/routes/UI and editor discovery; dashboard, memory, pipeline templates, bulk delete, auto‑mode resume, many UI adjustments, and corresponding type/API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (UI)
participant Server as Automaker Server
participant Cache as CodexModelCacheService
participant AppServer as CodexAppServerService
participant CLI as Codex CLI
UI->>Server: GET /api/codex/models?refresh=true
Server->>Cache: check cache (cachedAt, TTL, forceRefresh?)
alt cache valid & not force-refresh
Cache-->>Server: return cached models
Server-->>UI: 200 { models, cachedAt }
else cache miss or refresh
Server->>AppServer: executeJsonRpc(requestFn => sendRequest('model/list'))
AppServer->>CLI: spawn Codex CLI (app-server)
CLI-->>AppServer: JSON‑RPC stdout lines
AppServer-->>Server: AppServerModelResponse
Server->>Cache: persist models (atomic write), update cachedAt
Server-->>UI: 200 { models, cachedAt }
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 @webdevcody, 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 significant changes to the application's AI model management and Codex CLI integration. The 'AI Profiles' feature has been entirely removed, streamlining the model selection process. Concurrently, the integration with the Codex CLI has been substantially upgraded to leverage its 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 a significant and well-executed refactoring. It removes the hardcoded 'AI Profiles' feature and replaces it with a dynamic model fetching system for Codex, which is a great architectural improvement. The new services for interacting with the codex app-server and caching models are robust and well-designed. Additionally, the cleanup of numerous console.log statements in favor of a structured logger improves maintainability. I've identified a couple of areas for improvement: a potential infinite loop in a React effect and an opportunity to improve error logging in the JWT parsing logic. Overall, this is a solid update that simplifies the user experience and modernizes the model management system.
| useEffect(() => { | ||
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) { | ||
| fetchCodexModels(); | ||
| } | ||
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]); |
There was a problem hiding this comment.
This useEffect could cause an infinite loop of fetch requests if fetchCodexModels fails. When an error occurs, codexModelsLoading becomes false and codexModels.length remains 0, which will trigger this effect again on the next render. To prevent this, you should also check if there's an existing error before attempting to fetch, and include codexModelsError in the dependency array.
| useEffect(() => { | |
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) { | |
| fetchCodexModels(); | |
| } | |
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]); | |
| useEffect(() => { | |
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading && !codexModelsError) { | |
| fetchCodexModels(); | |
| } | |
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels, codexModelsError]); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/ui/src/components/views/settings-view/codex/codex-usage-section.tsx (1)
1-1: Remove@ts-nocheckand resolve underlying type errors.This directive suppresses all TypeScript type checking in the file. With
strict: trueenabled intsconfig.json, investigate and fix the specific type issues rather than blanket-suppressing them.apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
77-90: Avoid hard-coding'codex-gpt-5.2-codex'as a fallback default.If
codexModelsis empty (or stale), selecting the Codex provider will set a potentially invalid model id. Prefer:
- “don’t change selected model” until models are loaded, or
- kick off
fetchCodexModels(true)and select once available, or- store a “last selected codex model” and fall back to that.
86-89: AddresolveModelString()to normalize the Claude default model selection.Line 88 passes the alias
'sonnet'directly toonModelSelect(). Per repo guidelines for TypeScript files, model aliases should be converted to full model names usingresolveModelString()from@automaker/model-resolver. Update to:onModelSelect(resolveModelString('sonnet'));Add the import:
import { resolveModelString } from '@automaker/model-resolver';apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
188-199: Type safety issue: Unsafe cast toModelAliaswhen value can beCursorModelIdorCodexModelId.The
modelstate variable is typed asModelAlias(Claude aliases only), butPhaseModelEntry.modelcan beModelAlias | CursorModelId | CodexModelId. The cast at line 191 (setModel(entry.model as ModelAlias)) is unsafe and prevents Cursor or Codex models from being selected in the mass-edit dialog. Either:
- Widen the
modelstate type toModelAlias | CursorModelId | CodexModelId, or- Validate that only Claude models are supported and document the limitation.
Note:
resolveModelString()should not be applied here. The codebase intentionally persists model values in mixed format (aliases for Claude, full IDs for others) and resolves them only at execution time (getChatModel()).
🤖 Fix all issues with AI agents
In @apps/server/src/routes/codex/index.ts:
- Around line 59-87: The /models route handler (router.get('/models')) returns
503 when models are unavailable which is inconsistent with the /usage endpoint;
change the unavailability branch that currently sends res.status(503).json(...)
to instead return a 200 JSON payload matching /usage (i.e., res.json({ success:
false, error: 'Codex CLI not available or not authenticated', message: "..."}))
so the UI handling is consistent; keep the catch block returning 500 for server
errors and continue using modelCacheService.getModelsWithMetadata as before.
In @apps/server/src/services/codex-usage-service.ts:
- Around line 156-180: The code assigns -1 sentinel values for
limit/used/remaining on result.rateLimits.primary and .secondary which can break
UI consumers; change those fields to null (or undefined) instead of -1 and
update the CodexRateLimitWindow type so limit, used, and remaining are optional
or nullable, then update the construction in the block that reads
rateLimitsResult to set limit/used/remaining = null (or omit them) for both
primary and secondary, and run a quick search for any consumers of
result.rateLimits (or functions referencing CodexRateLimitWindow) to ensure they
handle null/undefined values or adjust those consumers accordingly.
In @apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx:
- Around line 152-153: The component is subscribing to the whole Zustand store
causing unnecessary re-renders; change the useAppStore call to select only
defaultPlanningMode and defaultRequirePlanApproval and use shallow comparison.
Specifically, update the call to useAppStore(...) to pass a selector that
returns { defaultPlanningMode, defaultRequirePlanApproval } and import/pass
shallow (from 'zustand/shallow') as the equality function so only those two
values trigger re-renders.
In @apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx:
- Around line 15-18: The code incorrectly casts PhaseModelEntry.model to
ModelAlias when handling PhaseModelSelector output; instead widen the local
state used by setModel to a broader type (e.g., string or a shared "agent model"
union) so you no longer assert entry.model as ModelAlias, update the state
declaration and any handlers that call setModel to accept string/AgentModel
instead of ModelAlias, and ensure Feature.model remains consistent by
converting/validating the model value only where you deterministically require a
Claude-only ModelAlias (using isCursorModel or explicit checks) rather than
global casts.
In @apps/ui/src/components/views/board-view/shared/model-selector.tsx:
- Around line 45-51: The useEffect mount block calls fetchCodexModels() without
handling rejections which can produce unhandled promise errors; update the
effect to call fetchCodexModels inside an async IIFE or attach a .catch handler
and log or handle the error (e.g., set loading/error state) so failures from
fetchCodexModels are caught; keep the existing dependency list
(isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels) and
reference the same symbols (useEffect, fetchCodexModels, isCodexAvailable,
codexModels, codexModelsLoading) when adding the try/catch or .catch.
In @apps/ui/src/components/views/github-issues-view.tsx:
- Around line 103-104: The model property is currently hardcoded as 'opus';
import resolveModelString from '@automaker/model-resolver' and replace the
literal with resolveModelString('opus') so the model alias is converted to the
canonical model name (update the model assignment where model: 'opus' is
declared and add the import for resolveModelString at the top of the file).
In @apps/ui/src/store/app-store.ts:
- Around line 4-5: Add the missing codex.getModels signature to the ElectronAPI
type so its interface includes getModels: (refresh?: boolean) => Promise<{
success: boolean; models?: string[]; error?: string }>, and in the
fetchCodexModels function (which currently calls getElectronAPI()) wrap or gate
the call with an SSR check (e.g., only call getElectronAPI() when typeof window
!== 'undefined' or ensure fetchCodexModels is invoked from client-only code like
a useEffect) so getElectronAPI() is not called during SSR.
🧹 Nitpick comments (9)
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
94-101: Silent error handling may hide issues during development.The
.catch(() => {})swallows all errors without any logging, making it difficult to debug issues when Codex model fetching fails. Consider at minimum logging the error for debugging purposes.💡 Suggested improvement
useEffect(() => { if (codexModels.length === 0 && !codexModelsLoading) { fetchCodexModels().catch(() => { - // Silently fail - user will see empty Codex section + // User will see empty Codex section + console.debug('[PhaseModelSelector] Failed to fetch Codex models'); }); } }, [codexModels.length, codexModelsLoading, fetchCodexModels]);apps/server/src/services/codex-model-cache-service.ts (2)
148-175: Silent empty array returns may hide service issues.When
appServerService.isAvailable()returns false orresponse.datais missing, the method returns an empty array without distinguishing between "service unavailable" and "no models exist". Consider logging these distinct cases for operational visibility.💡 Add differentiated logging
private async doRefresh(): Promise<CodexModel[]> { try { // Check if app-server is available const isAvailable = await this.appServerService.isAvailable(); if (!isAvailable) { + logger.info('[doRefresh] App-server not available'); return []; } // Fetch models from app-server const response = await this.appServerService.getModels(); if (!response || !response.data) { + logger.info('[doRefresh] No model data in response'); return []; }
180-190: Consider derivingsupportsVisionfrom the source model data when available.Currently hardcoded as
truewith the assumption that all Codex models support vision. While this is accurate today, if app-server adds non-vision models in the future, theAppServerModelinterface would need to include asupportsVisionfield so the cache service can pass it through dynamically. For now, document this assumption clearly, or add a comment noting that app-server changes would be required to support mixed vision support.apps/server/src/services/codex-usage-service.ts (1)
130-147: Plan type normalization logic is duplicated.The pattern of normalizing
planTypeto lowercase and checking againstaccountPlanTypeArrayappears in bothfetchFromAppServer(lines 134-147) andgetPlanTypeFromAuthFile(lines 276-283). Consider extracting a helper method.♻️ Extract normalization helper
private normalizePlanType(rawPlanType: string | undefined): CodexPlanType { if (!rawPlanType) return 'unknown'; const normalized = rawPlanType.toLowerCase() as CodexPlanType; return this.accountPlanTypeArray.includes(normalized) ? normalized : 'unknown'; }apps/ui/src/routes/__root.tsx (1)
65-74: Good non-blocking Codex bootstrap; consider addingsetupCompleteas an additional guard.Right now you bootstrap as soon as
authChecked && isAuthenticatedand Codex CLI is available; if Codex status can be “temporarily” true during setup flows, you may want to also requiresetupCompleteto reduce background work/noise.Also applies to: 435-448
apps/ui/src/lib/http-api-client.ts (1)
2054-2075: Prefer a sharedCodexModeltype instead of duplicating the response shape inline.This reduces drift risk between server/store/UI types (especially for fields like
tier,supportsVision,isDefault).libs/types/src/codex-app-server.ts (1)
70-87: Make JSON-RPC id type/spec compliance explicit (and consider a discriminatedresult|errorunion).If Codex app-server always uses numeric ids, add a comment stating that; otherwise consider:
id: string | number | nulltype JsonRpcResponse<T> = { id: ...; result: T; error?: never } | { id: ...; error: ...; result?: never }apps/server/src/services/codex-app-server-service.ts (2)
28-31: Cache the discovered CLI path consistently (avoid repeated findCodexCliPath calls).
Right nowisAvailable()caches, butexecuteJsonRpc()doesn’t persist the discoveredcliPathback tothis.cachedCliPath, so callers that don’t callisAvailable()first will re-probe each time.Proposed patch
- const cliPath = this.cachedCliPath || (await findCodexCliPath()); + const cliPath = this.cachedCliPath || (await findCodexCliPath()); + this.cachedCliPath = cliPath;Also applies to: 84-88
36-64: Public RPC wrappers look fine; consider surfacing “force refresh” for models if the UI expects it.
getModels()always requestsmodel/listwith{}and returns cached/CLI results only; if you need parity with UIforceRefresh, this is where it would hook in.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
.gitignoreapps/server/src/index.tsapps/server/src/lib/codex-auth.tsapps/server/src/providers/codex-provider.tsapps/server/src/routes/codex/index.tsapps/server/src/services/codex-app-server-service.tsapps/server/src/services/codex-model-cache-service.tsapps/server/src/services/codex-usage-service.tsapps/server/src/services/settings-service.tsapps/server/src/types/settings.tsapps/ui/scripts/setup-e2e-fixtures.mjsapps/ui/src/components/layout/sidebar.tsxapps/ui/src/components/layout/sidebar/constants.tsapps/ui/src/components/layout/sidebar/hooks/use-navigation.tsapps/ui/src/components/ui/keyboard-map.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/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/shared/index.tsapps/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/board-view/shared/profile-select.tsxapps/ui/src/components/views/board-view/shared/profile-typeahead.tsxapps/ui/src/components/views/github-issues-view.tsxapps/ui/src/components/views/profiles-view.tsxapps/ui/src/components/views/profiles-view/components/index.tsapps/ui/src/components/views/profiles-view/components/profile-form.tsxapps/ui/src/components/views/profiles-view/components/profiles-header.tsxapps/ui/src/components/views/profiles-view/components/sortable-profile-card.tsxapps/ui/src/components/views/profiles-view/constants.tsapps/ui/src/components/views/profiles-view/utils.tsapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/codex/codex-usage-section.tsxapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/wiki-view.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/lib/codex-usage-format.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/routes/__root.tsxapps/ui/src/routes/profiles.tsxapps/ui/src/store/app-store.tsapps/ui/tests/profiles/profiles-crud.spec.tsapps/ui/tests/utils/core/constants.tsapps/ui/tests/utils/git/worktree.tsapps/ui/tests/utils/index.tsapps/ui/tests/utils/project/setup.tsapps/ui/tests/utils/views/profiles.tsdocs/settings-api-migration.mdlibs/types/src/codex-app-server.tslibs/types/src/index.tslibs/types/src/settings.ts
💤 Files with no reviewable changes (29)
- apps/ui/src/components/layout/sidebar/constants.ts
- apps/ui/src/components/views/board-view/shared/profile-quick-select.tsx
- apps/server/src/types/settings.ts
- apps/ui/tests/utils/project/setup.ts
- apps/ui/tests/utils/index.ts
- apps/ui/src/components/views/board-view/shared/index.ts
- apps/ui/src/components/layout/sidebar/hooks/use-navigation.ts
- apps/ui/scripts/setup-e2e-fixtures.mjs
- apps/ui/src/hooks/use-settings-sync.ts
- apps/ui/src/routes/profiles.tsx
- apps/ui/src/components/views/board-view.tsx
- apps/ui/src/components/views/profiles-view/components/sortable-profile-card.tsx
- apps/ui/src/components/views/profiles-view.tsx
- apps/ui/tests/profiles/profiles-crud.spec.ts
- apps/ui/src/components/views/profiles-view/components/profile-form.tsx
- apps/ui/src/components/views/profiles-view/utils.ts
- apps/ui/src/components/views/wiki-view.tsx
- apps/ui/src/components/views/profiles-view/components/profiles-header.tsx
- apps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx
- apps/ui/tests/utils/core/constants.ts
- apps/ui/src/components/views/settings-view.tsx
- apps/ui/src/components/views/board-view/shared/profile-select.tsx
- apps/ui/src/components/views/profiles-view/constants.ts
- apps/ui/src/components/views/profiles-view/components/index.ts
- apps/ui/src/components/views/board-view/shared/profile-typeahead.tsx
- apps/server/src/services/settings-service.ts
- apps/ui/tests/utils/git/worktree.ts
- apps/ui/src/components/ui/keyboard-map.tsx
- apps/ui/tests/utils/views/profiles.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/codex-app-server.tsapps/server/src/services/codex-app-server-service.tsapps/ui/src/routes/__root.tsxlibs/types/src/index.tsapps/ui/src/components/layout/sidebar.tsxapps/ui/src/lib/http-api-client.tsapps/ui/src/lib/electron.tsapps/server/src/routes/codex/index.tsapps/ui/src/components/views/github-issues-view.tsxapps/server/src/lib/codex-auth.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/lib/codex-usage-format.tsapps/ui/src/components/views/settings-view/codex/codex-usage-section.tsxapps/server/src/services/codex-model-cache-service.tsapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/server/src/index.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxlibs/types/src/settings.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/services/codex-usage-service.tsapps/server/src/providers/codex-provider.tsapps/ui/src/store/app-store.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/codex-app-server.tsapps/server/src/services/codex-app-server-service.tsapps/ui/src/routes/__root.tsxlibs/types/src/index.tsapps/ui/src/components/layout/sidebar.tsxapps/ui/src/lib/http-api-client.tsapps/ui/src/lib/electron.tsapps/server/src/routes/codex/index.tsapps/ui/src/components/views/github-issues-view.tsxapps/server/src/lib/codex-auth.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/lib/codex-usage-format.tsapps/ui/src/components/views/settings-view/codex/codex-usage-section.tsxapps/server/src/services/codex-model-cache-service.tsapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/server/src/index.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxlibs/types/src/settings.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/server/src/services/codex-usage-service.tsapps/server/src/providers/codex-provider.tsapps/ui/src/store/app-store.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/codex-app-server-service.tsapps/server/src/routes/codex/index.tsapps/server/src/lib/codex-auth.tsapps/server/src/services/codex-model-cache-service.tsapps/server/src/index.tsapps/server/src/services/codex-usage-service.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 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:
libs/types/src/codex-app-server.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/routes/__root.tsxapps/ui/src/components/layout/sidebar.tsxapps/ui/src/components/views/github-issues-view.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/settings-view/codex/codex-usage-section.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.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: Organize server code following modular pattern with routes/, services/, providers/, and lib/ directories
Applied to files:
apps/server/src/index.ts
🧬 Code graph analysis (9)
apps/server/src/services/codex-app-server-service.ts (1)
libs/types/src/codex-app-server.ts (4)
AppServerModelResponse(12-15)AppServerAccountResponse(36-39)AppServerRateLimitsResponse(51-53)JsonRpcRequest(70-74)
apps/ui/src/routes/__root.tsx (1)
apps/ui/src/store/setup-store.ts (1)
useSetupStore(221-292)
apps/ui/src/components/views/github-issues-view.tsx (1)
apps/ui/src/store/app-store.ts (1)
useAppStore(1195-3088)
apps/ui/src/components/views/settings-view/codex/codex-usage-section.tsx (1)
apps/ui/src/lib/codex-usage-format.ts (1)
formatCodexPlanType(72-75)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
apps/ui/src/components/views/board-view/dialogs/index.ts (1)
MassEditDialog(10-10)
apps/ui/src/components/views/board-view/shared/model-selector.tsx (6)
libs/types/src/index.ts (2)
ModelOption(145-145)ModelProvider(115-115)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
ModelOption(11-18)libs/types/src/model-display.ts (1)
ModelOption(17-28)apps/server/src/types/settings.ts (1)
ModelProvider(14-14)libs/types/src/settings.ts (1)
ModelProvider(102-102)apps/ui/src/lib/utils.ts (1)
cn(6-8)
apps/ui/src/hooks/use-settings-migration.ts (2)
apps/server/src/routes/github/routes/validation-common.ts (1)
logger(174-174)apps/server/src/routes/settings/common.ts (1)
logger(12-12)
apps/server/src/services/codex-usage-service.ts (1)
apps/server/src/services/codex-app-server-service.ts (1)
CodexAppServerService(22-212)
apps/ui/src/store/app-store.ts (2)
apps/ui/src/lib/http-api-client.ts (1)
get(768-795)apps/ui/src/lib/electron.ts (1)
getElectronAPI(902-911)
🔇 Additional comments (21)
.gitignore (1)
76-77: ✅ Good security practice: ignore Codex config with API keys.The pattern correctly excludes the Codex configuration file to prevent accidental commits of sensitive credentials. The specific path pattern and descriptive comment are clear.
Consider whether other files in the
.codex/directory should also be ignored, or if this pattern is complete. If the directory may contain other sensitive data, a broader pattern like.codex/might be safer.apps/ui/src/components/layout/sidebar.tsx (1)
62-63: LGTM! Clean removal of AI Profiles feature flag.The removal of
hideAiProfilesfrom the destructuring is consistent with the PR objective to remove the AI Profiles feature. The flag is not referenced elsewhere in this file, and the change integrates cleanly with the related updates touseNavigationandSIDEBAR_FEATURE_FLAGS.apps/server/src/providers/codex-provider.ts (1)
662-662: No duplicate logger initialization exists. The file contains only oneconst logger = createLogger('CodexProvider');statement at line 662. The othercreateLoggermatch is the import statement. The original concern is unfounded.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (3)
172-181: Tier badge logic differs from server-sideinferTierimplementation.The server's
CodexModelCacheService.inferTier()checks for'max'or'gpt-5.2-codex'for premium,'mini'for basic, and defaults to standard. However, the UI transformation here usesmodel.tier === 'premium'andmodel.tier === 'basic'directly from the store data, which should already be transformed by the server. This looks correct assuming the server provides the tier field.
1-1: Good migration from React namespace to named imports.Using named imports (
useEffect, useMemo, useRef, useState) instead ofReact.useEffect, etc. is the modern, tree-shaking-friendly approach.
80-87: Store integration for Codex models is well-structured.The destructured store access cleanly separates Codex-related state (
codexModels,codexModelsLoading,fetchCodexModels) from existing functionality.apps/server/src/services/codex-model-cache-service.ts (2)
112-127: Thread-safe refresh deduplication is well-implemented.The
inFlightRefreshpattern correctly prevents concurrent refresh requests and ensures the promise is cleared in thefinallyblock regardless of success or failure.
231-257: Atomic write pattern is correctly implemented.Using temp file with timestamp suffix and rename ensures cache file integrity. The cleanup in the catch block handles partial write failures appropriately.
libs/types/src/settings.ts (1)
335-336: Documentation accurately reflects AI profiles removal.The GlobalSettings description update correctly removes mention of AI profiles, aligning with the feature removal across the codebase.
apps/server/src/services/codex-usage-service.ts (1)
40-52: Constructor with optional dependency injection is well-designed.Allowing
appServerServiceto be optional enables graceful degradation when the app-server is unavailable, falling back to auth file parsing.apps/ui/src/components/views/settings-view/codex/codex-usage-section.tsx (1)
206-212: Simplified plan display correctly removes credits.The conditional rendering now only checks
planType, which aligns with the removal of credits from theCodexUsageDatainterface. The UI layout is cleaner with the consolidated single-line display.docs/settings-api-migration.md (1)
174-174: Documentation accurately reflects AI profiles removal from migration logic.The migration description now correctly omits
aiProfilesfrom the list of arrays that fall back to localStorage when missing from server data.apps/server/src/routes/codex/index.ts (2)
8-11: Clean dependency injection pattern for route factory.Accepting both
usageServiceandmodelCacheServiceas parameters enables proper testing and decouples the routes from service instantiation.
62-63: Force refresh via query parameter is well-implemented.The
req.query.refresh === 'true'check provides a clean API for cache bypass when needed.libs/types/src/index.ts (1)
33-46: LGTM: clean type re-export surface for Codex app-server JSON-RPC types.apps/server/src/index.ts (1)
173-188: Service cleanup is handled internally per-request—no shutdown hooks needed.The
CodexAppServerService.executeJsonRpc()method already cleans up resources atomically: it closes the readline interface (line 195) and kills the child process (line 196) after each request, with a finally block ensuring cleanup even on errors. Since processes are spawned on-demand and cleaned up within each method call, there are no long-lived resources requiring SIGINT/SIGTERM handlers.apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (2)
37-38: Good move: model capability checks now come from shared@automaker/types.
Keeps Claude-only planning / reasoning-effort gating consistent across UI surfaces.Also applies to: 56-56
509-516: Model-only selector layout looks correct post “AI Profiles” removal.
No obvious regressions in the dialog flow from this segment alone.apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (2)
39-40: Good: capability checks centralized via@automaker/types.
171-171: Reset-to-opus + Model selector wiring looks consistent.
No profile coupling remains in this segment, and planning mode gating continues to be driven offisClaudeModel(modelEntry.model).Also applies to: 485-492
apps/ui/src/hooks/use-settings-migration.ts (1)
294-296: Logging updates align with AI Profiles removal; no sensitive data exposed.
Project counts are safe/low-noise compared to dumping payloads.Also applies to: 376-377, 406-407
| // Get available Codex models (cached) | ||
| router.get('/models', async (req: Request, res: Response) => { | ||
| try { | ||
| const forceRefresh = req.query.refresh === 'true'; | ||
| const { models, cachedAt } = await modelCacheService.getModelsWithMetadata(forceRefresh); | ||
|
|
||
| if (models.length === 0) { | ||
| res.status(503).json({ | ||
| success: false, | ||
| error: 'Codex CLI not available or not authenticated', | ||
| message: "Please install Codex CLI and run 'codex login' to authenticate", | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| res.json({ | ||
| success: true, | ||
| models, | ||
| cachedAt, | ||
| }); | ||
| } catch (error) { | ||
| logger.error('Error fetching models:', error); | ||
| const message = error instanceof Error ? error.message : 'Unknown error'; | ||
| res.status(500).json({ | ||
| success: false, | ||
| error: message, | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Inconsistent HTTP status codes between /usage and /models endpoints.
The /usage endpoint uses 200 with an error payload for unavailability (to avoid triggering logout), but /models uses 503. This inconsistency may require different error handling patterns in the UI. Consider aligning both endpoints to use the same pattern, or document the rationale for the difference.
💡 Option: Align with /usage pattern
if (models.length === 0) {
- res.status(503).json({
- success: false,
+ // Use 200 + error payload to avoid triggering Automaker session logout
+ res.status(200).json({
+ success: false,
error: 'Codex CLI not available or not authenticated',
message: "Please install Codex CLI and run 'codex login' to authenticate",
+ models: [],
});
return;
}🤖 Prompt for AI Agents
In @apps/server/src/routes/codex/index.ts around lines 59 - 87, The /models
route handler (router.get('/models')) returns 503 when models are unavailable
which is inconsistent with the /usage endpoint; change the unavailability branch
that currently sends res.status(503).json(...) to instead return a 200 JSON
payload matching /usage (i.e., res.json({ success: false, error: 'Codex CLI not
available or not authenticated', message: "..."})) so the UI handling is
consistent; keep the catch block returning 500 for server errors and continue
using modelCacheService.getModelsWithMetadata as before.
| private async executeJsonRpc<T>( | ||
| requestFn: (sendRequest: <R>(method: string, params?: unknown) => Promise<R>) => Promise<T> | ||
| ): Promise<T | null> { | ||
| let childProcess: ChildProcess | null = null; | ||
|
|
||
| try { | ||
| const cliPath = this.cachedCliPath || (await findCodexCliPath()); | ||
|
|
||
| if (!cliPath) { | ||
| return null; | ||
| } | ||
|
|
||
| // On Windows, .cmd files must be run through shell | ||
| const needsShell = process.platform === 'win32' && cliPath.toLowerCase().endsWith('.cmd'); | ||
|
|
||
| childProcess = spawn(cliPath, ['app-server'], { | ||
| cwd: process.cwd(), | ||
| env: { | ||
| ...process.env, | ||
| TERM: 'dumb', | ||
| }, | ||
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| shell: needsShell, | ||
| }); | ||
|
|
||
| if (!childProcess.stdin || !childProcess.stdout) { | ||
| throw new Error('Failed to create stdio pipes'); | ||
| } | ||
|
|
||
| // Setup readline for reading JSONL responses | ||
| const rl = readline.createInterface({ | ||
| input: childProcess.stdout, | ||
| crlfDelay: Infinity, | ||
| }); | ||
|
|
||
| // Message ID counter for JSON-RPC | ||
| let messageId = 0; | ||
| const pendingRequests = new Map< | ||
| number, | ||
| { | ||
| resolve: (value: unknown) => void; | ||
| reject: (error: Error) => void; | ||
| timeout: NodeJS.Timeout; | ||
| } | ||
| >(); | ||
|
|
||
| // Process incoming messages | ||
| rl.on('line', (line) => { | ||
| if (!line.trim()) return; | ||
|
|
||
| try { | ||
| const message = JSON.parse(line); | ||
|
|
||
| // Handle response to our request | ||
| if ('id' in message && message.id !== undefined) { | ||
| const pending = pendingRequests.get(message.id); | ||
| if (pending) { | ||
| clearTimeout(pending.timeout); | ||
| pendingRequests.delete(message.id); | ||
| if (message.error) { | ||
| pending.reject(new Error(message.error.message || 'Unknown error')); | ||
| } else { | ||
| pending.resolve(message.result); | ||
| } | ||
| } | ||
| } | ||
| // Ignore notifications (no id field) | ||
| } catch { | ||
| // Ignore parse errors for non-JSON lines | ||
| } | ||
| }); | ||
|
|
||
| // Helper to send JSON-RPC request and wait for response | ||
| const sendRequest = <R>(method: string, params?: unknown): Promise<R> => { | ||
| return new Promise((resolve, reject) => { | ||
| const id = ++messageId; | ||
| const request: JsonRpcRequest = { | ||
| method, | ||
| id, | ||
| params: params ?? {}, | ||
| }; | ||
|
|
||
| // Set timeout for request (10 seconds) | ||
| const timeout = setTimeout(() => { | ||
| pendingRequests.delete(id); | ||
| reject(new Error(`Request timeout: ${method}`)); | ||
| }, 10000); | ||
|
|
||
| pendingRequests.set(id, { | ||
| resolve: resolve as (value: unknown) => void, | ||
| reject, | ||
| timeout, | ||
| }); | ||
|
|
||
| childProcess!.stdin!.write(JSON.stringify(request) + '\n'); | ||
| }); | ||
| }; | ||
|
|
||
| // Helper to send notification (no response expected) | ||
| const sendNotification = (method: string, params?: unknown): void => { | ||
| const notification = params ? { method, params } : { method }; | ||
| childProcess!.stdin!.write(JSON.stringify(notification) + '\n'); | ||
| }; | ||
|
|
||
| // 1. Initialize the app-server | ||
| await sendRequest('initialize', { | ||
| clientInfo: { | ||
| name: 'automaker', | ||
| title: 'AutoMaker', | ||
| version: '1.0.0', | ||
| }, | ||
| }); | ||
|
|
||
| // 2. Send initialized notification | ||
| sendNotification('initialized'); | ||
|
|
||
| // 3. Execute user-provided requests | ||
| const result = await requestFn(sendRequest); | ||
|
|
||
| // Clean up | ||
| rl.close(); | ||
| childProcess.kill('SIGTERM'); | ||
|
|
||
| return result; | ||
| } catch (error) { | ||
| logger.error('[executeJsonRpc] Failed:', error); | ||
| return null; | ||
| } finally { | ||
| // Ensure process is killed | ||
| if (childProcess && !childProcess.killed) { | ||
| childProcess.kill('SIGTERM'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and its actual content
if [ -f "apps/server/src/services/codex-app-server-service.ts" ]; then
wc -l "apps/server/src/services/codex-app-server-service.ts"
echo "---"
sed -n '78,211p' "apps/server/src/services/codex-app-server-service.ts"
fiRepository: AutoMaker-Org/automaker
Length of output: 4093
🏁 Script executed:
# Find lib/events.ts and understand createEventEmitter usage
find . -name "events.ts" -type f | head -20Repository: AutoMaker-Org/automaker
Length of output: 98
🏁 Script executed:
# Check how createEventEmitter is used in other server services
rg "createEventEmitter" --type ts -B 2 -A 5 apps/server/srcRepository: AutoMaker-Org/automaker
Length of output: 1595
🏁 Script executed:
# Check CodexAppServerService constructor and class definition
sed -n '1,100p' "apps/server/src/services/codex-app-server-service.ts"Repository: AutoMaker-Org/automaker
Length of output: 3077
🏁 Script executed:
# Check how the service is instantiated in index.ts
rg "CodexAppServerService" apps/server/src --type ts -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 4884
🏁 Script executed:
# Look at the full events.ts to understand EventEmitter interface
cat "apps/server/src/lib/events.ts"Repository: AutoMaker-Org/automaker
Length of output: 1041
🏁 Script executed:
# Check if other services follow the events pattern
rg "constructor.*events" apps/server/src/services --type ts -B 1 -A 3 | head -40Repository: AutoMaker-Org/automaker
Length of output: 910
🏁 Script executed:
# Look at AgentService to see how it uses events
sed -n '1,50p' "apps/server/src/services/agent-service.ts"Repository: AutoMaker-Org/automaker
Length of output: 1468
Harden child-process lifecycle: reject pending RPCs on exit, clear all timers, handle readline/stdout close, and use platform-aware process termination.
The current flow can leave in-flight sendRequest() promises hanging if the process exits early or stdout closes, and kill('SIGTERM') is not reliably portable on Windows (especially with shell-spawned .cmd files). Additionally, readline is only closed on the success path, creating resource leaks on errors.
Key fixes:
- Add
childProcess.on('error'|'close'|'exit')andrl.on('close')/stdout.on('close')handlers to reject all pending requests and clear their timeouts. - Move
rl.close()and timeout cleanup to thefinallyblock to ensure cleanup on all paths. - Use platform-aware termination: call
.kill()on Windows,.kill('SIGTERM')on other platforms; optionally escalate toSIGKILLafter a grace period. - Inject
EventEmitterinto the constructor and emit lifecycle events (per guideline: apps/server services should stream events to frontend via WebSocket usingcreateEventEmitter()). Currently, this service never emits any events.
Suggested changes
// Process incoming messages
rl.on('line', (line) => {
if (!line.trim()) return;
@@
} catch {
// Ignore parse errors for non-JSON lines
}
});
+ const failAllPending = (err: Error) => {
+ for (const [id, pending] of pendingRequests.entries()) {
+ clearTimeout(pending.timeout);
+ pending.reject(err);
+ pendingRequests.delete(id);
+ }
+ };
+
+ childProcess.on('error', (err) => failAllPending(err instanceof Error ? err : new Error(String(err))));
+ childProcess.on('close', (code, signal) =>
+ failAllPending(new Error(`Codex app-server closed (code=${code ?? 'null'}, signal=${signal ?? 'null'})`))
+ );
+ rl.on('close', () => failAllPending(new Error('Codex app-server stdout closed')));
@@
// Clean up
rl.close();
- childProcess.kill('SIGTERM');
+ if (process.platform === 'win32') childProcess.kill();
+ else childProcess.kill('SIGTERM');
return result;
} catch (error) {
logger.error('[executeJsonRpc] Failed:', error);
return null;
} finally {
// Ensure process is killed
if (childProcess && !childProcess.killed) {
- childProcess.kill('SIGTERM');
+ if (process.platform === 'win32') childProcess.kill();
+ else childProcess.kill('SIGTERM');
}
+ // Clean up readline if still open
+ if (rl && !rl.closed) {
+ rl.close();
+ }
+ // Clear any remaining timeouts
+ for (const pending of pendingRequests.values()) {
+ clearTimeout(pending.timeout);
+ }
}| // Add rate limit info if available | ||
| if (rateLimitsResult?.rateLimits?.primary) { | ||
| const primary = rateLimitsResult.rateLimits.primary; | ||
| result.rateLimits!.primary = { | ||
| limit: -1, // Not provided by API | ||
| used: -1, // Not provided by API | ||
| remaining: -1, // Not provided by API | ||
| usedPercent: primary.usedPercent, | ||
| windowDurationMins: primary.windowDurationMins, | ||
| resetsAt: primary.resetsAt, | ||
| }; | ||
| } | ||
|
|
||
| // Add secondary rate limit if available | ||
| if (rateLimitsResult?.rateLimits?.secondary) { | ||
| const secondary = rateLimitsResult.rateLimits.secondary; | ||
| result.rateLimits!.secondary = { | ||
| limit: -1, // Not provided by API | ||
| used: -1, // Not provided by API | ||
| remaining: -1, // Not provided by API | ||
| usedPercent: secondary.usedPercent, | ||
| windowDurationMins: secondary.windowDurationMins, | ||
| resetsAt: secondary.resetsAt, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Sentinel value -1 for missing rate limit fields may cause UI issues.
Using -1 for limit, used, and remaining when "Not provided by API" could cause display issues if the UI doesn't explicitly handle this case (e.g., showing "-1 remaining" or negative progress bars). Consider using null or undefined instead, or ensure all UI consumers handle this sentinel.
💡 Alternative approach using optional fields
if (rateLimitsResult?.rateLimits?.primary) {
const primary = rateLimitsResult.rateLimits.primary;
result.rateLimits!.primary = {
- limit: -1, // Not provided by API
- used: -1, // Not provided by API
- remaining: -1, // Not provided by API
+ limit: undefined,
+ used: undefined,
+ remaining: undefined,
usedPercent: primary.usedPercent,
windowDurationMins: primary.windowDurationMins,
resetsAt: primary.resetsAt,
};
}This would require updating CodexRateLimitWindow interface to make these fields optional.
📝 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.
| // Add rate limit info if available | |
| if (rateLimitsResult?.rateLimits?.primary) { | |
| const primary = rateLimitsResult.rateLimits.primary; | |
| result.rateLimits!.primary = { | |
| limit: -1, // Not provided by API | |
| used: -1, // Not provided by API | |
| remaining: -1, // Not provided by API | |
| usedPercent: primary.usedPercent, | |
| windowDurationMins: primary.windowDurationMins, | |
| resetsAt: primary.resetsAt, | |
| }; | |
| } | |
| // Add secondary rate limit if available | |
| if (rateLimitsResult?.rateLimits?.secondary) { | |
| const secondary = rateLimitsResult.rateLimits.secondary; | |
| result.rateLimits!.secondary = { | |
| limit: -1, // Not provided by API | |
| used: -1, // Not provided by API | |
| remaining: -1, // Not provided by API | |
| usedPercent: secondary.usedPercent, | |
| windowDurationMins: secondary.windowDurationMins, | |
| resetsAt: secondary.resetsAt, | |
| }; | |
| } | |
| // Add rate limit info if available | |
| if (rateLimitsResult?.rateLimits?.primary) { | |
| const primary = rateLimitsResult.rateLimits.primary; | |
| result.rateLimits!.primary = { | |
| limit: undefined, // Not provided by API | |
| used: undefined, // Not provided by API | |
| remaining: undefined, // Not provided by API | |
| usedPercent: primary.usedPercent, | |
| windowDurationMins: primary.windowDurationMins, | |
| resetsAt: primary.resetsAt, | |
| }; | |
| } | |
| // Add secondary rate limit if available | |
| if (rateLimitsResult?.rateLimits?.secondary) { | |
| const secondary = rateLimitsResult.rateLimits.secondary; | |
| result.rateLimits!.secondary = { | |
| limit: undefined, // Not provided by API | |
| used: undefined, // Not provided by API | |
| remaining: undefined, // Not provided by API | |
| usedPercent: secondary.usedPercent, | |
| windowDurationMins: secondary.windowDurationMins, | |
| resetsAt: secondary.resetsAt, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In @apps/server/src/services/codex-usage-service.ts around lines 156 - 180, The
code assigns -1 sentinel values for limit/used/remaining on
result.rateLimits.primary and .secondary which can break UI consumers; change
those fields to null (or undefined) instead of -1 and update the
CodexRateLimitWindow type so limit, used, and remaining are optional or
nullable, then update the construction in the block that reads rateLimitsResult
to set limit/used/remaining = null (or omit them) for both primary and
secondary, and run a quick search for any consumers of result.rateLimits (or
functions referencing CodexRateLimitWindow) to ensure they handle null/undefined
values or adjust those consumers accordingly.
| const { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore(); | ||
|
|
There was a problem hiding this comment.
Avoid subscribing to the entire Zustand store here (unnecessary re-renders).
const { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore(); will re-render this dialog on any store change. Use a selector (and shallow compare) instead.
Proposed patch
- const { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore();
+ const { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore((s) => ({
+ defaultPlanningMode: s.defaultPlanningMode,
+ defaultRequirePlanApproval: s.defaultRequirePlanApproval,
+ }));📝 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 { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore(); | |
| const { defaultPlanningMode, defaultRequirePlanApproval } = useAppStore((s) => ({ | |
| defaultPlanningMode: s.defaultPlanningMode, | |
| defaultRequirePlanApproval: s.defaultRequirePlanApproval, | |
| })); |
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
around lines 152 - 153, The component is subscribing to the whole Zustand store
causing unnecessary re-renders; change the useAppStore call to select only
defaultPlanningMode and defaultRequirePlanApproval and use shallow comparison.
Specifically, update the call to useAppStore(...) to pass a selector that
returns { defaultPlanningMode, defaultRequirePlanApproval } and import/pass
shallow (from 'zustand/shallow') as the equality function so only those two
values trigger re-renders.
| import { Feature, ModelAlias, ThinkingLevel, PlanningMode } from '@/store/app-store'; | ||
| import { TestingTabContent, PrioritySelect, PlanningModeSelect } from '../shared'; | ||
| import { PhaseModelSelector } from '@/components/views/settings-view/model-defaults/phase-model-selector'; | ||
| import { isCursorModel, PROVIDER_PREFIXES, type PhaseModelEntry } from '@automaker/types'; | ||
| import { isCursorModel, type PhaseModelEntry } from '@automaker/types'; |
There was a problem hiding this comment.
Avoid casting PhaseModelEntry.model to ModelAlias unless the selector is Claude-only.
If PhaseModelSelector can emit Cursor/Codex models, setModel(entry.model as ModelAlias) is lying to the type system. Prefer widening local state to string (or a shared “agent model” type) and keep Feature.model consistent.
Also applies to: 188-199
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx around
lines 15 - 18, The code incorrectly casts PhaseModelEntry.model to ModelAlias
when handling PhaseModelSelector output; instead widen the local state used by
setModel to a broader type (e.g., string or a shared "agent model" union) so you
no longer assert entry.model as ModelAlias, update the state declaration and any
handlers that call setModel to accept string/AgentModel instead of ModelAlias,
and ensure Feature.model remains consistent by converting/validating the model
value only where you deterministically require a Claude-only ModelAlias (using
isCursorModel or explicit checks) rather than global casts.
| // Fetch Codex models on mount | ||
| useEffect(() => { | ||
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) { | ||
| fetchCodexModels(); | ||
| } | ||
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]); | ||
|
|
There was a problem hiding this comment.
Catch fetchCodexModels() errors in the mount effect to avoid unhandled rejections.
Proposed diff
useEffect(() => {
if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) {
- fetchCodexModels();
+ fetchCodexModels().catch(() => {
+ // store sets codexModelsError; avoid unhandled rejection noise
+ });
}
}, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]);📝 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.
| // Fetch Codex models on mount | |
| useEffect(() => { | |
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) { | |
| fetchCodexModels(); | |
| } | |
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]); | |
| // Fetch Codex models on mount | |
| useEffect(() => { | |
| if (isCodexAvailable && codexModels.length === 0 && !codexModelsLoading) { | |
| fetchCodexModels().catch(() => { | |
| // store sets codexModelsError; avoid unhandled rejection noise | |
| }); | |
| } | |
| }, [isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels]); |
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/board-view/shared/model-selector.tsx around
lines 45 - 51, The useEffect mount block calls fetchCodexModels() without
handling rejections which can produce unhandled promise errors; update the
effect to call fetchCodexModels inside an async IIFE or attach a .catch handler
and log or handle the error (e.g., set loading/error state) so failures from
fetchCodexModels are caught; keep the existing dependency list
(isCodexAvailable, codexModels.length, codexModelsLoading, fetchCodexModels) and
reference the same symbols (useEffect, fetchCodexModels, isCodexAvailable,
codexModels, codexModelsLoading) when adding the try/catch or .catch.
| model: 'opus', | ||
| thinkingLevel: 'none' as const, |
There was a problem hiding this comment.
Use resolveModelString() to convert model aliases.
The hardcoded 'opus' model alias should be resolved using resolveModelString() from @automaker/model-resolver to ensure consistent model name resolution across the application.
As per coding guidelines: "Use resolveModelString() from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names."
📦 Proposed fix
First, import the resolver at the top of the file:
+import { resolveModelString } from '@automaker/model-resolver';Then, update the model assignment:
const feature = {
id: `issue-${issue.number}-${crypto.randomUUID()}`,
title: issue.title,
description,
category: 'From GitHub',
status: 'backlog' as const,
passes: false,
priority: getFeaturePriority(validation.estimatedComplexity),
- model: 'opus',
+ model: resolveModelString('opus'),
thinkingLevel: 'none' as const,
branchName: currentBranch,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};🤖 Prompt for AI Agents
In @apps/ui/src/components/views/github-issues-view.tsx around lines 103 - 104,
The model property is currently hardcoded as 'opus'; import resolveModelString
from '@automaker/model-resolver' and replace the literal with
resolveModelString('opus') so the model alias is converted to the canonical
model name (update the model assignment where model: 'opus' is declared and add
the import for resolveModelString at the top of the file).
| import { getElectronAPI } from '@/lib/electron'; | ||
| import { createLogger } from '@automaker/utils/logger'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the ElectronAPI type definition
rg -n --type=ts -C5 "interface\s+ElectronAPI|type\s+ElectronAPI" | head -60Repository: AutoMaker-Org/automaker
Length of output: 1392
🏁 Script executed:
# Search for codex.getModels usage and definition
rg -n --type=ts "codex\.getModels|getModels.*codex" -C3Repository: AutoMaker-Org/automaker
Length of output: 544
🏁 Script executed:
# Find the app-store implementation of fetchCodexModels at the specified lines
fd -e ts -x wc -l {} \; apps/ui/src/store/app-store.tsRepository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Look for the codex interface/type definition
rg -n --type=ts "codex\s*:\s*\{" -C5Repository: AutoMaker-Org/automaker
Length of output: 2475
🏁 Script executed:
# Search for SSR guards or window checks around fetchCodexModels
rg -n --type=ts "typeof window|window\s*===|getElectronAPI" -C3Repository: AutoMaker-Org/automaker
Length of output: 50380
🏁 Script executed:
# Get the complete codex interface definition from electron.d.ts
sed -n '599,620p' apps/ui/src/types/electron.d.tsRepository: AutoMaker-Org/automaker
Length of output: 486
🏁 Script executed:
# Check the full fetchCodexModels implementation in app-store.ts
sed -n '2939,2960p' apps/ui/src/store/app-store.tsRepository: AutoMaker-Org/automaker
Length of output: 717
🏁 Script executed:
# Search for codex.getModels definition in the entire codebase
rg -n "codex.*getModels|getModels.*=.*async" --type=tsRepository: AutoMaker-Org/automaker
Length of output: 252
🏁 Script executed:
# Look for where codex is extended or modified beyond the type definition
rg -n "codex\s*=|codex\s*\{" --type=ts -B2 -A5 | head -80Repository: AutoMaker-Org/automaker
Length of output: 1167
Fix type definition mismatch and add SSR guard to fetchCodexModels.
The codex.getModels() method exists in the HTTP API client implementation but is missing from the ElectronAPI type definition in apps/ui/src/types/electron.d.ts (currently only codex.getUsage() is declared). Additionally, fetchCodexModels() calls getElectronAPI() without an SSR guard—getElectronAPI() throws with "Cannot get API during SSR" when typeof window === 'undefined'.
Two fixes required:
- Add
getModels: (refresh?: boolean) => Promise<{ success: boolean; models?: string[]; error?: string }>to thecodexinterface in the ElectronAPI type definition. - Guard the
getElectronAPI()call infetchCodexModels()withtypeof window !== 'undefined'or call from client-only code (e.g., effect hooks withuseEffectwithout SSR dependencies).
🤖 Prompt for AI Agents
In @apps/ui/src/store/app-store.ts around lines 4 - 5, Add the missing
codex.getModels signature to the ElectronAPI type so its interface includes
getModels: (refresh?: boolean) => Promise<{ success: boolean; models?: string[];
error?: string }>, and in the fetchCodexModels function (which currently calls
getElectronAPI()) wrap or gate the call with an SSR check (e.g., only call
getElectronAPI() when typeof window !== 'undefined' or ensure fetchCodexModels
is invoked from client-only code like a useEffect) so getElectronAPI() is not
called during SSR.
- Added a new DashboardView component for improved project management. - Updated sidebar navigation to redirect to the dashboard instead of the home page. - Removed ProjectActions from the sidebar for a cleaner interface. - Enhanced BoardView to conditionally render the WorktreePanel based on visibility settings. - Introduced worktree panel visibility management per project in the app store. - Updated project settings to include worktree panel visibility and favorite status. - Adjusted navigation logic to ensure users are directed to the appropriate view based on project state.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/hooks/use-settings-migration.ts (2)
289-307: Logging change looks fine; consider log-level/noise.Project-count logging is useful, but it’s currently
infoand happens on startup + migrations. If this is too noisy in real usage, considerdebug(or guard behind a dev flag).Also applies to: 374-407
497-510: NormalizeisFavoriteto a boolean during hydration.
ref.isFavoritemay beundefined(older settings / partial data). Coerce tofalseso downstream UI/state doesn’t need to special-case.Proposed fix
- const projects = (settings.projects ?? []).map((ref) => ({ + const projects = (settings.projects ?? []).map((ref) => ({ id: ref.id, name: ref.name, path: ref.path, lastOpened: ref.lastOpened, theme: ref.theme, - isFavorite: ref.isFavorite, + isFavorite: Boolean(ref.isFavorite), features: [], // Features are loaded separately when project is opened }));
🤖 Fix all issues with AI agents
In @apps/ui/src/components/views/dashboard-view.tsx:
- Around line 1-41: The code uses the React namespace for event typing (e.g.
React.MouseEvent) without importing React which can break TS; import the
MouseEvent type from React and update occurrences (e.g. replace
React.MouseEvent<HTMLButtonElement> with MouseEvent<HTMLButtonElement> or import
type { MouseEvent } from 'react') for all handlers in this file (including the
handlers around the section that spans the NewProjectModal/WorkspacePickerModal
actions). Ensure you add a single `import type { MouseEvent } from 'react'` at
the top and update the function parameter types where React.MouseEvent is
referenced.
- Around line 216-303: The code builds file paths with string concatenation in
handleCreateBlankProject (projectPath and the .automaker file path), which
breaks on Windows; replace those concatenations with platform-safe path joins
(e.g., import path from 'path' and use path.join(parentDir, projectName) for
projectPath and path.join(projectPath, '.automaker', 'app_spec.txt') for the
writeFile target), and update any other `${projectPath}/...` usages in that
function to path.join to ensure correct separators across OSes.
- Around line 304-461: Both handleCreateFromTemplate and
handleCreateFromCustomUrl build the app_spec.txt path with string interpolation
(`\`${projectPath}/.automaker/app_spec.txt\``) which breaks on Windows; change
those to build the path using a path-join utility (e.g., Node's
path.join(projectPath, '.automaker', 'app_spec.txt') or an exposed Electron API
like api.pathJoin) and pass that joined path into the existing api.writeFile
calls so the path is correct across platforms.
In @apps/ui/src/routes/__root.tsx:
- Around line 429-441: The redirect in useEffect should wait for auth/setup
resolution to avoid churn: add the auth readiness guard (authChecked) and a gate
for user allowed into app (e.g., isAuthenticated && setupComplete or a provided
isAllowedToEnterApp flag) to the conditional and dependency list; update the
useEffect that currently references isMounted, location.pathname,
currentProject, navigate to only run when authChecked is true and the user is
allowed, then perform the same navigate({ to: '/board' }) or navigate({ to:
'/dashboard' }) logic.
🧹 Nitpick comments (3)
apps/ui/src/hooks/use-project-settings-loader.ts (1)
76-89: Missing dependency in useEffect.The
setWorktreePanelVisiblesetter is used inside the effect but not included in the dependency array. While Zustand setters are typically stable references, for consistency with the existing pattern (other setters aren't listed either), this is acceptable. However, consider adding all setters to the dependency array for React best practices, or suppress the lint warning explicitly if this is intentional.🔧 Option: Add setters to dependency array for completeness
loadProjectSettings(); - }, [currentProject?.path]); + }, [ + currentProject?.path, + setBoardBackground, + setCardOpacity, + setColumnOpacity, + setColumnBorderEnabled, + setCardGlassmorphism, + setCardBorderEnabled, + setCardBorderOpacity, + setHideScrollbar, + setWorktreePanelVisible, + ]); }apps/ui/src/lib/http-api-client.ts (1)
2054-2076: Avoid type drift:codex.getModels()return type is duplicated inline.The API addition looks fine, but the inline
modelsshape (tier,hasThinking,supportsVision, etc.) will likely be repeated (store/UI). Consider exporting a sharedCodexModeltype from@automaker/types(or another shared package) and reusing it here + in the store to prevent drift.apps/ui/src/routes/__root.tsx (1)
84-88:isDashboardRouteshould likely support nested dashboard routes.If you ever introduce nested dashboard routes (e.g.
/dashboard/foo), the exact match will fall back to the “sidebar” layout unexpectedly.Proposed fix
- const isDashboardRoute = location.pathname === '/dashboard'; + const isDashboardRoute = location.pathname === '/dashboard' || location.pathname.startsWith('/dashboard/');Also applies to: 524-540
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/ui/src/components/layout/sidebar.tsxapps/ui/src/components/layout/sidebar/components/automaker-logo.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/components/views/dashboard-view.tsxapps/ui/src/components/views/setup-view.tsxapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/routes/__root.tsxapps/ui/src/routes/dashboard.tsxapps/ui/src/store/app-store.tslibs/types/src/settings.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/src/lib/electron.ts
- apps/ui/src/components/views/board-view.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/layout/sidebar.tsxapps/ui/src/routes/dashboard.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/routes/__root.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/layout/sidebar/components/automaker-logo.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/setup-view.tsxapps/ui/src/components/views/dashboard-view.tsxapps/ui/src/lib/http-api-client.tslibs/types/src/settings.tsapps/ui/src/store/app-store.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/layout/sidebar.tsxapps/ui/src/routes/dashboard.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/routes/__root.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/layout/sidebar/components/automaker-logo.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/components/views/setup-view.tsxapps/ui/src/components/views/dashboard-view.tsxapps/ui/src/lib/http-api-client.tslibs/types/src/settings.tsapps/ui/src/store/app-store.ts
🧠 Learnings (2)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/layout/sidebar.tsxapps/ui/src/routes/dashboard.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/routes/__root.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/layout/sidebar/components/automaker-logo.tsxapps/ui/src/components/views/setup-view.tsxapps/ui/src/components/views/dashboard-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: Frontend UI must use TanStack Router for file-based routing, organize components in components/views/, implement stores with Zustand, and use custom hooks in hooks/ directory
Applied to files:
apps/ui/src/routes/dashboard.tsx
🧬 Code graph analysis (3)
apps/ui/src/routes/__root.tsx (1)
apps/ui/src/components/dialogs/sandbox-risk-dialog.tsx (1)
SandboxRiskDialog(28-108)
apps/ui/src/hooks/use-settings-migration.ts (2)
apps/server/src/routes/github/routes/validation-common.ts (1)
logger(174-174)apps/server/src/routes/settings/common.ts (1)
logger(12-12)
apps/ui/src/store/app-store.ts (4)
libs/types/src/index.ts (1)
PipelineConfig(210-210)libs/types/src/pipeline.ts (1)
PipelineConfig(15-18)apps/ui/src/lib/http-api-client.ts (1)
get(768-795)apps/ui/src/lib/electron.ts (1)
getElectronAPI(902-911)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (14)
apps/ui/src/components/layout/sidebar.tsx (1)
61-62: LGTM! Removal of hideAiProfiles is complete.The removal of
hideAiProfilesfrom the destructuring aligns with the PR's objective to remove the AI Profiles feature. The code correctly no longer references this flag in theuseNavigationhook or elsewhere in the component, andhideAiProfileshas been completely removed across the codebase.apps/ui/src/components/views/setup-view.tsx (1)
86-91: LGTM!The post-setup redirect to
/dashboardaligns with the new dashboard route introduced in this PR. The log message is appropriately updated to reflect the new destination.apps/ui/src/components/layout/sidebar/components/automaker-logo.tsx (1)
35-35: LGTM!Navigation to
/dashboardis consistent with the new dashboard-centric flow introduced in this PR.apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (1)
1-14: LGTM!Clean simplification by removing internal collapse state. Visibility control is now properly managed at the parent level (board-header), following a better separation of concerns.
apps/ui/src/components/views/board-view/board-header.tsx (2)
53-74: LGTM with a note on error handling.Good optimistic update pattern - updating local store immediately before persisting to server. The fire-and-forget error handling is acceptable for a non-critical UI preference like panel visibility. If stricter consistency is desired in the future, consider rolling back the local state on server error.
100-114: LGTM!The Worktrees toggle UI follows the established pattern of other header controls, with appropriate conditional rendering based on
isMountedto prevent hydration issues.libs/types/src/settings.ts (2)
297-298: LGTM!The
isFavoriteoptional property is a clean addition for dashboard favorites functionality.
600-602: LGTM!The
worktreePanelVisibleoptional property correctly supports per-project UI state persistence, defaulting totruewhen not set (as implemented in the UI code).apps/ui/src/routes/dashboard.tsx (1)
1-6: LGTM!Clean route definition following TanStack Router conventions. The file-based routing pattern aligns with the project's architecture guidelines. Based on learnings, this correctly organizes the route and uses the appropriate component from
components/views/.apps/ui/src/store/app-store.ts (3)
642-655: Codex model fetch flow looks solid; verify API contract stays in sync.Throttling + loading guards are good. Main risk is contract drift between
api.codex.getModels()return shape andAppState.codexModelsshape. Ensure a single sharedCodexModeltype is used across http-api-client and app-store, or document why separate definitions exist.Also verify persistence flow for
toggleProjectFavoriteandworktreePanelVisiblestate mutations (lines 2954-3000).
659-662: Persistence is correctly implemented. ThesetWorktreePanelVisiblestore action properly updates local state, and the toggle handler inboard-header.tsx(lines 61–68) immediately persists the change viahttpClient.settings.updateProject(). Settings are also correctly loaded from the server viause-project-settings-loader.ts(lines 77–80) when a project loads. No action needed.Likely an incorrect or invalid review comment.
823-824: No action needed—projects are automatically synced to the server.
toggleProjectFavorite()relies on the centralizeduseSettingsSynchook that subscribes to store changes at the app root. Sinceprojectsis inSETTINGS_FIELDS_TO_SYNC, any modification to the projects array (including favorite toggles) triggers a debounced sync to the server. Favorites persist across reloads.Likely an incorrect or invalid review comment.
apps/ui/src/lib/http-api-client.ts (1)
1927-1959: The implementation ofworktreePanelVisibleis complete and properly handles persistence, default values, and the end-to-end read/write path:
- Default: Documented as
truein type definition (libs/types/src/settings.ts) and enforced via?? truefallback in all accessors (app-store.tsgetter, UI components).- Persistence: Write path exists in
board-header.tsx(callsupdateProjectAPI on toggle); read path inuse-project-settings-loader.ts(applies server response to store on project load).- No reset risk: Per-project state is maintained in the store (
worktreePanelVisibleByProject) and synced with server on changes, preventing loss of state on reload.No action required.
apps/ui/src/routes/__root.tsx (1)
442-455: Code review verified. The Codex bootstrap effect is safe and won't spam fetches.
fetchCodexModels()includes proper guards:
- Skips if already loading (
codexModelsLoadingcheck)- Skips if fetched within the last 5 minutes (even without
forceRefresh)This prevents repeated refetch loops even if
codexCliStatuschanges frequently, since the underlying API call is protected by both a loading state and a 5-minute cache window.
| import { useState, useCallback } from 'react'; | ||
| import { createLogger } from '@automaker/utils/logger'; | ||
| import { useNavigate } from '@tanstack/react-router'; | ||
| import { useAppStore, type ThemeMode } from '@/store/app-store'; | ||
| import { useOSDetection } from '@/hooks/use-os-detection'; | ||
| import { getElectronAPI, isElectron } from '@/lib/electron'; | ||
| import { initializeProject } from '@/lib/project-init'; | ||
| import { getHttpApiClient } from '@/lib/http-api-client'; | ||
| import { isMac } from '@/lib/utils'; | ||
| import { toast } from 'sonner'; | ||
| import { Button } from '@/components/ui/button'; | ||
| import { NewProjectModal } from '@/components/dialogs/new-project-modal'; | ||
| import { WorkspacePickerModal } from '@/components/dialogs/workspace-picker-modal'; | ||
| import type { StarterTemplate } from '@/lib/templates'; | ||
| import { | ||
| FolderOpen, | ||
| Plus, | ||
| Folder, | ||
| Star, | ||
| Clock, | ||
| Loader2, | ||
| ChevronDown, | ||
| MessageSquare, | ||
| Settings, | ||
| MoreVertical, | ||
| Trash2, | ||
| } from 'lucide-react'; | ||
| import { | ||
| DropdownMenu, | ||
| DropdownMenuContent, | ||
| DropdownMenuItem, | ||
| DropdownMenuTrigger, | ||
| } from '@/components/ui/dropdown-menu'; | ||
| import { | ||
| Dialog, | ||
| DialogContent, | ||
| DialogDescription, | ||
| DialogFooter, | ||
| DialogHeader, | ||
| DialogTitle, | ||
| } from '@/components/ui/dialog'; |
There was a problem hiding this comment.
Fix React.MouseEvent typing (don’t rely on React namespace being in scope).
This file references React.MouseEvent but doesn’t import React (or MouseEvent type). Depending on TS settings, this can fail typechecking.
Proposed fix
-import { useState, useCallback } from 'react';
+import { useState, useCallback, type MouseEvent as ReactMouseEvent } from 'react';
@@
- const handleToggleFavorite = useCallback(
- (e: React.MouseEvent, projectId: string) => {
+ const handleToggleFavorite = useCallback(
+ (e: ReactMouseEvent, projectId: string) => {
e.stopPropagation();
toggleProjectFavorite(projectId);
},
[toggleProjectFavorite]
);
@@
- const handleRemoveProject = useCallback(
- (e: React.MouseEvent, project: { id: string; name: string }) => {
+ const handleRemoveProject = useCallback(
+ (e: ReactMouseEvent, project: { id: string; name: string }) => {
e.stopPropagation();
setProjectToRemove(project);
},
[]
);Also applies to: 182-197
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/dashboard-view.tsx around lines 1 - 41, The
code uses the React namespace for event typing (e.g. React.MouseEvent) without
importing React which can break TS; import the MouseEvent type from React and
update occurrences (e.g. replace React.MouseEvent<HTMLButtonElement> with
MouseEvent<HTMLButtonElement> or import type { MouseEvent } from 'react') for
all handlers in this file (including the handlers around the section that spans
the NewProjectModal/WorkspacePickerModal actions). Ensure you add a single
`import type { MouseEvent } from 'react'` at the top and update the function
parameter types where React.MouseEvent is referenced.
| const handleCreateBlankProject = async (projectName: string, parentDir: string) => { | ||
| setIsCreating(true); | ||
| try { | ||
| const api = getElectronAPI(); | ||
| const projectPath = `${parentDir}/${projectName}`; | ||
|
|
||
| const parentExists = await api.exists(parentDir); | ||
| if (!parentExists) { | ||
| toast.error('Parent directory does not exist', { | ||
| description: `Cannot create project in non-existent directory: ${parentDir}`, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const parentStat = await api.stat(parentDir); | ||
| if (parentStat && !parentStat.stats?.isDirectory) { | ||
| toast.error('Parent path is not a directory', { | ||
| description: `${parentDir} is not a directory`, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const mkdirResult = await api.mkdir(projectPath); | ||
| if (!mkdirResult.success) { | ||
| toast.error('Failed to create project directory', { | ||
| description: mkdirResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const initResult = await initializeProject(projectPath); | ||
| if (!initResult.success) { | ||
| toast.error('Failed to initialize project', { | ||
| description: initResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| await api.writeFile( | ||
| `${projectPath}/.automaker/app_spec.txt`, | ||
| `<project_specification> | ||
| <project_name>${projectName}</project_name> | ||
| <overview> | ||
| Describe your project here. This file will be analyzed by an AI agent | ||
| to understand your project structure and tech stack. | ||
| </overview> | ||
| <technology_stack> | ||
| <!-- The AI agent will fill this in after analyzing your project --> | ||
| </technology_stack> | ||
| <core_capabilities> | ||
| <!-- List core features and capabilities --> | ||
| </core_capabilities> | ||
| <implemented_features> | ||
| <!-- The AI agent will populate this based on code analysis --> | ||
| </implemented_features> | ||
| </project_specification>` | ||
| ); | ||
|
|
||
| const project = { | ||
| id: `project-${Date.now()}`, | ||
| name: projectName, | ||
| path: projectPath, | ||
| lastOpened: new Date().toISOString(), | ||
| }; | ||
|
|
||
| addProject(project); | ||
| setCurrentProject(project); | ||
| setShowNewProjectModal(false); | ||
|
|
||
| toast.success('Project created', { | ||
| description: `Created ${projectName}`, | ||
| }); | ||
|
|
||
| navigate({ to: '/board' }); | ||
| } catch (error) { | ||
| logger.error('Failed to create project:', error); | ||
| toast.error('Failed to create project', { | ||
| description: error instanceof Error ? error.message : 'Unknown error', | ||
| }); | ||
| } finally { | ||
| setIsCreating(false); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Windows path bug: building projectPath with "/" will break on C:\... style parents.
const projectPath = \${parentDir}/${projectName}`;can produce invalid/mixed paths on Windows and may causemkdir/writeFile` to fail.
Proposed fix
const handleCreateBlankProject = async (projectName: string, parentDir: string) => {
setIsCreating(true);
try {
const api = getElectronAPI();
- const projectPath = `${parentDir}/${projectName}`;
+ const sep = parentDir.includes('\\') ? '\\' : '/';
+ const normalizedParentDir = parentDir.replace(/[\\/]+$/, '');
+ const projectPath = `${normalizedParentDir}${sep}${projectName}`;
@@
await api.writeFile(
- `${projectPath}/.automaker/app_spec.txt`,
+ `${projectPath}${sep}.automaker${sep}app_spec.txt`,
`<project_specification>
<project_name>${projectName}</project_name>🤖 Prompt for AI Agents
In @apps/ui/src/components/views/dashboard-view.tsx around lines 216 - 303, The
code builds file paths with string concatenation in handleCreateBlankProject
(projectPath and the .automaker file path), which breaks on Windows; replace
those concatenations with platform-safe path joins (e.g., import path from
'path' and use path.join(parentDir, projectName) for projectPath and
path.join(projectPath, '.automaker', 'app_spec.txt') for the writeFile target),
and update any other `${projectPath}/...` usages in that function to path.join
to ensure correct separators across OSes.
| const handleCreateFromTemplate = async ( | ||
| template: StarterTemplate, | ||
| projectName: string, | ||
| parentDir: string | ||
| ) => { | ||
| setIsCreating(true); | ||
| try { | ||
| const httpClient = getHttpApiClient(); | ||
| const api = getElectronAPI(); | ||
|
|
||
| const cloneResult = await httpClient.templates.clone( | ||
| template.repoUrl, | ||
| projectName, | ||
| parentDir | ||
| ); | ||
| if (!cloneResult.success || !cloneResult.projectPath) { | ||
| toast.error('Failed to clone template', { | ||
| description: cloneResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const projectPath = cloneResult.projectPath; | ||
| const initResult = await initializeProject(projectPath); | ||
| if (!initResult.success) { | ||
| toast.error('Failed to initialize project', { | ||
| description: initResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| await api.writeFile( | ||
| `${projectPath}/.automaker/app_spec.txt`, | ||
| `<project_specification> | ||
| <project_name>${projectName}</project_name> | ||
| <overview> | ||
| This project was created from the "${template.name}" starter template. | ||
| ${template.description} | ||
| </overview> | ||
| <technology_stack> | ||
| ${template.techStack.map((tech) => `<technology>${tech}</technology>`).join('\n ')} | ||
| </technology_stack> | ||
| <core_capabilities> | ||
| ${template.features.map((feature) => `<capability>${feature}</capability>`).join('\n ')} | ||
| </core_capabilities> | ||
| <implemented_features> | ||
| <!-- The AI agent will populate this based on code analysis --> | ||
| </implemented_features> | ||
| </project_specification>` | ||
| ); | ||
|
|
||
| const project = { | ||
| id: `project-${Date.now()}`, | ||
| name: projectName, | ||
| path: projectPath, | ||
| lastOpened: new Date().toISOString(), | ||
| }; | ||
|
|
||
| addProject(project); | ||
| setCurrentProject(project); | ||
| setShowNewProjectModal(false); | ||
|
|
||
| toast.success('Project created from template', { | ||
| description: `Created ${projectName} from ${template.name}`, | ||
| }); | ||
|
|
||
| navigate({ to: '/board' }); | ||
| } catch (error) { | ||
| logger.error('Failed to create project from template:', error); | ||
| toast.error('Failed to create project', { | ||
| description: error instanceof Error ? error.message : 'Unknown error', | ||
| }); | ||
| } finally { | ||
| setIsCreating(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleCreateFromCustomUrl = async ( | ||
| repoUrl: string, | ||
| projectName: string, | ||
| parentDir: string | ||
| ) => { | ||
| setIsCreating(true); | ||
| try { | ||
| const httpClient = getHttpApiClient(); | ||
| const api = getElectronAPI(); | ||
|
|
||
| const cloneResult = await httpClient.templates.clone(repoUrl, projectName, parentDir); | ||
| if (!cloneResult.success || !cloneResult.projectPath) { | ||
| toast.error('Failed to clone repository', { | ||
| description: cloneResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const projectPath = cloneResult.projectPath; | ||
| const initResult = await initializeProject(projectPath); | ||
| if (!initResult.success) { | ||
| toast.error('Failed to initialize project', { | ||
| description: initResult.error || 'Unknown error occurred', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| await api.writeFile( | ||
| `${projectPath}/.automaker/app_spec.txt`, | ||
| `<project_specification> | ||
| <project_name>${projectName}</project_name> | ||
| <overview> | ||
| This project was cloned from ${repoUrl}. | ||
| The AI agent will analyze the project structure. | ||
| </overview> | ||
| <technology_stack> | ||
| <!-- The AI agent will fill this in after analyzing your project --> | ||
| </technology_stack> | ||
| <core_capabilities> | ||
| <!-- List core features and capabilities --> | ||
| </core_capabilities> | ||
| <implemented_features> | ||
| <!-- The AI agent will populate this based on code analysis --> | ||
| </implemented_features> | ||
| </project_specification>` | ||
| ); | ||
|
|
||
| const project = { | ||
| id: `project-${Date.now()}`, | ||
| name: projectName, | ||
| path: projectPath, | ||
| lastOpened: new Date().toISOString(), | ||
| }; | ||
|
|
||
| addProject(project); | ||
| setCurrentProject(project); | ||
| setShowNewProjectModal(false); | ||
|
|
||
| toast.success('Project created from repository', { | ||
| description: `Created ${projectName}`, | ||
| }); | ||
|
|
||
| navigate({ to: '/board' }); | ||
| } catch (error) { | ||
| logger.error('Failed to create project from custom URL:', error); | ||
| toast.error('Failed to create project', { | ||
| description: error instanceof Error ? error.message : 'Unknown error', | ||
| }); | ||
| } finally { | ||
| setIsCreating(false); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Same Windows path issue exists in template/custom-url flows when writing .automaker/app_spec.txt.
You construct \${projectPath}/.automaker/app_spec.txt`` in multiple flows; on Windows this can break the first-run experience.
Proposed fix (apply similarly in both flows)
- await api.writeFile(
- `${projectPath}/.automaker/app_spec.txt`,
+ const sep = projectPath.includes('\\') ? '\\' : '/';
+ await api.writeFile(
+ `${projectPath}${sep}.automaker${sep}app_spec.txt`,
`<project_specification>🤖 Prompt for AI Agents
In @apps/ui/src/components/views/dashboard-view.tsx around lines 304 - 461, Both
handleCreateFromTemplate and handleCreateFromCustomUrl build the app_spec.txt
path with string interpolation (`\`${projectPath}/.automaker/app_spec.txt\``)
which breaks on Windows; change those to build the path using a path-join
utility (e.g., Node's path.join(projectPath, '.automaker', 'app_spec.txt') or an
exposed Electron API like api.pathJoin) and pass that joined path into the
existing api.writeFile calls so the path is correct across platforms.
- Removed the add feature shortcut from BoardHeader and integrated the add feature functionality directly into the KanbanBoard and BoardView components. - Added a floating action button for adding features in the KanbanBoard's backlog column. - Updated KanbanColumn to support a footer action for enhanced UI consistency. - Cleaned up unused imports and props related to feature addition.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/views/board-view/components/kanban-column.tsx (1)
79-91: Fix footer padding to match actual button height and prevent content from being hidden — The currentpb-14(56px) is insufficient; the footer button (h-9= 36px) plus padding (pt-6= 24px,p-2= 8px) totals ~68px, leaving the last card partially hidden. Additionally, the absolute overlay atz-20can block pointer events near the bottom.Change
pb-14topb-[72px]and restructure the footer to isolate pointer events:Proposed fix
- footerAction && 'pb-14' + footerAction && 'pb-[72px]'{footerAction && ( - <div className="absolute bottom-0 left-0 right-0 z-20 p-2 bg-gradient-to-t from-card/95 via-card/80 to-transparent pt-6"> - {footerAction} + <div className="absolute bottom-0 left-0 right-0 z-20 bg-gradient-to-t from-card/95 via-card/80 to-transparent pt-6 pointer-events-none"> + <div className="p-2 pointer-events-auto">{footerAction}</div> </div> )}Applies to lines 79-91 and 93-98.
🤖 Fix all issues with AI agents
In @apps/ui/src/components/views/board-view/kanban-board.tsx:
- Around line 132-164: The Select/Drag Button is rendered as enabled even when
onToggleSelectionMode is undefined; change the Button
(data-testid="selection-mode-button") to check for onToggleSelectionMode and, if
falsy, render it disabled: remove or avoid calling onClick, add the disabled
prop (or aria-disabled), set an appropriate title like "Selection not
available", and apply a disabled style/class (e.g., opacity-50
cursor-not-allowed) instead of the active classes that reference isSelectionMode
so the button is visibly and functionally inert when onToggleSelectionMode is
not provided.
🧹 Nitpick comments (1)
apps/ui/src/components/views/board-view.tsx (1)
98-100: Optional: narrow the store selector to current project to reduce unrelated re-renders.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-column.tsxapps/ui/src/components/views/board-view/kanban-board.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/board-view/components/kanban-column.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.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:
apps/ui/src/components/views/board-view/components/kanban-column.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsx
🧠 Learnings (1)
📚 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/components/kanban-column.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-header.tsx
🧬 Code graph analysis (3)
apps/ui/src/components/views/board-view/kanban-board.tsx (1)
apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/src/components/views/board-view.tsx (1)
apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (1)
WorktreePanel(16-258)
apps/ui/src/components/views/board-view/board-header.tsx (1)
apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2332-2337)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (8)
apps/ui/src/components/views/board-view/components/kanban-column.tsx (1)
13-15:footerActionprop addition looks good, but verify it can’t obscure content — the prop wiring is straightforward.Also applies to: 29-30
apps/ui/src/components/views/board-view/kanban-board.tsx (2)
46-47:onAddFeaturesignature change: please verify all call sites updated (required prop).Also applies to: 82-83
188-201: Footer “Add Feature” action wiring is clean — consistent handler + test id.apps/ui/src/components/views/board-view/board-header.tsx (2)
13-16: Prop surface change (projectPath) is fine; just ensure all BoardHeader call sites updated.Also applies to: 29-39
94-109: Mount-gated Worktrees toggle to avoid hydration issues is a good call.apps/ui/src/components/views/board-view.tsx (3)
1142-1145:projectPathwiring intoBoardHeaderlooks correct.
1160-1193: WorktreePanel conditional rendering matches the new per-project toggle.
1249-1250:onAddFeatureis now consistently wired from BoardView → KanbanBoard.
| // Worktree panel visibility (per-project) | ||
| const worktreePanelVisibleByProject = useAppStore((state) => state.worktreePanelVisibleByProject); | ||
| const setWorktreePanelVisible = useAppStore((state) => state.setWorktreePanelVisible); | ||
| const isWorktreePanelVisible = worktreePanelVisibleByProject[projectPath] ?? true; | ||
|
|
||
| const handleWorktreePanelToggle = useCallback( | ||
| async (visible: boolean) => { | ||
| // Update local store | ||
| setWorktreePanelVisible(projectPath, visible); | ||
|
|
||
| // Persist to server | ||
| try { | ||
| const httpClient = getHttpApiClient(); | ||
| await httpClient.settings.updateProject(projectPath, { | ||
| worktreePanelVisible: visible, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Failed to persist worktree panel visibility:', error); | ||
| } | ||
| }, | ||
| [projectPath, setWorktreePanelVisible] | ||
| ); |
There was a problem hiding this comment.
Persist toggle needs rollback + out-of-order protection — otherwise a failed request or quick double-toggle can leave server and UI diverged.
Proposed fix (rollback on error + last-write-wins)
-import { useState, useCallback } from 'react';
+import { useState, useCallback, useRef } from 'react';
@@
const setWorktreePanelVisible = useAppStore((state) => state.setWorktreePanelVisible);
const isWorktreePanelVisible = worktreePanelVisibleByProject[projectPath] ?? true;
+ const persistSeqRef = useRef(0);
const handleWorktreePanelToggle = useCallback(
async (visible: boolean) => {
+ const prev = worktreePanelVisibleByProject[projectPath] ?? true;
+ const seq = ++persistSeqRef.current;
+
// Update local store
setWorktreePanelVisible(projectPath, visible);
// Persist to server
try {
const httpClient = getHttpApiClient();
await httpClient.settings.updateProject(projectPath, {
worktreePanelVisible: visible,
});
} catch (error) {
+ // Only rollback if this is still the latest attempted persist
+ if (persistSeqRef.current === seq) {
+ setWorktreePanelVisible(projectPath, prev);
+ }
console.error('Failed to persist worktree panel visibility:', error);
}
},
- [projectPath, setWorktreePanelVisible]
+ [projectPath, setWorktreePanelVisible, worktreePanelVisibleByProject]
);| <div className="flex items-center gap-1"> | ||
| <Button | ||
| variant="default" | ||
| size="sm" | ||
| className="h-6 w-6 p-0" | ||
| onClick={onAddFeature} | ||
| title="Add Feature" | ||
| data-testid="add-feature-button" | ||
| > | ||
| <Plus className="w-3.5 h-3.5" /> | ||
| </Button> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className={`h-6 px-2 text-xs ${isSelectionMode ? 'text-primary bg-primary/10' : 'text-muted-foreground hover:text-foreground'}`} | ||
| onClick={onToggleSelectionMode} | ||
| title={isSelectionMode ? 'Switch to Drag Mode' : 'Select Multiple'} | ||
| data-testid="selection-mode-button" | ||
| > | ||
| {isSelectionMode ? ( | ||
| <> | ||
| <GripVertical className="w-3.5 h-3.5 mr-1" /> | ||
| Drag | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <CheckSquare className="w-3.5 h-3.5 mr-1" /> | ||
| Select | ||
| </> | ||
| )} | ||
| </Button> | ||
| </div> | ||
| ) : column.id === 'in_progress' ? ( |
There was a problem hiding this comment.
Don’t render an enabled “Select/Drag” button if onToggleSelectionMode is undefined — it becomes a silent no-op.
Proposed fix
@@
<Button
variant="ghost"
size="sm"
className={`h-6 px-2 text-xs ${isSelectionMode ? 'text-primary bg-primary/10' : 'text-muted-foreground hover:text-foreground'}`}
- onClick={onToggleSelectionMode}
+ onClick={onToggleSelectionMode}
+ disabled={!onToggleSelectionMode}
+ aria-pressed={isSelectionMode}
title={isSelectionMode ? 'Switch to Drag Mode' : 'Select Multiple'}
data-testid="selection-mode-button"
>🤖 Prompt for AI Agents
In @apps/ui/src/components/views/board-view/kanban-board.tsx around lines 132 -
164, The Select/Drag Button is rendered as enabled even when
onToggleSelectionMode is undefined; change the Button
(data-testid="selection-mode-button") to check for onToggleSelectionMode and, if
falsy, render it disabled: remove or avoid calling onClick, add the disabled
prop (or aria-disabled), set an appropriate title like "Selection not
available", and apply a disabled style/class (e.g., opacity-50
cursor-not-allowed) instead of the active classes that reference isSelectionMode
so the button is visibly and functionally inert when onToggleSelectionMode is
not provided.
- Eliminated kanbanCardDetailLevel from the SettingsService, app state, and various UI components including BoardView and BoardControls. - Updated related hooks and API client to reflect the removal of kanbanCardDetailLevel. - Cleaned up imports and props associated with kanbanCardDetailLevel across the codebase for improved clarity and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/src/store/app-store.ts (1)
2948-2987: Consider adding validation and improving error handling.The implementation has several areas for improvement:
Line 2956: The 5-minute cache duration (300000ms) is hardcoded. Consider making it configurable via a constant or app setting.
Lines 2974-2979: No validation of the returned models structure before setting state. If the API returns malformed data, it could cause runtime issues elsewhere in the app.
Lines 2980-2985: Errors are stored in state but not logged. Consider adding
logger.error()to aid debugging.♻️ Proposed improvements
+ // At the top of the file, add a constant + const CODEX_MODELS_CACHE_DURATION_MS = 5 * 60 * 1000; // 5 minutes fetchCodexModels: async (forceRefresh = false) => { const { codexModelsLastFetched, codexModelsLoading } = get(); // Skip if already loading if (codexModelsLoading) return; // Skip if recently fetched (< 5 minutes ago) and not forcing refresh - if (!forceRefresh && codexModelsLastFetched && Date.now() - codexModelsLastFetched < 300000) { + if (!forceRefresh && codexModelsLastFetched && Date.now() - codexModelsLastFetched < CODEX_MODELS_CACHE_DURATION_MS) { return; } set({ codexModelsLoading: true, codexModelsError: null }); try { const api = getElectronAPI(); if (!api.codex) { throw new Error('Codex API not available'); } const result = await api.codex.getModels(forceRefresh); if (!result.success) { throw new Error(result.error || 'Failed to fetch Codex models'); } + // Validate models structure + const models = result.models || []; + if (models.length > 0 && !models[0].id) { + throw new Error('Invalid Codex models structure: missing id field'); + } set({ - codexModels: result.models || [], + codexModels: models, codexModelsLastFetched: Date.now(), codexModelsLoading: false, codexModelsError: null, }); } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + logger.error('Failed to fetch Codex models:', errorMessage); set({ codexModelsError: errorMessage, codexModelsLoading: false, }); } },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/server/src/services/settings-service.tsapps/server/src/types/settings.tsapps/ui/scripts/setup-e2e-fixtures.mjsapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/board-controls.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsxapps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsdocs/settings-api-migration.mdlibs/types/src/index.tslibs/types/src/settings.ts
💤 Files with no reviewable changes (4)
- apps/ui/src/hooks/use-settings-sync.ts
- apps/ui/scripts/setup-e2e-fixtures.mjs
- apps/server/src/services/settings-service.ts
- apps/server/src/types/settings.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/settings-api-migration.md
- libs/types/src/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/board-view/board-controls.tsxapps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsxlibs/types/src/settings.tsapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/lib/http-api-client.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/store/app-store.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/board-controls.tsxapps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsxlibs/types/src/settings.tsapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/lib/http-api-client.tsapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/store/app-store.ts
🧠 Learnings (1)
📚 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/board-controls.tsxapps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsxapps/ui/src/components/views/board-view/board-header.tsxapps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view.tsx
🧬 Code graph analysis (5)
apps/ui/src/components/views/board-view/board-header.tsx (1)
apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2331-2336)
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx (1)
apps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsx (1)
SummaryDialog(24-71)
apps/ui/src/components/views/board-view/kanban-board.tsx (1)
apps/ui/src/store/app-store.ts (1)
formatShortcut(162-207)
apps/ui/src/components/views/board-view.tsx (1)
apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (1)
WorktreePanel(16-258)
apps/ui/src/hooks/use-settings-migration.ts (2)
apps/server/src/routes/github/routes/validation-common.ts (1)
logger(174-174)apps/server/src/routes/settings/common.ts (1)
logger(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (33)
libs/types/src/settings.ts (3)
331-336: GlobalSettings doc update looks consistent with the new feature set.
No concerns in this change.
575-598: Verify thatProjectSettings.worktreePanelVisibledefaulting is correctly implemented throughout the codebase. The field is documented to default totruewhen undefined, but verification of the actual UI and server logic implementations is needed to confirm all code paths useworktreePanelVisible ?? trueto handle missing values in older settings files.
283-296: Remove verification request —isFavoritehandling is already correct.The code already treats
undefinedas "not favorited" through JavaScript's natural falsy behavior. The sorting logic (a.isFavorite && !b.isFavorite), filtering (p.isFavoriteand!p.isFavorite), and toggle operations (!p.isFavorite) all work correctly with undefined values. No explicit?? falsecoalescing is needed.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/board-view/board-controls.tsx (1)
1-117: Kanban Card Detail Level feature removal is complete and properly integrated.Verification confirms:
- No remaining references to
kanbanCardDetailLeveloronDetailLevelChangein the codebase- Call site in
BoardViewcorrectly uses only the 6 updated props- All icon imports (
ImageIcon,Archive,Columns3,Network) are used- No orphaned code or broken references
The component is ready for production.
apps/ui/src/components/views/board-view/kanban-board.tsx (4)
6-7: LGTM!The imports are correctly added for the new functionality -
useAppStoreandformatShortcutfrom the app store, and thePlusicon for the Add Feature buttons.
93-96: LGTM!The keyboard shortcut retrieval and default fallback are well-implemented. Using
'N'as a sensible default ensures the shortcut hint displays even if the user hasn't customized shortcuts.
136-167: LGTM!The backlog header controls are well-structured with the Add Feature button alongside the selection mode toggle. The conditional rendering and styling are appropriate.
192-208: LGTM!The footer action button provides a discoverable entry point for adding features with a helpful keyboard shortcut hint. The
formatShortcut(addFeatureShortcut, true)call correctly formats the shortcut for display.apps/ui/src/lib/http-api-client.ts (2)
1953-1953: LGTM!The
worktreePanelVisiblefield is correctly added to the project settings response type, enabling per-project persistence of worktree panel visibility.
2056-2074: LGTM!The
codex.getModelsAPI method is well-implemented with:
- Clear type definitions for the response structure
- Optional
refreshparameter with sensible default- Clean URL construction with conditional query parameter
apps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsx (1)
34-37: LGTM!The additions correctly enable text selection within the dialog and prevent event bubbling that could interfere with the dialog's usability (e.g., closing the dialog or initiating drag operations when selecting text).
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx (4)
3-3: LGTM!Import correctly updated to use
ThinkingLevelfrom the app store, aligning with the removal of AI-profile related fields.
123-157: LGTM!The backlog card rendering is now unconditionally displayed based on feature status, removing the previous detail-level gating. This simplifies the component logic.
255-259: LGTM!The text selection improvements (
select-text cursor-textclasses andstopPropagationhandlers) enable users to select and copy summary text without triggering drag or other parent interactions.
296-306: Verify the fallback SummaryDialog rendering is reachable.The fallback return renders
SummaryDialogbut there's no UI element (like an expand button) in this path to setisSummaryDialogOpentotrue. This means the dialog will never be visible in scenarios where this fallback is reached. Is this intentional as a placeholder for future functionality, or should the fallback returnnullinstead?apps/ui/src/hooks/use-settings-migration.ts (4)
293-294: LGTM!Logging simplified to focus on project counts, consistent with the removal of AI-profile related data.
375-376: LGTM!Hook logging simplified consistently with the migration function changes.
405-406: LGTM!Server settings logging updated to match the simplified approach.
506-506: LGTM!The
isFavoritefield is correctly propagated during project hydration, enabling per-project favorite state to persist through the settings migration flow.apps/ui/src/components/views/board-view/board-header.tsx (4)
1-12: LGTM!Imports correctly updated to support the new Worktrees toggle (Popover, GitBranch icon, useCallback, getHttpApiClient).
48-69: LGTM!The worktree panel visibility logic is well-implemented:
- Per-project visibility state from store
- Sensible default of
true(visible)- Async handler with proper memoization
- Server persistence with error handling
95-109: LGTM!The Worktrees toggle UI is clean and provides a clear, accessible control for showing/hiding the worktree panel per project.
111-152: LGTM!Moving the concurrency control into a Popover is a good UX improvement - it reduces visual clutter in the header while keeping the control accessible. The implementation correctly preserves all existing functionality.
apps/ui/src/components/views/board-view.tsx (6)
10-27: LGTM!The
DialogAwarePointerSensoris a clean solution to prevent drag-and-drop from interfering with dialog interactions. Using[role="dialog"]as the selector is semantically correct and framework-agnostic.
113-114: LGTM!Subscribing to
worktreePanelVisibleByProjectensures the component re-renders when visibility changes for any project.
266-266: LGTM!Correctly updated to use the new
DialogAwarePointerSensorwhile preserving the existing activation constraint configuration.
1157-1159: LGTM!The
projectPathprop is correctly passed toBoardHeader, enabling per-project worktree visibility control.
1175-1207: LGTM!The WorktreePanel is now conditionally rendered based on the per-project visibility setting. The nullish coalescing operator (
?? true) provides a sensible default of visible when no setting exists.
1262-1262: LGTM!The
onAddFeatureprop is correctly wired to open the add dialog, completing the integration with the KanbanBoard's new Add Feature buttons.apps/ui/src/store/app-store.ts (4)
4-4: LGTM!The import is correctly used in the
fetchCodexModelsimplementation.
3094-3107: LGTM!The worktree panel visibility implementation follows the established pattern for per-project settings and correctly defaults to visible when not explicitly set.
1437-1452: TheisFavoriteproperty is correctly defined on theProjecttype (as an optional boolean inapps/ui/src/lib/electron.ts), and the implementation is type-safe.
639-651: CodexModelConfig from @automaker/types lacks thetierandisDefaultproperties.CodexModelConfig (available in @automaker/types) contains only
id,label,description,hasThinking, andsupportsVision. The inline type here includes additional properties (tierandisDefault) that are returned by the API and used by the UI (e.g., model-selector.tsx). The inline type cannot be replaced with CodexModelConfig without losing these essential properties.Likely an incorrect or invalid review comment.
- Integrated Tooltip components into AddFeatureDialog, EditFeatureDialog, and MassEditDialog to provide user guidance on planning mode availability. - Updated the rendering logic for planning mode selection to conditionally display tooltips when planning modes are not supported. - Improved user experience by clarifying the conditions under which planning modes can be utilized.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (1)
131-133: Fix planning state/UI mismatch when planning is unsupported; tooltip needs a focusable trigger.When planning is unsupported you render a disabled selector with
mode="skip", but you saveplanningModefrom state (which may still be'full'|'spec', etc.). Same forrequirePlanApproval. Also, tooltip likely won’t show for keyboard users due to a non-focusable trigger wrapper.Proposed fix
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip'; -import { isClaudeModel, supportsReasoningEffort } from '@automaker/types'; +import { isClaudeModel, supportsReasoningEffort } from '@automaker/types'; +import { resolveModelString } from '@automaker/model-resolver'; @@ - const modelSupportsPlanningMode = isClaudeModel(modelEntry.model); + const resolvedModel = resolveModelString(modelEntry.model); + const modelSupportsPlanningMode = isClaudeModel(resolvedModel); + + useEffect(() => { + if (!modelSupportsPlanningMode) { + setPlanningMode('skip'); + setRequirePlanApproval(false); + } + }, [modelSupportsPlanningMode]); + + useEffect(() => { + if (planningMode === 'skip' || planningMode === 'lite') setRequirePlanApproval(false); + }, [planningMode]);- <TooltipTrigger asChild> - <div> + <TooltipTrigger asChild> + <span tabIndex={0} aria-disabled="true" className="block"> <PlanningModeSelect mode="skip" onModeChange={() => {}} testIdPrefix="edit-feature-planning" compact disabled /> - </div> + </span> </TooltipTrigger>Also applies to: 520-557
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
39-56: Import and useresolveModelString()to normalize model aliases before capability checks.
modelEntry.modelinitializes as'opus'(alias) and is passed toisClaudeModel(),supportsReasoningEffort(), andmodelSupportsThinking()without resolution. Per coding guidelines, resolve aliases to full model names first usingresolveModelString()from@automaker/model-resolver. While the code currently works because these functions accept aliases, this pattern violates the prescribed guideline and risks brittleness if function signatures change.
🤖 Fix all issues with AI agents
In @apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx:
- Around line 577-602: The Require approval checkbox is only disabled in the UI
but requirePlanApproval can stay true and be saved; update the component to
clear/normalize requirePlanApproval whenever planningMode or
modelSupportsPlanningMode changes (use the existing setRequirePlanApproval to
set false when planningMode is 'skip' or 'lite' or modelSupportsPlanningMode is
false), and also ensure the final save/path that builds the updates (the
function that assembles the feature updates before submit) normalizes
requirePlanApproval to false if planningMode is 'skip'/'lite' or
modelSupportsPlanningMode is false so it cannot persist.
🧹 Nitpick comments (3)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
190-203: Avoidas ModelAliascasts fromPhaseModelEntry(type drift risk).
PhaseModelSelectoremits aPhaseModelEntry, but you’re forcingentry.model as ModelAlias. IfPhaseModelEntry.modelis (or becomes) a canonical id (or a non-alias), this will compile but break gating/updates. Prefer aligning state type to whatPhaseModelSelectoractually returns (or normalize viaresolveModelString()and store both alias + resolved if you need both).apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
153-173: Dialog-open reset sets model alias directly; ensure it matchesPhaseModelSelectorexpectations.
setModelEntry({ model: 'opus' })on open is fine ifPhaseModelSelectoruses aliases, but if it emits canonical ids you’ll end up mixing formats. Strongly prefer normalizing (store resolved id, or store both).apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (1)
37-58: Follow guidelines and useresolveModelString()for model normalization consistency.While
isClaudeModel()andsupportsReasoningEffort()correctly handle model aliases ('opus', 'haiku', etc.), per coding guidelines you should normalize aliases viaresolveModelString()before using them in capability checks. This ensures consistency across the codebase and future-proofs the code against changes to how these functions process their inputs.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-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:
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-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: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx
🧬 Code graph analysis (1)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (4)
apps/ui/src/components/views/board-view/dialogs/index.ts (1)
MassEditDialog(10-10)libs/types/src/index.ts (1)
isClaudeModel(195-195)apps/ui/src/components/views/board-view/shared/planning-mode-select.tsx (1)
PlanningModeSelect(78-161)apps/ui/src/lib/utils.ts (1)
cn(6-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (1)
apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
15-20: TheisCursorModel()andisClaudeModel()functions are designed to accept both model aliases (e.g.,'sonnet','opus') and full model IDs. Per their JSDoc comments and implementation, they explicitly handle aliases by checking againstCLAUDE_MODEL_MAPkeys. No conversion viaresolveModelString()is needed. The same pattern is already used throughout the codebase inadd-feature-dialog.tsxandedit-feature-dialog.tsxwithout issues. The../sharedimport is a valid relative reference to the local barrel export and not an "old relative path" concern.Likely an incorrect or invalid review comment.
| <div className="space-y-1.5"> | ||
| <Label className="text-xs text-muted-foreground">Model</Label> | ||
| <PhaseModelSelector | ||
| value={modelEntry} | ||
| onChange={handleModelChange} | ||
| compact | ||
| align="end" | ||
| /> | ||
| </div> | ||
|
|
||
| <div | ||
| className={cn( | ||
| 'grid gap-3', | ||
| modelSupportsPlanningMode ? 'grid-cols-2' : 'grid-cols-1' | ||
| )} | ||
| > | ||
| {modelSupportsPlanningMode && ( | ||
| <div className="space-y-1.5"> | ||
| <Label className="text-xs text-muted-foreground">Planning</Label> | ||
| <div className="grid gap-3 grid-cols-2"> | ||
| <div className="space-y-1.5"> | ||
| <Label | ||
| className={cn( | ||
| 'text-xs text-muted-foreground', | ||
| !modelSupportsPlanningMode && 'opacity-50' | ||
| )} | ||
| > | ||
| Planning | ||
| </Label> | ||
| {modelSupportsPlanningMode ? ( | ||
| <PlanningModeSelect | ||
| mode={planningMode} | ||
| onModeChange={setPlanningMode} | ||
| testIdPrefix="add-feature-planning" | ||
| compact | ||
| /> | ||
| </div> | ||
| )} | ||
| ) : ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div> | ||
| <PlanningModeSelect | ||
| mode="skip" | ||
| onModeChange={() => {}} | ||
| testIdPrefix="add-feature-planning" | ||
| compact | ||
| disabled | ||
| /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>Planning modes are only available for Claude Provider</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Fix planning state/UI mismatch when planning is unsupported; tooltip needs a focusable trigger.
Same issue as EditFeature: when unsupported you render disabled PlanningModeSelect mode="skip", but FeatureData.planningMode still comes from state and can persist a non-skip value. Also, tooltip trigger likely isn’t keyboard accessible. Normalize planningMode/requirePlanApproval when model/mode changes, and wrap the tooltip trigger in a focusable element.
Proposed fix
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip';
import {
getAncestors,
formatAncestorContextForPrompt,
type AncestorContext,
} from '@automaker/dependency-resolver';
+import { resolveModelString } from '@automaker/model-resolver';
@@
- const modelSupportsPlanningMode = isClaudeModel(modelEntry.model);
+ const resolvedModel = resolveModelString(modelEntry.model);
+ const modelSupportsPlanningMode = isClaudeModel(resolvedModel);
+
+ useEffect(() => {
+ if (!modelSupportsPlanningMode) {
+ setPlanningMode('skip');
+ setRequirePlanApproval(false);
+ }
+ }, [modelSupportsPlanningMode]);
+
+ useEffect(() => {
+ if (planningMode === 'skip' || planningMode === 'lite') setRequirePlanApproval(false);
+ }, [planningMode]);- <TooltipTrigger asChild>
- <div>
+ <TooltipTrigger asChild>
+ <span tabIndex={0} aria-disabled="true" className="block">
<PlanningModeSelect
mode="skip"
onModeChange={() => {}}
testIdPrefix="add-feature-planning"
compact
disabled
/>
- </div>
+ </span>
</TooltipTrigger>Also applies to: 551-576
| <div className="flex items-center gap-2"> | ||
| <Checkbox | ||
| id="edit-feature-require-approval" | ||
| checked={requirePlanApproval} | ||
| onCheckedChange={(checked) => setRequirePlanApproval(!!checked)} | ||
| disabled={ | ||
| !modelSupportsPlanningMode || | ||
| planningMode === 'skip' || | ||
| planningMode === 'lite' | ||
| } | ||
| data-testid="edit-feature-require-approval-checkbox" | ||
| /> | ||
| <Label | ||
| htmlFor="edit-feature-require-approval" | ||
| className={cn( | ||
| 'text-xs font-normal', | ||
| !modelSupportsPlanningMode || | ||
| planningMode === 'skip' || | ||
| planningMode === 'lite' | ||
| ? 'cursor-not-allowed text-muted-foreground' | ||
| : 'cursor-pointer' | ||
| )} | ||
| > | ||
| Require approval | ||
| </Label> | ||
| </div> |
There was a problem hiding this comment.
Ensure “Require approval” can’t persist while planning is skip/lite or unsupported.
You disable the checkbox, but requirePlanApproval can remain true in state and be saved (especially after model/mode switches). Consider normalizing it on planningMode/model changes (see previous diff), and/or normalizing before building the final updates.
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx
around lines 577 - 602, The Require approval checkbox is only disabled in the UI
but requirePlanApproval can stay true and be saved; update the component to
clear/normalize requirePlanApproval whenever planningMode or
modelSupportsPlanningMode changes (use the existing setRequirePlanApproval to
set false when planningMode is 'skip' or 'lite' or modelSupportsPlanningMode is
false), and also ensure the final save/path that builds the updates (the
function that assembles the feature updates before submit) normalizes
requirePlanApproval to false if planningMode is 'skip'/'lite' or
modelSupportsPlanningMode is false so it cannot persist.
| {modelSupportsPlanningMode ? ( | ||
| <FieldWrapper | ||
| label="Planning Mode" | ||
| isMixed={mixedValues.planningMode || mixedValues.requirePlanApproval} | ||
| willApply={applyState.planningMode || applyState.requirePlanApproval} | ||
| onApplyChange={(apply) => | ||
| setApplyState((prev) => ({ | ||
| ...prev, | ||
| planningMode: apply, | ||
| requirePlanApproval: apply, | ||
| })) | ||
| } | ||
| > | ||
| <PlanningModeSelect | ||
| mode={planningMode} | ||
| onModeChange={(newMode) => { | ||
| setPlanningMode(newMode); | ||
| // Auto-suggest approval based on mode, but user can override | ||
| setRequirePlanApproval(newMode === 'spec' || newMode === 'full'); | ||
| }} | ||
| requireApproval={requirePlanApproval} | ||
| onRequireApprovalChange={setRequirePlanApproval} | ||
| testIdPrefix="mass-edit-planning" | ||
| /> | ||
| </FieldWrapper> | ||
| ) : ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div | ||
| className={cn( | ||
| 'p-3 rounded-lg border transition-colors border-border bg-muted/20 opacity-50 cursor-not-allowed' | ||
| )} | ||
| > | ||
| <div className="flex items-center justify-between mb-3"> | ||
| <div className="flex items-center gap-2"> | ||
| <Checkbox checked={false} disabled className="opacity-50" /> | ||
| <Label className="text-sm font-medium text-muted-foreground"> | ||
| Planning Mode | ||
| </Label> | ||
| </div> | ||
| </div> | ||
| <div className="opacity-50 pointer-events-none"> | ||
| <PlanningModeSelect | ||
| mode="skip" | ||
| onModeChange={() => {}} | ||
| testIdPrefix="mass-edit-planning" | ||
| disabled | ||
| /> | ||
| </div> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>Planning modes are only available for Claude Provider</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} |
There was a problem hiding this comment.
Clear/normalize planning apply-state when planning becomes unsupported + make tooltip keyboard-accessible.
If the user enabled applyState.planningMode/requirePlanApproval while on a Claude model, then switches to a non-Claude model, those apply flags can remain true (but the UI becomes a disabled placeholder). That can cause “Apply” to push planning settings unexpectedly. Also, the tooltip trigger is a non-focusable div, so keyboard users likely won’t get the explanation.
Proposed fix
export function MassEditDialog({ open, onClose, selectedFeatures, onApply }: MassEditDialogProps) {
const [isApplying, setIsApplying] = useState(false);
@@
const modelSupportsPlanningMode = isClaudeModel(resolvedModel);
+
+ useEffect(() => {
+ if (!modelSupportsPlanningMode) {
+ setPlanningMode('skip');
+ setRequirePlanApproval(false);
+ setApplyState((prev) => ({ ...prev, planningMode: false, requirePlanApproval: false }));
+ }
+ }, [modelSupportsPlanningMode]);- <TooltipProvider>
- <Tooltip>
- <TooltipTrigger asChild>
+ <TooltipProvider>
+ <Tooltip>
+ <TooltipTrigger asChild>
+ <span tabIndex={0} aria-disabled="true" className="block">
<div
className={cn(
'p-3 rounded-lg border transition-colors border-border bg-muted/20 opacity-50 cursor-not-allowed'
)}
>
@@
</div>
- </TooltipTrigger>
+ </span>
+ </TooltipTrigger>
<TooltipContent>
<p>Planning modes are only available for Claude Provider</p>
</TooltipContent>
</Tooltip>
</TooltipProvider>
This commit integrates OpenCode as a new AI provider and updates all provider icons with their official brand colors for better visual recognition. **OpenCode Provider Integration:** - Add OpencodeProvider class with CLI-based execution - Support for OpenCode native models (opencode/) and Bedrock models - Proper event normalization for OpenCode streaming format - Correct CLI arguments: --format json (not stream-json) - Event structure: type, part.text, sessionID fields **Provider Icons:** - Add official OpenCode icon (white square frame from opencode.ai) - Add DeepSeek icon (blue whale #4D6BFE) - Add Qwen icon (purple gradient #6336E7 → #6F69F7) - Add Amazon Nova icon (AWS orange #FF9900) - Add Mistral icon (rainbow gradient gold→red) - Add Meta icon (blue #1877F2) - Update existing icons with brand colors: * Claude: #d97757 (terra cotta) * OpenAI/Codex: #74aa9c (teal-green) * Cursor: #5E9EFF (bright blue) **Settings UI Updates:** - Update settings navigation to show OpenCode icon - Update model configuration to use provider-specific icons - Differentiate between OpenCode free models and Bedrock-hosted models - All AI models now display their official brand logos **Model Resolution:** - Add isOpencodeModel() function to detect OpenCode models - Support patterns: opencode/, opencode-*, amazon-bedrock/* - Update getProviderFromModel to recognize opencode provider Note: Some unit tests in opencode-provider.test.ts need updating to match the new event structure and CLI argument format.
Sync with latest upstream changes: - feat: enhance feature dialogs with planning mode tooltips - refactor: remove kanbanCardDetailLevel from settings and UI components - refactor: streamline feature addition in BoardView and KanbanBoard - feat: implement dashboard view and enhance sidebar navigation
Update normalizeEvent tests to match new OpenCode API: - text events use type='text' with part.text instead of text-delta - tool_call events use type='tool_call' with part containing call_id, name, args - tool_result events use type='tool_result' with part - tool_error events use type='tool_error' with part - step_finish events use type='step_finish' with part Update buildCliArgs tests: - Remove expectations for -q flag (no longer used) - Remove expectations for -c flag (cwd set at subprocess level) - Remove expectations for - final arg (prompt via stdin) - Update format to 'json' instead of 'stream-json' Remaining 8 test failures are in integration tests that use executeQuery and require more extensive mock data updates.
Fix all 8 remaining test failures: 1. Update executeQuery integration tests to use new OpenCode event format: - text events use type='text' with part.text - tool_call events use type='tool_call' with part containing call_id, name, args - tool_result events use type='tool_result' with part - step_finish events use type='step_finish' with part - Use sessionID field instead of session_id 2. Fix step_finish event handling: - Include result field in successful completion response - Check for reason === 'error' to detect failed steps - Provide default error message when error field is missing 3. Update model test expectations: - Model 'opencode/big-pickle' stays as-is (not stripped to 'big-pickle') - PROVIDER_PREFIXES only strips 'opencode-' prefix, not 'opencode/' All 84 tests now pass successfully!
There was a problem hiding this comment.
Actionable comments posted: 1
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/components/ui/provider-icon.tsx (1)
324-333: Add missing icon entries fordeepseek,qwen, andnovatoiconMap.The
iconMapis typed asRecord<ProviderIconKey, ComponentType<...>>, which should include all 9 providers defined inPROVIDER_ICON_KEYS, but currently only has 6 entries. The icon components (DeepSeekIcon,QwenIcon,NovaIcon) exist and are exported; they just need to be mapped.Diff
const iconMap: Record<ProviderIconKey, ComponentType<{ className?: string }>> = { anthropic: AnthropicIcon, openai: OpenAIIcon, cursor: CursorIcon, gemini: GeminiIcon, grok: GrokIcon, opencode: OpenCodeIcon, + deepseek: DeepSeekIcon, + qwen: QwenIcon, + nova: NovaIcon, };
🤖 Fix all issues with AI agents
In @apps/server/src/providers/opencode-provider.ts:
- Around line 126-134: The OpenCodeFinishStepEvent.part type lacks a typed
result property, so part.result is treated as unknown and causes TS2322 when
assigned to result.result; add a properly typed optional result property to the
part interface (e.g. part?: { type: 'step-finish'; reason?: string; error?:
string; result?: { result: string; [key: string]: unknown }; [key: string]:
unknown }) so the compiler knows result.result is a string and assignments
succeed; update the OpenCodeFinishStepEvent definition accordingly.
🧹 Nitpick comments (2)
apps/ui/src/components/ui/provider-icon.tsx (2)
138-211: Duplicate path data violates DRY principle.
DeepSeekIcon,QwenIcon, andNovaIconhave their path data duplicated—once inPROVIDER_ICON_DEFINITIONS(lines 62-76) and again in these inline SVG implementations. This is inconsistent withOpenCodeIcon,AnthropicIcon, etc., which use the centralizedProviderIconcomponent.♻️ Refactor to use ProviderIcon component
-export function DeepSeekIcon({ - className, - title, - ...props -}: { - className?: string; - title?: string; -}) { - const hasAccessibleLabel = Boolean(title); - - return ( - <svg - viewBox="0 0 24 24" - className={cn('inline-block', className)} - role={hasAccessibleLabel ? 'img' : 'presentation'} - aria-hidden={!hasAccessibleLabel} - focusable="false" - {...props} - > - {title && <title>{title}</title>} - <path - d="M23.748 4.482c-.254-.124..." - fill="#4D6BFE" - /> - </svg> - ); -} +export function DeepSeekIcon(props: Omit<ProviderIconProps, 'provider'>) { + return <ProviderIcon provider={PROVIDER_ICON_KEYS.deepseek} {...props} />; +} -export function QwenIcon({ className, title, ...props }: { className?: string; title?: string }) { - // ... inline SVG implementation -} +export function QwenIcon(props: Omit<ProviderIconProps, 'provider'>) { + return <ProviderIcon provider={PROVIDER_ICON_KEYS.qwen} {...props} />; +} -export function NovaIcon({ className, title, ...props }: { className?: string; title?: string }) { - // ... inline SVG implementation -} +export function NovaIcon(props: Omit<ProviderIconProps, 'provider'>) { + return <ProviderIcon provider={PROVIDER_ICON_KEYS.nova} {...props} />; +}Note:
QwenIconuses alinearGradient. You'll need to extendProviderIconDefinitionto support gradients, or keepQwenIconas a special case with inline SVG.
214-268:MistralIconandMetaIconare not integrated into the provider icon system.These components have standalone inline SVG implementations but lack entries in
PROVIDER_ICON_KEYSandPROVIDER_ICON_DEFINITIONS. If they're intended to be part of the centralized icon system, consider addingmistralandmetato the type definitions and icon mappings for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/server/src/providers/opencode-provider.tsapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxlibs/model-resolver/src/resolver.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/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxlibs/model-resolver/src/resolver.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/ui/provider-icon.tsxapps/server/src/providers/opencode-provider.tsapps/server/tests/unit/providers/opencode-provider.test.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/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxlibs/model-resolver/src/resolver.tsapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/ui/provider-icon.tsxapps/server/src/providers/opencode-provider.tsapps/server/tests/unit/providers/opencode-provider.test.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/opencode-provider.ts
🧠 Learnings (2)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/ui/provider-icon.tsx
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
libs/model-resolver/src/resolver.tsapps/ui/src/components/ui/provider-icon.tsx
🧬 Code graph analysis (3)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
apps/ui/src/components/ui/provider-icon.tsx (1)
OpenCodeIcon(134-136)
libs/model-resolver/src/resolver.ts (2)
libs/types/src/index.ts (1)
isOpencodeModel(197-197)libs/types/src/provider-utils.ts (1)
isOpencodeModel(102-123)
apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (1)
apps/ui/src/components/ui/provider-icon.tsx (7)
OpenCodeIcon(134-136)AnthropicIcon(114-116)DeepSeekIcon(138-164)NovaIcon(193-212)MetaIcon(249-268)MistralIcon(214-247)QwenIcon(166-191)
🪛 GitHub Actions: E2E Tests
apps/server/src/providers/opencode-provider.ts
[error] 387-387: TS2322: Type '{}' is not assignable to type 'string'.
🪛 GitHub Actions: PR Build Check
apps/server/src/providers/opencode-provider.ts
[error] 387-387: Type '{}' is not assignable to type 'string'. (TS2322)
🔇 Additional comments (18)
libs/model-resolver/src/resolver.ts (2)
72-77: OpenCode passthrough branch is correct and placed in the right spot.
It prevents Claude-alias resolution from accidentally rewriting OpenCode model identifiers and keeps routing consistent with existing provider-prefixed passthrough behavior.
11-22: No action required. Verification confirmsisOpencodeModelis properly exported from@automaker/types(both inprovider-utils.ts:102and re-exported viaindex.ts:197), and there is no circular dependency—@automaker/model-resolvercorrectly depends on@automaker/typesin a single direction with no reciprocal dependency. The import follows coding guidelines by using the shared package.apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
2-5: LGTM!The icon swap from
BottoOpenCodeIconaligns with the centralized provider icon system introduced in this PR. The import path is appropriate for intra-app usage.Also applies to: 173-173
apps/ui/src/components/views/settings-view/config/navigation.ts (1)
18-18: LGTM!Consistent use of
OpenCodeIconfor the OpenCode provider navigation item, aligning with the centralized provider icon system.Also applies to: 50-50
apps/ui/src/components/ui/provider-icon.tsx (2)
105-109: LGTM!The path rendering correctly applies
fillfrom the definition with a sensiblecurrentColorfallback, and optionalfillRulesupport.
289-292: Verify: Allamazon-bedrock-*models map toopencodeicon.The condition
modelStr.includes('amazon-bedrock')matches all Bedrock providers (anthropic, deepseek, amazon, meta, mistral, qwen), returning theopencodeicon for all of them. Is this intentional, or should different Bedrock providers show their respective icons (e.g.,DeepSeekIconforamazon-bedrock-deepseek)?apps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsx (2)
14-22: LGTM!Clean mapping of OpenCode providers to their respective icons. The
getProviderIconfunction correctly returns provider-specific icons for the configuration view, providing good visual differentiation between Bedrock providers.Also applies to: 36-55
121-124: LGTM!Appropriate use of
OpenCodeIconfor the Model Configuration header.apps/server/src/providers/opencode-provider.ts (4)
240-257: LGTM on CLI argument construction.The argument building logic correctly:
- Uses
runsubcommand- Sets
--format jsonfor streaming output- Handles model selection with prefix stripping
- Delegates prompt handling to stdin via
buildSubprocessOptions
273-308: LGTM on prompt extraction and subprocess options.The
extractPromptTextmethod properly handles both string and array prompts, filtering out non-text content blocks. Passing the prompt via stdin inbuildSubprocessOptionsis a good approach to avoid shell escaping issues with special characters.
324-391: LGTM on event normalization logic (with the type fix).The normalization logic properly maps OpenCode event types to
ProviderMessageformat. The handling ofstep_finishcorrectly distinguishes between success and error states by checking bothpart?.errorandpart?.reason === 'error'.Once the missing
resultproperty is added toOpenCodeFinishStepEvent.part(flagged separately), this implementation is sound.
246-251: No changes needed.The OpenCode provider correctly handles model strings without requiring
resolveModelString(). All available models ingetAvailableModels()(lines 476–586) are full, provider-specific model strings like'opencode/big-pickle'and'amazon-bedrock/anthropic.claude-sonnet-4-5-20250929-v1:0', not aliases. The provider's test suite confirms it only receives and processes full model strings with optional provider prefixes (e.g.,'opencode-...') thatstripProviderPrefix()correctly handles. TheresolveModelString()function from the coding guidelines is designed for Claude model aliases ('sonnet','haiku','opus') and returns Claude-format model strings, which would not match OpenCode's model format. The current implementation is correct.apps/server/tests/unit/providers/opencode-provider.test.ts (6)
1-35: LGTM on test setup and mocking.The test file properly mocks
@automaker/platformfunctions and resets state between tests withbeforeEach. The mock setup appropriately isolates the provider for unit testing.
167-232: LGTM on buildCliArgs tests.Tests properly verify the updated CLI argument format:
--format json(updated fromstream-json)- Model prefix stripping works correctly
- Working directory is not passed as a CLI argument
Good coverage for model handling variations including the
opencode-prefix stripping.
238-573: LGTM on normalizeEvent tests.Comprehensive test coverage for all event types with the new OpenCode event format:
textevents withpart.texttool_call/tool_result/tool_errorwith nestedpartobjectsstep_start/step_finishhandlingsessionID→session_idmapping- Edge cases: empty text, missing fields, generated tool IDs
580-779: LGTM on executeQuery integration tests.Tests properly verify:
- Streaming event flow
- Tool use/result ID matching
- Prompt passed via
stdinData- CLI args construction with format and model
- Handling of unknown events (filtered out)
- Error handling when CLI is not installed
1035-1088: LGTM on session_id preservation tests.Good end-to-end test verifying that
sessionIDfrom OpenCode events is correctly propagated assession_idthrough the fullexecuteQueryflow across all event types.
1090-1218: LGTM on edge case tests for normalizeEvent.Thorough coverage of edge cases:
- Empty/null/complex nested args for tool_call
- Empty output for tool_result
- Whitespace-only and multiline text
- Error precedence when step_finish has both result and error
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx (1)
173-191: Approve currentBranch parameter and addsetModeto dependency array.The
currentBranchparameter is correct—it matches the optionalbranchNameparameter inapi.backlogPlan.apply(), and adding it to the dependency array ensures the callback updates when the branch changes.However,
setModeis called on lines 157 and 181 but is missing from the dependency array. While state setters fromuseStateare technically stable and won't cause runtime errors, ESLint's exhaustive-deps rule requires all used variables to be listed. AddsetModeto the dependency array:], [ projectPath, pendingPlanResult, selectedChanges, setPendingPlanResult, onPlanApplied, onClose, currentBranch, setMode, ]);apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (1)
1-1: Remove the@ts-nocheckdirective from line 1.The file imports
WorktreeInfofrom the correct local../typesmodule and uses only properties that exist in that type definition (e.g.,worktree.branch,worktree.isMain,worktree.hasWorktree,worktree.pr). No type conflicts exist in this file that would require suppressing TypeScript checks. The directive can be safely removed.(Note: A separate architectural issue exists where
WorktreeInfohas conflicting definitions acrosselectron.d.ts,settings.ts, andworktree-panel/types.ts, but this file is not affected by that inconsistency.)
🤖 Fix all issues with AI agents
In @apps/server/src/routes/worktree/routes/delete.ts:
- Around line 51-70: The response currently reports branch: branchName even when
deletion is skipped for an invalid name; introduce a boolean flag (e.g.,
branchDeleted = false) before the branch deletion block, set branchDeleted =
true only after a successful execGitCommand(['branch', '-D', branchName],
projectPath), keep it false when isValidBranchName(branchName) fails or an
exception occurs, and change the returned payload to include branchDeleted and
only include branchName (or set branch to null) when branchDeleted is true so
callers aren’t misled; update references around isValidBranchName,
execGitCommand, deleteBranch, branchName, and logger.warn accordingly.
In @apps/server/src/routes/worktree/routes/init-script.ts:
- Around line 39-47: The GET /init-script handler in init-script.ts reads
projectPath from req.query without using the existing validatePathParams
middleware (which currently validates req.body only), so add consistent path
validation: either extend validatePathParams to accept a location flag (e.g.,
'body'|'query') or add a small middleware wrapper (e.g.,
validateQueryPathParams) that validates req.query.projectPath for path traversal
and existence, then attach that middleware to the GET route for /init-script (or
invoke the validation inline at the top of the handler before using
projectPath). Ensure you reference the same validation rules used by the
PUT/DELETE handlers and reuse the validatePathParams logic to avoid duplication.
In @apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx:
- Around line 66-67: The comment incorrectly claims currentBranch "defaults to
main" but no default is applied; fix by ensuring a default is set where the prop
is used: in the BacklogPlanDialog component (or where props are destructured)
apply currentBranch = currentBranch ?? 'main' or pass currentBranch ?? 'main'
into the API call (e.g., where featureLoader.create(...) is invoked) so the
server always receives 'main' when undefined; alternatively, if you prefer not
to change behavior, update the comment to remove the "defaults to main" text and
leave currentBranch as an optional possibly-undefined prop.
🧹 Nitpick comments (14)
apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx (1)
49-55: Minor consideration on effect dependencies.Including
defaultDeleteBranchin the dependency array means if a parent component changes this prop while the dialog is open, the user's checkbox selection will be overwritten. If this prop is unlikely to change mid-session, consider depending only onopen:useEffect(() => { if (open) { setDeleteBranch(defaultDeleteBranch); } - }, [open, defaultDeleteBranch]); + }, [open]); // eslint-disable-line react-hooks/exhaustive-depsAlternatively, the current implementation is fine if
defaultDeleteBranchis stable per dialog instance.apps/ui/src/components/ui/ansi-output.tsx (3)
31-32: Code 40 mapped to transparent instead of black.Standard ANSI code 40 represents black background, but it's mapped to
'transparent'here. This appears intentional for dark-themed UIs to avoid obscuring content, but worth documenting this deviation from the spec.📝 Consider adding a comment
const ANSI_BG_COLORS: Record<number, string> = { - 40: 'transparent', + 40: 'transparent', // Black bg intentionally transparent for dark-theme visibility
150-157: RGB color parsing could be more defensive.The check for RGB mode verifies
codes[i + 4] !== undefinedbut doesn't validate thatcodes[i + 2]andcodes[i + 3]are also defined. Malformed sequences could produce invalid CSS likergb(undefined, undefined, 255).🛡️ Defensive check for all RGB components
- } else if (codes[i + 1] === 2 && codes[i + 4] !== undefined) { + } else if ( + codes[i + 1] === 2 && + codes[i + 2] !== undefined && + codes[i + 3] !== undefined && + codes[i + 4] !== undefined + ) { // RGB mode: 38;2;r;g;b const r = codes[i + 2]; const g = codes[i + 3]; const b = codes[i + 4];Apply the same pattern to lines 165-171 for background colors.
211-248: Consider bounds checking for edge cases.The function assumes
indexis in the valid 0-255 range. Negative indices would returnundefinedfrom the array lookup, and indices > 255 would produce invalid RGB values from the grayscale formula. While unlikely with well-formed ANSI input, a bounds check would be safer.🛡️ Optional bounds check
function get256Color(index: number): string { + // Clamp to valid 256-color range + if (index < 0 || index > 255) { + return 'inherit'; + } + // 0-15: Standard colors if (index < 16) {apps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsx (1)
183-193: Consider addingrole="alert"for screen reader accessibility.The error display looks great visually, but adding
role="alert"would ensure screen readers announce the error immediately when it appears, improving accessibility for users who rely on assistive technology.♿ Proposed accessibility improvement
{error && ( - <div className="flex items-start gap-2 p-3 rounded-md bg-destructive/10 border border-destructive/20"> + <div role="alert" className="flex items-start gap-2 p-3 rounded-md bg-destructive/10 border border-destructive/20"> <AlertCircle className="w-4 h-4 text-destructive mt-0.5 flex-shrink-0" />apps/server/src/routes/backlog-plan/routes/apply.ts (1)
12-158: Consider emitting events for progress updates.Per coding guidelines, server operations should use
createEventEmitter()fromlib/events.tsfor WebSocket streaming. While this endpoint returns synchronously, emitting progress events (e.g., after each feature add/update/delete) could improve UX by showing real-time progress in the frontend, especially for larger backlog plans.This is optional—verify if the frontend would benefit from real-time progress updates during backlog plan application.
apps/ui/src/hooks/use-project-settings-loader.ts (1)
115-115: Consider adding the new setters to the dependency array.While Zustand store selectors typically return stable function references, ESLint's
react-hooks/exhaustive-depsrule may flag the missing dependencies. The existing setters (setBoardBackground,setCardOpacity, etc.) are also missing from the array, so this appears to be an intentional pattern in this file.apps/server/src/routes/worktree/common.ts (1)
142-149: Consider migrating remaining git commands toexecGitCommandfor consistency.While the current usages of
execAsyncinhasCommits(line 144) andensureInitialCommit(lines 193, 197) are safe because they use hardcoded strings or constants, migrating them toexecGitCommandwould provide consistency and defense-in-depth.♻️ Example refactor for hasCommits
export async function hasCommits(repoPath: string): Promise<boolean> { try { - await execAsync('git rev-parse --verify HEAD', { cwd: repoPath }); + await execGitCommand(['rev-parse', '--verify', 'HEAD'], repoPath); return true; } catch { return false; } }Also applies to: 188-210
apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (1)
87-104: Consider adding a refresh mechanism for init script state.The
hasInitScriptstate is only checked whenprojectPathchanges. If a user creates or deletes an init script in Settings while this panel is open, the state won't update until they switch projects.Consider exposing a refresh trigger or subscribing to WebSocket events for init script changes to keep the UI in sync.
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
227-227: Handle case when no editors are available.The entire "Open in Editor" section is conditionally rendered only when
effectiveDefaultEditorexists. However, if no editors are detected, users have no way to copy the path or know that no editors were found. Consider showing a fallback option or message.💡 Suggested enhancement
- {effectiveDefaultEditor && ( + {effectiveDefaultEditor ? ( <DropdownMenuSub> {/* ... existing split button ... */} </DropdownMenuSub> + ) : ( + <DropdownMenuItem + onClick={async () => { + try { + await navigator.clipboard.writeText(worktree.path); + toast.success('Path copied to clipboard'); + } catch { + toast.error('Failed to copy path to clipboard'); + } + }} + className="text-xs" + > + <Copy className="w-3.5 h-3.5 mr-2" /> + Copy Path + </DropdownMenuItem> )}apps/ui/src/components/views/board-view/init-script-indicator.tsx (3)
82-86: Collapse/expand icons appear semantically inverted.When
showLogsis true (logs visible), showingChevronDownsuggests "expand more" rather than "collapse". Typically,ChevronUpindicates content can be collapsed, andChevronDownindicates it can be expanded.Consider swapping the icons for clearer UX
{showLogs ? ( - <ChevronDown className="w-4 h-4 text-muted-foreground" /> + <ChevronUp className="w-4 h-4 text-muted-foreground" /> ) : ( - <ChevronUp className="w-4 h-4 text-muted-foreground" /> + <ChevronDown className="w-4 h-4 text-muted-foreground" /> )}
158-170: Effect may run excessively due to unstableallStatesreference.
allStatesis derived fromgetInitScriptStatesForProject(projectPath)on every render, likely returning a new array reference each time. This causes the effect to re-run on every render, repeatedly checking and potentially updatingdismissedKeys.Consider memoizing
allStatesor using a more stable dependency.Memoize allStates to stabilize the dependency
+ // Memoize to avoid effect re-runs on every render + const allStates = useMemo( + () => getInitScriptStatesForProject(projectPath), + [getInitScriptStatesForProject, projectPath] + ); - // Get all init script states for this project - const allStates = getInitScriptStatesForProject(projectPath);
175-177: Fragile key parsing assumes fixed format.The key parsing logic
key.split('::')[1]assumes the format is always"projectPath::branch". If the branch name contains::or the format changes, this will extract incorrect data silently.Consider using a more robust approach or documenting the key format contract.
apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx (1)
221-234: Repeated async persistence pattern could be extracted.The toggle handlers at Lines 221-234, 260-272, and 297-309 all follow the same pattern:
- Check
currentProject?.path- Update local store
- Persist to server via
httpClient.settings.updateProject- Catch and log errors
Consider extracting a helper to reduce duplication, though the current approach is clear and maintainable.
📜 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 (42)
apps/server/src/index.tsapps/server/src/lib/worktree-metadata.tsapps/server/src/routes/backlog-plan/routes/apply.tsapps/server/src/routes/worktree/common.tsapps/server/src/routes/worktree/index.tsapps/server/src/routes/worktree/middleware.tsapps/server/src/routes/worktree/routes/create.tsapps/server/src/routes/worktree/routes/delete.tsapps/server/src/routes/worktree/routes/init-script.tsapps/server/src/services/init-script-service.tsapps/ui/package.jsonapps/ui/src/components/ui/ansi-output.tsxapps/ui/src/components/ui/shell-syntax-editor.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/backlog-plan-dialog.tsxapps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsxapps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/init-script-indicator.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsxapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/components/views/settings-view/worktrees/index.tsapps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsxapps/ui/src/hooks/use-init-script-events.tsapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/src/types/electron.d.tsdocs/worktree-init-script-example.shlibs/platform/src/index.tslibs/platform/src/system-paths.tslibs/types/src/event.tslibs/types/src/settings.ts
💤 Files with no reviewable changes (1)
- apps/ui/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/server/src/routes/worktree/middleware.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/views/settings-view/config/navigation.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/routes/backlog-plan/routes/apply.tsapps/ui/src/components/views/settings-view/worktrees/index.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-init-script-events.tsapps/ui/src/components/ui/ansi-output.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsxapps/ui/src/components/ui/shell-syntax-editor.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsxapps/server/src/routes/worktree/routes/delete.tsapps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsxapps/server/src/index.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/init-script-indicator.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/lib/electron.tsapps/server/src/routes/worktree/index.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/server/src/lib/worktree-metadata.tsapps/server/src/routes/worktree/routes/create.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/components/views/settings-view.tsxapps/server/src/services/init-script-service.tsapps/ui/src/components/views/board-view.tsxapps/server/src/routes/worktree/routes/init-script.tsapps/server/src/routes/worktree/common.tsapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/board-header.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/backlog-plan/routes/apply.tsapps/server/src/routes/worktree/routes/delete.tsapps/server/src/index.tsapps/server/src/routes/worktree/index.tsapps/server/src/lib/worktree-metadata.tsapps/server/src/routes/worktree/routes/create.tsapps/server/src/services/init-script-service.tsapps/server/src/routes/worktree/routes/init-script.tsapps/server/src/routes/worktree/common.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/routes/backlog-plan/routes/apply.tsapps/ui/src/components/views/settings-view/worktrees/index.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/hooks/use-init-script-events.tsapps/ui/src/components/ui/ansi-output.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsxapps/ui/src/components/ui/shell-syntax-editor.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsxapps/server/src/routes/worktree/routes/delete.tsapps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsxapps/server/src/index.tsapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/init-script-indicator.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/lib/electron.tsapps/server/src/routes/worktree/index.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/server/src/lib/worktree-metadata.tsapps/server/src/routes/worktree/routes/create.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/components/views/settings-view.tsxapps/server/src/services/init-script-service.tsapps/ui/src/components/views/board-view.tsxapps/server/src/routes/worktree/routes/init-script.tsapps/server/src/routes/worktree/common.tsapps/ui/src/hooks/use-project-settings-loader.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/board-header.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 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/ui/src/hooks/use-init-script-events.tsapps/server/src/index.tsapps/server/src/routes/worktree/index.tsapps/server/src/routes/worktree/routes/create.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/ui/ansi-output.tsxapps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsxapps/ui/src/components/ui/shell-syntax-editor.tsxapps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsxapps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsxapps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/init-script-indicator.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/board-view.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/board-header.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/routes/worktree/routes/delete.tsapps/server/src/routes/worktree/routes/create.tsapps/ui/src/components/views/board-view.tsxapps/server/src/routes/worktree/common.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: Organize server code following modular pattern with routes/, services/, providers/, and lib/ directories
Applied to files:
apps/server/src/index.ts
🧬 Code graph analysis (11)
apps/server/src/routes/backlog-plan/routes/apply.ts (1)
libs/types/src/backlog-plan.ts (1)
BacklogPlanResult(29-33)
apps/ui/src/hooks/use-init-script-events.ts (1)
apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2411-2416)
apps/ui/src/components/ui/ansi-output.tsx (1)
apps/ui/src/lib/utils.ts (1)
cn(6-8)
apps/ui/src/components/ui/shell-syntax-editor.tsx (1)
apps/ui/src/lib/utils.ts (1)
cn(6-8)
apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx (4)
apps/ui/src/lib/api-fetch.ts (3)
apiGet(109-115)apiPut(132-139)apiDelete(144-147)apps/ui/src/lib/utils.ts (1)
cn(6-8)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2411-2416)apps/ui/src/components/ui/shell-syntax-editor.tsx (1)
ShellSyntaxEditor(106-142)
apps/ui/src/components/views/board-view/init-script-indicator.tsx (3)
apps/ui/src/store/app-store.ts (1)
InitScriptState(476-481)apps/ui/src/lib/utils.ts (1)
cn(6-8)apps/ui/src/components/ui/ansi-output.tsx (1)
AnsiOutput(250-276)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (4)
apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(625-630)libs/types/src/index.ts (1)
WorktreeInfo(127-127)libs/types/src/settings.ts (1)
WorktreeInfo(560-571)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (4)
apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2411-2416)apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(625-630)libs/types/src/settings.ts (1)
WorktreeInfo(560-571)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
apps/ui/src/components/views/settings-view.tsx (2)
apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx (1)
WorktreesSection(35-430)apps/ui/src/components/views/settings-view/worktrees/index.ts (1)
WorktreesSection(1-1)
apps/ui/src/components/views/board-view.tsx (4)
apps/ui/src/store/app-store.ts (1)
useAppStore(1278-3340)apps/ui/src/hooks/use-init-script-events.ts (1)
useInitScriptEvents(32-79)apps/ui/src/lib/http-api-client.ts (1)
getHttpApiClient(2411-2416)apps/ui/src/components/views/board-view/init-script-indicator.tsx (1)
InitScriptIndicator(140-209)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (2)
useAvailableEditors(12-74)useEffectiveDefaultEditor(80-101)apps/ui/src/components/icons/editor-icons.tsx (1)
getEditorIcon(172-220)
🪛 Biome (2.1.2)
apps/ui/src/components/ui/ansi-output.tsx
[error] 69-69: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 69-69: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 69-69: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 69-69: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 69-69: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 78-78: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 78-78: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 78-78: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 78-78: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 78-78: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 92-92: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (78)
apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx (3)
1-1: LGTM!The
useEffectimport is correctly added to support the new dialog state reset logic.
33-34: LGTM!Clean addition of the optional prop with appropriate JSDoc documentation.
44-46: LGTM!Proper initialization with a sensible default value.
apps/ui/src/components/views/board-view/dialogs/plan-settings-dialog.tsx (3)
12-17: LGTM!The interface is well-defined with clear, descriptive prop names following the standard controlled dialog pattern.
19-67: LGTM!The component implementation is clean and well-structured:
- Proper accessibility with matching
htmlForandidattributes- Test IDs included for both the dialog and toggle
- Fully controlled state via props
- Clear, descriptive help text for the toggle behavior
1-9: No changes needed. The imports inplan-settings-dialog.tsxfollow the established pattern used throughout the codebase. All other dialog files in the same directory import UI components from@/components/ui/*identically. No shared@automaker/*UI package exists for these components—they are local to theapps/uiproject.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/board-view/dialogs/worktree-settings-dialog.tsx (2)
12-17: LGTM!The interface is well-defined with clear TypeScript types following the controlled component pattern.
19-67: Well-structured dialog component.The implementation follows good practices:
- Proper accessibility linking between Label and Switch via matching
id/htmlFor- Testing support with
data-testidattributes- Clear separation of concerns with controlled props pattern
- Helpful description text for users
apps/ui/package.json (1)
45-46: Invalid version specified:@codemirror/language@6.12.1does not exist.The version
6.12.1is not published on npm for@codemirror/language. The latest available version is6.11.3. Additionally, there's a version specifier inconsistency: existing CodeMirror dependencies (@codemirror/lang-xmlon line 44,@codemirror/theme-one-darkon line 47) use exact versions, while these new packages use caret ranges. Change to:- "@codemirror/language": "^6.12.1", - "@codemirror/legacy-modes": "^6.5.2", + "@codemirror/language": "6.11.3", + "@codemirror/legacy-modes": "6.5.2",Likely an incorrect or invalid review comment.
apps/ui/src/components/ui/ansi-output.tsx (3)
1-7: LGTM!Imports and interface definition are clean. The
@/lib/utilspath alias is appropriate for app-internal utilities.
62-79: Static analysis warnings are false positives.The Biome warnings about control characters in regex are false positives. Parsing ANSI escape sequences inherently requires matching control characters (
\x1b,\x07). The existingeslint-disablecomments correctly acknowledge this.
250-276: LGTM!The component implementation is clean:
useMemocorrectly memoizes the expensive parsing operation- Using
key={index}is acceptable since segments are positional and won't reorder- Inline styles are appropriate for dynamic ANSI-derived colors
whitespace-pre-wrapandbreak-wordscorrectly preserve terminal output formattingapps/ui/src/components/views/board-view/dialogs/create-worktree-dialog.tsx (4)
1-15: LGTM!Imports are well-organized. The
AlertCircleicon addition fromlucide-reactis appropriate for the enhanced error display.
17-78: Well-structured error parsing with good UX considerations.The
parseWorktreeErrorfunction provides comprehensive coverage of common git/worktree errors with actionable user-friendly messages. The default fallback (line 76) that strips prefixes and takes the first line is a good approach for unknown errors.
98-100: Good use of structured error state.The type change from
string | nullto{ title: string; description?: string } | nullenables richer error feedback while maintaining backward compatibility with errors that only need a title.
121-126: MissingsetIsLoading(false)before early return causes stuck loading state.If the worktree API is unavailable, the function returns early at line 125 after
setIsLoading(true)was called at line 118. Since thisreturnbypasses thefinallyblock, the loading state is never cleared, leaving the UI stuck with a disabled button and spinner.🐛 Proposed fix
const api = getElectronAPI(); if (!api?.worktree?.create) { setError({ title: 'Worktree API not available' }); + setIsLoading(false); return; }Likely an incorrect or invalid review comment.
apps/server/src/routes/backlog-plan/routes/apply.ts (2)
15-29: LGTM - Clean validation for optional branchName.The destructuring and validation logic correctly handles edge cases: whitespace-only strings are normalized to
undefined, and valid strings are trimmed. This ensures consistent behavior downstream.
89-97: LGTM - Branch scoping for new features.The
branchNameis correctly propagated only to new feature creation, not to updates. This is appropriate since existing features should retain their original branch context.apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx (1)
72-82: LGTM - Clean prop integration.The new
currentBranchprop is cleanly integrated into the component's destructuring.apps/ui/src/hooks/use-project-settings-loader.ts (2)
20-25: LGTM!The new setters are correctly extracted from the app store using individual selectors, consistent with the existing pattern in this hook.
81-106: LGTM!The new per-project settings are applied correctly with proper undefined checks, following the same defensive pattern used for other settings. The implementation aligns with the store actions defined in
apps/ui/src/store/app-store.ts.apps/server/src/lib/worktree-metadata.ts (1)
24-29: LGTM!The new optional fields are well-typed with clear documentation. The status union type appropriately covers the init script lifecycle states, and the optional nature ensures backward compatibility with existing metadata files.
apps/ui/src/hooks/use-init-script-events.ts (4)
1-4: LGTM!Imports are correctly using shared packages (
@/store/app-store,@/lib/http-api-client,@/lib/utils) as per coding guidelines.
6-26: LGTM!The payload interfaces are well-defined and cover all necessary fields for each event type. Good use of union type for the
typefield inInitScriptOutputPayload.
36-78: LGTM on the overall hook structure.The hook correctly:
- Guards against null projectPath
- Uses pathsEqual for cross-platform path comparison
- Returns the unsubscribe function for proper cleanup
- Includes all dependencies in the useEffect dependency array
66-72: No action needed—setInitScriptStatecorrectly merges state fields.The implementation in
app-store.tsuses object spread syntax ({ ...current, ...state }) to merge the incoming partial state with existing state. When theworktree:init-completedevent callssetInitScriptStatewith onlystatusanderror, thebranchandoutputfields from the current state are preserved. This is the correct behavior.apps/server/src/routes/worktree/common.ts (2)
6-6: LGTM!Good use of
spawnProcessfrom@automaker/platformfor secure command execution.
14-50: Excellent security improvement with clear documentation.The
execGitCommandfunction properly prevents command injection by using argument arrays instead of string interpolation. The documentation with examples clearly illustrates the safe vs. unsafe patterns.apps/ui/src/components/ui/shell-syntax-editor.tsx (6)
1-8: LGTM!Imports are well-organized with CodeMirror packages and the
cnutility from shared packages as per coding guidelines.
10-18: LGTM!Clean props interface with sensible optional properties for customization. Good inclusion of
data-testidfor testing support.
21-46: LGTM!Good use of CSS variables for theme compatibility, with sensible OKLCH fallbacks. This ensures the editor integrates well with the app's theming system.
49-97: LGTM!Comprehensive theme configuration covering all essential editor elements. Good use of CSS variables for colors and transparent backgrounds to blend with the container.
100-104: LGTM!Defining the extensions array at module scope is efficient since these are static configurations that don't need to be recreated on each render.
106-141: LGTM!The component implementation is clean and well-structured. Good defaults for
minHeightand proper propagation of all customization props to CodeMirror.apps/ui/src/lib/electron.ts (7)
436-443: LGTM! Per-project scoping for spec regeneration operations.The addition of optional
projectPathparameter tostopandstatusmethods enables per-project state management, which aligns with the broader PR goal of per-project worktree capabilities.
537-540: LGTM! NewresumeInterruptedmethod for auto-mode recovery.This addition supports resuming interrupted features on a per-project basis, which is a useful capability for recovering from crashes or manual stops.
737-750: LGTM! Codex models API with caching support.The new
getModelsmethod provides model metadata with caching info (cachedAt), aligning with the TTL/persistent model cache mentioned in the PR summary.
1649-1716: LGTM! Editor discovery and selection enhancements.The mock implementation properly mirrors the expected real API with:
editorCommandparameter foropenInEditor- New
getAvailableEditorsandrefreshEditorsmethods- Comprehensive editor name mapping for display purposes
1773-1813: LGTM! Init script support for worktrees.The mock implementations for
getInitScript,setInitScript,deleteInitScript,runInitScript, andonInitScriptEventproperly support the new worktree initialization script feature.
2167-2170: LGTM! Mock implementation forresumeInterrupted.The mock correctly returns a success response with an appropriate message for the no-op case.
3100-3107: LGTM!isFavoriteproperty for dashboard pinning.The new optional
isFavoriteproperty on theProjectinterface supports pinning projects to the top of the dashboard, as described in the AI summary.apps/ui/src/components/views/settings-view/worktrees/index.ts (1)
1-1: LGTM! Clean barrel export for worktrees module.Standard re-export pattern that exposes
WorktreesSectionfor the settings view integration.apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts (1)
3-22: LGTM! Type extension for worktrees settings navigation.The new
'worktrees'option inSettingsViewIdenables navigation to the new worktrees settings section.apps/server/src/routes/worktree/routes/delete.ts (1)
9-13: LGTM! Good security-focused imports.The new imports for
isValidBranchNameandexecGitCommandsupport the injection-prevention pattern. The logger provides observability for security-relevant events.As per coding guidelines, server operations should emit events via
createEventEmitter()fromlib/events.tsfor streaming to the frontend. This route doesn't appear to need streaming events (it's a simple delete operation), but verify if any status updates should be emitted.apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (2)
39-49: LGTM! New props for editor selection and init script support.The API changes are well-structured:
onOpenInEditornow accepts optionaleditorCommandfor editor selectiononRunInitScriptandhasInitScriptenable init script execution from the worktree actionsThese align with the broader init-script and editor discovery features in this PR.
340-342: LGTM! Props correctly forwarded to child component.The new
onRunInitScriptandhasInitScriptprops are properly passed toWorktreeActionsDropdown.apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx (1)
140-165: LGTM!The
handleRunInitScriptimplementation correctly:
- Guards against missing
projectPath- Uses proper error handling with user-friendly toast notifications
- Is memoized with the correct dependency array
- Delegates success feedback to WebSocket events as noted in the comment
apps/server/src/index.ts (4)
173-175: LGTM!Service instantiation order is correct -
codexAppServerServiceis created first since bothcodexModelCacheServiceandcodexUsageServicedepend on it.
183-187: LGTM!Good use of
voidwith.catch()for non-blocking background initialization. This ensures the server starts immediately while the model cache warms up asynchronously, with errors properly logged.
600-618: LGTM!The global error handlers follow Node.js best practices:
unhandledRejection: Logs and continues (appropriate since the process state is still valid)uncaughtException: Logs and exits (correct since process state is undefined after an uncaught exception)
220-220: Events emitter correctly passed to worktree routes.This aligns with the coding guideline requiring
createEventEmitter()usage for server operations that stream events to the frontend via WebSocket. Based on learnings.apps/ui/src/components/views/settings-view.tsx (1)
162-165: LGTM!Clean integration of the new
WorktreesSection. The props correctly wire the globaluseWorktreesstate and its setter from the app store.apps/server/src/routes/worktree/routes/create.ts (3)
101-119: Good security practice: Input validation for branch names.The validation prevents command injection by restricting branch names to safe characters. Also correctly validates
baseBranchwhen provided, with an exception forHEAD.
170-190: LGTM!Excellent security improvement using array-based arguments with
execGitCommandinstead of string concatenation. This eliminates shell injection risks even if validation were bypassed.
213-222: Non-blocking init script execution pattern is correct.The response is sent immediately, and the init script runs asynchronously with errors logged. This ensures the API remains responsive while long-running scripts execute. Success/progress is appropriately streamed via WebSocket events.
apps/server/src/routes/worktree/index.ts (2)
101-109: LGTM!Init script routes are well-structured:
- Appropriate HTTP methods (GET/PUT/DELETE/POST)
- Path validation middleware applied to mutation operations
- Events emitter correctly passed to
createRunInitScriptHandlerfor WebSocket streaming
102-102: Cannot applyvalidatePathParamsto a GET endpoint with query parameters.The middleware
validatePathParamsis designed exclusively for request body parameters and reads fromreq.body. The GET /init-script endpoint uses query parameters (req.query.projectPath), which are incompatible with this middleware. GET requests typically do not have bodies, so this solution is architecturally infeasible.If path validation is needed for the query parameter, a separate query parameter validator would need to be created or the endpoint should be refactored to use POST with body parameters like the PUT and DELETE variants.
Likely an incorrect or invalid review comment.
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
226-273: LGTM!Well-implemented split-button dropdown for editor selection:
- Main area opens in the effective default editor
- Submenu provides access to other editors and Copy Path
- Icons are resolved outside JSX to avoid unnecessary re-renders
- Clipboard operation includes proper error handling with user feedback
274-279: LGTM!The Re-run Init Script option is correctly gated:
- Only shown for non-main worktrees (
!worktree.isMain)- Only shown when an init script exists (
hasInitScript)apps/server/src/routes/worktree/routes/init-script.ts (3)
36-76: LGTM!The GET handler correctly:
- Returns
exists: true/falseto differentiate between "no script" and "empty script"- Uses secure filesystem operations
- Has proper error handling with distinct responses for missing file vs unexpected errors
105-128: Good defense-in-depth with size limits and pattern warnings.The 1MB size limit prevents disk exhaustion attacks, and the dangerous pattern logging provides visibility without blocking legitimate use cases. The patterns cover common attack vectors (
rm -rf /, curl/wget piping to shell).Note: The regex
\/(?!\s*\$)catchesrm -rf /but would missrm -rf /*orrm -rf /home. Consider extending if more coverage is desired.
247-261: Non-blocking execution with immediate acknowledgment.The fire-and-forget pattern is appropriate here since progress is streamed via WebSocket events. The immediate response provides good UX while long-running scripts execute in the background.
apps/ui/src/components/views/board-view/board-header.tsx (2)
83-99: Optimistic update with fire-and-forget persistence looks appropriate.The pattern of updating local state first then persisting to server is a good UX choice. The error is logged but doesn't revert UI state - this is acceptable for a non-critical setting, but consider whether users should be notified of persistence failures beyond console logging.
136-158: Well-structured worktrees toggle integration.Good use of:
isMountedcheck to prevent hydration issuesdata-testidattributes for testability- Consistent styling with
controlContainerClassapps/ui/src/components/views/board-view.tsx (4)
1-1:@ts-nochecksuppresses all TypeScript errors.This disables TypeScript checking for the entire file, which can hide real type errors. Consider addressing the underlying type issues and removing this directive, or at minimum using more targeted
@ts-expect-errorcomments for specific lines.Is there a plan to remove
@ts-nocheckand address the type issues?
14-27: Clean implementation of dialog-aware drag sensor.The
DialogAwarePointerSensorcorrectly prevents drag operations from within dialogs by checking for[role="dialog"]ancestor. Good use of optional chaining onclosest.
523-560: Bulk delete implementation handles partial failures well.Good handling of:
- Partial success scenarios (some deletions succeed, others fail)
- Separate toast messages for success and failure counts
- Proper cleanup with
exitSelectionMode()andloadFeatures()The direct
useAppStore.getState().removeFeature()call at Line 538 is intentional to get fresh state reference - this is a valid Zustand pattern.
265-266: Init script events hook properly integrated.The
useInitScriptEventshook is correctly invoked with the project path, handling the null case via?? null. This sets up the WebSocket subscription for init script status updates.apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (4)
1-1:@ts-nocheckpresent in this file as well.Same concern as board-view.tsx - TypeScript checking is disabled for the entire file.
59-83: Well-documented work mode derivation logic.The
getDefaultWorkModefunction has clear JSDoc explaining the priority order:
forceCurrentBranchMode→ 'current'selectedNonMainWorktreeBranch→ 'custom'useWorktreessetting → 'auto' or 'current'Clean and easy to reason about.
443-471: Enhancement history integration looks correct.The history tracking logic:
- Preserves the original text before enhancement (unless it's a duplicate)
- Records the enhanced text with the enhancement mode
- Uses timestamps to track when changes occurred
The duplicate check at Line 454 prevents redundant entries when the same text is enhanced multiple times.
501-527: Good UX for disabled planning mode.The tooltip wrapper around the disabled
PlanningModeSelectprovides clear feedback to users about why planning modes are unavailable for non-Claude models. The wrapper<div>is necessary since disabled elements don't propagate pointer events for tooltips.apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx (1)
362-376: Helpful placeholder content in shell editor.The placeholder script provides good examples of common initialization tasks (npm install, pnpm, environment file setup) that guide users on what to include in their init scripts.
apps/server/src/services/init-script-service.ts (5)
82-87: Case-insensitive check for Windows system32 path.The check
bashInPath.toLowerCase().includes('system32')correctly handles case variations in Windows paths. Good defensive programming.
194-229: Excellent security practice with sandboxed environment.The safe environment construction:
- Explicitly allowlists only necessary environment variables
- Prevents exposure of sensitive credentials (API keys)
- Includes automaker-specific variables for script use
- Properly handles platform-specific variables
This is a strong security pattern for subprocess execution.
231-236: Script execution is safe from command injection.The script path is:
- Constructed from
path.join()with a fixed structure- Validated via
secureFs.access()which respectsALLOWED_ROOT_DIRECTORY- Passed as an argument to spawn (not string interpolation)
This prevents command injection attacks.
336-347: Singleton accessor is straightforward and appropriate.The lazy initialization singleton pattern is suitable for this stateful service that caches shell command detection results.
187-192: No changes needed. Theemitterparameter passed torunInitScriptis created viacreateEventEmitter()in the server's main entry point and injected through the route handler chain, which correctly implements the coding guideline.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx
Outdated
Show resolved
Hide resolved
| const response = await apiDelete<{ success: boolean; error?: string }>( | ||
| '/api/worktree/init-script', | ||
| { | ||
| body: { projectPath: currentProject.path }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the apiDelete function signature
rg -n "export async function apiDelete" apps/ui/src/lib/api-fetch.ts -A 15Repository: AutoMaker-Org/automaker
Length of output: 944
🏁 Script executed:
#!/bin/bash
# Check ApiFetchOptions type definition
rg -n "type ApiFetchOptions|interface ApiFetchOptions" apps/ui/src/lib/api-fetch.ts -A 20Repository: AutoMaker-Org/automaker
Length of output: 851
🏁 Script executed:
#!/bin/bash
# Search for other uses of apiDelete with body parameter in the codebase
rg -n "apiDelete" apps/ui/src -A 2 | grep -B 2 -A 2 "body:"Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for all uses of apiDelete to see the pattern of how it's typically called
rg -n "apiDelete" apps/ui/src -B 1 -A 3 | head -100Repository: AutoMaker-Org/automaker
Length of output: 3796
🏁 Script executed:
#!/bin/bash
# Check how apiFetch function handles the body parameter
rg -n "function apiFetch|const apiFetch" apps/ui/src/lib/api-fetch.ts -A 25 | head -50Repository: AutoMaker-Org/automaker
Length of output: 906
Consider alternative to DELETE with body for better compatibility.
The apiDelete call correctly uses the body parameter—ApiFetchOptions explicitly supports body?: unknown, and apiFetch properly handles it (lines 98-100 in api-fetch.ts stringify the body before passing to fetch).
However, HTTP DELETE with a request body is non-standard and may be rejected by proxies, load balancers, or legacy servers. Consider passing projectPath as a query parameter instead (/api/worktree/init-script?projectPath=...) or using a dedicated endpoint design that avoids body with DELETE.
- Update isOpencodeModel() to detect dynamic models with provider/model format (e.g., github-copilot/gpt-4o, google/gemini-2.5-pro, zai-coding-plan/glm-4.7) - Update resolveModelString() to recognize and pass through OpenCode models - Update enhance route to route OpenCode models to OpenCode provider - Fix OpenCode CLI command format: use --format json (not stream-json) - Remove unsupported -q and - flags from CLI arguments - Update normalizeEvent() to handle actual OpenCode JSON event format - Add dynamic model configuration UI with provider grouping - Cache providers and models in app store for snappier navigation - Show authenticated providers in OpenCode CLI status card Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update error event interface to handle nested error objects with name/data/message structure from OpenCode CLI - Extract meaningful error messages from provider errors in normalizeEvent - Add error type handling in executeWithProvider to throw errors with actual provider messages instead of returning empty response Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add icon detection for dynamic OpenCode provider models (provider/model format) - Support zai-coding-plan, github-copilot, google, xai, and other providers - Detect model type from name (glm, claude, gpt, gemini, grok, etc.) - Fix React.useMemo → useMemo to resolve "React is not defined" error Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Convert execFileSync to async execFile in fetchModelsFromCli and fetchAuthenticatedProviders to avoid blocking the event loop - Remove unused opencode-dynamic-providers.tsx component - Use regex for more robust model ID validation in parseModelsOutput Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test expected 4 enhancement modes but there are now 5 after adding the ux-reviewer mode. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace busy-wait loop in refreshModels with Promise-based approach - Remove duplicate error logging in opencode-models.ts handlers - Fix multi-slash parsing in provider-icon.tsx (only handle exactly one slash) - Use dynamic icon resolution for selected OpenCode model in trigger - Fix misleading comment about merge precedence (static takes precedence) - Add enabledOpencodeModels and opencodeDefaultModel to settings sync - Add clarifying comments about session-only dynamic model settings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add enabledOpencodeModels and opencodeDefaultModel to the settings migration to ensure they are properly persisted like Cursor settings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feat: add dynamic model discovery and routing for OpenCode provider
- Introduced a flag to track if a branch was successfully deleted, improving response clarity. - Updated the response structure to include the new branchDeleted flag. - Enhanced projectPath validation in init-script to ensure it is a non-empty string before processing.
There was a problem hiding this comment.
Actionable comments posted: 4
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/components/views/board-view/dialogs/backlog-plan-dialog.tsx (1)
121-123: UseresolveModelString()to resolve model aliases before passing to the API.The code passes the model alias (e.g., "sonnet", "opus") directly to
api.backlogPlan.generate()without resolving it to the full model ID. Per the coding guidelines, model aliases must be resolved usingresolveModelString()from@automaker/model-resolverbefore API calls. This pattern is used consistently throughout the codebase (e.g., inmodel-config.ts'sgetChatModel()and server-side services).Import
resolveModelStringfrom@automaker/model-resolverand resolve the model:const effectiveModel = resolveModelString(effectiveModelEntry.model);
🤖 Fix all issues with AI agents
In @apps/server/src/routes/setup/routes/opencode-models.ts:
- Around line 61-103: Update createSetupRoutes to accept an events: EventEmitter
parameter and thread it into the handler factory createGetOpencodeModelsHandler
so the route has access to events; modify createGetOpencodeModelsHandler to
accept events and use it around calls to provider.refreshModels() (the two
places triggered by forceRefresh and by !provider.hasCachedModels()) to emit
start and complete events (e.g., events.emit('opencode-models:refresh-start', {
source: 'setup', forceRefresh }) before calling refreshModels and
events.emit('opencode-models:refresh-complete', { source: 'setup', forceRefresh,
count: models.length }) after success), and also emit a failure event in the
catch block if refreshModels throws; ensure the handler continues to return the
same ModelsResponse shape and that all references use the existing symbols
createSetupRoutes, createGetOpencodeModelsHandler, provider.refreshModels, and
provider.getAvailableModels/provider.hasCachedModels.
In @apps/server/src/routes/worktree/routes/init-script.ts:
- Line 4: The top-of-file doc comment incorrectly says "POST /init-script" while
this route is a reader; update that doc comment to "GET /init-script" so it
matches the implemented handler and other comments (edit the comment string near
the route declaration in init-script.ts that currently reads "POST /init-script
- Read the init script content" to use "GET" instead).
- Around line 259-265: The call to forceRunInitScript is fire-and-forget but
returns a Promise<void>, so failures can become unhandled rejections; change the
call to handle errors by attaching a rejection handler (or awaiting within an
async context) and log/emit the error using the existing emitter (events) and
logger: call forceRunInitScript({...}).catch(err => { events.emit('error', err)
and/or processLogger.error(...) }) or await inside an async wrapper and handle
try/catch, ensuring any thrown error is logged and an error event is emitted.
🧹 Nitpick comments (14)
apps/server/src/routes/worktree/routes/init-script.ts (1)
26-31: Consider reusinggetInitScriptPathfrom the service.This function duplicates the logic already present in
init-script-service.ts(lines 46-48). Importing and reusing the existing function would reduce duplication and ensure consistency if the path logic ever changes.Suggested change
-import { forceRunInitScript } from '../../../services/init-script-service.js'; +import { forceRunInitScript, getInitScriptPath } from '../../../services/init-script-service.js'; -/** Fixed path for init script within .automaker directory */ -const INIT_SCRIPT_FILENAME = 'worktree-init.sh'; - /** Maximum allowed size for init scripts (1MB) */ const MAX_SCRIPT_SIZE_BYTES = 1024 * 1024; - -/** - * Get the full path to the init script for a project - */ -function getInitScriptPath(projectPath: string): string { - return path.join(projectPath, '.automaker', INIT_SCRIPT_FILENAME); -}apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx (2)
49-54: Unused functionextractModel.This function is defined but never called in this file. Consider removing it to reduce dead code.
🧹 Remove unused function
-/** - * Extract model string from PhaseModelEntry or string - */ -function extractModel(entry: PhaseModelEntry | string): ModelAlias | CursorModelId { - if (typeof entry === 'string') { - return entry as ModelAlias | CursorModelId; - } - return entry.model; -}
327-329: Type suppression forindeterminateprop.The
@ts-expect-errorworks but indicates a gap in the Checkbox component's type definitions. If this pattern is used elsewhere, consider extending the component's props interface to includeindeterminate?: boolean.apps/server/src/routes/worktree/routes/delete.ts (2)
6-12: Unused imports ifexecGitCommandis adopted consistently.Lines 43–46 still use
execAsyncforgit rev-parse, while the rest of the file usesexecGitCommand. For consistency and to avoid mixed patterns, consider migrating the branch name retrieval to useexecGitCommandas well. Once migrated,execandpromisifyimports become dead code.♻️ Suggested refactor
-import { exec } from 'child_process'; -import { promisify } from 'util'; import { isGitRepo } from '@automaker/git-utils'; import { getErrorMessage, logError, isValidBranchName, execGitCommand } from '../common.js'; import { createLogger } from '@automaker/utils'; -const execAsync = promisify(exec); const logger = createLogger('Worktree');Then update lines 42–49:
let branchName: string | null = null; try { - const { stdout } = await execAsync('git rev-parse --abbrev-ref HEAD', { - cwd: worktreePath, - }); - branchName = stdout.trim(); + const stdout = await execGitCommand(['rev-parse', '--abbrev-ref', 'HEAD'], worktreePath); + branchName = stdout.trim(); } catch { // Could not get branch name }
15-88: Consider emitting events for worktree operations.As per coding guidelines, server operations should use
createEventEmitter()fromlib/events.tsto stream events to the frontend via WebSocket. If the UI needs real-time updates about worktree deletions (e.g., for progress indication or dashboard refresh), consider emitting an event after successful deletion.apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
3-3: Unused import:Botappears to be replaced byOpenCodeIcon.The
Boticon is imported but only used in the "Not Authenticated" block (line 286). IfBotis intentionally used there, this is fine. However, verify this is the intended icon for that state, asOpenCodeIconis now used for the header.
8-13: MoveOpenCodeProviderInfoto shared types package to prevent duplication and centralize the source of truth.This type is currently duplicated between
apps/server/src/providers/opencode-provider.tsand the local UI component. Add it to@automaker/typesand import from there instead of the local file, aligning with the codebase pattern of importing from shared packages.apps/server/src/providers/opencode-provider.ts (2)
904-932: Consider adding more premium model indicators.The tier inference logic identifies premium models but may miss newer flagship models. Consider making this configurable or updating the patterns periodically.
750-754: Add timeout handling for slow CLI responses.The
execFileAsynccall has a 30-second timeout, which is appropriate. However, consider logging when the timeout is hit to help debug slow CLI issues.🔧 Suggested improvement
const { stdout } = await execFileAsync(command, args, { encoding: 'utf-8', timeout: 30000, windowsHide: true, }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { + opencodeLogger.warn('OpenCode CLI models command timed out after 30s'); + } + opencodeLogger.error(`Failed to fetch models from CLI: ${error}`); + return []; }apps/ui/src/components/ui/provider-icon.tsx (3)
92-99: Placeholder entries formetaandmistralmay cause confusion.These entries have empty
pathvalues since they use standalone SVG components. Consider adding a comment explaining this, or removing them fromPROVIDER_ICON_DEFINITIONSif they're not used byProviderIcon.📝 Suggested documentation
+ // Note: meta and mistral use custom standalone SVG components with multiple paths/colors + // These placeholder entries prevent TypeScript errors when iterating over ProviderIconKey meta: { viewBox: '0 0 24 24', - path: '', + path: '', // Uses MetaIcon standalone component }, mistral: { viewBox: '0 0 24 24', - path: '', + path: '', // Uses MistralIcon standalone component },
181-207:DeepSeekIconduplicates path data fromPROVIDER_ICON_DEFINITIONS.The path in
DeepSeekIconis the same asPROVIDER_ICON_DEFINITIONS.deepseek.path. Consider usingProviderIcondirectly likeOpenCodeIcondoes, which reduces duplication.♻️ Suggested refactor
-export function DeepSeekIcon({ - className, - title, - ...props -}: { - className?: string; - title?: string; -}) { - const hasAccessibleLabel = Boolean(title); - - return ( - <svg - viewBox="0 0 24 24" - className={cn('inline-block', className)} - role={hasAccessibleLabel ? 'img' : 'presentation'} - aria-hidden={!hasAccessibleLabel} - focusable="false" - {...props} - > - {title && <title>{title}</title>} - <path - d="M23.748 4.482c-.254-.124..." - fill="#4D6BFE" - /> - </svg> - ); -} +export function DeepSeekIcon(props: Omit<ProviderIconProps, 'provider'>) { + return <ProviderIcon provider={PROVIDER_ICON_KEYS.deepseek} {...props} />; +}
404-524: Complex model-to-icon resolution logic - consider extracting patterns.The
getUnderlyingModelIconfunction has grown complex with many conditional branches. Consider extracting the pattern matching into a data structure for maintainability.apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
176-182: Silent failure on Codex models fetch - consider user feedback.The
fetchCodexModelserror is silently caught. While this prevents crashes, users may not understand why the Codex section is empty. Consider adding a loading state or minimal error indicator.apps/server/src/routes/setup/routes/opencode-models.ts (1)
21-29: Consider adding a reset mechanism for testability.The module-level singleton cannot be reset between tests, which may cause test pollution. Consider exposing a reset function for testing purposes.
♻️ Proposed enhancement
// Singleton provider instance for caching let providerInstance: OpencodeProvider | null = null; function getProvider(): OpencodeProvider { if (!providerInstance) { providerInstance = new OpencodeProvider(); } return providerInstance; } + +/** + * Resets the provider instance (for testing purposes only) + * @internal + */ +export function resetProviderInstance(): void { + providerInstance = null; +}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/server/src/providers/opencode-provider.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/server/src/routes/worktree/routes/delete.tsapps/server/src/routes/worktree/routes/init-script.tsapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/ui/provider-icon.tsxapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/views/settings-view/providers/opencode-model-configuration.tsxapps/ui/src/components/views/settings-view/providers/opencode-settings-tab.tsxapps/ui/src/components/views/setup-view/steps/opencode-setup-step.tsxapps/ui/src/components/views/setup-view/steps/providers-setup-step.tsxapps/ui/src/hooks/use-settings-migration.tsapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tslibs/model-resolver/src/resolver.tslibs/prompts/tests/enhancement.test.tslibs/types/src/opencode-models.tslibs/types/src/provider-utils.tslibs/types/src/settings.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/routes/worktree/routes/delete.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/ui/provider-icon.tsxapps/server/src/routes/setup/index.tsapps/server/src/routes/worktree/routes/init-script.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/worktree/routes/delete.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/server/src/providers/opencode-provider.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/worktree/routes/init-script.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/routes/worktree/routes/delete.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/setup/routes/opencode-models.tsapps/server/tests/unit/providers/opencode-provider.test.tsapps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/ui/provider-icon.tsxapps/server/src/routes/setup/index.tsapps/server/src/routes/worktree/routes/init-script.ts
🧠 Learnings (3)
📚 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/routes/worktree/routes/delete.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: 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/tests/unit/providers/opencode-provider.test.tsapps/server/src/providers/opencode-provider.tsapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/ui/provider-icon.tsx
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsxapps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsxapps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsxapps/ui/src/components/ui/provider-icon.tsx
🧬 Code graph analysis (6)
apps/server/src/routes/worktree/routes/delete.ts (1)
apps/server/src/routes/worktree/common.ts (2)
execGitCommand(36-50)isValidBranchName(110-112)
apps/server/src/routes/enhance-prompt/routes/enhance.ts (1)
libs/types/src/provider-utils.ts (1)
isOpencodeModel(103-138)
apps/server/src/routes/setup/routes/opencode-models.ts (2)
apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(67-76)apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
OpenCodeProviderInfo(8-13)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (2)
apps/server/src/providers/opencode-provider.ts (1)
OpenCodeProviderInfo(67-76)apps/ui/src/components/ui/provider-icon.tsx (1)
OpenCodeIcon(177-179)
apps/server/src/providers/opencode-provider.ts (5)
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (1)
OpenCodeProviderInfo(8-13)libs/types/src/opencode-models.ts (1)
OpencodeProvider(16-16)apps/server/src/providers/index.ts (3)
OpencodeProvider(29-29)ModelDefinition(18-18)ExecuteOptions(15-15)libs/types/src/index.ts (3)
ModelDefinition(15-15)ExecuteOptions(10-10)ContentBlock(11-11)scripts/launcher-utils.mjs (1)
lines(190-190)
apps/server/src/routes/worktree/routes/init-script.ts (3)
apps/server/src/services/init-script-service.ts (4)
getInitScriptPath(47-49)getInitScriptPath(350-351)forceRunInitScript(317-333)forceRunInitScript(359-360)apps/server/src/lib/secure-fs.ts (1)
secureFs(8-39)apps/server/src/routes/worktree/common.ts (2)
logError(180-180)isValidBranchName(110-112)
🪛 Biome (2.1.2)
apps/server/src/providers/opencode-provider.ts
[error] 788-788: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 1044-1044: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (35)
apps/server/src/routes/worktree/routes/init-script.ts (2)
91-163: LGTM!The PUT handler has appropriate validation (projectPath required, content type check, size limit enforcement), uses secure filesystem operations, and correctly logs warnings for potentially dangerous patterns without blocking the operation.
168-196: LGTM!Clean implementation with proper validation and use of
force: trueto handle the case where the file doesn't exist gracefully.apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx (2)
66-68: LGTM!The new optional
currentBranchprop is well-documented and correctly defined in the interface.
173-195: LGTM!The
currentBranchprop is correctly threaded through to the API call with a sensible default of'main', and the dependency array properly includescurrentBranchto ensure the callback updates when the branch changes.apps/server/src/routes/worktree/routes/delete.ts (4)
51-57: LGTM!Good use of
execGitCommandwith array arguments to prevent command injection. The fallback toworktree prunewhen removal fails is reasonable error recovery.
59-74: LGTM!Good defensive coding:
- Branch name validation with
isValidBranchNameprevents injection- Protection of
main/masterbranches from accidental deletion- Proper warning logs for debugging invalid names and failed deletions
branchDeletedflag accurately tracks deletion outcome
76-83: LGTM!The response structure is clear, with
branchDeletedexplicitly indicating whether the branch was deleted, andbranchonly populated when deletion succeeded.
40-49: TheworktreePathparameter is validated via thevalidatePathParamsmiddleware before the handler executes. The middleware callsvalidatePath()which resolves the path and checks it againstALLOWED_ROOT_DIRECTORY, preventing path traversal attacks. While the/deleteroute does not use therequireValidWorktreemiddleware (which additionally checks if the path is a valid git repository), the path containment validation fromvalidatePathParamsis sufficient for safe use as thecwdargument inexecAsync. No security vulnerability exists here.Likely an incorrect or invalid review comment.
apps/ui/src/components/views/settings-view/cli-status/opencode-cli-status.tsx (3)
184-191: LGTM - Clean provider filtering and default value.The
providers = []default andauthenticatedProvidersfiltering are well-implemented. The early return with skeleton on missing status is appropriate.
260-299: LGTM - Authentication status rendering handles both direct auth and provider-based auth.The conditional logic correctly prioritizes direct
authStatus?.authenticatedover provider count, with appropriate fallback rendering for the "free tier ready" state.
301-337: LGTM - Dynamic providers section with proper badge rendering.The connected providers section correctly displays authenticated providers with their auth method badges. The pluralization logic for "Provider(s)" is handled properly.
apps/server/src/routes/enhance-prompt/routes/enhance.ts (3)
101-133: LGTM - Provider execution with proper error handling.The renamed
executeWithProviderfunction cleanly abstracts provider execution for both Cursor and OpenCode models. The error handling (lines 114-118) properly catches and rethrows provider errors with meaningful messages.
191-197: LGTM - Enhancement mode expansion to includeux-reviewer.The systemPromptMap correctly includes the new
ux-reviewermode, maintaining consistency with the existing pattern.
214-227: LGTM - OpenCode model routing.The routing logic correctly identifies OpenCode models and uses the combined prompt approach. The comment on line 225 accurately explains why prompts are combined for OpenCode CLI.
apps/server/src/providers/opencode-provider.ts (5)
78-83: LGTM - Cache duration and validation patterns are well-defined.The 5-minute cache duration is reasonable for model discovery. The regex patterns for model ID validation are appropriately strict.
671-688: LGTM - Concurrent refresh prevention with promise reuse.The
refreshModels()method correctly prevents duplicate concurrent refreshes by reusing the existing promise. Thefinallyblock properly cleans up state.
786-815: Static analysis flag: Control character in regex is intentional for ANSI escape codes.The regex
/\x1b\[[0-9;]*m/gon line 788 is correct for stripping ANSI escape sequences from CLI output. The\x1b(ESC character) is necessary to match ANSI codes. This is a false positive from the static analysis tool.
1041-1046: Same ANSI escape code pattern - intentional control character usage.Line 1044 uses the same ANSI stripping regex pattern as line 788. This is intentional and correct for parsing CLI output.
324-341: The--format jsonargument is correct for OpenCode CLI.OpenCode CLI supports
--format jsonwhich outputs raw JSON events in newline-delimited JSON/JSONL format, suitable for streaming and programmatic consumption. The code and comment are accurate.apps/server/tests/unit/providers/opencode-provider.test.ts (4)
54-67: LGTM - Updated model count and default model assertions.Tests correctly reflect the updated model set (5 models) and verify Big Pickle as the default OpenCode model.
106-122: LGTM - New parseModelsOutput test covers nested provider IDs.The test correctly verifies that nested provider model IDs like
openrouter/anthropic/claude-3.5-sonnetare parsed with the first segment as the provider.
232-284: LGTM - Event normalization tests updated for new format.Tests correctly use the new event structure with
type: 'text'and nestedpartobjects containing the actual data.
1195-1210: Good edge case coverage: error takes precedence when both result and error are present.This test verifies the important behavior that when a step_finish event contains both a result and an error, the error handling takes precedence.
apps/ui/src/components/ui/provider-icon.tsx (1)
144-148: LGTM - ProviderIcon now supports custom fill and fillRule.The component correctly applies
fillRulewhen defined and defaults tocurrentColorfor fill, maintaining backward compatibility.apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (4)
254-262: LGTM - Memoized Codex model transformation.The transformation correctly maps store models to component format with appropriate tier-based badges.
367-386: LGTM - Static and dynamic OpenCode models merged with deduplication.The merge logic correctly prioritizes static models over dynamic ones when IDs overlap, preventing duplicates.
442-527: Well-structured OpenCode sectioning with priority ordering.The
opencodeSectionsmemoization creates a clean two-tier grouping (Free/Dynamic) with provider-based subgroups. The priority ordering for dynamic providers is a good UX consideration.
1193-1219: LGTM - OpenCode sections render with proper Fragment usage.The rendering correctly uses
Fragmentfor clean JSX structure and applies conditional styling for dynamic sections.apps/server/src/routes/setup/index.ts (2)
27-32: LGTM - Clean import of OpenCode model handlers.The four handler factories are imported consistently with the existing pattern.
74-79: LGTM - RESTful route registration for OpenCode model discovery.The routes follow RESTful conventions:
- GET
/opencode/modelsfor listing models- POST
/opencode/models/refreshfor triggering refresh- GET
/opencode/providersfor listing providers- POST
/opencode/cache/clearfor cache invalidationapps/server/src/routes/setup/routes/opencode-models.ts (5)
10-19: LGTM!Imports correctly use shared packages (
@automaker/types,@automaker/utils) as per coding guidelines. The relative import from the local provider is appropriate for intra-app dependencies.
31-50: LGTM!Response types are well-structured with consistent success/error patterns.
105-132: LGTM!The refresh handler follows a consistent pattern with proper error handling. The event emission consideration from the previous handler applies here as well.
134-164: LGTM!The handler correctly fetches and filters providers, returning both the full list and authenticated subset for client flexibility.
166-189: LGTM!Cache clearing handler is properly implemented. Consider the event emission suggestion from earlier for notifying the UI of cache state changes.
| export function createGetOpencodeModelsHandler() { | ||
| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const provider = getProvider(); | ||
| const forceRefresh = req.query.refresh === 'true'; | ||
|
|
||
| let models: ModelDefinition[]; | ||
| let cached = true; | ||
|
|
||
| if (forceRefresh) { | ||
| models = await provider.refreshModels(); | ||
| cached = false; | ||
| } else { | ||
| // Check if we have cached models | ||
| const cachedModels = provider.getAvailableModels(); | ||
|
|
||
| // If cache only has default models (provider.hasCachedModels() would be false), | ||
| // trigger a refresh to get dynamic models | ||
| if (!provider.hasCachedModels()) { | ||
| models = await provider.refreshModels(); | ||
| cached = false; | ||
| } else { | ||
| models = cachedModels; | ||
| } | ||
| } | ||
|
|
||
| const response: ModelsResponse = { | ||
| success: true, | ||
| models, | ||
| count: models.length, | ||
| cached, | ||
| }; | ||
|
|
||
| res.json(response); | ||
| } catch (error) { | ||
| logError(error, 'Get OpenCode models failed'); | ||
| res.status(500).json({ | ||
| success: false, | ||
| error: getErrorMessage(error), | ||
| } as ModelsResponse); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Check if other refresh routes use createEventEmitter
rg -n "createEventEmitter|eventEmitter" apps/server/src/routes/ -A 3Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check if lib/events.ts exists and how it's used
fd "events.ts" apps/server/src/Repository: AutoMaker-Org/automaker
Length of output: 96
🏁 Script executed:
# Search for model refresh operations and how they handle events
rg -n "refreshModels|refresh" apps/server/src/ -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 16766
🏁 Script executed:
# Check the events.ts API
cat apps/server/src/lib/events.tsRepository: AutoMaker-Org/automaker
Length of output: 1041
🏁 Script executed:
# Search for createEventEmitter usage anywhere in apps/server/src
rg -n "createEventEmitter" apps/server/src/Repository: AutoMaker-Org/automaker
Length of output: 333
🏁 Script executed:
# Also search for eventEmitter usage more broadly
rg -n "eventEmitter" apps/server/src/ | head -50Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Search for how events object is being used in the app
rg -n "events\." apps/server/src/ -B 2 -A 2 | head -100Repository: AutoMaker-Org/automaker
Length of output: 8453
🏁 Script executed:
# Check if handlers receive events as a parameter
rg -n "Handler.*events|emit\(" apps/server/src/routes/ -B 1 -A 1Repository: AutoMaker-Org/automaker
Length of output: 19508
🏁 Script executed:
# Look at how index.ts uses the events object
sed -n '160,200p' apps/server/src/index.tsRepository: AutoMaker-Org/automaker
Length of output: 1820
🏁 Script executed:
# Check the setup routes index.ts to see if it should receive events
sed -n '1,100p' apps/server/src/routes/setup/index.tsRepository: AutoMaker-Org/automaker
Length of output: 4625
🏁 Script executed:
# Check how other route modules are initialized with events
rg -n "export.*function.*router" apps/server/src/routes/*/index.ts -A 2Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check how routes are mounted in main index.ts
rg -n "createSetupRoutes|createWorktreeRoutes|createSuggestionsRoutes" apps/server/src/index.ts -B 2 -A 2Repository: AutoMaker-Org/automaker
Length of output: 1319
🏁 Script executed:
# Look at the full routing setup section in index.ts
sed -n '180,250p' apps/server/src/index.tsRepository: AutoMaker-Org/automaker
Length of output: 3275
🏁 Script executed:
# Verify that setup routes actually don't receive events in current implementation
grep -n "createSetupRoutes" apps/server/src/index.tsRepository: AutoMaker-Org/automaker
Length of output: 178
🏁 Script executed:
# Check if there's any event emission in setup route handlers
rg -n "emit" apps/server/src/routes/setup/Repository: AutoMaker-Org/automaker
Length of output: 49
Add event emission to setup routes per coding guidelines.
Coding guidelines mandate using createEventEmitter() from lib/events.ts for ALL server operations. The setup routes currently don't receive the events parameter, unlike other routes (worktree, suggestions, github, ideation, backlog-plan) that do.
Update createSetupRoutes() to accept an events: EventEmitter parameter and pass it to handler factories. Model refresh operations at lines 71, 80, and 114 should emit events (e.g., events.emit('opencode-models:refresh-start', {...}) and events.emit('opencode-models:refresh-complete', {...})) to stream refresh status to the frontend via WebSocket.
🤖 Prompt for AI Agents
In @apps/server/src/routes/setup/routes/opencode-models.ts around lines 61 -
103, Update createSetupRoutes to accept an events: EventEmitter parameter and
thread it into the handler factory createGetOpencodeModelsHandler so the route
has access to events; modify createGetOpencodeModelsHandler to accept events and
use it around calls to provider.refreshModels() (the two places triggered by
forceRefresh and by !provider.hasCachedModels()) to emit start and complete
events (e.g., events.emit('opencode-models:refresh-start', { source: 'setup',
forceRefresh }) before calling refreshModels and
events.emit('opencode-models:refresh-complete', { source: 'setup', forceRefresh,
count: models.length }) after success), and also emit a failure event in the
catch block if refreshModels throws; ensure the handler continues to return the
same ModelsResponse shape and that all references use the existing symbols
createSetupRoutes, createGetOpencodeModelsHandler, provider.refreshModels, and
provider.getAvailableModels/provider.hasCachedModels.
| /** | ||
| * Init Script routes - Read/write/run the worktree-init.sh file | ||
| * | ||
| * POST /init-script - Read the init script content |
There was a problem hiding this comment.
Doc comment typo: POST should be GET.
The comment says "POST /init-script" for reading, but the implementation and other comments indicate this should be GET.
Suggested fix
- * POST /init-script - Read the init script content
+ * GET /init-script - Read the init script content📝 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.
| * POST /init-script - Read the init script content | |
| * GET /init-script - Read the init script content |
🤖 Prompt for AI Agents
In @apps/server/src/routes/worktree/routes/init-script.ts at line 4, The
top-of-file doc comment incorrectly says "POST /init-script" while this route is
a reader; update that doc comment to "GET /init-script" so it matches the
implemented handler and other comments (edit the comment string near the route
declaration in init-script.ts that currently reads "POST /init-script - Read the
init script content" to use "GET" instead).
| } catch { | ||
| // File doesn't exist | ||
| res.json({ | ||
| success: true, | ||
| exists: false, | ||
| content: '', | ||
| path: scriptPath, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Catch block assumes all errors mean file doesn't exist.
This catches all exceptions and treats them as "file not found." Other errors like permission denied (EACCES) or I/O errors would be silently masked, returning exists: false instead of surfacing the actual problem.
Suggested fix to differentiate error types
- } catch {
- // File doesn't exist
- res.json({
- success: true,
- exists: false,
- content: '',
- path: scriptPath,
- });
+ } catch (err) {
+ // Only treat ENOENT as "file doesn't exist"
+ if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
+ res.json({
+ success: true,
+ exists: false,
+ content: '',
+ path: scriptPath,
+ });
+ } else {
+ throw err; // Re-throw other errors to be handled by outer catch
+ }
}📝 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.
| } catch { | |
| // File doesn't exist | |
| res.json({ | |
| success: true, | |
| exists: false, | |
| content: '', | |
| path: scriptPath, | |
| }); | |
| } | |
| } catch (err) { | |
| // Only treat ENOENT as "file doesn't exist" | |
| if ((err as NodeJS.ErrnoException).code === 'ENOENT') { | |
| res.json({ | |
| success: true, | |
| exists: false, | |
| content: '', | |
| path: scriptPath, | |
| }); | |
| } else { | |
| throw err; // Re-throw other errors to be handled by outer catch | |
| } | |
| } |
| // Run the script asynchronously (non-blocking) | ||
| forceRunInitScript({ | ||
| projectPath, | ||
| worktreePath, | ||
| branch, | ||
| emitter: events, | ||
| }); |
There was a problem hiding this comment.
Missing error handling for async forceRunInitScript call.
forceRunInitScript returns a Promise<void> but is called without await and without a .catch() handler. If the promise rejects after the response is sent, it will cause an unhandled promise rejection. While progress is streamed via WebSocket events, startup failures or unexpected errors should still be caught and logged.
Suggested fix
// Run the script asynchronously (non-blocking)
forceRunInitScript({
projectPath,
worktreePath,
branch,
emitter: events,
- });
+ }).catch((err) => {
+ logError(err, `Init script execution failed for branch "${branch}"`);
+ });🤖 Prompt for AI Agents
In @apps/server/src/routes/worktree/routes/init-script.ts around lines 259 -
265, The call to forceRunInitScript is fire-and-forget but returns a
Promise<void>, so failures can become unhandled rejections; change the call to
handle errors by attaching a rejection handler (or awaiting within an async
context) and log/emit the error using the existing emitter (events) and logger:
call forceRunInitScript({...}).catch(err => { events.emit('error', err) and/or
processLogger.error(...) }) or await inside an async wrapper and handle
try/catch, ensuring any thrown error is logged and an error event is emitted.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/tests/unit/lib/enhancement-prompts.test.ts (1)
154-161: Hardcoded mode array is missing 'ux-reviewer'.This hardcoded array was not updated when
ENHANCEMENT_MODESwas introduced, causing inconsistent test coverage. ThegetExamplesfunction won't be tested with the'ux-reviewer'mode.🐛 Proposed fix
it('should return arrays with example objects', () => { - const modes: EnhancementMode[] = ['improve', 'technical', 'simplify', 'acceptance']; - modes.forEach((mode) => { + ENHANCEMENT_MODES.forEach((mode) => { const examples = getExamples(mode); expect(Array.isArray(examples)).toBe(true); expect(examples.length).toBeGreaterThan(0); }); });
🧹 Nitpick comments (3)
apps/server/tests/unit/lib/enhancement-prompts.test.ts (2)
20-27: Consider deriving ENHANCEMENT_MODES from the source of truth.Defining a local
ENHANCEMENT_MODESconstant risks going out of sync with the actual implementation. Consider importing it from@/lib/enhancement-prompts.jsif exported, or deriving it fromgetAvailableEnhancementModes()which the tests already import.♻️ Suggested approach
-const ENHANCEMENT_MODES: EnhancementMode[] = [ - 'improve', - 'technical', - 'simplify', - 'acceptance', - 'ux-reviewer', -]; +// Derive from the source of truth to avoid duplication +const ENHANCEMENT_MODES = getAvailableEnhancementModes();
137-143: Add 'ux-reviewer' mode coverage to getSystemPrompt test.The test omits the 'ux-reviewer' mode defined in
ENHANCEMENT_MODES. Refactor to iterate overENHANCEMENT_MODESto cover all modes consistently with other test cases (e.g.,buildUserPromptat line 195).apps/server/tests/unit/routes/app-spec/common.test.ts (1)
10-64: Consider adding a test for multi-project isolation.Since the API now supports per-project state management, consider adding a test that verifies state isolation between different project paths (e.g., setting one project to running doesn't affect another project's state).
🧪 Example test for multi-project isolation
it('should maintain separate state for different projects', () => { const projectA = '/tmp/project-a'; const projectB = '/tmp/project-b'; const controllerA = new AbortController(); setRunningState(projectA, true, controllerA); setRunningState(projectB, false, null); expect(getSpecRegenerationStatus(projectA).isRunning).toBe(true); expect(getSpecRegenerationStatus(projectA).currentAbortController).toBe(controllerA); expect(getSpecRegenerationStatus(projectB).isRunning).toBe(false); expect(getSpecRegenerationStatus(projectB).currentAbortController).toBe(null); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/server/tests/unit/lib/enhancement-prompts.test.tsapps/server/tests/unit/routes/app-spec/common.test.tsapps/server/tests/unit/services/dev-server-service.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/server/tests/unit/routes/app-spec/common.test.tsapps/server/tests/unit/services/dev-server-service.test.tsapps/server/tests/unit/lib/enhancement-prompts.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/server/tests/unit/routes/app-spec/common.test.tsapps/server/tests/unit/services/dev-server-service.test.tsapps/server/tests/unit/lib/enhancement-prompts.test.ts
🧬 Code graph analysis (1)
apps/server/tests/unit/routes/app-spec/common.test.ts (1)
apps/server/src/routes/app-spec/common.ts (1)
getSpecRegenerationStatus(16-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (10)
apps/server/tests/unit/lib/enhancement-prompts.test.ts (3)
195-200: Good use of the centralized ENHANCEMENT_MODES constant.This correctly iterates over all enhancement modes, ensuring test coverage scales with new modes.
209-223: Test coverage for 'ux-reviewer' validity is good.The addition of
isValidEnhancementMode('ux-reviewer')assertion at line 215 properly validates the new mode.
226-233: LGTM!The refactored assertions correctly use
ENHANCEMENT_MODESfor both length and membership checks, ensuring this test stays in sync with mode additions.apps/server/tests/unit/services/dev-server-service.test.ts (1)
8-12: LGTM - Mock scaffolding forexecFileadded.The mock is correctly structured alongside the existing
spawnandexecSyncmocks. Note thatexecFileis not currently imported (line 27) or used in the service—this is preparatory scaffolding for potential future use in Codex app-server workflows.apps/server/tests/unit/routes/app-spec/common.test.ts (6)
1-9: LGTM!The import uses the project's path alias (
@/) which is appropriate for intra-app imports. TheTEST_PROJECT_PATHconstant is a clean approach for parameterizing the tests with a consistent project path.
11-14: LGTM!The
beforeEachproperly resets the per-project state before each test, ensuring test isolation.
16-39: LGTM!Tests are correctly parameterized with the project path and verify the expected state transitions for
isRunningandcurrentAbortController.
48-63: LGTM!Comprehensive test covering multiple state transitions with proper verification at each step.
66-103: LGTM!The
getErrorMessagetests provide thorough coverage including edge cases for empty messages, custom properties, and subclassed errors.
41-46: Approved - The implementation and tests are correct.The test correctly demonstrates that when
running=trueandnullis explicitly passed, the existing abort controller is preserved. This is intentional behavior: the function only updates the abort controller map when a truthy controller is provided (line 53-54:if (controller) { abortControllers.set(...) }). Whenrunning=false, both maps are always cleared regardless of the controller argument. This is sensible API design, not counterintuitive—it allows callers to preserve the existing controller by not updating it while remaining in running state, and to clear everything when stopping.
DhanushSantosh
force-pushed
the
v0.10.0rc
branch
from
54d0509 to
fbab1d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/server/src/lib/claude-sdk.ts:
- Around line 21-26: The prompt property in the ClaudeQueryParams interface is
too broad and must be narrowed to the SDK type expected by claudeQuery; change
ClaudeQueryParams.prompt from string | AsyncIterable<unknown> to string |
AsyncIterable<SDKUserMessage>, import or reference SDKUserMessage from the
Claude SDK, and update any callers if needed so they pass
AsyncIterable<SDKUserMessage> when using claudeQuery.
🧹 Nitpick comments (3)
apps/server/src/routes/suggestions/generate-suggestions.ts (1)
316-332: Consider adding optional chaining for safer property access.The
ClaudeStreamMessageinterface inclaude-sdk.tsdefinesmessageas optional (message?: {...}). While themsg.type === 'assistant'check implies content exists, TypeScript may not narrow this correctly without optional chaining.♻️ Suggested improvement
for await (const msg of stream) { - if (msg.type === 'assistant' && msg.message.content) { - for (const block of msg.message.content) { + if (msg.type === 'assistant' && msg.message?.content) { + for (const block of msg.message.content) {apps/server/src/services/auto-mode-service.ts (1)
3064-3143: Consider parallel resumption for interrupted features.The
resumeInterruptedFeaturesmethod resumes features sequentially withawait this.resumeFeature(...). If many features were interrupted, this could cause significant startup delay. Consider whether parallel resumption (respectingmaxConcurrency) would be more appropriate.♻️ Alternative: parallel resumption with concurrency control
// Resume each interrupted feature - for (const feature of interruptedFeatures) { - try { - logger.info(`Resuming feature: ${feature.id} (${feature.title})`); - // Use resumeFeature which will detect the existing context and continue - await this.resumeFeature(projectPath, feature.id, true); - } catch (error) { - logger.error(`Failed to resume feature ${feature.id}:`, error); - // Continue with other features - } - } + // Resume features in parallel (fire-and-forget, similar to normal auto-loop) + for (const feature of interruptedFeatures) { + logger.info(`Resuming feature: ${feature.id} (${feature.title})`); + this.resumeFeature(projectPath, feature.id, true).catch((error) => { + logger.error(`Failed to resume feature ${feature.id}:`, error); + }); + }apps/server/src/providers/claude-provider.ts (1)
129-129: Consider adding fallbackText for mock mode consistency.Unlike other call sites (e.g.,
describe-image.ts,generate-spec.ts,generate-title.ts), this invocation ofqueryClaudeomitsfallbackText. In mock mode,queryClaudewill use an empty/default fallback, which may cause tests or mock scenarios to behave unexpectedly.Since this is the provider layer handling arbitrary prompts, generating a meaningful fallback is less straightforward. If intentional, consider adding a brief comment explaining that fallback is omitted at this layer because the caller should handle mock scenarios.
💡 Optional: Add clarifying comment
const stream = queryClaude({ prompt: promptPayload, options: sdkOptions }); + // Note: fallbackText omitted - callers using ClaudeProvider in mock mode should + // provide their own fallback handling or use queryClaude directly with fallbackText
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/server/src/lib/claude-sdk.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/routes/context/routes/describe-file.tsapps/server/src/routes/context/routes/describe-image.tsapps/server/src/routes/enhance-prompt/routes/enhance.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/setup/routes/verify-claude-auth.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/services/auto-mode-service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/server/src/routes/enhance-prompt/routes/enhance.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/routes/context/routes/describe-file.tsapps/server/src/lib/claude-sdk.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/services/auto-mode-service.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/setup/routes/verify-claude-auth.tsapps/server/src/routes/context/routes/describe-image.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/context/routes/describe-file.tsapps/server/src/lib/claude-sdk.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/services/auto-mode-service.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/setup/routes/verify-claude-auth.tsapps/server/src/routes/context/routes/describe-image.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/routes/context/routes/describe-file.tsapps/server/src/lib/claude-sdk.tsapps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/routes/app-spec/generate-spec.tsapps/server/src/providers/claude-provider.tsapps/server/src/routes/app-spec/generate-features-from-spec.tsapps/server/src/routes/features/routes/generate-title.tsapps/server/src/services/auto-mode-service.tsapps/server/src/routes/github/routes/validate-issue.tsapps/server/src/routes/setup/routes/verify-claude-auth.tsapps/server/src/routes/context/routes/describe-image.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T05:07:48.147Z
Learnt from: CR
Repo: AutoMaker-Org/automaker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T05:07:48.147Z
Learning: Applies to **/*.{ts,tsx} : Use `resolveModelString()` from automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Applied to files:
apps/server/src/routes/suggestions/generate-suggestions.tsapps/server/src/services/auto-mode-service.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: 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 (9)
apps/server/src/routes/context/routes/describe-file.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/suggestions/generate-suggestions.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/app-spec/generate-spec.ts (3)
libs/types/src/index.ts (1)
SpecOutput(88-88)apps/server/src/lib/app-spec-format.ts (2)
SpecOutput(9-9)specToXml(27-100)apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/providers/claude-provider.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/app-spec/generate-features-from-spec.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/features/routes/generate-title.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/services/auto-mode-service.ts (2)
libs/platform/src/paths.ts (4)
ensureAutomakerDir(198-202)getExecutionStatePath(185-187)getFeaturesDir(36-38)getFeatureDir(49-51)apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/setup/routes/verify-claude-auth.ts (2)
apps/server/src/lib/auth-utils.ts (1)
AuthRateLimiter(226-263)apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
apps/server/src/routes/context/routes/describe-image.ts (1)
apps/server/src/lib/claude-sdk.ts (1)
queryClaude(68-80)
🪛 GitHub Actions: E2E Tests
apps/server/src/lib/claude-sdk.ts
[error] 75-75: Type 'string | AsyncIterable' is not assignable to type 'string | AsyncIterable'. (tsc) during command 'npm run build --workspace=apps/server'
🪛 GitHub Actions: PR Build Check
apps/server/src/lib/claude-sdk.ts
[error] 75-75: Type 'string | AsyncIterable' is not assignable to type 'string | AsyncIterable'.
🔇 Additional comments (25)
apps/server/src/routes/github/routes/validate-issue.ts (2)
46-54: LGTM - Mock validation constants are well-structured.The mock data constants follow a consistent pattern across the codebase and appropriately provide fallback values for test scenarios. The structured approach with separate constants for verdict, confidence, and reasoning improves maintainability.
218-223: LGTM - queryClaude integration looks correct.The
queryClaudewrapper is invoked with appropriate parameters includingfallbackTextandfallbackStructuredOutputfor mock mode support. This aligns with the pattern established inapps/server/src/lib/claude-sdk.ts.apps/server/src/routes/setup/routes/verify-claude-auth.ts (2)
16-20: LGTM - Clean import and mock constant addition.The
queryClaudeimport andMOCK_AUTH_SUCCESS_RESPONSEconstant are appropriately minimal for auth verification purposes.
157-166: LGTM - Auth verification uses queryClaude correctly.The fallback text
'ok'is appropriate for verifying authentication in mock mode, matching the expected response from the prompt.apps/server/src/lib/claude-sdk.ts (2)
37-65: LGTM - Mock stream implementation is well-designed.The
createMockStreamfunction correctly creates an async generator that yields both an assistant message with text content and a result message with optional structured output. This mirrors the expected stream format from the real Claude SDK.
68-79: LGTM - queryClaude wrapper logic is correct.The function properly checks for mock mode and either delegates to the real SDK or returns a mock stream. The fallback text normalization ensures a sensible default.
apps/server/src/routes/suggestions/generate-suggestions.ts (2)
28-43: LGTM - Mock suggestions data is well-structured.The mock constants and JSON response follow the established pattern and match the expected schema for suggestions.
299-314: LGTM - queryClaude integration with fallback data.Both
fallbackTextandfallbackStructuredOutputare provided, ensuring consistent behavior in mock mode with proper structured output for parsing.apps/server/src/services/auto-mode-service.ts (3)
211-232: LGTM - ExecutionState interface is well-designed for recovery.The interface captures all necessary state for recovering after server restart, including auto-loop status, concurrency settings, and running feature IDs. The version field enables future migrations.
3006-3026: LGTM - State persistence implementation is robust.The
saveExecutionStatemethod properly ensures the directory exists before writing and includes appropriate error handling with logging.
3196-3216: LGTM - Learning extraction uses queryClaude correctly.The mock fallback for learning extraction appropriately returns an empty learnings array, which is handled gracefully by the parsing logic below.
apps/server/src/routes/context/routes/describe-file.ts (2)
27-27: LGTM - File-specific mock description helper is well-designed.The
buildMockFileDescriptionfunction generates meaningful fallback descriptions that include the filename, making mock responses more useful for testing and debugging.Also applies to: 53-56
255-259: LGTM - queryClaude integration with async generator prompt.The implementation correctly passes an async generator for the prompt (matching the multi-part content pattern) and provides a file-specific fallback text.
apps/server/src/routes/app-spec/generate-features-from-spec.ts (2)
25-43: LGTM - Mock feature data matches expected schema.The mock response includes all required fields (id, category, title, description, priority, complexity, dependencies) matching the JSON schema defined in the prompt.
206-210: LGTM - queryClaude call for feature generation.The integration is correct. Since this endpoint parses the response as JSON text (not using structured output), only providing
fallbackTextis appropriate.apps/server/src/routes/context/routes/describe-image.ts (3)
19-19: LGTM!Import of
queryClaudealigns with the codebase-wide transition to the centralized Claude SDK wrapper.
27-27: LGTM!The mock image description helper is well-structured with a clear prefix constant and deterministic output based on the filename. This ensures consistent fallback behavior during mock mode.
Also applies to: 44-47
425-429: LGTM!The
queryClaudeintegration correctly passes thefallbackTextgenerated frombuildMockImageDescription(imagePath), providing a deterministic mock response when mock mode is enabled.apps/server/src/providers/claude-provider.ts (1)
8-11: LGTM!Import restructuring is correct:
Optionsis now type-only (as it should be for a TypeScript type), andqueryClaudeis imported from the local wrapper module.apps/server/src/routes/app-spec/generate-spec.ts (3)
28-28: LGTM!Import aligns with the codebase-wide shift to the
queryClaudewrapper.
31-47: LGTM!The mock spec constants and
buildMockSpecOutputhelper are well-structured:
- Uses meaningful default values that satisfy the
SpecOutputschema- Incorporates the actual
projectOverviewwhen available, providing more realistic mock output- Empty
implemented_featuresarray is valid per the schema
222-228: LGTM!Excellent integration of
queryClaudewith bothfallbackText(XML viaspecToXml) andfallbackStructuredOutput(the mock object). This ensures both the text extraction path and the structured output path work correctly in mock mode.apps/server/src/routes/features/routes/generate-title.ts (3)
10-10: LGTM!Import of
queryClaudealigns with the codebase-wide transition to the centralized Claude SDK wrapper.
13-15: LGTM!The fallback title helper is well-implemented:
- Clear constants for configuration
- Handles edge case where description has fewer words than the max
- Adds ellipsis suffix only when truncation occurs
Also applies to: 40-50
107-117: LGTM!The
queryClaudeintegration correctly providesfallbackTextusing thebuildFallbackTitlehelper, ensuring deterministic mock responses during testing.
apps/server/src/lib/claude-sdk.ts
Outdated
| export interface ClaudeQueryParams { | ||
| prompt: string | AsyncIterable<unknown>; | ||
| options: Options; | ||
| fallbackText?: string; | ||
| fallbackStructuredOutput?: unknown; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "claude-sdk.ts" -type fRepository: AutoMaker-Org/automaker
Length of output: 102
🏁 Script executed:
cat -n apps/server/src/lib/claude-sdk.ts | head -80Repository: AutoMaker-Org/automaker
Length of output: 2777
🌐 Web query:
SDKUserMessage type @anthropic-ai/claude-agent-sdk documentation
💡 Result:
SDKUserMessage (TypeScript)
type SDKUserMessage = {
type: 'user';
uuid?: UUID;
session_id: string;
message: APIUserMessage; // From Anthropic SDK
parent_tool_use_id: string | null;
}.
Sources: Claude Agent SDK TypeScript reference. [1][2]
References
- Claude Agent SDK — Agent SDK reference (TypeScript) (message types). [1]
- GitHub repo for claude-agent-sdk (package docs). [2]
🏁 Script executed:
# Check if there's a node_modules or package.json that shows the SDK version
find . -name "package.json" -path "*/node_modules/@anthropic-ai/claude-agent-sdk*" -o -name "package.json" | head -5Repository: AutoMaker-Org/automaker
Length of output: 221
🏁 Script executed:
# Look for .d.ts files or type definitions from the SDK
find . -type f -name "*.d.ts" | grep -i "claude-agent-sdk" | head -10Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Check the main package.json to see SDK version
cat package.json 2>/dev/null | grep -A 2 "claude-agent-sdk" || cat apps/server/package.json 2>/dev/null | grep -A 2 "claude-agent-sdk"Repository: AutoMaker-Org/automaker
Length of output: 198
🌐 Web query:
@anthropic-ai/claude-agent-sdk query function type signature AsyncIterable parameter
💡 Result:
TypeScript signature (from the Claude Agent SDK docs):
function query({
prompt,
options
}: {
prompt: string | AsyncIterable;
options?: Options;
}): Query
The AsyncIterable parameter should yield SDKUserMessage objects (type shape shown in the docs — { type: 'user'; uuid?: UUID; session_id: string; message: APIUserMessage; parent_tool_use_id: string | null }). The function returns a Query (an AsyncGenerator<SDKMessage, void> with interrupt() and setPermissionMode()). [1][2]
🏁 Script executed:
# Check if there's a tsconfig.json that shows type checking configuration
find . -name "tsconfig.json" -type f | xargs -I {} sh -c 'echo "File: {}"; head -20 {}'Repository: AutoMaker-Org/automaker
Length of output: 2614
🏁 Script executed:
# Check for any existing TypeScript errors or build logs
find . -name "*.log" -type f | head -5Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Verify the actual usage - does the code pass AsyncIterable<unknown> to claudeQuery?
grep -n "claudeQuery" apps/server/src/lib/claude-sdk.tsRepository: AutoMaker-Org/automaker
Length of output: 201
Fix type mismatch causing build failure.
The prompt parameter in ClaudeQueryParams is typed as string | AsyncIterable<unknown>, but the Claude SDK's query function expects string | AsyncIterable<SDKUserMessage>. With strict TypeScript checking enabled, this causes a type error at line 75 when passing the parameter to claudeQuery().
Proposed fix
-import { query as claudeQuery, type Options } from '@anthropic-ai/claude-agent-sdk';
+import { query as claudeQuery, type Options, type SDKUserMessage } from '@anthropic-ai/claude-agent-sdk';
export interface ClaudeQueryParams {
- prompt: string | AsyncIterable<unknown>;
+ prompt: string | AsyncIterable<SDKUserMessage>;
options: Options;
fallbackText?: string;
fallbackStructuredOutput?: unknown;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface ClaudeQueryParams { | |
| prompt: string | AsyncIterable<unknown>; | |
| options: Options; | |
| fallbackText?: string; | |
| fallbackStructuredOutput?: unknown; | |
| } | |
| export interface ClaudeQueryParams { | |
| prompt: string | AsyncIterable<SDKUserMessage>; | |
| options: Options; | |
| fallbackText?: string; | |
| fallbackStructuredOutput?: unknown; | |
| } |
🤖 Prompt for AI Agents
In @apps/server/src/lib/claude-sdk.ts around lines 21 - 26, The prompt property
in the ClaudeQueryParams interface is too broad and must be narrowed to the SDK
type expected by claudeQuery; change ClaudeQueryParams.prompt from string |
AsyncIterable<unknown> to string | AsyncIterable<SDKUserMessage>, import or
reference SDKUserMessage from the Claude SDK, and update any callers if needed
so they pass AsyncIterable<SDKUserMessage> when using claudeQuery.
Summary by CodeRabbit
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.