feat(agent-cli): add Agent CLI selection feature replacing Claude config#24
Merged
newbe36524 merged 1 commit intomainfrom Feb 26, 2026
Merged
feat(agent-cli): add Agent CLI selection feature replacing Claude config#24newbe36524 merged 1 commit intomainfrom
newbe36524 merged 1 commit intomainfrom
Conversation
Add new Agent CLI selection system that simplifies from the previous Claude configuration approach. Users can now select their preferred AI programming CLI (Claude Code, with support for future CLIs) with automatic detection. Features: - AgentCliManager for CLI selection and detection - IPC handlers for agent-cli operations - Redux slice for Agent CLI state management - Onboarding step for Agent CLI selection with skip option - Centralized documentation links configuration - PreconditionError component for onboarding error handling Co-Authored-By: Hagicode <noreply@hagicode.com>
📝 WalkthroughWalkthroughThis PR introduces Agent CLI selection and detection functionality, replacing the previous Claude configuration management system. It adds an AgentCliManager for CLI availability detection and persistence, integrates it into the onboarding flow via a new AgentCliSelection step, simplifies the DependencyManager to AI-driven handling, updates LlmInstallationManager to remove Claude config dependencies, and adds corresponding IPC handlers, Redux store management, and internationalization support. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as AgentCliSelection<br/>Component
participant Preload as Preload API
participant IPC as IPC Handlers
participant Manager as AgentCliManager
participant Store as Electron Store
participant Detection as CLI Detection
participant Redux as Redux Store
User->>UI: Select Agent CLI
UI->>Preload: agentCliSave({cliType})
Preload->>IPC: ipcRenderer.invoke('agentCli:save')
IPC->>Manager: saveSelection(cliType)
Manager->>Store: Write selection to store
Manager-->>IPC: { success: true }
IPC-->>Preload: Return success
Preload-->>UI: Resolve promise
UI->>Preload: agentCliDetect(cliType)
Preload->>IPC: ipcRenderer.invoke('agentCli:detect')
IPC->>Manager: detectAvailability(cliType)
Manager->>Detection: Execute CLI command<br/>(e.g., claude --version)
Detection-->>Manager: { detected, version? }
Manager-->>IPC: Return detection result
IPC-->>Preload: Return result
Preload-->>UI: Update detection state
UI->>Redux: Dispatch selectAgentCli(cliType)
Redux->>Redux: Update selectedCliType state
Redux-->>UI: Notify listeners
UI->>User: Display selected CLI with<br/>detection status
User->>UI: Click Next
UI->>Redux: selectCanProceed query
Redux-->>UI: true (selection valid)
UI->>User: Proceed to LLM Installation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tsconfig.json (1)
2-24:⚠️ Potential issue | 🟠 MajorAdd
baseUrlandpathsconfiguration to enable TypeScript path resolution for@/*imports.The renderer codebase extensively uses
@/*import aliases (70+ occurrences across components, store, utilities), buttsconfig.jsonlacks the requiredcompilerOptions.baseUrlandcompilerOptions.pathsmapping. Withstrict: trueenabled, TypeScript cannot resolve these imports, causing type-checking failures in editors and builds despite runtime bundler support.♻️ Proposed fix
"compilerOptions": { + "baseUrl": ".", + "paths": { + "@/*": ["src/renderer/*"] + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` around lines 2 - 24, Add compilerOptions.baseUrl and compilerOptions.paths to tsconfig.json so the TypeScript compiler can resolve the '@/*' aliases used in the renderer; specifically set compilerOptions.baseUrl to the project root (e.g., "./") and add a compilerOptions.paths entry mapping "@/*" to the renderer source (e.g., "src/renderer/*") or to the appropriate src folder if aliases target a different root, then rerun type-checking; update the existing tsconfig.json's compilerOptions section (the same object that contains "strict", "moduleResolution", etc.) to include baseUrl and paths for '@/*'.
🟡 Minor comments (12)
src/renderer/components/onboarding/steps/LlmInstallation.tsx-301-302 (1)
301-302:⚠️ Potential issue | 🟡 MinorLocalize hardcoded Chinese labels in the onboarding UI.
Line 301 and Line 368 bypass i18n, so these labels won’t adapt to non-Chinese locales.
🌐 Suggested i18n-safe change
- 当前区域: {getRegionInfo(executedRegion || selectedRegion).name} + {t('installationConfirmation.currentRegion')}: {getRegionInfo(executedRegion || selectedRegion).name} ... - <span className="text-sm font-medium">重试区域:</span> + <span className="text-sm font-medium">{t('llmInstallation.retryRegion')}:</span>Also applies to: 368-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/onboarding/steps/LlmInstallation.tsx` around lines 301 - 302, Replace hardcoded Chinese UI labels in the LlmInstallation component with i18n lookups: find the JSX that renders "当前区域: {getRegionInfo(executedRegion || selectedRegion).name}" and the other hardcoded label around the later occurrence, and wrap those strings with the app's translation function (e.g., t('...') or useTranslation hook) so the label text comes from the locale files; update any nearby static text similarly and add matching keys to the locale resource bundle used by the project.src/renderer/i18n/locales/zh-CN/pages.json-106-106 (1)
106-106:⚠️ Potential issue | 🟡 Minor
showMoreVersionswording mismatches the runtime value semantics.Line 106 currently reads like a total count, but the UI passes remaining count. This will display incorrect meaning to users.
💡 Suggested localization fix
- "showMoreVersions": "显示更多版本 (共 {{count}} 个)", + "showMoreVersions": "显示更多版本 (还剩 {{count}} 个)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/zh-CN/pages.json` at line 106, The localization string "showMoreVersions" currently reads like a total count but the UI passes a remaining count; update the value for the "showMoreVersions" key so it clearly expresses "remaining" (e.g., use wording like "显示更多版本（还有 {{count}} 个）" or "显示更多版本（剩余 {{count}} 个）") to match the runtime semantics in pages.json.src/main/web-service-manager.ts-788-789 (1)
788-789:⚠️ Potential issue | 🟡 MinorUse case-insensitive
.ps1detection for startup branching.Line 788 currently checks
endsWith('.ps1');.PS1or mixed-case filenames can miss the PowerShell path.🔧 Suggested robustness fix
- if (process.platform === 'win32' && scriptPath.endsWith('.ps1')) { + if (process.platform === 'win32' && scriptPath.toLowerCase().endsWith('.ps1')) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/web-service-manager.ts` around lines 788 - 789, Update the startup branching that checks the script extension by making the `.ps1` test case-insensitive: locate the conditional that examines scriptPath (the if where process.platform === 'win32' && scriptPath.endsWith('.ps1')) and change the extension detection to compare a lowercase extension (for example via scriptPath.toLowerCase().endsWith('.ps1') or using path.extname(scriptPath).toLowerCase() === '.ps1') so filenames like .PS1 or mixed-case are correctly detected and the PowerShellExecutor path is used.vite.config.mjs-20-22 (1)
20-22:⚠️ Potential issue | 🟡 MinorRemove unused and misleading wildcard-style alias keys.
'@types/*'is treated literally by Vite (not as a glob pattern) and is unused in the codebase. Remove it along with the redundant'@types/agent-cli'entry, relying only on the base'@types'alias for subpath resolution.♻️ Suggested alias cleanup
alias: { '@types': path.resolve(__dirname, './src/types'), - '@types/*': path.resolve(__dirname, './src/types/*'), - '@types/agent-cli': path.resolve(__dirname, './src/types/agent-cli.ts'), '@': path.resolve(__dirname, './src/renderer'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.mjs` around lines 20 - 22, Remove the misleading wildcard-style alias entries from the Vite aliases: delete the '@types/*' and '@types/agent-cli' alias keys and keep only the base '@types' alias so subpath resolution uses '@types' (the strings '@types/*' and '@types/agent-cli' appear in the alias object in vite.config.mjs and should be removed).src/renderer/store/index.ts-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorUse
@/*alias for the new store slice import.
./slices/agentCliSliceshould follow renderer alias convention.As per coding guidelines, "src/renderer/**/*.{ts,tsx}: Use `@/*` path aliases for imports within the renderer process".♻️ Proposed fix
-import agentCliReducer from './slices/agentCliSlice'; +import agentCliReducer from '@/store/slices/agentCliSlice';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/index.ts` at line 11, Update the import of the agentCli slice to use the renderer path alias instead of a relative path: replace the current import that references './slices/agentCliSlice' (the agentCliReducer import) with the equivalent '@/...' alias path that points to the same slice under the renderer root (e.g., '@/store/slices/agentCliSlice') so it follows the "src/renderer/**/*" alias convention.src/main/dependency-manager.ts-198-208 (1)
198-208:⚠️ Potential issue | 🟡 MinorFix progress reporting to increment
currentper dependency.
currentis always1, so consumers never get real progress advancement.♻️ Proposed fix
- for (const dep of depsToCheck) { + for (const [index, dep] of depsToCheck.entries()) { results.failed.push({ dependency: dep.name, error: 'Installation now handled by AI', }); onProgress?.({ - current: 1, + current: index + 1, total: depsToCheck.length, dependency: dep.name, status: 'error', }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/dependency-manager.ts` around lines 198 - 208, In the for-loop over depsToCheck (where you push into results.failed and call onProgress), current is hard-coded as 1; change it to report the actual progress by using the loop index or a counter (e.g., set current to i+1 when iterating with for (let i=0; i<depsToCheck.length; i++) or increment a counter before calling onProgress) and keep total as depsToCheck.length so onProgress receives increasing current values for each dependency; update the calls around results.failed and onProgress to use that computed current value (refer to depsToCheck, results.failed, and the onProgress call).src/renderer/i18n/index.ts-17-24 (1)
17-24:⚠️ Potential issue | 🟡 MinorUse renderer alias imports instead of relative paths for new namespace resources.
Please align these new imports with the renderer alias convention.
As per coding guidelines, "src/renderer/**/*.{ts,tsx}: Use `@/*` path aliases for imports within the renderer process".♻️ Proposed fix
-import zhCNAgentCli from './locales/zh-CN/agent-cli.json'; +import zhCNAgentCli from '@/i18n/locales/zh-CN/agent-cli.json'; @@ -import enUSAgentCli from './locales/en-US/agent-cli.json'; +import enUSAgentCli from '@/i18n/locales/en-US/agent-cli.json';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/index.ts` around lines 17 - 24, Replace the relative locale imports in this module with renderer alias imports: update the import statements for zhCNAgentCli, enUSCommon, enUSComponents, enUSPages, enUSUi, enUSOnboarding, and enUSAgentCli so they import from the renderer alias path (use `@/renderer/i18n/locales/`... or the project’s configured `@/`* alias) instead of './locales/...'; ensure the import specifiers match the existing filenames (e.g., zh-CN/agent-cli.json, en-US/common.json, etc.) and retain the same imported identifiers in index.ts.src/main/agent-cli-manager.ts-94-100 (1)
94-100:⚠️ Potential issue | 🟡 MinorTreat Windows “command not found” output as expected not-installed case
On Windows, missing
claudeoften reports"is not recognized..."with exit code 1. Right now that path is logged as detection error. Handle it as a normaldetected: falsecase to reduce false error noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/agent-cli-manager.ts` around lines 94 - 100, The detection logic in AgentCliManager currently treats Windows' "is not recognized" output as an error; update the CLI detection branch (where cliType is logged) to also treat error.message patterns like "is not recognized" or "not recognized as an internal or external command" (case-insensitive) — and the common Windows exit code 1 case with that message — as a normal not-installed result (return { detected: false }) instead of logging a detection error; keep other errors using the existing log.error path.src/renderer/store/slices/claudeConfigSlice.ts-2-2 (1)
2-2:⚠️ Potential issue | 🟡 MinorSwitch renderer import to
@/*aliasPlease update this new import to use the renderer alias convention instead of a relative path.
As per coding guidelines
src/renderer/**/*.{ts,tsx}: Use@/*path aliases for imports within the renderer process.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/slices/claudeConfigSlice.ts` at line 2, Update the import of AgentCliType in claudeConfigSlice.ts to use the renderer path alias instead of a relative path: replace the relative module '../../../types/agent-cli' used in the import statement that references AgentCliType with the aliased path (e.g. '@/types/agent-cli') so it follows the renderer convention for imports.src/renderer/store/slices/agentCliSlice.ts-2-2 (1)
2-2:⚠️ Potential issue | 🟡 MinorUse renderer alias import for agent-cli types
Please replace this new relative import with
@/*alias style.As per coding guidelines
src/renderer/**/*.{ts,tsx}: Use@/*path aliases for imports within the renderer process.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/slices/agentCliSlice.ts` at line 2, Replace the relative import of AgentCliType and CliDetectionResult with the renderer alias import by changing the import of AgentCliType and CliDetectionResult from '../../../types/agent-cli' to '@/types/agent-cli' in the file containing the agentCliSlice code; ensure the symbols AgentCliType and CliDetectionResult are referenced unchanged and the project's tsconfig/webpack alias already supports '@/'.src/renderer/store/slices/agentCliSlice.ts-33-39 (1)
33-39:⚠️ Potential issue | 🟡 MinorClear stale error state during detection lifecycle
If a previous detection failed,
erroris never cleared onstartDetection/setDetectionResult, so the UI can keep showing an outdated error after success.Suggested reducer tweak
setDetectionResult: (state, action: PayloadAction<CliDetectionResult>) => { state.cliDetected = action.payload.detected; state.isDetecting = false; + state.error = null; }, startDetection: (state) => { state.isDetecting = true; + state.error = null; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/slices/agentCliSlice.ts` around lines 33 - 39, The reducers startDetection and setDetectionResult in agentCliSlice leave a stale state.error from prior failures; update startDetection to clear state.error (e.g., set to undefined/null) when detection begins and update setDetectionResult to clear state.error when a successful detection occurs (or always clear it when setting a new result) so the UI no longer shows outdated errors; refer to the reducer functions startDetection and setDetectionResult and the error property on the slice state to locate where to clear the error.src/renderer/components/onboarding/OnboardingWizard.tsx-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorUse renderer alias imports for new onboarding step import
Please switch this new renderer import to
@/*alias style instead of a relative path.As per coding guidelines
src/renderer/**/*.{ts,tsx}: Use@/*path aliases for imports within the renderer process.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/onboarding/OnboardingWizard.tsx` at line 17, Replace the relative import of AgentCliSelectionStep with the renderer alias path: change the import from "./steps/AgentCliSelection" to use the `@/`* alias (import AgentCliSelectionStep from '@/components/onboarding/steps/AgentCliSelection') so the OnboardingWizard.tsx module follows the renderer aliasing convention; update the import statement where AgentCliSelectionStep is referenced.
🧹 Nitpick comments (4)
src/renderer/i18n/locales/zh-CN/agent-cli.json (1)
24-25: Avoid hardcoding a Unix-only config path in localized help text.
~/.claude/settings.jsonis platform-specific and can confuse Windows users. Prefer a generic key + platform-specific rendering or docs link.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/zh-CN/agent-cli.json` around lines 24 - 25, The localized string configLocation currently hardcodes a Unix path (~/.claude/settings.json); update the zh-CN agent-cli.json entry for "configLocation" to remove the platform-specific path and use a neutral placeholder or descriptive text (e.g., "配置位置: {configPath}" or "配置位置: 在您的主目录下的 Claude 设置文件") and ensure the UI/renderer replaces {configPath} with a platform-specific path at runtime; also adjust "configInstructions" if needed to point users to a platform-aware docs link or to the same placeholder usage so Windows users aren't shown a Unix-only path.src/renderer/i18n/locales/en-US/onboarding.json (1)
255-258: Prefer dynamic interpolation over hardcoded step numbers in error text.Hardcoding
"Step 2"can go stale if onboarding order changes; use an interpolated step label/index from flow state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/en-US/onboarding.json` around lines 255 - 258, Replace the hardcoded "Step 2" string in the preconditionError locale entry with an interpolation placeholder (e.g. change preconditionError.message to use "{{stepLabel}}" or "{{stepIndex}}") and update any consuming code to pass the dynamic value from the onboarding flow state when rendering (ensure the i18n lookup for "preconditionError.message" receives the step label/index from the flow state so the message and action remain correct if onboarding order changes).src/main/ipc/agentCliHandlers.ts (1)
14-17: Useunknowntype in catch clauses instead ofanyand narrow toErrorfor type-safe message extraction.This file has 6 catch blocks (lines 14-17, 24-27, 35-38, 45-48, 55-58, 65-68) using
catch (error: any), which violates the strict type safety requirement enabled in your TypeScript configuration. Replaceanywithunknownand narrow the type when accessing properties likeerror.message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/agentCliHandlers.ts` around lines 14 - 17, Replace all catch clauses that use `catch (error: any)` with `catch (error: unknown)` in the functions inside agentCliHandlers (the six catch blocks around saving/loading Agent CLI selection and related handlers), and when returning/logging use a type-narrowing check such as `if (error instanceof Error) { message = error.message } else { message = String(error) }` so you safely access the message for the processLogger/console.error and the returned `{ success: false, error: ... }` payloads.src/preload/index.ts (1)
301-306: UseAgentCliTypein preload API signatures instead of rawstring.Methods at lines 301-306 accept
stringforcliType, which loosens the IPC contract and allows invalid values at compile time. Per the coding guideline requiring strict type safety, these should be typed with the existingAgentCliTypeenum fromsrc/types/agent-cli.ts.Type-safe direction
+import type { AgentCliType } from '../types/agent-cli.js'; ... - agentCliSave: (data: { cliType: string }) => ipcRenderer.invoke('agentCli:save', data), + agentCliSave: (data: { cliType: AgentCliType }) => ipcRenderer.invoke('agentCli:save', data), ... - agentCliDetect: (cliType: string) => ipcRenderer.invoke('agentCli:detect', cliType), + agentCliDetect: (cliType: AgentCliType) => ipcRenderer.invoke('agentCli:detect', cliType), - agentCliGetCommand: (cliType: string) => ipcRenderer.invoke('agentCli:getCommand', cliType), + agentCliGetCommand: (cliType: AgentCliType) => ipcRenderer.invoke('agentCli:getCommand', cliType),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/preload/index.ts` around lines 301 - 306, Update the preload API to use the AgentCliType enum instead of raw strings: import AgentCliType and change the signatures so data.cliType in agentCliSave is typed as AgentCliType and the parameters of agentCliDetect and agentCliGetCommand use AgentCliType (leave agentCliLoad/agentCliSkip/agentCliGetSelected unchanged); ensure the import for AgentCliType from the existing agent-cli types module is added and adjust any downstream callers or type assertions to satisfy the new enum types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/dependency-manager.ts`:
- Around line 92-100: The mapping currently hardcodes installed: false; replace
that with a real installation-state value instead of a false negative by calling
a resolver (e.g., introduce a helper like
resolveDependencyInstallationState(dep) or isDependencyInstalled(dep)) and
return either a boolean true/false when you can determine it or a distinct
token/string (e.g., "delegated" or "unknown") when checks are
delegated/indeterminate; update the returned object in the function that calls
mapDependencyType/formatRequiredVersion to set installed:
this.resolveDependencyInstallationState(dep) (or similar) and implement that
helper to detect delegated checks (inspect dep.installHint/flags) and to run
actual checks when available.
In `@src/main/llm-installation-manager.ts`:
- Around line 87-88: The call to logPromptDetails(resolvedPromptPath,
promptContent) (and the similar calls at lines ~228-232) is unguarded and causes
prompt text to be written to persistent logs; wrap these calls in a debug-mode
guard so they only execute when debug/log-level is enabled (e.g., if
(this.isDebugEnabled()) or this.logger.isDebugEnabled()) or alternatively move
the check inside logPromptDetails to early-return when not in debug mode;
specifically update usages of logPromptDetails and/or the logPromptDetails
method to check the instance's debug/log-level flag before logging
resolvedPromptPath or promptContent.
- Around line 145-147: The constructedCommand string in LlmInstallationManager
(variable constructedCommand) is vulnerable to shell injection when using
execAsync; replace string-interpolated shell execution with an argv-based
process invocation (e.g., spawn/execFile or execa with argument array) for both
macOS (the osascript call built with constructedCommand) and the Linux path
around the other similar block (the section referenced at 172-176). Stop
interpolating escapedPrompt into a single shell string—pass the prompt as a
separate argument to the child process API (or build a safe args array for
osascript), and call exec/spawn/execFile with the command and args so the
runtime handles quoting/escaping instead of using the shell.
In `@src/main/onboarding-manager.ts`:
- Around line 291-293: The call to
dependencyManager.checkFromManifest(dependencies, null) (used when computing
initialStatus and the later occurrence) passes null which forces every
dependency to show as “not installed”; replace the null with the actual parsed
manifest/context object (the parsed entry point or equivalent manifest context
variable available in the onboarding flow) so checkFromManifest receives real
manifest context and returns accurate statuses; update both places where
checkFromManifest is called with null (the initialStatus assignment and the
later call) to pass that parsed entry point/manifest variable (named where
parsed entry point is produced in this module) instead.
In `@src/main/utils/powershell-executor.ts`:
- Around line 409-415: The spawnService() function attaches the ChildProcess
'error' handler only when onError is provided, which can cause unhandled 'error'
events and crashes; update spawnService() to always attach child.on('error',
...) so the child process always has an error listener, and inside that handler
call onError(error) only if onError is defined (i.e., unconditional listener
that conditionally invokes the callback). Locate the child variable and the
existing conditional block around child.on('error', ...) and replace it so the
listener is always registered but still forwards the error to the onError
parameter when present.
In `@src/main/web-service-manager.ts`:
- Around line 815-839: When the child exits during startup (inside onExit) fail
fast instead of only logging: detect StartupPhase.Spawning or WaitingListening
and immediately transition to the error state by setting this.status = 'error',
incrementing this.restartCount, nulling this.process, and then invoke the same
startup-failure path that start() uses (i.e., call or reuse the existing failure
handler invoked in the later exit-handling block so any pending start() promise
is rejected / the startup timeout is aborted). Update onExit (and the analogous
block referenced later) to call that shared failure method (or invoke the same
code path) so start() returns immediately with an error instead of waiting for
the port-listen timeout.
In `@src/renderer/App.tsx`:
- Around line 39-44: Replace the stringly-typed IPC method signatures in App.tsx
to use the shared domain types by importing AgentCliType,
StoredAgentCliSelection, and CliDetectionResult from the `@types/agent-cli` alias
and updating the following symbols: agentCliSave should accept (data: { cliType:
AgentCliType }), agentCliLoad should return Promise<StoredAgentCliSelection>,
agentCliDetect should return Promise<CliDetectionResult>, agentCliGetCommand and
agentCliGetSelected should use AgentCliType for parameters/returns where
applicable (e.g., agentCliGetCommand(cliType: AgentCliType): Promise<string> and
agentCliGetSelected(): Promise<AgentCliType | null>), leaving agentCliSkip and
boolean success results intact; apply the same typing changes in
src/preload/index.ts so the preload and renderer IPC contracts match the shared
types.
In `@src/renderer/components/onboarding/PreconditionError.tsx`:
- Line 2: The import of Button in PreconditionError.tsx uses a relative path;
replace the relative import "import { Button } from '../ui/button';" with the
project alias form that maps to src/renderer by using the `@/` prefix (e.g. import
Button from '@/components/ui/button' or equivalent according to your alias
config) so the Button symbol is imported via the `@/`* renderer alias rather than
a relative path.
In `@src/renderer/components/onboarding/steps/AgentCliSelection.tsx`:
- Around line 240-264: The skip confirmation overlay is missing dialog semantics
and keyboard/focus management; update the component that renders showSkipDialog
to render a proper modal with role="dialog", aria-modal="true", aria-labelledby
pointed to the title and aria-describedby to the message, and use a focus trap:
create refs for the dialog container and the first actionable button, set
initial focus to the cancel or confirm button in a useEffect when showSkipDialog
becomes true, and restore focus when it closes; also handle Escape key to call
setShowSkipDialog(false) and ensure confirmSkip and the Button components are
reachable/operable via keyboard so screen readers and keyboard-only users can
interact correctly.
- Around line 5-19: The imports in AgentCliSelection.tsx use deep
renderer-relative paths; update them to use the renderer alias `@/`*: replace
'../../ui/button' with '@/renderer/components/ui/button' (or the project's
equivalent `@/ui` path), '../../../../types/agent-cli' with '@/types/agent-cli'
for AgentCliType and getAllCliConfigs, and change
'../../../store/slices/agentCliSlice' and '../../../store' to
'@/store/slices/agentCliSlice' and '@/store' respectively so symbols like
Button, AgentCliType, getAllCliConfigs, selectAgentCli, skipAgentCli,
setDetectionResult, startDetection, selectSelectedCliType, selectIsSkipped,
selectCliDetected, selectIsDetecting, selectCanProceed, and
AppDispatch/StoredAgentCliSelection/CliDetectionResult are all imported via `@/`*
aliases.
In `@src/renderer/components/onboarding/steps/LlmInstallation.tsx`:
- Around line 8-10: The imports at the top of LlmInstallation.tsx use deep
relative paths; update the import statements that bring in
selectClaudeConfigDetected, selectClaudeConfigSource, selectClaudeProvider,
selectSelectedCliType, selectIsSkipped and PreconditionError to use the renderer
alias (`@/`*) instead of "../../../" style paths so they resolve via the `@/` alias
for renderer-internal modules—locate the import lines in LlmInstallation.tsx and
rewrite them to import from the corresponding `@/` paths (e.g., `@/store/`...,
`@/renderer/`... or the canonical `@/` paths used elsewhere in the project).
In `@tsconfig.json`:
- Around line 21-23: The include patterns use unsupported brace expansion
("src/renderer/**/*.{ts,tsx}") and you also lack the path alias mapping for
"@/"; update the tsconfig.json by replacing the brace-expanded include with
explicit patterns (e.g., "src/renderer/**/*.ts" and "src/renderer/**/*.tsx") and
add a compilerOptions.paths entry for the "@/..." alias (map "@" or "@/ *" to
the appropriate "src/renderer" or "src/*" folder) so the TypeScript compiler can
resolve imports; modify the "include" array and add/extend
"compilerOptions.paths" accordingly.
---
Outside diff comments:
In `@tsconfig.json`:
- Around line 2-24: Add compilerOptions.baseUrl and compilerOptions.paths to
tsconfig.json so the TypeScript compiler can resolve the '@/*' aliases used in
the renderer; specifically set compilerOptions.baseUrl to the project root
(e.g., "./") and add a compilerOptions.paths entry mapping "@/*" to the renderer
source (e.g., "src/renderer/*") or to the appropriate src folder if aliases
target a different root, then rerun type-checking; update the existing
tsconfig.json's compilerOptions section (the same object that contains "strict",
"moduleResolution", etc.) to include baseUrl and paths for '@/*'.
---
Minor comments:
In `@src/main/agent-cli-manager.ts`:
- Around line 94-100: The detection logic in AgentCliManager currently treats
Windows' "is not recognized" output as an error; update the CLI detection branch
(where cliType is logged) to also treat error.message patterns like "is not
recognized" or "not recognized as an internal or external command"
(case-insensitive) — and the common Windows exit code 1 case with that message —
as a normal not-installed result (return { detected: false }) instead of logging
a detection error; keep other errors using the existing log.error path.
In `@src/main/dependency-manager.ts`:
- Around line 198-208: In the for-loop over depsToCheck (where you push into
results.failed and call onProgress), current is hard-coded as 1; change it to
report the actual progress by using the loop index or a counter (e.g., set
current to i+1 when iterating with for (let i=0; i<depsToCheck.length; i++) or
increment a counter before calling onProgress) and keep total as
depsToCheck.length so onProgress receives increasing current values for each
dependency; update the calls around results.failed and onProgress to use that
computed current value (refer to depsToCheck, results.failed, and the onProgress
call).
In `@src/main/web-service-manager.ts`:
- Around line 788-789: Update the startup branching that checks the script
extension by making the `.ps1` test case-insensitive: locate the conditional
that examines scriptPath (the if where process.platform === 'win32' &&
scriptPath.endsWith('.ps1')) and change the extension detection to compare a
lowercase extension (for example via scriptPath.toLowerCase().endsWith('.ps1')
or using path.extname(scriptPath).toLowerCase() === '.ps1') so filenames like
.PS1 or mixed-case are correctly detected and the PowerShellExecutor path is
used.
In `@src/renderer/components/onboarding/OnboardingWizard.tsx`:
- Line 17: Replace the relative import of AgentCliSelectionStep with the
renderer alias path: change the import from "./steps/AgentCliSelection" to use
the `@/`* alias (import AgentCliSelectionStep from
'@/components/onboarding/steps/AgentCliSelection') so the OnboardingWizard.tsx
module follows the renderer aliasing convention; update the import statement
where AgentCliSelectionStep is referenced.
In `@src/renderer/components/onboarding/steps/LlmInstallation.tsx`:
- Around line 301-302: Replace hardcoded Chinese UI labels in the
LlmInstallation component with i18n lookups: find the JSX that renders "当前区域:
{getRegionInfo(executedRegion || selectedRegion).name}" and the other hardcoded
label around the later occurrence, and wrap those strings with the app's
translation function (e.g., t('...') or useTranslation hook) so the label text
comes from the locale files; update any nearby static text similarly and add
matching keys to the locale resource bundle used by the project.
In `@src/renderer/i18n/index.ts`:
- Around line 17-24: Replace the relative locale imports in this module with
renderer alias imports: update the import statements for zhCNAgentCli,
enUSCommon, enUSComponents, enUSPages, enUSUi, enUSOnboarding, and enUSAgentCli
so they import from the renderer alias path (use `@/renderer/i18n/locales/`... or
the project’s configured `@/`* alias) instead of './locales/...'; ensure the
import specifiers match the existing filenames (e.g., zh-CN/agent-cli.json,
en-US/common.json, etc.) and retain the same imported identifiers in index.ts.
In `@src/renderer/i18n/locales/zh-CN/pages.json`:
- Line 106: The localization string "showMoreVersions" currently reads like a
total count but the UI passes a remaining count; update the value for the
"showMoreVersions" key so it clearly expresses "remaining" (e.g., use wording
like "显示更多版本（还有 {{count}} 个）" or "显示更多版本（剩余 {{count}} 个）") to match the runtime
semantics in pages.json.
In `@src/renderer/store/index.ts`:
- Line 11: Update the import of the agentCli slice to use the renderer path
alias instead of a relative path: replace the current import that references
'./slices/agentCliSlice' (the agentCliReducer import) with the equivalent
'@/...' alias path that points to the same slice under the renderer root (e.g.,
'@/store/slices/agentCliSlice') so it follows the "src/renderer/**/*" alias
convention.
In `@src/renderer/store/slices/agentCliSlice.ts`:
- Line 2: Replace the relative import of AgentCliType and CliDetectionResult
with the renderer alias import by changing the import of AgentCliType and
CliDetectionResult from '../../../types/agent-cli' to '@/types/agent-cli' in the
file containing the agentCliSlice code; ensure the symbols AgentCliType and
CliDetectionResult are referenced unchanged and the project's tsconfig/webpack
alias already supports '@/'.
- Around line 33-39: The reducers startDetection and setDetectionResult in
agentCliSlice leave a stale state.error from prior failures; update
startDetection to clear state.error (e.g., set to undefined/null) when detection
begins and update setDetectionResult to clear state.error when a successful
detection occurs (or always clear it when setting a new result) so the UI no
longer shows outdated errors; refer to the reducer functions startDetection and
setDetectionResult and the error property on the slice state to locate where to
clear the error.
In `@src/renderer/store/slices/claudeConfigSlice.ts`:
- Line 2: Update the import of AgentCliType in claudeConfigSlice.ts to use the
renderer path alias instead of a relative path: replace the relative module
'../../../types/agent-cli' used in the import statement that references
AgentCliType with the aliased path (e.g. '@/types/agent-cli') so it follows the
renderer convention for imports.
In `@vite.config.mjs`:
- Around line 20-22: Remove the misleading wildcard-style alias entries from the
Vite aliases: delete the '@types/*' and '@types/agent-cli' alias keys and keep
only the base '@types' alias so subpath resolution uses '@types' (the strings
'@types/*' and '@types/agent-cli' appear in the alias object in vite.config.mjs
and should be removed).
---
Nitpick comments:
In `@src/main/ipc/agentCliHandlers.ts`:
- Around line 14-17: Replace all catch clauses that use `catch (error: any)`
with `catch (error: unknown)` in the functions inside agentCliHandlers (the six
catch blocks around saving/loading Agent CLI selection and related handlers),
and when returning/logging use a type-narrowing check such as `if (error
instanceof Error) { message = error.message } else { message = String(error) }`
so you safely access the message for the processLogger/console.error and the
returned `{ success: false, error: ... }` payloads.
In `@src/preload/index.ts`:
- Around line 301-306: Update the preload API to use the AgentCliType enum
instead of raw strings: import AgentCliType and change the signatures so
data.cliType in agentCliSave is typed as AgentCliType and the parameters of
agentCliDetect and agentCliGetCommand use AgentCliType (leave
agentCliLoad/agentCliSkip/agentCliGetSelected unchanged); ensure the import for
AgentCliType from the existing agent-cli types module is added and adjust any
downstream callers or type assertions to satisfy the new enum types.
In `@src/renderer/i18n/locales/en-US/onboarding.json`:
- Around line 255-258: Replace the hardcoded "Step 2" string in the
preconditionError locale entry with an interpolation placeholder (e.g. change
preconditionError.message to use "{{stepLabel}}" or "{{stepIndex}}") and update
any consuming code to pass the dynamic value from the onboarding flow state when
rendering (ensure the i18n lookup for "preconditionError.message" receives the
step label/index from the flow state so the message and action remain correct if
onboarding order changes).
In `@src/renderer/i18n/locales/zh-CN/agent-cli.json`:
- Around line 24-25: The localized string configLocation currently hardcodes a
Unix path (~/.claude/settings.json); update the zh-CN agent-cli.json entry for
"configLocation" to remove the platform-specific path and use a neutral
placeholder or descriptive text (e.g., "配置位置: {configPath}" or "配置位置: 在您的主目录下的
Claude 设置文件") and ensure the UI/renderer replaces {configPath} with a
platform-specific path at runtime; also adjust "configInstructions" if needed to
point users to a platform-aware docs link or to the same placeholder usage so
Windows users aren't shown a Unix-only path.
ℹ️ Review info
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 (38)
src/main/agent-cli-manager.tssrc/main/claude-config-manager.tssrc/main/dependency-manager.tssrc/main/ipc/agentCliHandlers.tssrc/main/llm-installation-manager.tssrc/main/main.tssrc/main/manifest-reader.tssrc/main/onboarding-manager.tssrc/main/utils/powershell-executor.tssrc/main/version-manager.tssrc/main/web-service-manager.tssrc/preload/index.tssrc/renderer/App.tsxsrc/renderer/components/VersionManagementPage.tsxsrc/renderer/components/onboarding/OnboardingWizard.tsxsrc/renderer/components/onboarding/PreconditionError.tsxsrc/renderer/components/onboarding/steps/AgentCliSelection.tsxsrc/renderer/components/onboarding/steps/ClaudeConfig.tsxsrc/renderer/components/onboarding/steps/LlmInstallation.tsxsrc/renderer/i18n/config.tssrc/renderer/i18n/index.tssrc/renderer/i18n/locales/en-US/agent-cli.jsonsrc/renderer/i18n/locales/en-US/claude.jsonsrc/renderer/i18n/locales/en-US/onboarding.jsonsrc/renderer/i18n/locales/en-US/pages.jsonsrc/renderer/i18n/locales/zh-CN/agent-cli.jsonsrc/renderer/i18n/locales/zh-CN/claude.jsonsrc/renderer/i18n/locales/zh-CN/onboarding.jsonsrc/renderer/i18n/locales/zh-CN/pages.jsonsrc/renderer/store/index.tssrc/renderer/store/slices/agentCliSlice.tssrc/renderer/store/slices/claudeConfigSlice.tssrc/types/agent-cli.tssrc/types/claude-config.tssrc/types/doc-links.tssrc/types/onboarding.tstsconfig.jsonvite.config.mjs
💤 Files with no reviewable changes (6)
- src/main/manifest-reader.ts
- src/renderer/components/onboarding/steps/ClaudeConfig.tsx
- src/renderer/i18n/locales/zh-CN/claude.json
- src/main/claude-config-manager.ts
- src/renderer/i18n/locales/en-US/claude.json
- src/types/claude-config.ts
| export class AgentCliManager { | ||
| private static readonly STORE_KEY = 'agentCliSelection'; | ||
|
|
||
| constructor(private store: any) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/agent-cli-manager.ts | head -120Repository: HagiCode-org/desktop
Length of output: 4011
🏁 Script executed:
rg "darwin-arm64" src/main/agent-cli-manager.tsRepository: HagiCode-org/desktop
Length of output: 46
🏁 Script executed:
rg "from.*electron-store" src/ -A 2 | head -30Repository: HagiCode-org/desktop
Length of output: 1547
🏁 Script executed:
cat -n src/main/config.ts | head -40Repository: HagiCode-org/desktop
Length of output: 1067
🏁 Script executed:
cat -n src/main/config.ts | sed -n '40,60p'Repository: HagiCode-org/desktop
Length of output: 671
🏁 Script executed:
rg "AgentCliManager" src/ -B 3 -A 3Repository: HagiCode-org/desktop
Length of output: 5661
🏁 Script executed:
rg "StoredAgentCliSelection" src/types/ -A 5Repository: HagiCode-org/desktop
Length of output: 330
Replace any types with proper Store and Error types
Using any bypasses strict typing for persisted onboarding/CLI state. Type the store as Store<StoredAgentCliSelection> and narrow caught errors from unknown.
Suggested type-safe implementation
import { exec } from 'child_process';
import { promisify } from 'util';
import log from 'electron-log';
+import Store from 'electron-store';
import {
AgentCliType,
StoredAgentCliSelection,
CliDetectionResult,
} from '../types/agent-cli.js';
const execAsync = promisify(exec);
export class AgentCliManager {
private static readonly STORE_KEY = 'agentCliSelection';
- constructor(private store: any) {}
+ constructor(private store: Store<StoredAgentCliSelection>) {}
// ... other methods ...
try {
// ...
- } catch (error: any) {
+ } catch (error: unknown) {
+ const err = error as { message?: string; code?: string };
- if (error.message?.includes('not found') || error.code === 'ENOENT') {
+ if (err.message?.includes('not found') || err.code === 'ENOENT') {Violates the coding guideline for src/**/*.{ts,tsx}: Maintain strict type safety with TypeScript strict mode enabled. Also applies to line 92.
Comment on lines
+92
to
+100
| return dependencies.map(dep => ({ | ||
| key: dep.key, | ||
| name: dep.name, | ||
| type: this.mapDependencyType(dep.key, dep.type), | ||
| installed: false, | ||
| requiredVersion: this.formatRequiredVersion(dep.versionConstraints), | ||
| description: dep.description, | ||
| downloadUrl: dep.installHint, | ||
| }; | ||
|
|
||
| // If no entryPoint available, skip check | ||
| if (!entryPoint || !this.workingDirectory) { | ||
| log.warn('[DependencyManager] No entryPoint or working directory available for check'); | ||
| return result; | ||
| } | ||
|
|
||
| try { | ||
| // Resolve check script path | ||
| const { manifestReader } = await import('./manifest-reader.js'); | ||
| const scriptPath = manifestReader.resolveScriptPath(entryPoint.check, this.workingDirectory); | ||
|
|
||
| log.info('[DependencyManager] Checking dependency using script:', scriptPath); | ||
|
|
||
| // Execute check script with real-time output | ||
| const resultSession = await this.executeEntryPointScript(scriptPath, this.workingDirectory, onOutput); | ||
| const parsedResult = this.parseResultSession(resultSession); | ||
|
|
||
| // Update result based on result.json | ||
| result.installed = parsedResult.success; | ||
| result.version = parsedResult.version; | ||
|
|
||
| // Check version constraints if version detected | ||
| if (parsedResult.version) { | ||
| result.versionMismatch = !this.checkVersionConstraints( | ||
| parsedResult.version, | ||
| dep.versionConstraints | ||
| ); | ||
| } | ||
|
|
||
| console.log(`[DependencyManager] ${dep.name}: installed=${result.installed}, version=${parsedResult.version}, mismatch=${result.versionMismatch}`); | ||
| } catch (error) { | ||
| // Script not found or failed | ||
| console.log(`[DependencyManager] ${dep.name}: not installed (check failed)`, error); | ||
| result.installed = false; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| /** | ||
| * Check if installed version satisfies version constraints | ||
| * @param installedVersion - The installed version string | ||
| * @param constraints - Version constraints from manifest | ||
| * @returns true if version satisfies constraints | ||
| */ | ||
| private checkVersionConstraints(installedVersion: string, constraints: ParsedDependency['versionConstraints']): boolean { | ||
| // If exact version is required, check exact match | ||
| if (constraints.exact) { | ||
| return this.isExactVersionMatch(installedVersion, constraints.exact); | ||
| } | ||
|
|
||
| // Check min version | ||
| if (constraints.min && !this.isVersionSatisfied(installedVersion, constraints.min)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check max version | ||
| if (constraints.max && !this.isMaxVersionSatisfied(installedVersion, constraints.max)) { | ||
| return false; | ||
| } | ||
|
|
||
| // For dotnet runtime-specific check | ||
| if (constraints.runtime?.min) { | ||
| // Handle special case like "10.0.0+" - check if at least this version | ||
| if (!this.isVersionSatisfied(installedVersion, constraints.runtime.min)) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Check exact version match (including pre-release tags) | ||
| * @param installed - Installed version | ||
| * @param required - Required exact version | ||
| * @returns true if exact match | ||
| */ | ||
| private isExactVersionMatch(installed: string, required: string): boolean { | ||
| // Remove 'v' prefix if present | ||
| const cleanInstalled = installed.replace(/^v/, ''); | ||
| const cleanRequired = required.replace(/^v/, ''); | ||
|
|
||
| // Direct string comparison for exact match | ||
| return cleanInstalled === cleanRequired; | ||
| } | ||
|
|
||
| /** | ||
| * Check if installed version is less than or equal to max version | ||
| * @param installedVersion - Installed version | ||
| * @param maxVersion - Maximum allowed version | ||
| * @returns true if installed <= max | ||
| */ | ||
| private isMaxVersionSatisfied(installedVersion: string, maxVersion: string): boolean { | ||
| const parseVersion = (v: string) => { | ||
| // Remove 'v' prefix if present | ||
| const cleanVersion = v.replace(/^v/, ''); | ||
| // Handle pre-release versions (e.g., "0.1.0-alpha.9") | ||
| const parts = cleanVersion.split('-')[0].split('.').map(Number); | ||
| return parts; | ||
| }; | ||
|
|
||
| const installed = parseVersion(installedVersion); | ||
| const max = parseVersion(maxVersion); | ||
|
|
||
| for (let i = 0; i < Math.max(installed.length, max.length); i++) { | ||
| const ins = installed[i] || 0; | ||
| const mx = max[i] || 0; | ||
|
|
||
| if (ins < mx) return true; | ||
| if (ins > mx) return false; | ||
| } | ||
|
|
||
| return true; // Equal versions | ||
| })); |
There was a problem hiding this comment.
Do not hardcode all dependency checks to installed: false.
This returns deterministic false negatives for every dependency and can produce incorrect onboarding/install decisions. If checks are delegated, return a distinct “delegated/unknown” pathway instead of reporting a failed check state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/dependency-manager.ts` around lines 92 - 100, The mapping currently
hardcodes installed: false; replace that with a real installation-state value
instead of a false negative by calling a resolver (e.g., introduce a helper like
resolveDependencyInstallationState(dep) or isDependencyInstalled(dep)) and
return either a boolean true/false when you can determine it or a distinct
token/string (e.g., "delegated" or "unknown") when checks are
delegated/indeterminate; update the returned object in the function that calls
mapDependencyType/formatRequiredVersion to set installed:
this.resolveDependencyInstallationState(dep) (or similar) and implement that
helper to detect delegated checks (inspect dep.installHint/flags) and to run
actual checks when available.
Comment on lines
+87
to
+88
| // Log prompt details for debugging | ||
| this.logPromptDetails(resolvedPromptPath, promptContent); |
There was a problem hiding this comment.
Prompt content is logged even outside debug mode
Prompt preview/content length logging should be debug-gated; otherwise onboarding prompts can leak into persistent logs.
Suggested guard
- this.logPromptDetails(resolvedPromptPath, promptContent);
+ if (this.debugMode) {
+ this.logPromptDetails(resolvedPromptPath, promptContent);
+ }Also applies to: 228-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/llm-installation-manager.ts` around lines 87 - 88, The call to
logPromptDetails(resolvedPromptPath, promptContent) (and the similar calls at
lines ~228-232) is unguarded and causes prompt text to be written to persistent
logs; wrap these calls in a debug-mode guard so they only execute when
debug/log-level is enabled (e.g., if (this.isDebugEnabled()) or
this.logger.isDebugEnabled()) or alternatively move the check inside
logPromptDetails to early-return when not in debug mode; specifically update
usages of logPromptDetails and/or the logPromptDetails method to check the
instance's debug/log-level flag before logging resolvedPromptPath or
promptContent.
Comment on lines
+145
to
+147
| constructedCommand = `osascript -e 'tell application "Terminal" to do script "claude \\"${escapedPrompt}\\"; read -p \\"Press enter to exit...\\"; exit"'`; | ||
| log.info('[LlmInstallationManager] macOS command:', constructedCommand); | ||
| await execAsync(constructedCommand); |
There was a problem hiding this comment.
Harden terminal command construction against shell injection
The macOS/Linux execution path builds shell command strings from prompt-derived text. Escaping is incomplete, so crafted filenames/content can break quoting and execute unintended shell fragments. Please switch to argv-based process execution (or fully robust escaping with no shell interpolation).
Also applies to: 172-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/llm-installation-manager.ts` around lines 145 - 147, The
constructedCommand string in LlmInstallationManager (variable
constructedCommand) is vulnerable to shell injection when using execAsync;
replace string-interpolated shell execution with an argv-based process
invocation (e.g., spawn/execFile or execa with argument array) for both macOS
(the osascript call built with constructedCommand) and the Linux path around the
other similar block (the section referenced at 172-176). Stop interpolating
escapedPrompt into a single shell string—pass the prompt as a separate argument
to the child process API (or build a safe args array for osascript), and call
exec/spawn/execFile with the command and args so the runtime handles
quoting/escaping instead of using the shell.
Comment on lines
+291
to
293
| // Get initial status (now all return as not installed) | ||
| const initialStatus = await this.dependencyManager.checkFromManifest(dependencies, null); | ||
|
|
There was a problem hiding this comment.
checkFromManifest(..., null) degrades dependency status accuracy
Passing null here causes status to collapse to “not installed” for all dependencies, which can force redundant installs and misleading progress/status display. Please pass the parsed entry point (or equivalent manifest context) so checks remain meaningful.
Suggested adjustment
+ const entryPoint = manifestReader.parseEntryPoint(manifest);
- const initialStatus = await this.dependencyManager.checkFromManifest(dependencies, null);
+ const initialStatus = await this.dependencyManager.checkFromManifest(dependencies, entryPoint);
...
- let status = await this.dependencyManager.checkFromManifest(dependencies, null);
+ let status = await this.dependencyManager.checkFromManifest(dependencies, entryPoint);Also applies to: 446-447
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/onboarding-manager.ts` around lines 291 - 293, The call to
dependencyManager.checkFromManifest(dependencies, null) (used when computing
initialStatus and the later occurrence) passes null which forces every
dependency to show as “not installed”; replace the null with the actual parsed
manifest/context object (the parsed entry point or equivalent manifest context
variable available in the onboarding flow) so checkFromManifest receives real
manifest context and returns accurate statuses; update both places where
checkFromManifest is called with null (the initialStatus assignment and the
later call) to pass that parsed entry point/manifest variable (named where
parsed entry point is produced in this module) instead.
| @@ -0,0 +1,66 @@ | |||
| import { AlertTriangle } from 'lucide-react'; | |||
| import { Button } from '../ui/button'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use @/* alias imports in renderer components.
Line 2 uses a relative renderer import. Please switch this to the project alias form to match renderer import policy.
As per coding guidelines src/renderer/**/*.{ts,tsx}: Use @/* path aliases for imports within the renderer process.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/onboarding/PreconditionError.tsx` at line 2, The
import of Button in PreconditionError.tsx uses a relative path; replace the
relative import "import { Button } from '../ui/button';" with the project alias
form that maps to src/renderer by using the `@/` prefix (e.g. import Button from
'@/components/ui/button' or equivalent according to your alias config) so the
Button symbol is imported via the `@/`* renderer alias rather than a relative
path.
Comment on lines
+5
to
+19
| import { Button } from '../../ui/button'; | ||
| import { AgentCliType, getAllCliConfigs } from '../../../../types/agent-cli'; | ||
| import { | ||
| selectAgentCli, | ||
| skipAgentCli, | ||
| setDetectionResult, | ||
| startDetection, | ||
| selectSelectedCliType, | ||
| selectIsSkipped, | ||
| selectCliDetected, | ||
| selectIsDetecting, | ||
| selectCanProceed, | ||
| } from '../../../store/slices/agentCliSlice'; | ||
| import type { AppDispatch } from '../../../store'; | ||
| import type { StoredAgentCliSelection, CliDetectionResult } from '../../../../types/agent-cli'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Convert renderer-relative imports to @/* aliases.
These imports are inside renderer code and should use the alias convention instead of deep relative paths.
As per coding guidelines src/renderer/**/*.{ts,tsx}: Use @/* path aliases for imports within the renderer process.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/onboarding/steps/AgentCliSelection.tsx` around lines
5 - 19, The imports in AgentCliSelection.tsx use deep renderer-relative paths;
update them to use the renderer alias `@/`*: replace '../../ui/button' with
'@/renderer/components/ui/button' (or the project's equivalent `@/ui` path),
'../../../../types/agent-cli' with '@/types/agent-cli' for AgentCliType and
getAllCliConfigs, and change '../../../store/slices/agentCliSlice' and
'../../../store' to '@/store/slices/agentCliSlice' and '@/store' respectively so
symbols like Button, AgentCliType, getAllCliConfigs, selectAgentCli,
skipAgentCli, setDetectionResult, startDetection, selectSelectedCliType,
selectIsSkipped, selectCliDetected, selectIsDetecting, selectCanProceed, and
AppDispatch/StoredAgentCliSelection/CliDetectionResult are all imported via `@/`*
aliases.
Comment on lines
+240
to
+264
| {showSkipDialog && ( | ||
| <div className="fixed inset-0 bg-background/80 flex items-center justify-center z-50"> | ||
| <div className="bg-card border rounded-lg p-6 max-w-md shadow-lg"> | ||
| <h3 className="text-lg font-semibold mb-2"> | ||
| {t('skipConfirm.title')} | ||
| </h3> | ||
| <p className="text-muted-foreground mb-4"> | ||
| {t('skipConfirm.message')} | ||
| </p> | ||
| <div className="flex gap-3 justify-end"> | ||
| <Button | ||
| variant="outline" | ||
| onClick={() => setShowSkipDialog(false)} | ||
| > | ||
| {t('cancel')} | ||
| </Button> | ||
| <Button | ||
| onClick={confirmSkip} | ||
| > | ||
| {t('skipConfirm.confirm')} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Skip confirmation overlay is not an accessible dialog yet.
This modal lacks dialog semantics and keyboard handling (e.g., proper dialog role/aria labeling and focus management), which can block keyboard and assistive-tech users from completing the step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/onboarding/steps/AgentCliSelection.tsx` around lines
240 - 264, The skip confirmation overlay is missing dialog semantics and
keyboard/focus management; update the component that renders showSkipDialog to
render a proper modal with role="dialog", aria-modal="true", aria-labelledby
pointed to the title and aria-describedby to the message, and use a focus trap:
create refs for the dialog container and the first actionable button, set
initial focus to the cancel or confirm button in a useEffect when showSkipDialog
becomes true, and restore focus when it closes; also handle Escape key to call
setShowSkipDialog(false) and ensure confirmSkip and the Button components are
reachable/operable via keyboard so screen readers and keyboard-only users can
interact correctly.
Comment on lines
+8
to
+10
| import { selectClaudeConfigDetected, selectClaudeConfigSource, selectClaudeProvider } from '../../../store/slices/llmInstallationSlice'; | ||
| import { selectSelectedCliType, selectIsSkipped } from '../../../store/slices/agentCliSlice'; | ||
| import PreconditionError from '../PreconditionError'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use @/* aliases for renderer-internal imports in this file.
These changed imports are still using deep relative paths for renderer modules.
♻️ Suggested import normalization
-import { selectClaudeConfigDetected, selectClaudeConfigSource, selectClaudeProvider } from '../../../store/slices/llmInstallationSlice';
-import { selectSelectedCliType, selectIsSkipped } from '../../../store/slices/agentCliSlice';
-import PreconditionError from '../PreconditionError';
+import { selectClaudeConfigDetected, selectClaudeConfigSource, selectClaudeProvider } from '@/store/slices/llmInstallationSlice';
+import { selectSelectedCliType, selectIsSkipped } from '@/store/slices/agentCliSlice';
+import PreconditionError from '@/components/onboarding/PreconditionError';
import { OnboardingStep } from '../../../../types/onboarding';
-import { setCurrentStep } from '../../../store/slices/onboardingSlice';
+import { setCurrentStep } from '@/store/slices/onboardingSlice';As per coding guidelines src/renderer/**/*.{ts,tsx}: Use @/* path aliases for imports within the renderer process.
Also applies to: 12-12
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/onboarding/steps/LlmInstallation.tsx` around lines 8
- 10, The imports at the top of LlmInstallation.tsx use deep relative paths;
update the import statements that bring in selectClaudeConfigDetected,
selectClaudeConfigSource, selectClaudeProvider, selectSelectedCliType,
selectIsSkipped and PreconditionError to use the renderer alias (`@/`*) instead of
"../../../" style paths so they resolve via the `@/` alias for renderer-internal
modules—locate the import lines in LlmInstallation.tsx and rewrite them to
import from the corresponding `@/` paths (e.g., `@/store/`..., `@/renderer/`... or the
canonical `@/` paths used elsewhere in the project).
Comment on lines
+21
to
+23
| "src/main/**/*.ts", | ||
| "src/renderer/**/*.{ts,tsx}", | ||
| "src/types/**/*.ts" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TypeScript tsconfig include brace expansion glob patterns support documentation
💡 Result:
TypeScript’s tsconfig.json does not document or support “brace expansion” in include/exclude patterns (e.g. src/**/*.{ts,tsx} or src/**/*.ts{,x}).
The official docs define include/exclude globbing as supporting only these wildcards: *, ?, and **/ (recursive directories). No {...} / comma-list brace expansion is listed. [1][2]
What to do instead: list multiple patterns explicitly:
{
"include": ["src/**/*.ts", "src/**/*.tsx"]
}Sources: TypeScript TSConfig Reference (include section) [1] and “Glob support in tsconfig.json” (TypeScript wiki) [2].
🏁 Script executed:
fd tsconfig.json | head -5Repository: HagiCode-org/desktop
Length of output: 77
🏁 Script executed:
cat -n tsconfig.jsonRepository: HagiCode-org/desktop
Length of output: 940
🏁 Script executed:
# Check if there are `@/`* imports in renderer files to confirm the need for paths config
rg "@/" src/renderer --type ts --type tsx -l | head -5Repository: HagiCode-org/desktop
Length of output: 91
🏁 Script executed:
# Check if there are `@/`* imports in renderer files to confirm the need for paths config
rg "@/" src/renderer -l | head -10Repository: HagiCode-org/desktop
Length of output: 479
🏁 Script executed:
# Check actual `@/`* imports in one of these files
head -30 src/renderer/components/BlogFeedCard.tsx | grep -E "^import|^from"Repository: HagiCode-org/desktop
Length of output: 662
Fix unsupported brace expansion and add missing path alias configuration.
TypeScript's include glob patterns do not support brace expansion like {ts,tsx}. The pattern "src/renderer/**/*.{ts,tsx}" is unsupported and should use explicit patterns. Additionally, the @/* path aliases are actively used in renderer files but the compilerOptions.paths configuration is missing, breaking type resolution.
♻️ Proposed fix
"compilerOptions": {
"target": "ES2020",
"module": "ESNext",
"lib": ["ES2020", "DOM", "DOM.Iterable"],
"jsx": "react-jsx",
"outDir": "./dist",
"strict": true,
"esModuleInterop": true,
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,
"resolveJsonModule": true,
"moduleResolution": "Bundler",
"allowSyntheticDefaultImports": true,
"types": ["node"],
"declaration": true,
"sourceMap": true,
- "noEmit": false
+ "noEmit": false,
+ "paths": {
+ "@/*": ["src/renderer/*"]
+ }
},
"include": [
"src/main/**/*.ts",
- "src/renderer/**/*.{ts,tsx}",
+ "src/renderer/**/*.ts",
+ "src/renderer/**/*.tsx",
"src/types/**/*.ts"
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` around lines 21 - 23, The include patterns use unsupported
brace expansion ("src/renderer/**/*.{ts,tsx}") and you also lack the path alias
mapping for "@/"; update the tsconfig.json by replacing the brace-expanded
include with explicit patterns (e.g., "src/renderer/**/*.ts" and
"src/renderer/**/*.tsx") and add a compilerOptions.paths entry for the "@/..."
alias (map "@" or "@/ *" to the appropriate "src/renderer" or "src/*" folder) so
the TypeScript compiler can resolve imports; modify the "include" array and
add/extend "compilerOptions.paths" accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add new Agent CLI selection system that simplifies from the previous Claude configuration approach. Users can now select their preferred AI programming CLI (Claude Code, with support for future CLIs) with automatic detection.
Features:
Summary by CodeRabbit
New Features
Removed Features
UI Improvements