Conversation
Adds comprehensive agent configuration documentation for HagiCode Desktop and removes outdated documentation files (CLAUDE.md and openspec/AGENTS.md). The new AGENTS.md provides detailed project context, tech stack, structure, and development conventions. Co-Authored-By: Hagicode <noreply@hagicode.com>
Co-Authored-By: Hagicode <noreply@hagicode.com>
Implement comprehensive preset configuration management system with: - Remote preset fetching from docs.hagicode.com with fallback to bundled presets - Local caching with 24-hour TTL and atomic writes - Provider preset management with region-aware model mappings - IPC handlers for renderer process integration Co-Authored-By: Hagicode <noreply@hagicode.com>
📝 WalkthroughWalkthroughThis PR introduces comprehensive Claude Code AI integration and an LLM-assisted installation system. It adds preset management services (PresetLoader, PresetCacheManager), Claude configuration detection and validation (ClaudeConfigManager), and a multi-step LLM installation workflow with region support. Updates include Windows PowerShell execution support, architecture-aware platform detection (linux-arm64, osx-arm64), new onboarding steps with installation wizards, and extensive Redux state management with internationalization. Changes
Sequence DiagramsequenceDiagram
actor User
participant Renderer as Renderer Process
participant IPC as IPC Bridge
participant Main as Main Process
participant PresetSvc as Preset<br/>Services
participant ClaudeConfig as Claude<br/>Config Manager
participant ExtAPI as External<br/>API/CLI
User->>Renderer: Initiate Claude Setup
Renderer->>IPC: Detect existing Claude config
IPC->>Main: Handle detectClaudeConfig
Main->>ClaudeConfig: detectExistingConfig()
ClaudeConfig->>ExtAPI: Check env/settings.json
ExtAPI-->>ClaudeConfig: Config/API Key found
ClaudeConfig-->>Main: DetectedConfig result
Main-->>IPC: Return config data
IPC-->>Renderer: Config details + provider
Renderer->>IPC: Fetch presets
IPC->>Main: Handle presetFetch
Main->>PresetSvc: loadPreset(forceRefresh)
PresetSvc->>ExtAPI: Fetch from CDN
ExtAPI-->>PresetSvc: Provider configs
PresetSvc->>PresetSvc: Cache locally
PresetSvc-->>Main: PresetIndex
Main-->>IPC: Provider data
IPC-->>Renderer: Preset + models
User->>Renderer: Enter API key & validate
Renderer->>IPC: Validate API key
IPC->>Main: Handle validateApiKey
Main->>ClaudeConfig: validateApiKey(provider, key)
ClaudeConfig->>ExtAPI: Run claude --version
ExtAPI-->>ClaudeConfig: Success/error
ClaudeConfig-->>Main: ValidationResult
Main-->>IPC: Validation status
IPC-->>Renderer: Valid/invalid feedback
User->>Renderer: Save config
Renderer->>IPC: Save Claude config
IPC->>Main: Handle saveClaudeConfig
Main->>ClaudeConfig: saveConfig(config)
ClaudeConfig->>ExtAPI: Write settings.json
ExtAPI-->>ClaudeConfig: Save + backup
ClaudeConfig-->>Main: Success/error
Main-->>IPC: Save result
IPC-->>Renderer: Confirmation
Renderer->>IPC: Load LLM prompt
IPC->>Main: Handle llmLoadPrompt
Main->>ClaudeConfig: Verify config exists
Main->>ExtAPI: Load region manifest
ExtAPI-->>Main: Prompt content
Main-->>IPC: Prompt + version
IPC-->>Renderer: Ready for API call
User->>Renderer: Call Claude API
Renderer->>IPC: Call Claude with prompt
IPC->>Main: Handle llmCallApi
Main->>ExtAPI: Open terminal → claude CLI
ExtAPI->>ExtAPI: Execute: claude read-prompt
ExtAPI-->>Main: Completion status
Main-->>IPC: ApiCallResult
IPC-->>Renderer: Installation initiated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 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
🧪 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: 9
Note
Due to the large number of review comments, Critical 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 (8)
src/renderer/components/SystemManagementView.tsx (2)
31-31:⚠️ Potential issue | 🟠 Major
onActiveVersionChangedreturn type should reflect the actual unsubscribe function it returns.The interface declares a
voidreturn, but the cleanup at lines 55–58 guards ontypeof unsubscribeVersion === 'function'and invokes it. TypeScript seesunsubscribeVersionasvoid, so the guard is always false and the listener is never cleaned up, causing a leak on every re-mount.🐛 Proposed fix
- onActiveVersionChanged: (callback: (version: InstalledVersion | null) => void) => void; + onActiveVersionChanged: (callback: (version: InstalledVersion | null) => void) => (() => void) | void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SystemManagementView.tsx` at line 31, The interface method onActiveVersionChanged currently returns void but the component expects and calls an unsubscribe function (stored in unsubscribeVersion) on cleanup; change the method signature to return a function type like () => void (or a named Unsubscribe type) instead of void so TypeScript recognizes the returned callback and the cleanup guard (checking typeof unsubscribeVersion === 'function' and invoking it) will work correctly; update the declaration of onActiveVersionChanged and any implementations to return the unsubscribe function used by the component.
5-5:⚠️ Potential issue | 🟡 MinorRemove unused
AlertCircleimport from line 5.
AlertCircleis imported but not referenced anywhere in the component. BothCheckCircleandAlertCircleare fully supported in lucide-react 0.562.0 and do not require any version concerns—only remove the unused import to clean up the dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SystemManagementView.tsx` at line 5, Remove the unused AlertCircle import from the import statement that currently brings in Package, CheckCircle, AlertCircle, Activity (update the import that references AlertCircle in SystemManagementView); leave the other imports (Package, CheckCircle, Activity) intact to avoid breaking usages elsewhere and save linting warnings by deleting only the AlertCircle symbol from the import list.src/main/web-service-manager.ts (1)
779-786:⚠️ Potential issue | 🔴 CriticalWindows startup path still bypasses PowerShell direct invocation.
start()(Line 785) still uses generic spawn options where Windows forcesshell=true(Line 433), while default Windows script is nowstart.ps1(Line 389). This bypasses the newPowerShellExecutorpath and can fail under script execution policy / command resolution differences.🛠️ Suggested patch direction
private getSpawnOptions() { const scriptPath = this.getStartupScriptPath(); @@ if (process.platform === 'win32') { - options.shell = true; // Required for executing .bat files + options.shell = !scriptPath.endsWith('.ps1'); options.detached = true; options.windowsHide = true; } else { @@ private getSpawnCommand(): { command: string; args: string[] } { const scriptPath = this.getStartupScriptPath(); const args = this.getPlatformSpecificArgs(); + if (process.platform === 'win32' && scriptPath.endsWith('.ps1')) { + return { + command: 'powershell.exe', + args: ['-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', scriptPath, ...args], + }; + } + if (process.platform === 'win32' && scriptPath.includes(' ')) { return { command: `"${scriptPath}"`, args }; }Also applies to: 430-434, 389-390
src/renderer/i18n/index.ts (1)
17-48:⚠️ Potential issue | 🟠 Major
installationnamespace JSON files exist but are never loaded into i18n.The PR adds
src/renderer/i18n/locales/en-US/installation.jsonandsrc/renderer/i18n/locales/zh-CN/installation.json, but neither this file norconfig.tsregisters aninstallationnamespace. Anyt('installation:…')call will return the raw key string.You need to:
- Import
zhCNInstallationandenUSInstallationhere.- Add
installation: zhCNInstallation/installation: enUSInstallationto the resource objects.- Add
'installation'to thensarray inconfig.ts.Proposed fix
import zhCNClaude from './locales/zh-CN/claude.json'; +import zhCNInstallation from './locales/zh-CN/installation.json'; import enUSClaude from './locales/en-US/claude.json'; +import enUSInstallation from './locales/en-US/installation.json'; // ... inside resources: 'zh-CN': { ... claude: zhCNClaude, + installation: zhCNInstallation, }, 'en-US': { ... claude: enUSClaude, + installation: enUSInstallation, },#!/bin/bash # Check if any component actually references the 'installation' namespace rg -n "installation:" --type=ts --type=tsx -g '!*.json' || rg -n "'installation'" --type=ts -g '!*.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 - 48, Import the new locale files as zhCNInstallation and enUSInstallation, add them to the i18next resources object under the 'installation' namespace (e.g., resources['zh-CN'].installation = zhCNInstallation and resources['en-US'].installation = enUSInstallation) so the resource objects in index.ts include installation, and update the i18n config's ns array (i18nConfig.ns in config.ts) to include the string 'installation' so t('installation:...') resolves.src/renderer/store/index.ts (1)
92-116:⚠️ Potential issue | 🟠 MajorCreate initialization thunk for
claudeConfigslice and dispatch at startup.The
claudeConfigandllmInstallationslices are registered in the store but have no corresponding initialization thunks dispatched at startup (lines 95–116), unlike all other slices. AdetectExistingConfigasync thunk exists inclaudeConfigSlice.ts(line 40) to detect and load existing Claude configuration. Since the onboarding flow depends onclaudeConfigstate (e.g.,selectCanGoNextchecksclaudeConfig.isValidanduseExistingConfig), dispatchdetectExistingConfigat startup to ensure existing configurations are detected and available during onboarding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/index.ts` around lines 92 - 116, Add and dispatch an initialization thunk for the claudeConfig slice so existing Claude configuration is detected before onboarding runs: create a new thunk (e.g., initializeClaudeConfig) that calls/awaits the existing detectExistingConfig async thunk from claudeConfigSlice.ts, import and dispatch initializeClaudeConfig in the startup sequence before checkOnboardingTrigger(), and likewise add/dispatch an initialization thunk for llmInstallation (e.g., initializeLLMInstallation) if that slice also needs startup detection; ensure you reference detectExistingConfig, initializeClaudeConfig, initializeLLMInstallation, and checkOnboardingTrigger when updating the store initialization order.src/main/package-sources/github-release-source.ts (1)
329-334:⚠️ Potential issue | 🔴 Critical
extractVersionFromFilenamedoesn't handle the new-nort.zipformat — returns'unknown'.The regex on line 332 only matches the old format
hagicode-{version}-(linux|windows|osx).zip. For new-format filenames likehagicode-0.1.0-beta.11-linux-x64-nort.zip, this will return'unknown'as the version string, which will propagate into theVersionobjects.Proposed fix
private extractVersionFromFilename(filename: string): string { - // Try to extract from tag name (remove 'v' prefix if present) - // Format: hagicode-{version}-{platform}.zip - const match = filename.match(/^hagicode-([0-9]\.[0-9]\.[0-9](?:-[a-zA-Z0-9\.]+)?)-(linux|windows|osx)\.zip$/); - return match ? match[1] : 'unknown'; + // New format: hagicode-{version}-{platform}-nort.zip + const newMatch = filename.match(/^hagicode-([0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9\.]+)?)-(linux-x64|linux-arm64|win-x64|osx-x64|osx-arm64)-nort\.zip$/); + if (newMatch) return newMatch[1]; + + // Old format: hagicode-{version}-{platform}.zip + const match = filename.match(/^hagicode-([0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9\.]+)?)-(linux|windows|osx)\.zip$/); + return match ? match[1] : 'unknown'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/package-sources/github-release-source.ts` around lines 329 - 334, extractVersionFromFilename currently only matches the old three-platform pattern and fails on filenames like "hagicode-0.1.0-beta.11-linux-x64-nort.zip"; update the regex in extractVersionFromFilename to capture the full semver (with optional pre-release) immediately after "hagicode-" and allow an arbitrary dash-separated platform suffix (including "-nort") before ".zip". For example, change the pattern used in extractVersionFromFilename so group 1 captures /[0-9]+\.[0-9]+\.[0-9]+(?:-[A-Za-z0-9\.]+)?/ and the rest of the filename is matched as a flexible dash-separated platform segment (e.g. something like /^hagicode-([0-9]+\.[0-9]+\.[0-9]+(?:-[A-Za-z0-9\.]+)?)-[a-z0-9\-]+(?:-nort)?\.zip$/i or a variant that allows multiple dash parts) so the function returns the proper version instead of 'unknown'.src/main/package-sources/local-folder-source.ts (1)
205-241:⚠️ Potential issue | 🟠 MajorLegacy platform normalization is incompatible with canonical filtering
Fallback parsing returns
linux/windows/osx, but filtering at Line 77 expectslinux-x64/win-x64/osx-x64(or arm64 variants). That breaks backward compatibility for old filenames.Suggested fix
- if (match) { - return match[2]; - } + if (match) { + const p = match[2].toLowerCase(); + if (p.includes('linux')) return 'linux-x64'; + if (p.includes('win')) return 'win-x64'; + if (p.includes('osx') || p.includes('darwin') || p.includes('mac')) return 'osx-x64'; + return 'unknown'; + } @@ - if (oldPlatform.includes('linux') || oldPlatform.includes('ubuntu')) { - return 'linux'; - } - if (oldPlatform.includes('win')) { - return 'windows'; - } - if (oldPlatform.includes('darwin') || oldPlatform.includes('mac') || oldPlatform.includes('osx')) { - return 'osx'; - } + if (oldPlatform.includes('linux') || oldPlatform.includes('ubuntu')) return oldPlatform.includes('arm64') ? 'linux-arm64' : 'linux-x64'; + if (oldPlatform.includes('win')) return 'win-x64'; + if (oldPlatform.includes('darwin') || oldPlatform.includes('mac') || oldPlatform.includes('osx')) return oldPlatform.includes('arm64') ? 'osx-arm64' : 'osx-x64'; @@ - if (rawPlatform.includes('linux') || rawPlatform.includes('ubuntu')) { - return 'linux'; - } - if (rawPlatform.includes('win')) { - return 'windows'; - } - if (rawPlatform.includes('darwin') || rawPlatform.includes('mac') || rawPlatform.includes('osx')) { - return 'osx'; - } + if (rawPlatform.includes('linux') || rawPlatform.includes('ubuntu')) return rawPlatform.includes('arm64') ? 'linux-arm64' : 'linux-x64'; + if (rawPlatform.includes('win')) return 'win-x64'; + if (rawPlatform.includes('darwin') || rawPlatform.includes('mac') || rawPlatform.includes('osx')) return rawPlatform.includes('arm64') ? 'osx-arm64' : 'osx-x64';Also applies to: 75-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/package-sources/local-folder-source.ts` around lines 205 - 241, The legacy platform-parsing blocks in local-folder-source.ts return non-canonical values like "linux/windows/osx" which break the canonical filter (expects "linux-x64/win-x64/osx-x64" or arm64 variants); update the two fallback mapping sections (the block that sets oldPlatform from the /^hagicode-...-([a-zA-Z]+)-x64\.zip$/ match and the block that sets rawPlatform from the /^hagicode-...-([a-zA-Z0-9\-]+)\.zip$/ match) to return canonical platform strings: map linux/ubuntu -> "linux-x64" (or "linux-arm64" if the raw/old platform string contains "arm" or "aarch64"), map win/windows -> "win-x64" or "win-arm64", map darwin/mac/osx -> "osx-x64" or "osx-arm64"; ensure the code uses the same normalized identifiers expected by the filter (use oldPlatform/rawPlatform variables to detect "arm"/"aarch64" and append "-arm64", otherwise append "-x64").src/renderer/components/VersionManagementPage.tsx (1)
70-85:⚠️ Potential issue | 🔴 CriticalUpdate
window.electronAPItype contract to match actual implementation in preload.The interface is missing 6 methods that are actively used:
getDependencyList,onVersionListChanged,llmGetManifestPath,llmCallApi,resetOnboarding, andcheckTriggerCondition. Additionally, the callback type signatures foronInstalledVersionsChangedandonActiveVersionChangedare incorrect—they return unsubscribe functions, notvoid, as evidenced by lines 158–166 where they are invoked as cleanup functions. With"strict": truein TypeScript config, these type mismatches will cause compilation failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/VersionManagementPage.tsx` around lines 70 - 85, Update the Window.electronAPI type to match the preload implementation: add the missing methods getDependencyList, onVersionListChanged, llmGetManifestPath, llmCallApi, resetOnboarding, and checkTriggerCondition with appropriate return types, and change the signatures of onInstalledVersionsChanged and onActiveVersionChanged so their callback registration functions return an unsubscribe function (e.g., () => void) instead of void; reference window.electronAPI and the listed method names when making these changes so TypeScript "strict" mode matches the runtime API.
🟡 Minor comments (15)
AGENTS.md-84-84 (1)
84-84:⚠️ Potential issue | 🟡 MinorAdd a language specifier to the fenced code block.
markdownlint rule MD040 requires all fenced code blocks to have an explicit language. Use
textfor the directory tree.📝 Proposed fix
-``` +```text src/ ├── main/ # Electron main process code🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 84, Add a language specifier to the fenced code block that contains the directory tree (the triple-backtick block starting with "src/") in AGENTS.md; change the opening fence from ``` to ```text so markdownlint MD040 is satisfied and the directory listing is explicitly marked as plain text.AGENTS.md-57-57 (1)
57-57:⚠️ Potential issue | 🟡 MinorUpdate tech stack entry from
React Router DOMtoReact Routerfor v7 consistency.In React Router v7,
react-router-domhas been consolidated intoreact-router. Whilereact-router-domstill re-exports fromreact-routerfor migration compatibility, the tech stack should list the canonical package name to accurately reflect the v7 setup and avoid misleading contributors.📝 Proposed fix
- - **React Router DOM**: 7.11.0 + - **React Router**: 7.11.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 57, Update the tech stack entry that currently reads "**React Router DOM**: 7.11.0" to the canonical package name for v7 by replacing it with "**React Router**: 7.11.0" in AGENTS.md so the stack accurately reflects the v7 consolidation; ensure no other occurrences of "React Router DOM" remain and preserve the version string.src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx-43-43 (1)
43-43:⚠️ Potential issue | 🟡 MinorDecorative icons should carry
aria-hidden="true".
HelpCircle(Line 43) andCheckCircle2(Line 56) are purely decorative — their meaning is conveyed by the adjacent text. Withoutaria-hidden, screen readers may announce them redundantly.♿ Proposed fix
- <HelpCircle className="h-6 w-6 text-primary" /> + <HelpCircle className="h-6 w-6 text-primary" aria-hidden="true" />- <CheckCircle2 className="h-5 w-5 text-green-500 mt-0.5 flex-shrink-0" /> + <CheckCircle2 className="h-5 w-5 text-green-500 mt-0.5 flex-shrink-0" aria-hidden="true" />Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx` at line 43, Add aria-hidden="true" to the decorative icon components in InstallationConfirmationDialog.tsx so screen readers ignore them: update the HelpCircle and CheckCircle2 JSX elements to include aria-hidden="true" (and optionally focusable={false} if the icon library supports it) to mark them as decorative while leaving the adjacent text unchanged.src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx-38-38 (1)
38-38:⚠️ Potential issue | 🟡 MinorAdd
onOpenChangehandler for consistency and explicit intent.All other Dialog components in the codebase explicitly handle
onOpenChange. Without it here, the design intent (whether intentionally blocking Escape or allowing it) is implicit. Add a handler to match the established pattern:Recommended fix
- <Dialog open={isOpen}> + <Dialog open={isOpen} onOpenChange={() => {}}>Or, if Escape should route to the help flow:
- <Dialog open={isOpen}> + <Dialog open={isOpen} onOpenChange={(open) => { if (!open) onNeedHelp(); }}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx` at line 38, The Dialog in InstallationConfirmationDialog currently sets open={isOpen} but lacks an onOpenChange handler; add an onOpenChange prop to the Dialog (e.g. onOpenChange={handleOpenChange}) and implement handleOpenChange to mirror other dialogs in the codebase: when open becomes false call the same close/update routine used elsewhere (eg. setIsOpen(false) or onClose()) or, if Escape should route to help, call the help-routing function (eg. routeToHelpFlow())—ensure the handler name (handleOpenChange) and referenced symbols (Dialog, isOpen, setIsOpen/onClose/routeToHelpFlow) match existing code so behavior is explicit and consistent.src/renderer/components/installation-wizard/StepTracker.tsx-25-38 (1)
25-38:⚠️ Potential issue | 🟡 MinorStep dots lack accessible labels — screen readers cannot convey progress.
The
<div>dots are rendered withoutrole,aria-label, oraria-current, making the progress indicator invisible to assistive technology. Consider at minimum addingaria-labelto each dot and anaria-liveregion for the step title.♿ Proposed fix
+ <div className="step-progress" role="list" aria-label="Installation steps"> {Array.from({ length: totalSteps }).map((_, index) => ( <div key={index} + role="listitem" + aria-label={`Step ${index + 1}${stepTitles[index] ? `: ${stepTitles[index]}` : ''}`} + aria-current={index === currentStep ? 'step' : undefined} className={`step-dot ${ index < currentStep ? 'completed' : index === currentStep ? 'active' : 'pending' }`} /> ))} + </div>src/renderer/components/installation-wizard/StepTracker.tsx-3-7 (1)
3-7:⚠️ Potential issue | 🟡 Minor
totalStepsandstepTitlesare independent props — a mismatch will silently produce blank titles or extra/missing dots.If
totalSteps !== stepTitles.length, line 23 rendersundefinedfor out-of-range indices, and the dot count diverges from the number of titles. Consider derivingtotalStepsfromstepTitles.lengthto keep them in sync.🛡️ Proposed fix
interface StepTrackerProps { currentStep: number; - totalSteps: number; stepTitles: string[]; } export const StepTracker: React.FC<StepTrackerProps> = ({ currentStep, - totalSteps, stepTitles, }) => { + const totalSteps = stepTitles.length; return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/installation-wizard/StepTracker.tsx` around lines 3 - 7, The props interface and rendering are vulnerable because totalSteps can diverge from stepTitles.length; update the component to derive totalSteps from stepTitles.length (remove totalSteps from StepTrackerProps or ignore it) and use stepTitles.length wherever the count is needed (e.g., in the loop that renders dots and in the active index checks such as currentStep vs total count); alternatively, add a runtime guard in the component that, if totalSteps !== stepTitles.length, logs a warning and sets an internal count = stepTitles.length to ensure the number of dots and titles always stay in sync (reference StepTrackerProps, currentStep, totalSteps, and stepTitles).src/renderer/i18n/locales/en-US/claude.json-39-49 (1)
39-49:⚠️ Potential issue | 🟡 Minor
invalidFormatvalidation message is Anthropic-specific but multiple providers are supported.Line 48 says keys "should start with sk-ant-", but the UI supports Zhipu AI, Aliyun, MiniMax, and custom endpoints — their API keys likely use different prefixes. This message could confuse users of non-Anthropic providers.
Consider making the message provider-aware or removing the format hint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/en-US/claude.json` around lines 39 - 49, The "invalidFormat" message in src/renderer/i18n/locales/en-US/claude.json is Anthropic-specific ("should start with sk-ant-") but the UI supports multiple providers; update the "validation.invalidFormat" entry to be provider-neutral (e.g., "Invalid API Key format") or implement provider-aware messaging (only append the "sk-ant-" hint when provider === "anthropic") so non-Anthropic users aren't confused; locate the "invalidFormat" key in the "validation" object and either remove the specific prefix hint or make the text dynamically chosen based on the selected provider.src/renderer/i18n/locales/zh-CN/claude.json-39-48 (1)
39-48:⚠️ Potential issue | 🟡 Minor
invalidFormatvalidation message is Anthropic-specific but sits in a genericvalidationblock.The message says API keys should start with
sk-ant-, which is only valid for Anthropic. Since this config supports multiple providers (Zhipu, Aliyun, custom), this message will mislead users using non-Anthropic providers. Consider either scoping this message per provider or making the text generic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/zh-CN/claude.json` around lines 39 - 48, The "invalidFormat" message in the "validation" block is Anthropic-specific (mentions sk-ant-) and will mislead other providers; update this by either moving "invalidFormat" into a provider-scoped translations object (e.g., under an "anthropic" key) or change its value to a generic message like "API Key 格式无效" and add provider-specific variants (e.g., "anthropic.invalidFormat") so each provider (Anthropic, Zhipu, Aliyun, custom) can have its own validation string; locate the "validation" object and the "invalidFormat" key and implement one of these two fixes, updating any consumers that read validation.invalidFormat to use the provider-scoped key when applicable.src/renderer/i18n/locales/zh-CN/claude.json-78-79 (1)
78-79:⚠️ Potential issue | 🟡 MinorDotted key
testClaude.subtextconflicts with i18next configuration.The i18n config explicitly sets
keySeparator: '.'(src/renderer/i18n/config.ts:29). The code callst('testClaude.subtext'), which i18next will interpret as a nested path lookup: accesstestClaudeobject then get itssubtextproperty. However, the JSON has a flat key"testClaude.subtext"instead of a nested structure, causing the translation lookup to fail.Fix by nesting the structure:
Nesting solution
- "testClaude": "测试配置", - "testClaude.subtext": "打开终端执行 claude hi", + "testClaude": { + "label": "测试配置", + "subtext": "打开终端执行 claude hi" + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/i18n/locales/zh-CN/claude.json` around lines 78 - 79, The translation key "testClaude.subtext" in claude.json is defined as a flat dotted key but i18next is configured with keySeparator: '.' so t('testClaude.subtext') expects a nested object; update the locale JSON to nest the keys so "testClaude" is an object with a "subtext" property (matching the call t('testClaude.subtext')), i.e., replace the flat "testClaude.subtext" entry with a nested structure under "testClaude" containing "subtext" so lookups in t(...) and the i18n config align.src/types/preset.ts-200-212 (1)
200-212:⚠️ Potential issue | 🟡 MinorSame
nullissue applies toisValidPresetIndex.- typeof i.types === 'object' + typeof i.types === 'object' && i.types !== null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/preset.ts` around lines 200 - 212, The isValidPresetIndex function treats null incorrectly because the initial check uses falsy check; change it to explicitly reject null and non-objects (use index === null || typeof index !== 'object'), and ensure the properties checks guard against null for nested objects by verifying i.types !== null in addition to typeof i.types === 'object'; update the function where it references isValidPresetIndex and the local const i to use these explicit null checks.src/renderer/components/onboarding/steps/LlmInstallation.tsx-170-184 (1)
170-184:⚠️ Potential issue | 🟡 MinorHardcoded Chinese strings in several places bypass i18n.
getRegionInfo(Lines 176, 182), Line 341 (当前区域:), and Line 408 (重试区域:) contain hardcoded Chinese text despite the component already usinguseTranslation. These should uset(...)keys for consistency.🤖 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 170 - 184, Replace the hardcoded Chinese strings by using the existing i18n translation function: inside getRegionInfo(...) replace description values ('使用国内 API（智谱 AI、阿里云等）' and '使用国际 API（Anthropic 官方）') with calls to t(...) (e.g. t('llmInstallation.region.cn.description') and t('llmInstallation.region.international.description')) and the icon/name should keep using t('llmInstallation.region.cn')/t('llmInstallation.region.international') as already done; likewise locate the literal texts "当前区域:" and "重试区域:" in the component and swap them for t('llmInstallation.currentRegion') and t('llmInstallation.retryRegion') (or your chosen i18n keys) so all user-facing strings go through useTranslation.src/main/llm-installation-manager.ts-198-203 (1)
198-203:⚠️ Potential issue | 🟡 MinorHardcoded Chinese error message bypasses i18n.
Line 201 uses a hardcoded Chinese string
'无法找到可用的终端模拟器'. This will be unintelligible to non-Chinese users. Use an English message or a localizable key.Suggested fix
- error: '无法找到可用的终端模拟器', + error: 'No available terminal emulator found',🤖 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 198 - 203, Replace the hardcoded Chinese error string returned when terminalFound is false with either an English message or a localization key; update the return object (the block that checks terminalFound and returns { success: false, error: ... }) to use a clear English message such as "No available terminal emulator found" or a localizable key like "error.noTerminalFound" (e.g., via your i18n helper), and ensure any callers that inspect this error use the new key/message.src/types/preset.ts-179-195 (1)
179-195:⚠️ Potential issue | 🟡 Minor
typeof ... === 'object'passes fornull, making validators unsound.
typeof null === 'object'in JavaScript, soapiUrl: nullanddefaultModels: nullwould pass validation, but downstream code expectsRecord<string, string>andModelMapping(non-null objects), leading to runtime crashes.Proposed fix
return ( typeof p.providerId === 'string' && typeof p.name === 'string' && typeof p.description === 'string' && - typeof p.apiUrl === 'object' && + typeof p.apiUrl === 'object' && p.apiUrl !== null && typeof p.recommended === 'boolean' && typeof p.region === 'string' && - typeof p.defaultModels === 'object' + typeof p.defaultModels === 'object' && p.defaultModels !== null );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/preset.ts` around lines 179 - 195, The validator is currently allowing null because it uses typeof ... === 'object'; update isValidProviderPreset to explicitly reject null and arrays for both apiUrl and defaultModels (e.g. check p.apiUrl !== null && !Array.isArray(p.apiUrl) && typeof p.apiUrl === 'object' and same for p.defaultModels) and optionally perform a shallow runtime check that apiUrl's values are strings (for Record<string,string>) and defaultModels is a non-null object (ModelMapping) to ensure downstream code won't see null.src/services/preset-loader.ts-191-192 (1)
191-192:⚠️ Potential issue | 🟡 MinorRequest path drops query string parameters.
Only
pathnameis sent. If a source URL includes query params (e.g., signed URLs, cache-busting), fetch will target the wrong resource.💡 Proposed fix
- path: url.pathname, + path: `${url.pathname}${url.search}`,- path: providerUrl.pathname, + path: `${providerUrl.pathname}${providerUrl.search}`,Also applies to: 252-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/preset-loader.ts` around lines 191 - 192, The request path currently drops query parameters by using "path: url.pathname"; change these occurrences to include the search string (e.g., "path: url.pathname + url.search" or concatenating url.search when present) so signed URLs and cache-busting queries are preserved; update both the instance setting path: url.pathname (around the GET request construction) and the other occurrence noted (the one at the later block) to use url.pathname + url.search.src/renderer/components/VersionManagementPage.tsx-353-356 (1)
353-356:⚠️ Potential issue | 🟡 MinorSuccess state is unreachable in current Claude call flow.
Line 354 sets status back to
idleon success, so thecompletedUI block at Lines 844-856 never renders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/VersionManagementPage.tsx` around lines 353 - 356, The success UI never appears because the Claude call flow resets the status to 'idle' on successful completion; change the success branch in VersionManagementPage.tsx to call setLlmCallStatus('completed') (using the existing LlmCallStatus values) instead of setLlmCallStatus('idle'), and ensure the catch/finally still sets 'failed' or leaves 'idle' appropriately on errors so the completed UI block that checks for 'completed' can render.
🧹 Nitpick comments (17)
src/renderer/store/slices/packageSourceSlice.ts (1)
55-57: Avoid committing a developer-specific absolute default path.Line 56 hardcodes a personal local path, which is brittle across dev machines and hard to maintain. Prefer an environment-driven default (or centralized config constant) with a safe empty fallback.
♻️ Suggested refactor
+const DEV_DEFAULT_PACKAGE_PATH = process.env.HAGI_RELEASE_PACKAGES_PATH ?? ''; + const initialState: PackageSourceState = { ... folderPath: process.env.NODE_ENV === 'development' - ? '/home/newbe36524/repos/newbe36524/hagicode-mono/repos/hagibuild/Release/release-packages' + ? DEV_DEFAULT_PACKAGE_PATH : '', ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/slices/packageSourceSlice.ts` around lines 55 - 57, The slice currently hardcodes a developer-specific absolute path in the folderPath default; replace that literal with an environment-driven or centralized-config fallback. Update the folderPath value (in packageSourceSlice.ts) to use something like process.env.DEV_PACKAGE_PATH || config.DEFAULT_PACKAGE_FOLDER || '' (or process.env.PACKAGE_FOLDER for production) instead of the hardcoded string so no personal local path is committed; reference the folderPath identifier to apply this change and, if you have a config module, introduce DEFAULT_PACKAGE_FOLDER there and use it from the slice.src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx (1)
26-29: JSDoc hardcodes Chinese strings that should reference i18n keys instead.The comments at lines 26–29 embed literal Chinese text, which will drift from the actual rendered strings as translations evolve.
♻️ Proposed fix
- * - Clear question: "AI 是否已经安装完成？" - * - Primary action: "是，继续" (Yes, continue) - * - Secondary action: "否，需要帮助" (No, need help) + * - Clear question: `installationConfirmation.title` + * - Primary action: `installationConfirmation.confirm` + * - Secondary action: `installationConfirmation.needHelp`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsx` around lines 26 - 29, Replace the hardcoded Chinese strings in the JSDoc comment above the InstallationConfirmationDialog component with references to the i18n keys used by the component (e.g., the keys for the question, primary action, and secondary action); locate the JSDoc block near the InstallationConfirmationDialog export/definition and change the literal "AI 是否已经安装完成？", "是，继续", and "否，需要帮助" to mention their i18n key names (like onboarding.installation.confirmation.question, onboarding.installation.confirmation.primary, onboarding.installation.confirmation.secondary) so the doc stays in sync with translations.src/renderer/components/SystemManagementView.tsx (1)
320-320:"✅ 就绪"is a hardcoded string; uset()to stay consistent with the rest of the file.Every other status label (
running,stopped,error) is localised viat(). This new string bypasses i18n and embeds an emoji that is visually redundant with the adjacentCheckCircleicon.♻️ Proposed fix
- <span className="text-primary">✅ 就绪</span> + <span className="text-primary">{t('status.ready')}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SystemManagementView.tsx` at line 320, Replace the hardcoded "✅ 就绪" string in the SystemManagementView component with a localized label using t(...) consistent with the other status labels (e.g., t('status.ready') or the existing key pattern used for 'running'/'stopped'/'error'), remove the redundant emoji so only the CheckCircle icon conveys status, and ensure the same span element (className="text-primary") renders the localized text; keep the existing i18n usage/imports (useTranslation/t) already used elsewhere in this file.nukeBuild/scripts/windows/start.ps1 (2)
23-25: Emptycatchblocks suppress potentially useful diagnostic information.PSScriptAnalyzer flags these as
PSAvoidUsingEmptyCatchBlock. While silently ignoring window-management failures is intentional, adding aWrite-Verboseor$null = $_with a comment makes intent explicit and suppresses the linter warning without changing behavior.♻️ Example
} catch { - # Silently ignore window hiding failures + Write-Verbose "Window hiding not available: $($_.Exception.Message)" }Also applies to: 42-42, 70-70, 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nukeBuild/scripts/windows/start.ps1` around lines 23 - 25, Replace the empty catch blocks that silently swallow exceptions (the catch after the window-hide logic and the other similar catch blocks) with an explicit no-op that documents intent and satisfies PSScriptAnalyzer: inside each catch, assign the exception to $null (e.g., $null = $_) or call Write-Verbose with the exception message and a brief comment explaining that window-management errors are intentionally ignored; update the three occurrences corresponding to the catch at the window-hiding area and the other two empty catch blocks so behavior is unchanged but the linter warning PSAvoidUsingEmptyCatchBlock is suppressed.
54-54:Write-Hostin error fallback paths —Write-Erroris more idiomatic.PSScriptAnalyzer flags
Write-HostasPSAvoidUsingWriteHost. In these fallback paths (where the MessageBox already failed),Write-Errorwrites to the error stream which is more appropriate for error reporting and can be captured/redirected.♻️ Example for the .NET-not-found fallback (line 54)
- Write-Host $message -ForegroundColor Red + Write-Error $message Read-Host "Press Enter to exit"Also applies to: 81-81, 114-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nukeBuild/scripts/windows/start.ps1` at line 54, Replace the use of Write-Host in the script's MessageBox fallback/error paths with Write-Error so errors go to the error stream and can be captured; specifically locate the .NET-not-found fallback (the branch that falls back after MessageBox failure) and other similar fallback branches where Write-Host is used (the other Write-Host occurrences) and change them to Write-Error, preserving the existing $message text and any foreground-color semantics by including the message string as the Write-Error argument (e.g., Write-Error $message) so the behavior remains the same but uses the error stream.nukeBuild/scripts/windows/check.ps1 (1)
244-261: Duplicate counting blocks and magic number5— minor cleanup opportunity.
$installedCountand$requiredInstalledCountare computed with the exact same five conditions (lines 247–257), making them always equal. Since all five dependencies declarerequired = $truein the result object, the distinction adds no value. The hardcoded5on lines 236 and 261 will silently become wrong if a dependency is added or removed.♻️ Proposed refactor
- $installedCount = 0 - $requiredInstalledCount = 0 - - if ($DotNetStatus.status -eq "installed") { $installedCount++ } - if ($NodeStatus.status -eq "installed") { $installedCount++ } - if ($NpmStatus.status -eq "installed") { $installedCount++ } - if ($ClaudeStatus.status -eq "installed") { $installedCount++ } - if ($OpenspecStatus.status -eq "installed") { $installedCount++ } - - if ($DotNetStatus.status -eq "installed") { $requiredInstalledCount++ } - if ($NodeStatus.status -eq "installed") { $requiredInstalledCount++ } - if ($NpmStatus.status -eq "installed") { $requiredInstalledCount++ } - if ($ClaudeStatus.status -eq "installed") { $requiredInstalledCount++ } - if ($OpenspecStatus.status -eq "installed") { $requiredInstalledCount++ } + $allStatuses = @($DotNetStatus, $NodeStatus, $NpmStatus, $ClaudeStatus, $OpenspecStatus) + $installedCount = ($allStatuses | Where-Object { $_.status -eq "installed" }).Count + $requiredInstalledCount = $installedCount # All dependencies are required + $totalCount = $allStatuses.CountAnd update the summary block:
- $Result.summary.total = 5 + $Result.summary.total = $totalCount $Result.summary.installed = $installedCount $Result.summary.requiredInstalled = $requiredInstalledCount - $Result.summary.ready = ($requiredInstalledCount -eq 5) + $Result.summary.ready = ($requiredInstalledCount -eq $totalCount)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nukeBuild/scripts/windows/check.ps1` around lines 244 - 261, Remove the duplicate counting and hard-coded "5": compute installed and requiredInstalled by iterating over the dependency status objects (e.g., $DotNetStatus, $NodeStatus, $NpmStatus, $ClaudeStatus, $OpenspecStatus) or, better, iterate the collection that holds dependency results, incrementing installed when status -eq "installed" and requiredInstalled when the dependency's required flag is $true; then set $Result.summary.installed and $Result.summary.requiredInstalled from those counts and set $Result.summary.ready by comparing requiredInstalled to the count of dependencies with required = $true (instead of using the magic number 5).src/main/manifest-reader.ts (1)
12-13: Asymmetric naming between the two LLM prompt fields.
llmPromptimplicitly means "CN region" whilellmPromptIntlis explicit about being international. This is unintuitive — a reader would expectllmPromptto be the default/universal field. Consider symmetric naming such asllmPromptCn/llmPromptIntlto make both variants equally explicit.♻️ Proposed rename
- llmPrompt?: string; // LLM prompt path for CN region - llmPromptIntl?: string; // LLM prompt path for international region + llmPromptCn?: string; // LLM prompt path for CN region + llmPromptIntl?: string; // LLM prompt path for international region🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/manifest-reader.ts` around lines 12 - 13, The two manifest fields use asymmetric names (llmPrompt vs llmPromptIntl); rename llmPrompt to llmPromptCn for symmetry with llmPromptIntl, then update all references/usages (types, deserialization, accessors, tests, and any consumers) to use llmPromptCn and llmPromptIntl (e.g., in manifest-reader.ts, code that reads the manifest, and any interfaces that expose these fields) ensuring backward-compatibility if needed (accept old llmPrompt when parsing and map it to llmPromptCn).nukeBuild/scripts/windows/install.ps1 (1)
8-8:Get-WmiObjectis deprecated in PowerShell 7+ — preferGet-CimInstance.
Get-WmiObjectwas removed in PowerShell 7. While the#Requires -Version 5.1guard limits execution to 5.1+, systems running PowerShell 7 will still execute the script and hit a runtime error on the default parameter value.Get-CimInstanceis the forward-compatible replacement.♻️ Proposed fix
- [string]$Arch = (Get-WmiObject -Class Win32_Processor).Architecture, + [string]$Arch = (Get-CimInstance -ClassName Win32_Processor).Architecture,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nukeBuild/scripts/windows/install.ps1` at line 8, The default parameter expression uses the deprecated Get-WmiObject which fails on PowerShell 7; update the default value for [string]$Arch to use Get-CimInstance instead of Get-WmiObject (targeting the Win32_Processor class) and remove the stray trailing comma so the expression is valid; ensure the expression is written as a single call that retrieves the Architecture property (e.g., use Get-CimInstance -ClassName Win32_Processor and read .Architecture) so the variable $Arch is populated in a forward-compatible way.src/main/utils/powershell-executor.ts (1)
264-266: Usepath.joinforresult.jsonpath construction.Line 264 currently uses string concatenation for file paths. Using
path.joinis safer and consistent with the rest of main-process path handling.♻️ Suggested patch
import { spawn } from 'node:child_process'; import fs from 'node:fs/promises'; +import path from 'node:path'; @@ - const resultPath = `${scriptDirectory}/result.json`; + const resultPath = path.join(scriptDirectory, 'result.json');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/utils/powershell-executor.ts` around lines 264 - 266, Replace the string-concatenated resultPath assignment with a path-joined construction: use path.join(scriptDirectory, 'result.json') to build resultPath (ensure the module imports path if not already), then continue reading with fs.readFile(resultPath, 'utf-8') and parsing into ResultSessionFile as before; update the reference to resultPath in this block (where resultPath, scriptDirectory, fs.readFile, and ResultSessionFile are used).src/renderer/store/thunks/webServiceThunks.ts (1)
74-75: UnifystartWebServiceresponse typing at the source interface.Line 74 narrows the local type, but the global
window.electronAPI.startWebServicedeclaration still advertiseswarning. Prefer updating the shared interface type to match the new contract so consumers don’t drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/store/thunks/webServiceThunks.ts` around lines 74 - 75, The local narrowing of the startWebService response in webServiceThunks.ts is masking a mismatch with the global declaration; update the shared declaration for window.electronAPI.startWebService to return the unified shape { success: boolean; error?: { type: string; details: string } } (remove/replace any existing "warning" variant) so all call sites see the same contract, and then remove the ad-hoc local type narrowing in the startWebService consumer to rely on the updated global type.src/assets/presets/bundle.json (1)
1-12: Ambiguous dual version fields and empty providers.
version("1.0.0") and_version("1.0") are both present with no clear semantic distinction. Consider documenting or consolidating them to avoid confusion for future maintainers.Also,
providersis{}— the bundle ships no actual provider data. If this is intentional as a placeholder, a brief comment in a companion doc or README would help clarify. Otherwise, ensure the consuming code (PresetLoader / PresetCacheManager) handles the empty-providers case gracefully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/presets/bundle.json` around lines 1 - 12, Consolidate or document the ambiguous dual-version fields and handle the empty providers: decide whether to keep only "version" or "_version" and update the JSON accordingly (or add a comment in a companion README clarifying their semantic difference), and if "types.claude-code.providers" is intentionally empty either populate it with at least a placeholder provider entry or add documentation stating it's a placeholder; additionally update the consuming code paths (PresetLoader and PresetCacheManager) to explicitly check for an empty providers object (treat {} as no providers) and handle that case without throwing (e.g., fallback to defaults or skip provider registration).src/renderer/components/installation-wizard/ApiKeyInput.tsx (1)
46-73: Minor accessibility gaps in the form.
- The
<input>lacks an associated<label>element (the<h3>is not semantically linked).- The show/hide
<button>has noaria-label, so screen readers will only announce its text content, which is acceptable but could be more descriptive (e.g., "Toggle API key visibility").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/installation-wizard/ApiKeyInput.tsx` around lines 46 - 73, Add accessible labeling for the API key input and the visibility toggle: give the input element a stable id and associate a visible <label> (or a visually-hidden label) with that id so it's semantically linked to the heading, and add an explicit aria-label to the show/hide button (referencing the showKey state and setShowKey toggle) such as "Toggle API key visibility" or a dynamic label reflecting current state; ensure these changes are applied in the ApiKeyInput component around the input, handleSubmit, apiKey, showKey, setShowKey and onCancel usage so keyboard and screen-reader users can identify and control the field.src/main/package-sources/github-release-source.ts (1)
339-389: Extract shared platform and version utilities to reduce duplication across package sources.
extractPlatformFromFilename,getCurrentPlatform,extractVersionFromFilename, andcompareVersionsare duplicated acrossversion-manager.ts,github-release-source.ts,local-folder-source.ts, andhttp-index-source.ts. These methods are nearly identical and should be extracted into a shared utility module to reduce maintenance burden and ensure consistency.Note:
path-manager.tshas similar methods with different signatures (Platformreturn type, error throwing instead of fallbacks) and should be refactored separately. Additionally,http-index-source.ts'scompareVersionshas more sophisticated prerelease version handling that should be preserved during extraction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/package-sources/github-release-source.ts` around lines 339 - 389, Multiple files duplicate platform/version logic; extract the shared utilities into a single module and update callers to import them. Create a new utility (e.g., packagePlatformVersionUtils) that exports extractPlatformFromFilename, getCurrentPlatform, extractVersionFromFilename and compareVersions, porting the implementation from github-release-source.ts but preserve the more advanced prerelease handling currently implemented in http-index-source.ts when merging compareVersions. Replace the duplicated implementations in version-manager.ts, github-release-source.ts, local-folder-source.ts and http-index-source.ts with imports from the new utility; leave path-manager.ts untouched (it has different signatures/behavior) and ensure all callers use the same return types and error/fallback semantics as the consolidated utility.src/types/claude-config.ts (1)
110-114: Consider reusing the existingModelMappingtype
ModelMappinghere duplicatessrc/types/preset.ts(haiku/sonnet/opus). Keeping both will drift over time; prefer a shared export.Suggested refactor
+import type { ModelMapping as PresetModelMapping } from './preset'; @@ -export interface ModelMapping { - haiku?: string; - sonnet?: string; - opus?: string; -} +export type ModelMapping = PresetModelMapping;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/claude-config.ts` around lines 110 - 114, The ModelMapping interface in claude-config.ts duplicates the same haiku/sonnet/opus shape defined in src/types/preset.ts; remove the local declaration and import/re-export the shared type instead so both modules use the single source of truth (replace the local export of ModelMapping with an import from the preset types and update any references to use that imported symbol).src/main/llm-installation-manager.ts (1)
121-121:mainWindowparameter typed asany.The
mainWindowparameter is typed asanybut actually receivesevent.sender(aWebContents) from the IPC handler inmain.ts(Line 1345). Consider typing it asElectron.WebContentsfor clarity and type safety. Note also thatmainWindowis not actually used inside the method body — it's only passed in but never referenced — so it could be removed entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/llm-installation-manager.ts` at line 121, The callClaudeAPI method currently declares a second parameter mainWindow typed as any but it's never used; either strongly type it as Electron.WebContents (replace any with Electron.WebContents) if you intend to use the sender, or remove the mainWindow parameter entirely from the callClaudeAPI signature and all callers to eliminate dead/unsafe code; update the declaration in llm-installation-manager.ts (function callClaudeAPI) and adjust the IPC caller in main.ts to stop passing event.sender if you remove the parameter.src/renderer/components/onboarding/steps/LlmInstallation.tsx (1)
147-168:getProviderDisplayNameandgetSourceDisplayNameare duplicated fromInstallationWizard.tsx.Both components define identical helper functions (Lines 147-168 here, Lines 113-133 in
InstallationWizard.tsx). Consider extracting them to a shared utility.🤖 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 147 - 168, Extract the duplicated helper functions getProviderDisplayName and getSourceDisplayName into a shared utility module (e.g., create a new util function file and export both functions), then replace the inline definitions in LlmInstallation.tsx and InstallationWizard.tsx with imports from that shared module; ensure the utility preserves the same return values and signatures (accepting provider: string | null and source: string | null) and update any existing references to use the imported getProviderDisplayName and getSourceDisplayName.src/types/preset.ts (1)
10-14: DuplicateModelMappinginterface.An identical
ModelMappinginterface already exists insrc/types/claude-config.ts(lines 109-113). Consider importing from there to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/preset.ts` around lines 10 - 14, Remove the duplicate ModelMapping interface from this file and instead import and re-export (or directly use) the single authoritative ModelMapping type defined in claude-config; update any local references in this file to use the imported ModelMapping type and ensure the file's export list (if any) still exposes ModelMapping where needed so there is only one source-of-truth for the ModelMapping interface.
ℹ️ 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 (62)
AGENTS.mdCLAUDE.mdnukeBuild/scripts/windows/check.ps1nukeBuild/scripts/windows/install.ps1nukeBuild/scripts/windows/start.ps1openspec/AGENTS.mdsrc/assets/presets/bundle.jsonsrc/ipc/handlers/preset-handlers.tssrc/main/claude-config-manager.tssrc/main/dependency-manager.tssrc/main/lib/api-endpoints.tssrc/main/llm-installation-manager.tssrc/main/main.tssrc/main/manifest-reader.tssrc/main/onboarding-manager.tssrc/main/package-sources/github-release-source.tssrc/main/package-sources/http-index-source.tssrc/main/package-sources/local-folder-source.tssrc/main/path-manager.tssrc/main/utils/powershell-executor.tssrc/main/version-manager.tssrc/main/web-service-manager.tssrc/preload/index.tssrc/renderer/App.tsxsrc/renderer/components/PackageSourceSelector.tsxsrc/renderer/components/SystemManagementView.tsxsrc/renderer/components/VersionManagementPage.tsxsrc/renderer/components/installation-wizard/ApiKeyInput.tsxsrc/renderer/components/installation-wizard/InstallationWizard.tsxsrc/renderer/components/installation-wizard/StepTracker.tsxsrc/renderer/components/installation-wizard/UserConfirmation.tsxsrc/renderer/components/installation-wizard/index.tssrc/renderer/components/onboarding/OnboardingWizard.tsxsrc/renderer/components/onboarding/steps/ClaudeConfig.tsxsrc/renderer/components/onboarding/steps/DependencyInstaller.tsxsrc/renderer/components/onboarding/steps/InstallationConfirmationDialog.tsxsrc/renderer/components/onboarding/steps/InstallationHelpDialog.tsxsrc/renderer/components/onboarding/steps/LlmInstallation.tsxsrc/renderer/components/onboarding/steps/WelcomeIntro.tsxsrc/renderer/i18n/config.tssrc/renderer/i18n/index.tssrc/renderer/i18n/locales/en-US/claude.jsonsrc/renderer/i18n/locales/en-US/installation.jsonsrc/renderer/i18n/locales/en-US/onboarding.jsonsrc/renderer/i18n/locales/en-US/pages.jsonsrc/renderer/i18n/locales/zh-CN/claude.jsonsrc/renderer/i18n/locales/zh-CN/installation.jsonsrc/renderer/i18n/locales/zh-CN/onboarding.jsonsrc/renderer/i18n/locales/zh-CN/pages.jsonsrc/renderer/lib/api-endpoints.tssrc/renderer/lib/promo-links.tssrc/renderer/store/index.tssrc/renderer/store/slices/claudeConfigSlice.tssrc/renderer/store/slices/llmInstallationSlice.tssrc/renderer/store/slices/onboardingSlice.tssrc/renderer/store/slices/packageSourceSlice.tssrc/renderer/store/thunks/webServiceThunks.tssrc/services/preset-cache-manager.tssrc/services/preset-loader.tssrc/types/claude-config.tssrc/types/onboarding.tssrc/types/preset.ts
💤 Files with no reviewable changes (4)
- openspec/AGENTS.md
- src/renderer/components/onboarding/steps/DependencyInstaller.tsx
- src/renderer/App.tsx
- CLAUDE.md
| * Save Claude configuration to electron-store | ||
| * Stores encrypted API key | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Store initialization and encryptionKey configuration
rg -n -C4 "new Store\(|encryptionKey|claudeConfig|STORE_KEY" src/main srcRepository: HagiCode-org/desktop
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# Find ConfigManager class and Store initialization with encryption
rg -n "class ConfigManager|configManager.*=" src/main/main.ts | head -20Repository: HagiCode-org/desktop
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Search for ConfigManager initialization and Store creation
rg -n "new ConfigManager|new Store" src/main/main.ts -B 5 -A 5 | head -80Repository: HagiCode-org/desktop
Length of output: 508
🏁 Script executed:
#!/bin/bash
# Find ConfigManager class definition
fd -e ts -e js | xargs rg -l "class ConfigManager" src/Repository: HagiCode-org/desktop
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Read ConfigManager class to see Store initialization
cat -n src/main/config-manager.ts | head -150Repository: HagiCode-org/desktop
Length of output: 6139
🏁 Script executed:
#!/bin/bash
# Find getStore method in ConfigManager
rg -n "getStore|Store" src/main/config-manager.tsRepository: HagiCode-org/desktop
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for getStore method definition
rg -n "getStore\(" src/main/ -B 2 -A 5Repository: HagiCode-org/desktop
Length of output: 6927
🏁 Script executed:
#!/bin/bash
# Search for electron-store with encryptionKey
rg -n "encryptionKey|new Store" src/main/ -B 2 -A 2Repository: HagiCode-org/desktop
Length of output: 1350
🏁 Script executed:
#!/bin/bash
# Read the full Store initialization in config.ts
cat -n src/main/config.ts | head -100Repository: HagiCode-org/desktop
Length of output: 2785
Confirm API key encryption is disabled. Add encryptionKey to the Store initialization.
The Store in src/main/config.ts (lines 45-48) is initialized without an encryptionKey parameter, which means electron-store stores data in plaintext. Since this Store instance is passed directly to ClaudeConfigManager (src/main/main.ts:2184), API keys saved via this.store.set() (lines 525, 552) are NOT encrypted at rest, contradicting the JSDoc comment claiming encrypted storage.
Configure the Store with an encryption key in the main process initialization:
this.store = new Store<AppConfig>({
defaults: defaultConfig,
name: 'hagicode-desktop-config',
encryptionKey: // derive/load from secure source
});Also applies to: 551-553
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/claude-config-manager.ts` around lines 505 - 507, The Store is being
created without encryption, so API keys saved by ClaudeConfigManager via
this.store.set(...) are stored plaintext; update the Store initialization (the
Store<AppConfig> created and assigned to this.store that is passed into
ClaudeConfigManager) to include an encryptionKey parameter derived or loaded
from a secure source (e.g., OS keychain or a securely derived key) so that saved
values (used in ClaudeConfigManager methods that call this.store.set) are
encrypted at rest; ensure the encryptionKey is provided when constructing the
Store instance and that ClaudeConfigManager continues to use that same
this.store for reads/writes.
| try { | ||
| await fs.access(settingsPath); | ||
| } catch { | ||
| return ConfigSource.Unknown; | ||
| } |
There was a problem hiding this comment.
First-time config save is blocked by Unknown source preservation.
When settings.json does not exist, detectConfigSource() returns Unknown, and shouldPreserveExistingConfig() returns true. That path skips generateSettingsFile, so initial setup may never create Claude settings.
💡 Proposed fix
async shouldPreserveExistingConfig(): Promise<boolean> {
+ const settingsPath = this.getClaudeSettingsPath();
+ const exists = await fs.access(settingsPath).then(() => true).catch(() => false);
+ if (!exists) {
+ return false; // no file to preserve; allow first-time write
+ }
+
const source = await this.detectConfigSource();Also applies to: 917-919
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/claude-config-manager.ts` around lines 866 - 870, detectConfigSource
currently returns ConfigSource.Unknown when settings.json is missing, which
makes shouldPreserveExistingConfig treat a first-time setup as "preserve" and
skip generateSettingsFile; change detectConfigSource (the fs.access catch block
around settingsPath) to return a distinct missing value (e.g.,
ConfigSource.Missing or ConfigSource.None) instead of Unknown, and update
shouldPreserveExistingConfig to only return true for actual existing/explicit
sources (e.g., ConfigSource.File, ConfigSource.User, ConfigSource.Env) so that a
missing settings.json allows generateSettingsFile to run; reference functions:
detectConfigSource(), shouldPreserveExistingConfig(), generateSettingsFile(),
the settingsPath check and the ConfigSource enum.
| // Claude Config APIs | ||
| claudeDetect: () => ipcRenderer.invoke('claude:detect'), | ||
| claudeValidate: (provider: string, apiKey: string, endpoint?: string) => ipcRenderer.invoke('claude:validate', provider, apiKey, endpoint), | ||
| claudeVerifyCli: () => ipcRenderer.invoke('claude:verify-cli'), | ||
| claudeSave: (config: any) => ipcRenderer.invoke('claude:save', config), | ||
| claudeGetStored: () => ipcRenderer.invoke('claude:get-stored'), | ||
| claudeDelete: () => ipcRenderer.invoke('claude:delete'), | ||
| claudeTest: () => ipcRenderer.invoke('claude:test'), | ||
| claudeListBackups: () => ipcRenderer.invoke('claude:list-backups'), | ||
| claudeRestoreFromBackup: (backupPath: string) => ipcRenderer.invoke('claude:restore-backup', backupPath), | ||
|
|
||
| // Preset Management APIs | ||
| presetFetch: () => ipcRenderer.invoke('preset:fetch'), | ||
| presetRefresh: () => ipcRenderer.invoke('preset:refresh'), | ||
| presetClearCache: () => ipcRenderer.invoke('preset:clear-cache'), | ||
| presetGetProvider: (providerId: string) => ipcRenderer.invoke('preset:get-provider', providerId), | ||
| presetGetAllProviders: () => ipcRenderer.invoke('preset:get-all-providers'), | ||
| presetGetCacheStats: () => ipcRenderer.invoke('preset:get-cache-stats'), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the restoreFromBackup implementation in the main process
rg -n -A 20 'restoreFromBackup' --type=ts -g '!**/preload/**' -g '!**/node_modules/**'Repository: HagiCode-org/desktop
Length of output: 2785
🏁 Script executed:
#!/bin/bash
# Get more context on the restoreFromBackup implementation
sed -n '806,870p' src/main/claude-config-manager.tsRepository: HagiCode-org/desktop
Length of output: 1886
claudeRestoreFromBackup — fix required
The restoreFromBackup handler in ClaudeConfigManager does not validate that backupPath is within an intended backup directory. While it checks file existence and JSON validity, it accepts any arbitrary file path from the renderer process. A malicious renderer could read or restore arbitrary config files from anywhere on the filesystem, as long as they contain valid JSON with env or providers fields.
Restrict the backup path by resolving it and ensuring it's within a designated backup directory:
const backupDir = path.join(this.appDataPath, 'backups');
const resolvedPath = path.resolve(backupPath);
if (!resolvedPath.startsWith(path.resolve(backupDir))) {
return { success: false, error: 'Invalid backup path' };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/preload/index.ts` around lines 289 - 306, The claudeRestoreFromBackup IPC
handler (claudeRestoreFromBackup) in ClaudeConfigManager accepts arbitrary file
paths from the renderer; change the restoreFromBackup implementation to restrict
restores to the app's designated backup directory by resolving the incoming
backupPath (use path.resolve) and comparing it against the resolved backup
directory (e.g., path.join(this.appDataPath, 'backups'))—if the resolved
backupPath is not inside that directory reject with an error (do not proceed
with file read/JSON restore); use a safe containment check (startsWith on
resolved paths or path.relative) and keep existing existence/JSON validation
after this containment check.
| const getStepIndex = () => { | ||
| switch (currentStep) { | ||
| case 'idle': | ||
| case 'preparing': | ||
| return 0; | ||
| case 'calling-api': | ||
| return 1; | ||
| case 'confirmation': | ||
| return 2; | ||
| case 'completed': | ||
| case 'installing': | ||
| return 3; | ||
| case 'error': | ||
| return getStepIndex(); | ||
| default: | ||
| return 0; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Infinite recursion: getStepIndex() calls itself when currentStep === 'error'.
Line 106 recurses unconditionally with no state change, causing a stack overflow crash whenever currentStep is 'error'. This will freeze/crash the UI.
The intent was likely to return the index of the step that was active before the error occurred, but the current code has no mechanism for that. A simple fix is to return a sensible default.
Proposed fix
case 'error':
- return getStepIndex();
+ return 0; // Fall back to the preparing step on error
default:📝 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 getStepIndex = () => { | |
| switch (currentStep) { | |
| case 'idle': | |
| case 'preparing': | |
| return 0; | |
| case 'calling-api': | |
| return 1; | |
| case 'confirmation': | |
| return 2; | |
| case 'completed': | |
| case 'installing': | |
| return 3; | |
| case 'error': | |
| return getStepIndex(); | |
| default: | |
| return 0; | |
| } | |
| }; | |
| const getStepIndex = () => { | |
| switch (currentStep) { | |
| case 'idle': | |
| case 'preparing': | |
| return 0; | |
| case 'calling-api': | |
| return 1; | |
| case 'confirmation': | |
| return 2; | |
| case 'completed': | |
| case 'installing': | |
| return 3; | |
| case 'error': | |
| return 0; // Fall back to the preparing step on error | |
| default: | |
| return 0; | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/installation-wizard/InstallationWizard.tsx` around
lines 93 - 110, The getStepIndex function currently recurses when currentStep
=== 'error'; replace that branch with a non-recursive fallback: either return a
sensible default index (e.g., 0 or 3) or map 'error' to a stored
previousStep/lastStep state (if you have a previousStep variable) and return its
corresponding index; update the switch in getStepIndex to use this fallback
instead of calling getStepIndex() to avoid infinite recursion.
| return ( | ||
| <div className="user-confirmation"> | ||
| <h3>Confirmation</h3> | ||
| <div className="status-message"> | ||
| <p>{statusMessage}</p> | ||
| </div> | ||
|
|
||
| <div className="actions"> | ||
| <button type="button" className="btn-secondary" onClick={onRetry} disabled={isLoading}> | ||
| Re-check | ||
| </button> | ||
| <button type="button" className="btn-primary" onClick={onConfirm} disabled={isLoading}> | ||
| Complete | ||
| </button> | ||
| </div> | ||
|
|
||
| <div className="cancel-link"> | ||
| <button type="button" className="btn-link" onClick={onCancel} disabled={isLoading}> | ||
| Cancel | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if a stylesheet defining btn-primary, btn-secondary, user-confirmation exists
rg -l "btn-primary|btn-secondary|user-confirmation" --type=css --type=scssRepository: HagiCode-org/desktop
Length of output: 92
🏁 Script executed:
# Also check for the component file and any related stylesheets
fd "UserConfirmation" -e tsx -e ts -e css -e scssRepository: HagiCode-org/desktop
Length of output: 128
🏁 Script executed:
# Check the project structure for styling approach
ls -la src/renderer/components/installation-wizard/ | head -20Repository: HagiCode-org/desktop
Length of output: 505
🏁 Script executed:
# Search for any global stylesheets or CSS definitions
fd -e css -e scss | head -20Repository: HagiCode-org/desktop
Length of output: 86
🏁 Script executed:
# Check the main CSS file for class definitions
cat src/renderer/index.cssRepository: HagiCode-org/desktop
Length of output: 7175
🏁 Script executed:
# Check UserConfirmation.tsx for imports and structure
cat -n src/renderer/components/installation-wizard/UserConfirmation.tsxRepository: HagiCode-org/desktop
Length of output: 1527
🏁 Script executed:
# Search for any CSS or style files containing these button classes
rg "btn-primary|btn-secondary|btn-link|user-confirmation" -A 2Repository: HagiCode-org/desktop
Length of output: 3092
🏁 Script executed:
# Check if there are any style files in the component directory or parent directories
find src/renderer -name "*.css" -o -name "*.scss" -o -name "*.module.css"Repository: HagiCode-org/desktop
Length of output: 86
Custom CSS classes are undefined — components render unstyled.
The classes btn-primary, btn-secondary, btn-link, user-confirmation, status-message, actions, and cancel-link are not defined anywhere in the codebase. The project imports Tailwind CSS and shadcn/tailwind.css in src/renderer/index.css, but no stylesheet (custom or otherwise) defines these classes. As a result, the component renders structurally correct but with no visual styling.
This issue also affects InstallationWizard.tsx and ApiKeyInput.tsx, which use the same undefined classes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/installation-wizard/UserConfirmation.tsx` around
lines 21 - 43, The UI classes used in the UserConfirmation component
(user-confirmation, status-message, actions, cancel-link, btn-primary,
btn-secondary, btn-link) are undefined so the component renders unstyled; fix by
either adding CSS definitions for those class selectors into the renderer
stylesheet (e.g., src/renderer/index.css) or replacing those class names inside
the UserConfirmation component with equivalent Tailwind utility classes (and
apply the same change or add the corresponding CSS for InstallationWizard and
ApiKeyInput to keep styling consistent), ensuring you update the JSX in
UserConfirmation (and the components InstallationWizard and ApiKeyInput) to
reference the chosen approach.
| useEffect(() => { | ||
| const detectRegion = async () => { | ||
| setIsDetectingRegion(true); | ||
| try { | ||
| // Try to detect region by calling a region detection API | ||
| const result = await window.electronAPI.llmGetRegion?.(); | ||
| if (result?.success && result.region) { | ||
| setDetectedRegion(result.region as Region); | ||
| setSelectedRegion(result.region as Region); | ||
| } else { | ||
| // Default to CN if detection fails | ||
| setDetectedRegion('cn'); | ||
| setSelectedRegion('cn'); | ||
| } | ||
| } catch (err) { | ||
| // Default to CN on error | ||
| setDetectedRegion('cn'); | ||
| setSelectedRegion('cn'); | ||
| } finally { | ||
| setIsDetectingRegion(false); | ||
| } | ||
| }; | ||
| detectRegion(); | ||
| }, []); |
There was a problem hiding this comment.
Region auto-detection never works: response shape mismatch and value case mismatch.
Two issues prevent region detection from succeeding:
-
Missing
successfield: Thellm:get-regionIPC handler inmain.ts(Line 1383) returns{ region: 'CN' | 'INTERNATIONAL' }with nosuccessproperty. Line 61 checksresult?.success, which is alwaysundefined/falsy, so it always falls through to the default'cn'. -
Case mismatch: Even if fixed, the backend returns
'CN'/'INTERNATIONAL'while this component'sRegiontype expects'cn'/'international'.
Proposed fix
- const result = await window.electronAPI.llmGetRegion?.();
- if (result?.success && result.region) {
- setDetectedRegion(result.region as Region);
- setSelectedRegion(result.region as Region);
+ const result = await window.electronAPI.llmGetRegion?.();
+ if (result?.region) {
+ const mapped: Region = result.region === 'CN' ? 'cn' : 'international';
+ setDetectedRegion(mapped);
+ setSelectedRegion(mapped);
} else {🤖 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 55
- 78, Region auto-detection fails because the component checks a non-existent
result.success and doesn't normalize backend region casing; update the
useEffect/detectRegion logic that calls window.electronAPI.llmGetRegion to (1)
check for result?.region rather than result?.success, (2) map backend values
'CN' -> 'cn' and 'INTERNATIONAL' -> 'international' (or otherwise
lowercase/normalize the string) before calling setDetectedRegion and
setSelectedRegion, and (3) fall back to 'cn' if result?.region is missing or
unrecognized so Region type expectations (setDetectedRegion/setSelectedRegion)
are satisfied.
| {llmWizardOpen && ( | ||
| <InstallationWizard | ||
| manifestPath="/path/to/manifest.json" | ||
| onClose={() => setLlmWizardOpen(false)} | ||
| onComplete={() => setLlmWizardOpen(false)} |
There was a problem hiding this comment.
Hardcoded manifest path blocks the wizard flow.
Line 1011 passes "/path/to/manifest.json", so the wizard cannot operate on a real version manifest.
💡 Proposed fix
+ const [llmWizardManifestPath, setLlmWizardManifestPath] = useState<string | null>(null);
+
+ const openLlmWizard = async (versionId: string) => {
+ const res = await window.electronAPI.llmGetManifestPath(versionId);
+ if (!res.success || !res.manifestPath) {
+ toast.error(res.error || t('versionManagement.claude.manifestLoadFailed'));
+ return;
+ }
+ setLlmWizardManifestPath(res.manifestPath);
+ setLlmWizardOpen(true);
+ };- <button
- onClick={() => setLlmWizardOpen(true)}
+ <button
+ onClick={() => activeVersion && openLlmWizard(activeVersion.id)}- {llmWizardOpen && (
+ {llmWizardOpen && llmWizardManifestPath && (
<InstallationWizard
- manifestPath="/path/to/manifest.json"
+ manifestPath={llmWizardManifestPath}
onClose={() => setLlmWizardOpen(false)}
onComplete={() => setLlmWizardOpen(false)}
/>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/VersionManagementPage.tsx` around lines 1009 - 1013,
The InstallationWizard is being given a hardcoded manifestPath
("/path/to/manifest.json") which prevents real manifests from being used;
replace the literal with the real manifest source (for example a
selectedVersion.manifestPath, currentVersion.manifestPath, or a value from app
config/context) so InstallationWizard receives a valid path, and keep the
onClose/onComplete calls to setLlmWizardOpen(false) unchanged; update the JSX
where llmWizardOpen and InstallationWizard are used to pass the actual manifest
path variable instead of the hardcoded string.
| "testClaude": "Test Configuration", | ||
| "testClaude.subtext": "Open terminal and execute claude hi", |
There was a problem hiding this comment.
"testClaude.subtext" key is broken due to keySeparator: '.'.
Since keySeparator is set to '.' in config.ts, i18next interprets testClaude.subtext as a nested path (testClaude → subtext), but it's defined as a flat key here. The lookup will fail silently and return the raw key string.
Either nest it properly or rename to avoid the dot:
Option A: Nest the key
- "testClaude": "Test Configuration",
- "testClaude.subtext": "Open terminal and execute claude hi",
+ "testClaude": {
+ "label": "Test Configuration",
+ "subtext": "Open terminal and execute claude hi"
+ },Option B: Use a different separator (e.g., underscore)
- "testClaude.subtext": "Open terminal and execute claude hi",
+ "testClaude_subtext": "Open terminal and execute claude hi",#!/bin/bash
# Check how testClaude.subtext is referenced in components
rg -n "testClaude" --type=ts --type=tsx -g '!*.json'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/i18n/locales/en-US/claude.json` around lines 78 - 79, The JSON
key "testClaude.subtext" conflicts with i18next's keySeparator setting in
config.ts; replace it with a nested key or a dot-free key and update usage
sites: either change the locale to nested form ("testClaude": { "subtext": "Open
terminal and execute claude hi" }) or rename to a flat key without dots (e.g.,
"testClaude_subtext": "...") and then update all code references that call
t('testClaude.subtext') to the new path (t('testClaude.subtext') ->
t('testClaude.subtext') for nesting or t('testClaude_subtext') for rename) so
lookups resolve correctly.
| "claude": "Configure Claude Code", | ||
| "download": "Download the latest Hagicode Server package", | ||
| "dependencies": "Install required system dependencies", | ||
| "launch": "Start the service and verify it's running" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "welcome\.steps\.(claude|download|dependencies|llmInstallation|launch)" --type=ts --type=tsx srcRepository: HagiCode-org/desktop
Length of output: 91
🏁 Script executed:
rg -n "welcome\.steps\.(claude|download|dependencies|llmInstallation|launch)" src --type=tsRepository: HagiCode-org/desktop
Length of output: 404
🏁 Script executed:
rg -n "welcome\.steps" src -t tsRepository: HagiCode-org/desktop
Length of output: 404
🏁 Script executed:
sed -n '25,40p' src/renderer/components/onboarding/steps/WelcomeIntro.tsx | cat -nRepository: HagiCode-org/desktop
Length of output: 630
Remove unused "dependencies" translation key from line 24
The welcome.steps.dependencies key is not consumed by the onboarding flow. The WelcomeIntro component only references 3 steps (claude, download, launch); the dependencies key should be deleted. Note that welcome.features.dependencies (used elsewhere for the feature list) is separate and should remain.
🤖 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 22 - 25, Remove
the unused welcome.steps.dependencies key from the en-US onboarding JSON: edit
the onboarding.json locale (the object containing "welcome.steps") and delete
the "dependencies": "Install required system dependencies" entry so only
"claude", "download", and "launch" remain; do not change or remove
welcome.features.dependencies (used elsewhere) and ensure JSON remains valid for
the WelcomeIntro component to consume the three steps.
Summary by CodeRabbit
New Features
Improvements
Documentation