feat(polish): multi-provider UX improvements#49
Conversation
The model-name validator in POST /api/agents/:id/start rejected any model string containing brackets, so `sonnet[1m]` (and other 1M-context variants) returned "Invalid model name" and broke delegate_task from the MCP orchestrator. Widen the regex to include `[` and `]` — matching the charset claude-provider.ts already uses for interactive sessions — and short-circuit the `default` Dorothy alias so the --model flag is omitted and the CLI picks its own default. Also updates mcp-orchestrator tool descriptions for start_agent and delegate_task to list the full accepted model set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
During an active agent task, the terminal output view jumped away from the bottom and auto-scroll stopped working. The ResizeObserver and fullscreen effects in useAgentDialogTerminal called term.scrollToBottom() unconditionally after fitAddon.fit(), so any layout change (sidebar toggle, fullscreen, window resize) hijacked the viewport even when the user had scrolled up. Track user scroll position with an isAtBottomRef updated via term.onScroll (with a 2-line tolerance), and guard every post-fit scrollToBottom on that ref. Expose a scrollToBottom callback and an isAtBottom flag from the hook. AgentTerminalDialog now renders a ChevronDown "Scroll to bottom" button — styled to match the existing glass buttons — whenever the user is scrolled up; clicking it re-anchors and re-enables auto-scroll. Resets the lock to true on every new session so fresh dialogs still open anchored at the bottom. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three compounding issues in agent-routes caused the orchestrator to see "not connected" errors when delegating to idle agents: 1. /start never killed the previous PTY before spawning a new one. The claude CLI stays attached to the TTY after each task, so every /start leaked another live claude process. Eventually spawns failed silently, which the orchestrator experienced as "not connected". 2. /message's reconnection path called initAgentPtyCallback, which spawns a bare `bash -l` shell — not a claude session. Messages were written into bash and silently dropped, and because the exit handler in agent-manager never emitted agentStatusEmitter, /wait hung until timeout. 3. /start's onExit handler only transitioned `running` → completed/error. If the PTY died while status was `waiting`, the status stayed `waiting` forever and blocked every future /wait. Fix: /start now kills any existing PTY before spawning, and onExit also transitions `waiting` → error. /message auto-respawns a fresh one-shot PTY with the message as the prompt when none is alive, registering the same onData/onExit handlers (including agentStatusEmitter.emit) as /start. Adds three regression tests covering each scenario. All 870 tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix: accept 1M-context model variants in agent-routes
fix: anchor terminal output and add scroll-to-bottom button
fix: reliable orchestrator reconnection to idle agents
Agents with a worktreePath could still edit files in the main workspace because agent:update let users add a worktree *after* the PTY had already been spawned with cwd=projectPath. The running bash kept its original cwd, and /message reused that stale PTY without any check. Fix is structural: track ptyCwd as ground truth at every spawn site and call killStalePty() at every PTY-reuse site. Any future code path that mutates worktreePath is now forced to respawn cleanly. - agent-manager: export killStalePty(); clear ptyCwd in loadAgents; record ptyCwd in initAgentPty - ipc-handlers: killStalePty on agent:start; killStalePty after mutating worktreePath in agent:update; record ptyCwd in agent:create and local provider recreation - agent-routes /start: pass the *raw* worktree path to pty.spawn's cwd option (the shell-escaped form is only for the `cd '...'` command string) and record ptyCwd - agent-routes /message: killStalePty before reusing PTY; fix cwd option and record ptyCwd on reconnect - slack-bot, telegram-bot, main.ts kanban startAgent: killStalePty before PTY init for every entry point that reuses agent PTYs - tests: regression coverage in agent-routes.test.ts under "BUG 4" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Orchestrator Mode now blocks Edit/Write/MultiEdit/NotebookEdit via --disallowed-tools, plumbed from NewChatModal through IPC, HTTP, Telegram, and Slack spawn paths. Super Agent is implicitly treated as orchestrator. Bash stays available so orchestrators can run git/gh. - Bypass-mode agents no longer hit the workspace trust dialog on first launch. New ensureProjectTrusted helper pre-populates ~/.claude.json's hasTrustDialogAccepted before every pty.spawn (IPC create/start, HTTP /start + /message reconnect, initAgentPty). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds OpenRouter, DeepSeek, MiMo, Moonshot, Qwen, and ZhipuAI GLM as first-class providers. Each uses the existing claude binary and injects ANTHROPIC_BASE_URL + ANTHROPIC_API_KEY into the PTY at spawn time, preferring the direct provider endpoint when its key is set and falling back to OpenRouter otherwise. - Extend AgentProvider union and AppSettings with per-provider enabled/apiKey fields. - Extend CLIProvider.getPtyEnvVars signature with optional appSettings and thread it through agent-manager + ipc-handlers spawn sites. - Register the six providers in the provider registry. - Add Settings > AI Providers section with one card per provider (name, docs link, enabled toggle, API key input, model list). Direct endpoints: - OpenRouter https://openrouter.ai/api/v1 - DeepSeek https://api.deepseek.com/v1 - MiMo https://api.mimo.com/v1 - Moonshot https://api.moonshot.cn/v1 - Qwen https://dashscope.aliyuncs.com/compatible-mode/v1 - ZhipuAI GLM https://open.bigmodel.cn/api/paas/v4/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Create src/lib/providers.ts as the single frontend source of truth for all 11 AI providers (claude, codex, gemini, opencode, pi, openrouter, deepseek, moonshot, mimo, qwen, zhipu) with id, label, typed icon, accent colour, model list, and default model per provider. - NewChatModal/StepModel: replace 5-provider hardcoded grid with a registry-driven map; derive PROVIDER_MODELS and PROVIDER_DEFAULT_MODEL from registry; replace sentinel-string icon rendering with typed ProviderIcon component. - Settings/GeneralSection: replace 3-option hardcoded Default Provider select with a registry-driven map; uses requiresCli flag to show "(not installed)" only for CLI providers detected as absent. Adding a new provider in future requires a single entry in src/lib/providers.ts — no UI sites need to be touched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
agent-routes.ts (the one-shot API path used by delegate_task / /start / /message) was hardcoding 'claude' as the binary and omitting provider env vars. Gaps vs the interactive ipc-handlers.ts path: - CLI binary: hardcoded 'claude', ignored appSettings.cliPaths custom path - Provider env vars: ANTHROPIC_BASE_URL / ANTHROPIC_API_KEY never injected for OpenRouter / DeepSeek / MiMo / Moonshot / Qwen / ZhipuAI agents - MCP config: only passed to super-agent / automation agents; all other agents (including new provider workers) got no --mcp-config flag - Skills in /message reconnect: /start injects the skills prompt prefix; the auto-respawn reconnect path did not Fixes: - Resolve provider via getProvider(agent.provider) and call resolveBinaryPath(appSettings) for the correct binary in both /start and /message reconnect - Call getPtyEnvVars(..., appSettings) and spread into PTY env in both spawn sites — injects ANTHROPIC_BASE_URL + ANTHROPIC_API_KEY for alt providers, CLAUDE_* vars for all - Pass --mcp-config to all agents whose provider uses getMcpConfigStrategy() === 'flag' (all 11 registered providers) - Inject skills prompt prefix in /message reconnect path (matches /start) - Add getAppSettings mock to agent-routes.test.ts (RouteContext already declared it; mock was missing the method) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two hardcoded provider lists missed in the initial registry pass: - src/app/skills/page.tsx: availableProviders=['claude','codex','gemini'] → derived from PROVIDER_REGISTRY.filter(p => p.requiresCli) so all current and future CLI providers (claude, codex, gemini, opencode, pi) appear in the TerminalDialog "Install to:" selector. Env-injection providers (openrouter, deepseek, mimo, moonshot, qwen, zhipu) are correctly excluded — they share Claude's skill directory via ANTHROPIC_BASE_URL and need no separate install step. - src/components/NewChatModal/StepTools.tsx: PROVIDER_IDS constant → same requiresCli derivation; drives the installed-on badge row shown next to each skill in the NewChatModal skills browser. - src/components/ProviderBadge.tsx: extend PROVIDER_CONFIG with opencode (OC) and pi (π) entries so TerminalDialog can render their badge icons when they appear in the install selector. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sweep of all src/app/** and src/components/** files for hardcoded provider arrays and badge-color ternaries. Every finding replaced with PROVIDER_REGISTRY / getProviderDef() from @/lib/providers. Changes: - src/lib/providers.ts: add `badgeClass` field (full concrete Tailwind class string) to ProviderDef interface and all 11 provider entries, to avoid dynamic class generation that Tailwind JIT would purge - src/components/AgentList/AgentCard.tsx: remove PROVIDER_ICONS const (only had claude/codex/pi); add ProviderIcon component backed by getProviderDef(); covers all 11 registry providers - src/components/AgentList/AgentDetailPanel.tsx: replace codex/gemini badge-color ternary with getProviderDef(agent.provider)?.badgeClass - src/components/Settings/McpSection.tsx: replace hardcoded `type Provider = 'claude' | 'codex' | 'gemini'` with AgentProvider; derive PROVIDER_TABS from PROVIDER_REGISTRY.filter(requiresCli); rewrite ProviderIcon() to use registry icon descriptors; drop hand-coded GeminiSvg - src/app/memory/page.tsx: replace codex/gemini badge-color ternary with getProviderDef(project.provider)?.badgeClass - src/app/usage/page.tsx: add TODO noting usage tracking is Claude-native (useClaude() reads Claude Code JSONL files; other providers don't generate them) All 868 tests pass. tsc --noEmit and tsc -p electron/tsconfig.json --noEmit both clean on src/ files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…all 11 providers - Add CLAUDE_PROVIDER: this.id to getPtyEnvVars() in all providers so usage tracking can record which provider each session used - Add skills?: string[] to buildScheduledScript() interface and all implementations so scheduled tasks can inject skill directives into their prompts (same prefix as interactive sessions use) - Update cli-provider.ts interface accordingly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
automation-handlers: - Fix getDefaultProvider() to accept all 11 providers via isValidProvider() instead of ['claude', 'codex', 'gemini'] whitelist - Accept explicit agentProvider param in automation:create IPC - Use provider-aware binary path in createAutomationLaunchdJob (was already OK) scheduler-handlers: - Expose provider field in scheduler:listTasks response (all 3 push sites) - Accept explicit provider in scheduler:createTask config - Provider resolution: explicit config > agent provider > default > claude ipc-handlers: - Fix local/Tasmania provider branch to set CLAUDE_PROVIDER='local' instead of inheriting 'claude' from getPtyEnvVars - Extend tokenStats reader to compute providerTotals breakdown (per-provider: in, out, cost, sessions); legacy rows without provider field default to 'claude' slack-bot: - Fix hardcoded 'claude' command in Slack "start <agent>" handler — now uses getProvider(agent.provider).resolveBinaryPath() - Fix hardcoded 'claude' command in Super Agent Slack handler - Both handlers now also pass MCP config flag and skills prefix statusline.ts: - Add provider field to token-stats.json entries using $CLAUDE_PROVIDER env var (set by getPtyEnvVars); defaults to 'claude' for legacy rows Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Gap 1: Replace three hardcoded ProviderBadge checks (claude/codex/gemini) in skills page with a map over CLI_PROVIDER_IDS, so all 5 CLI providers (claude, codex, gemini, opencode, pi) render badges automatically. Gap 2: AIProvidersSection already covers all 6 env-injection providers (openrouter, deepseek, moonshot, mimo, qwen, zhipu) with API key inputs. Gap 3: CLIPathsSection already has inputs and auto-detect for all 5 CLI providers (claude, codex, gemini, opencode, pi). No backend-blocked items. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ider breakdown; fix AgentCard title prop Task A — Usage page per-provider breakdown: - Remove TODO comment from usage/page.tsx - Add providerTotals to TokenStats interface in useClaude.ts - Add defaultProvider to ClaudeSettings in claude-code.ts - Render "Usage by Provider" table: sessions/tokens-in/tokens-out/cost, sorted by cost desc then sessions; empty state for fresh installs - Use getProviderDef() for icon + label rendering (all 11 providers) Task B — Scheduled task creation: provider + skills picker: - Add provider/skills fields to ScheduledTask type, useTaskForm INITIAL_FORM, and the createTask IPC call in useTaskForm.ts - Add provider/skills params to electron.d.ts scheduler.createTask + listTasks - Add skills to SchedulerTaskMetadata; thread skills through createLaunchdJob / createCronJob into buildScheduledScript call in scheduler-handlers.ts - Add provider dropdown (all 11 PROVIDER_REGISTRY entries, defaults to defaultProvider from settings) + skills chip-picker to CreateTaskModal.tsx - Display provider badge + skill count on TaskCard.tsx rows Task C — Automation creation: provider + skills picker: - Add agentProvider/agentSkills fields to automation form state and reset block - Pass agentProvider + agentSkills to automation:create IPC - Update electron.d.ts automation.create params - Add agentSkills to Automation.agent type in automation-handlers.ts; thread skills into buildScheduledScript call via createAutomationLaunchdJob - Add provider dropdown + skills chip-picker to the agent-config section of the automations form; show provider badge + skill count on list rows Task D — AgentCard.tsx title prop typecheck: - The TS2322 errors on lines 18/24 were introduced by c7e45c9 (merged from feat/providers): title on <svg> and Lucide <Cpu> are not valid direct props - Fix: wrap both in <span title={label}> instead Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat: multi-provider support (11 AI CLIs/APIs) with full UI and settings integration
… and usage - Fix duplicate React key warnings by filtering .worktrees/ paths from project lists - Rename ZhipuAI to Zai in registry, settings, and UI - Add SVG provider logos for OpenRouter, DeepSeek, Moonshot, MiMo, Qwen, Zai - Block provider selection without API key; show tooltip in model picker and default provider - Replace fixed model grid with dynamic dropdown in NewChatModal - Usage page: update header for multi-provider, add provider pricing table, add per-provider breakdown - Settings: remove Quick Info (duplicate of System), move AI Providers below Terminal - Merge OpenCode/Pi Terminal pages into CLI Paths section - Update Git co-author label for all providers, update Permissions/Skills descriptions - Add Terminal scope clarification, add electron.d.ts types for provider API keys Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@JeanBrasse is attempting to deploy a commit to the Charlie Rabiller's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces multi-provider LLM support with stale PTY lifecycle management, orchestrator mode for tool restrictions, skill injection into scheduled tasks and automations, and comprehensive frontend configuration UI for managing external AI provider credentials and defaults. Changes
Sequence DiagramssequenceDiagram
participant Client
participant AgentRoutes as Agent Routes
participant AgentManager as Agent Manager
participant Provider
participant PTY as Node-PTY
participant FileSystem as File System
Client->>AgentRoutes: POST /api/agents/:id/start
AgentRoutes->>AgentManager: killStalePty(agent)
AgentManager->>FileSystem: Compare agent.ptyCwd vs expected cwd
alt PTY mismatch detected
AgentManager->>PTY: Kill stale PTY process
AgentManager->>AgentManager: Clear ptyCwd & ptyId
end
AgentRoutes->>AgentManager: ensureProjectTrusted(cwd)
AgentManager->>FileSystem: Mark ~/.claude.json project trust
AgentRoutes->>Provider: getProvider(agent.provider)
Provider->>AgentRoutes: Return provider instance
AgentRoutes->>Provider: resolveBinaryPath(appSettings)
Provider->>AgentRoutes: Return CLI binary path
AgentRoutes->>Provider: buildInteractiveCommand(params + orchestratorMode)
Provider->>AgentRoutes: Return command with tool restrictions if orchestrator
AgentRoutes->>Provider: getPtyEnvVars(agentId, projectPath, skills, appSettings)
Provider->>AgentRoutes: Return env vars including CLAUDE_PROVIDER
AgentRoutes->>PTY: spawn(cmd, cwd, env)
PTY->>AgentRoutes: PTY process created
AgentRoutes->>AgentManager: Store agent.ptyCwd = cwd
AgentRoutes->>Client: Return { success, agent }
sequenceDiagram
participant Client
participant AgentRoutes
participant AgentManager
participant Provider
participant Prompt as Prompt Builder
Client->>AgentRoutes: POST /api/agents/:id/message (PTY missing)
AgentRoutes->>AgentManager: killStalePty(agent)
AgentManager->>AgentManager: Detect & clear stale PTY
AgentRoutes->>Provider: getProvider(agent.provider)
Provider->>AgentRoutes: Provider instance
AgentRoutes->>Prompt: Build prompt with skills injection
alt skills present
Prompt->>Prompt: Prepend "IMPORTANT: Use these skills" + list
else no skills
Prompt->>Prompt: Use raw prompt
end
AgentRoutes->>Provider: buildInteractiveCommand(params)
Provider->>Prompt: Return command
AgentRoutes->>AgentRoutes: Spawn auto-reconnect one-shot PTY
AgentRoutes->>AgentManager: Update agent.ptyCwd & ptyId
AgentRoutes->>Client: Return { success }
sequenceDiagram
participant User
participant SchedulerUI as Scheduler UI
participant SchedulerHandler as Scheduler Handler
participant Provider
participant FileSystem
participant CronJob as Cron/Launchd
User->>SchedulerUI: Create scheduled task + provider + skills
SchedulerUI->>SchedulerHandler: scheduler:createTask(config)
SchedulerHandler->>Provider: getProvider(config.provider)
Provider->>SchedulerHandler: Provider instance
SchedulerHandler->>Provider: buildScheduledScript({prompt, skills, ...})
Provider->>Provider: Build prompt with skills prefix if present
Provider->>SchedulerHandler: Return bash script with skill-annotated prompt
SchedulerHandler->>FileSystem: Save skills + provider to SchedulerTaskMetadata
SchedulerHandler->>CronJob: Register scheduled job with script
CronJob->>User: Task scheduled with provider & skills
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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: 8
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 (10)
electron/services/telegram-bot.ts (1)
1181-1188:⚠️ Potential issue | 🟠 MajorDon't reuse the active-session path after killing a stale PTY.
Lines 1181-1188 can tear down and recreate the Super Agent PTY, but the next branch still trusts
superAgent.status. If that status is stillrunning/waiting, Line 1210 writes raw input into a fresh shell instead of launching a new provider session. Reset the session state whenkillStalePty(...)swaps the PTY, or force the new-session branch in that case.Also applies to: 1197-1210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/services/telegram-bot.ts` around lines 1181 - 1188, The code may kill and recreate a Super Agent PTY via killStalePty(superAgent) but then still trusts superAgent.status and may write raw input into a fresh shell; after killStalePty(superAgent) detect that the agent's pty was replaced (compare previous superAgent.ptyId vs current or check ptyProcesses) and clear or reset the session-related state on superAgent (e.g., set session id/state to null or a sentinel and set superAgent.status to a non-running value) so the subsequent logic (including the branch that writes to an existing session) will instead take the "create new provider session" path; apply the same reset when initAgentPty(superAgent) returns a new ptyId to ensure new PTY never receives raw input intended for the old session.electron/providers/index.ts (1)
24-35:⚠️ Potential issue | 🟠 MajorDon't silently downgrade unknown providers to Claude.
With several external providers now registered, Line 46 turns any typo/rename/config drift into “run it with Claude anyway”. That is the wrong failure mode here: it can hit the wrong account, wrong model, or wrong billing path and is hard to diagnose. Keep the Claude fallback only for
undefined/localand reject unsupported ids explicitly.🛡️ Suggested fix
export function getProvider(id?: AgentProvider): CLIProvider { if (!id || id === 'local') { return providers.claude; } - return providers[id] || providers.claude; + const provider = providers[id]; + if (!provider) { + throw new Error(`Unsupported provider: ${id}`); + } + return provider; }Also applies to: 42-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/index.ts` around lines 24 - 35, The providers map currently lists available providers (providers: Record<string, CLIProvider>) and the code falls back to new ClaudeProvider() for unknown ids; change the lookup logic so that only the specific cases 'undefined' or 'local' still map to Claude, but any other unrecognized provider id causes an explicit rejection/throw (or returns an error) instead of silently using Claude; update the resolution path that currently uses ClaudeProvider as a generic fallback to validate the id against the providers map and throw a clear error mentioning the unsupported provider id, referencing the providers constant and ClaudeProvider to locate the implementation.src/components/CanvasView/hooks/useAgentActions.ts (1)
4-14:⚠️ Potential issue | 🟠 MajorForward the selected provider/model into
createAgent.
handleCreateAgentacceptsmodel,_provider,_localModel, and_obsidianVaultPaths, but the payload sent tocreateAgent(...)drops all of them. Agents created from this flow will ignore the user's provider/local-model/vault choices and fall back to the default configuration.💡 Suggested fix
-import type { AgentCharacter, AgentStatus } from '@/types/electron'; +import type { AgentCharacter, AgentProvider, AgentStatus } from '@/types/electron'; interface CreateAgentConfig { projectPath: string; skills: string[]; worktree?: { enabled: boolean; branchName: string }; character?: AgentCharacter; name?: string; secondaryProjectPath?: string; permissionMode?: 'normal' | 'auto' | 'bypass'; effort?: 'low' | 'medium' | 'high'; + provider?: AgentProvider; + model?: string; + localModel?: string; + obsidianVaultPaths?: string[]; orchestratorMode?: boolean; }- _provider?: string, - _localModel?: string, - _obsidianVaultPaths?: string[], + provider?: AgentProvider, + localModel?: string, + obsidianVaultPaths?: string[], effort?: 'low' | 'medium' | 'high', orchestratorMode?: boolean, ) => { try { - const agent = await createAgent({ projectPath, skills, worktree, character, name, secondaryProjectPath, permissionMode, effort, orchestratorMode }); + const agent = await createAgent({ + projectPath, + skills, + worktree, + character, + name, + secondaryProjectPath, + permissionMode, + effort, + provider, + model, + localModel, + obsidianVaultPaths, + orchestratorMode, + });Also applies to: 19-20, 68-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CanvasView/hooks/useAgentActions.ts` around lines 4 - 14, The create flow currently drops the selected model/provider/localModel/obsidianVaultPaths when calling createAgent from handleCreateAgent, so forward the user's choices through the payload: ensure handleCreateAgent passes model, _provider, _localModel and _obsidianVaultPaths into the object sent to createAgent (and update createAgent's parameter shape if needed to accept provider/localModel/obsidianVaultPaths), and propagate these fields where agents are constructed so created agents respect the selected provider, model and vault settings (check occurrences around handleCreateAgent and createAgent to update all three call sites mentioned).src/components/NewChatModal/index.tsx (1)
339-397:⚠️ Potential issue | 🟠 Major
handleSubmitcloses over a staleisOrchestratorvalue.
isOrchestratoris used in both the update payload and theonSubmit(...)call, but it is missing from this callback's dependency list. Toggling orchestrator off does not change any listed dependency, so Save/Start can still submitorchestratorMode: true.💡 Suggested fix
- }, [projectPath, prompt, selectedSkills, useWorktree, branchName, showSecondaryProject, selectedSecondaryProject, customSecondaryPath, model, permissionMode, effort, provider, localModel, selectedObsidianVaults, onSubmit, isEditMode, editAgent, onUpdate, onClose]); + }, [projectPath, prompt, selectedSkills, useWorktree, branchName, showSecondaryProject, selectedSecondaryProject, customSecondaryPath, model, permissionMode, effort, provider, localModel, selectedObsidianVaults, onSubmit, isEditMode, editAgent, onUpdate, onClose, isOrchestrator]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NewChatModal/index.tsx` around lines 339 - 397, The handleSubmit callback is closing over a stale isOrchestrator value (used in onUpdate payload and the onSubmit(...) call); update the useCallback dependency list for handleSubmit to include isOrchestrator so the latest value is captured, ensuring orchestratorMode is correct when calling onUpdate and onSubmit (references: handleSubmit, isOrchestrator, onSubmit, onUpdate).electron/providers/gemini-provider.ts (1)
350-375:⚠️ Potential issue | 🔴 CriticalEscape
promptWithSkillsbefore embedding it in the shell script.Unlike
buildScheduledCommand(), this path interpolates raw prompt text inside single quotes. A prompt or skill name containing'will break the generated script, and crafted input can execute extra shell commands when a scheduled task runs.💡 Suggested fix
const promptWithSkills = (params.skills && params.skills.length > 0) ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` : params.prompt; + const escapedPrompt = promptWithSkills.replace(/'/g, `'"'"'`); return `#!/bin/bash @@ - "${params.binaryPath}" --output-format stream-json --debug --include-directories "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 + "${params.binaryPath}" --output-format stream-json --debug --include-directories "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1 echo "=== Task completed at $(date) ===" >> "${params.logPath}" `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/gemini-provider.ts` around lines 350 - 375, The generated shell script embeds promptWithSkills directly inside single quotes, allowing single quotes in the prompt to break the script and enable shell injection; fix by producing an escapedPrompt (escape single quotes using the standard shell-quote technique: replace ' with '\'' or use a safe sh-quoting helper) before building the template and interpolate escapedPrompt instead of raw promptWithSkills in the returned string in gemini-provider.ts (update the code that constructs promptWithSkills and the template line that currently uses "'${promptWithSkills}'" to use the escaped value).electron/providers/pi-provider.ts (1)
209-235:⚠️ Potential issue | 🟠 MajorDon't add unsupported, unescaped skill prompts to Pi scheduled jobs.
This path now prepends
/<skill-name>guidance even thoughPiProvider.supportsSkills()still reportsfalse. It also injects the combined prompt into a single-quoted shell argument without escaping, so apostrophes in the prompt or skill names will break the script.Suggested fix
- const promptWithSkills = (params.skills && params.skills.length > 0) + const promptWithSkills = (this.supportsSkills() && params.skills && params.skills.length > 0) ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` : params.prompt; + const escapedPromptWithSkills = promptWithSkills.replace(/'/g, "'\\''"); @@ -"${params.binaryPath}" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 +"${params.binaryPath}" -p '${escapedPromptWithSkills}' >> "${params.logPath}" 2>&1 echo "=== Task completed at $(date) ===" >> "${params.logPath}" `; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/pi-provider.ts` around lines 209 - 235, The generated scheduled-job script wrongly injects an unescaped skill hint even when PiProvider.supportsSkills() is false and places the combined prompt into a single-quoted shell argument (breaking on apostrophes); fix by only prepending the skills hint when PiProvider.supportsSkills() returns true and by safely escaping or quoting the prompt when passing it to the CLI (avoid single-quoted literal with embedded apostrophes) — update the code that builds promptWithSkills (referencing params and promptWithSkills) to check PiProvider.supportsSkills() before adding the skills string and change the invocation that uses "${params.binaryPath}" -p '${promptWithSkills}' to emit a safely escaped argument (e.g., use a here-doc, printf %q equivalent, or properly escape single quotes) so prompt and skill names with apostrophes do not break the shell.src/components/Settings/GeneralSection.tsx (1)
48-68:⚠️ Potential issue | 🟠 MajorThe provider availability list here never stays in sync with the current settings state.
It is populated once from
appSettings.get()and then cached, while the dropdown only disables entries whose value is strictlyfalse. That means new providers are briefly selectable before detection finishes, and API-key changes in the same settings session will not re-enable/disable options.Also applies to: 344-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/GeneralSection.tsx` around lines 48 - 68, The installedProviders state is only populated once from appSettings.get() on mount, causing transient selectable providers and no updates when settings change; fix by creating a reusable updateInstalledProviders function that calls Promise.all([window.electronAPI?.cliPaths?.detect(), window.electronAPI?.appSettings?.get()]) and calls setInstalledProviders(...) with explicit booleans (defaulting to false) and then (1) call that function on mount, (2) initialize installedProviders to all false to avoid transient enabled UI, and (3) subscribe to settings changes (or add a useEffect that depends on your settings state / use window.electronAPI?.appSettings?.onChange if available) to re-run updateInstalledProviders whenever app settings change so the dropdown stays in sync (refer to setInstalledProviders and the Promise.all detection block in the current code).electron/providers/codex-provider.ts (1)
279-306:⚠️ Potential issue | 🟠 MajorEscape
promptWithSkillsbefore embedding it in the shell script.This now splices raw prompt text and skill names into a single-quoted Bash argument. Any apostrophe in either value will terminate the string and make the scheduled run fail.
Suggested fix
buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; @@ }): string { const flags = params.autonomous ? '--full-auto' : ''; const promptWithSkills = (params.skills && params.skills.length > 0) ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` : params.prompt; + const escapedPromptWithSkills = promptWithSkills.replace(/'/g, "'\\''"); @@ -"${params.binaryPath}" ${flags} --json exec '${promptWithSkills}' >> "${params.logPath}" 2>&1 +"${params.binaryPath}" ${flags} --json exec '${escapedPromptWithSkills}' >> "${params.logPath}" 2>&1 echo "=== Task completed at $(date) ===" >> "${params.logPath}" `; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/codex-provider.ts` around lines 279 - 306, The embedded promptWithSkills is injected into a single-quoted shell argument and can break the script if it contains apostrophes; update the code that builds promptWithSkills (the variable derived from params.prompt and params.skills) to escape single quotes before embedding (e.g. replace every ' with '\'' or implement a small shellEscape helper used where promptWithSkills is interpolated), then use the escaped value in the exec line ("${params.binaryPath}" ${flags} --json exec '<escapedPromptWithSkills>') so the generated bash stays syntactically valid.electron/providers/opencode-provider.ts (1)
210-237:⚠️ Potential issue | 🔴 CriticalShell-escape
promptWithSkillsbefore callingopencode run.The new skills prefix is concatenated into a single-quoted shell argument without escaping the skill names first. If any selected skill contains
', the generated script becomes invalid and can escape the quoted prompt at runtime. Build the full prompt from raw text, then escape it once before writing the command line.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/opencode-provider.ts` around lines 210 - 237, The generated script inserts promptWithSkills directly into a single-quoted shell argument which breaks if a skill contains a single quote; fix by building the full prompt (promptWithSkills) and then shell-escaping it before writing the command line — implement an escaping helper (e.g., escapeShellArg or inline replace to turn each ' into '\'' and wrap the result in single quotes) and use that escaped value in the command invocation where "${params.binaryPath}" run '...'; update the template to use escapedPrompt instead of raw promptWithSkills so the produced script is safe for any skill text.electron/providers/claude-provider.ts (1)
403-431:⚠️ Potential issue | 🔴 CriticalEscape the skills directive before embedding it in the bash script.
params.promptis already shell-quoted upstream, butparams.skills.join(', ')is interpolated raw into the same single-quoted-pargument here. A skill name containing'breaks the script and can escape the quoted prompt when the automation runs. Compose the full prompt from raw text and escape it once at this boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/claude-provider.ts` around lines 403 - 431, The embedded skills string can break the single-quoted -p argument if a skill contains a single quote; update the prompt composition so you build the full raw prompt (use params.prompt and params.skills to form fullPrompt instead of promptWithSkills) and then shell-escape that fullPrompt before inserting it into the bash script. Add or use a helper (e.g., shellEscape or escapeForSingleQuotes) and apply it to fullPrompt prior to the -p substitution (the location currently using -p '${promptWithSkills}'), ensuring single quotes inside skills are transformed into the safe sequence for single-quoted shell literals.
🟠 Major comments (14)
src/hooks/useClaude.ts-29-29 (1)
29-29:⚠️ Potential issue | 🟠 Major
providerTotalscan stay stale due to missing change detection insetData.You added the new field to
TokenStats, but the memo-like comparison infetchDatadoesn’t considertokenStats, so provider usage updates can be dropped when project/session counts are unchanged.Proposed fix
const rlChanged = JSON.stringify(prev.rateLimits) !== JSON.stringify(newData.rateLimits); if (rlChanged) return newData; + const tokenStatsChanged = + JSON.stringify(prev.tokenStats) !== JSON.stringify(newData.tokenStats); + if (tokenStatsChanged) return newData; // No significant changes return prev;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useClaude.ts` at line 29, The memo-like comparison in fetchData is ignoring changes in TokenStats.providerTotals so updates get dropped; update the comparison logic used before calling setData (in fetchData) to include tokenStats (or specifically tokenStats.providerTotals) in the equality check or perform a deep-compare of providerTotals, and then call setData when providerTotals differ; alternatively remove the memo gate for tokenStats and always merge/update tokenStats into the state in setData (referencing TokenStats, providerTotals, fetchData, and setData) so provider usage changes are never silently discarded.src/components/KanbanBoard/components/NewTaskModal.tsx-86-88 (1)
86-88:⚠️ Potential issue | 🟠 MajorWorktree filter is not cross-platform (
/only).This misses Windows-style paths (
\...\.worktrees\...), so duplicate project entries can still leak through there.Proposed fix
- const projectList = rawProjects.filter((p: Project) => !p.path.includes('/.worktrees/')); + const worktreePattern = /[\\/]\.worktrees[\\/]/; + const projectList = rawProjects.filter((p: Project) => !worktreePattern.test(p.path));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/KanbanBoard/components/NewTaskModal.tsx` around lines 86 - 88, The current filter in NewTaskModal.tsx uses p.path.includes('/.worktrees/') which only matches POSIX separators and misses Windows backslashes; update the filter for rawProjects -> projectList to detect ".worktrees" cross-platform by normalizing the path or using a robust check (e.g. import Node's path and call path.normalize(p.path) and then check for `${path.sep}.worktrees${path.sep}` or split by path.sep and test for a ".worktrees" segment, or use a regex like /[\/\\]\.worktrees[\/\\]/) so duplicate entries are reliably excluded; apply this change to the projectList construction that filters Project objects.src/app/recurring-tasks/hooks/useTaskForm.ts-21-22 (1)
21-22: 🛠️ Refactor suggestion | 🟠 MajorMake
defaultProviderpart of the hook contract.
useTaskFormis still the source of truth for what gets submitted, but it initializes and resetsproviderto'claude', and Line 83 always serializes that state. That leaves the page-leveldefaultProviderworking only if the modal imperatively syncs the hook on every open/reset. Seed and reset the hook from the configured default instead.♻️ Suggested refactor
-const INITIAL_FORM = { +const createInitialForm = (defaultProvider: string) => ({ agentId: '', projectPath: '', title: '', prompt: '', schedulePreset: 'daily', customCron: '', time: '09:00', days: ['1', '2', '3', '4', '5'], intervalDays: 2, selectedDays: ['1'] as string[], autonomous: true, useWorktree: false, notifyTelegram: false, notifySlack: false, - provider: 'claude', + provider: defaultProvider, skills: [] as string[], -}; +}); export function useTaskForm( agents: Agent[], loadTasks: () => Promise<void>, showToast: (msg: string, type: 'success' | 'error' | 'info') => void, + defaultProvider = 'claude', ) { const [showCreateForm, setShowCreateForm] = useState(false); - const [formData, setFormData] = useState(INITIAL_FORM); + const [formData, setFormData] = useState(() => createInitialForm(defaultProvider)); @@ - setFormData(INITIAL_FORM); + setFormData(createInitialForm(defaultProvider));- const taskForm = useTaskForm(scheduled.agents, scheduled.loadTasks, showToast); + const taskForm = useTaskForm(scheduled.agents, scheduled.loadTasks, showToast, defaultProvider);Also applies to: 30-31, 77-84, 93-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/recurring-tasks/hooks/useTaskForm.ts` around lines 21 - 22, The hook useTaskForm currently hardcodes provider to 'claude' and always serializes that value; change it so defaultProvider is accepted and used as part of the hook's contract: add defaultProvider to the hook parameters signature, initialize provider state from that default (instead of the fixed 'claude'), ensure reset/initialize logic in the hook (and any serialize/submit function referenced in useTaskForm) resets provider back to the passed defaultProvider, and replace any hardcoded 'claude' occurrences (including places around provider initialization, reset, and serialization) with the hook's provider state so the page-level defaultProvider drives the form value.src/hooks/useElectron.ts-295-297 (1)
295-297:⚠️ Potential issue | 🟠 MajorMake the worktree filter path-separator agnostic.
p.path.includes('/.worktrees/')only catches POSIX-style paths. On Windows, worktree paths are typically...\ .worktrees\..., so those entries will still leak into the list and the duplicate-key issue this PR is fixing will persist there.💡 Suggested fix
- // Filter out worktree paths to avoid duplicate React keys - setProjects(list.filter((p: { path: string }) => !p.path.includes('/.worktrees/'))); + // Filter out worktree paths to avoid duplicate React keys + setProjects(list.filter((p: { path: string }) => !/[\\/]\.worktrees[\\/]/.test(p.path)));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useElectron.ts` around lines 295 - 297, The filter currently only detects POSIX separators (p.path.includes('/.worktrees/')), so Windows paths with backslashes slip through; update the predicate used in setProjects to detect worktree segments with either separator (e.g. test p.path against a regexp like /[\/\\]\.worktrees[\/\\]/ or normalize separators before checking) so entries containing ".worktrees" with either '/' or '\' are excluded; change the filtering logic near window.electronAPI!.fs.listProjects() and the setProjects call to use the new check on p.path.src/app/automations/page.tsx-242-243 (1)
242-243:⚠️ Potential issue | 🟠 MajorBlock unavailable providers in the automation form.
This form exposes every registry entry and forwards the chosen/default provider straight into the create payload without checking whether its CLI or API key is actually available. That lets users save automations that only fail later when the job starts.
Also applies to: 1018-1028
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/automations/page.tsx` around lines 242 - 243, The form currently passes formData.agentProvider (or defaultProvider) and agentSkills into the create payload without validating availability, which allows saving automations that will later fail; update the form submit/validation logic (where agentProvider/defaultProvider and agentSkills are used) to verify the chosen provider has required credentials (CLI or API key) available in the registry before allowing selection or submission, block/disable unavailable providers in the provider selection UI, and return a validation error if an unavailable provider is chosen so the create payload never includes a provider lacking credentials.electron/services/slack-bot.ts-460-481 (1)
460-481:⚠️ Potential issue | 🟠 MajorUse the provider command builders here instead of a Claude-shaped command template.
These branches resolve a provider-specific binary, but they still append Claude-only flags and argument ordering. The Codex and Pi providers in this PR use different command shapes, so Slack-triggered launches will break as soon as the agent is not Claude.
Also applies to: 595-621
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/services/slack-bot.ts` around lines 460 - 481, Replace the manual Claude-shaped command string assembly around slackAgentProvider/resolveBinaryPath with the provider-specific command builder on slackAgentProvider (e.g., a buildCommand/buildLaunchCommand method); gather the same inputs you currently compute (binaryPath, skipPermissions flag derived from agent.permissionMode or agent.skipPermissions, mcpConfigPath when slackAgentProvider.getMcpConfigStrategy() === 'flag' and file exists, agent.secondaryProjectPath and the default homedir /.dorothy add-dir, disallowed tools when isSuperAgent(agent) || agent.orchestratorMode, and the session prompt which should include agent.skills when present and not isSuperAgent); pass these as structured options into the provider builder so quoting/flag ordering is handled by the provider implementation instead of concatenating Claude-only flags in this file.src/app/recurring-tasks/components/CreateTaskModal.tsx-35-36 (1)
35-36:⚠️ Potential issue | 🟠 MajorThis modal bypasses the new provider-availability guardrails.
It renders every
PROVIDER_REGISTRYentry with no disabled state, so recurring tasks can still be created for providers whose CLI or API key is missing. Pass the same availability map used byStepModeland disable or filter unsupported options here.Also applies to: 166-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/recurring-tasks/components/CreateTaskModal.tsx` around lines 35 - 36, The modal currently renders every PROVIDER_REGISTRY entry with no disabled state; update CreateTaskModal (and the rendering block around the later lines noted) to accept and use the same provider availability map that StepModel uses (e.g., pass in the availability map prop from the parent or reuse the existing availability prop) and either filter out unsupported providers or render them disabled based on that map; specifically, when iterating PROVIDER_REGISTRY to build options in CreateTaskModal, consult the availability lookup (the same shape StepModel uses) and set disabled=true or omit the option for providers marked unavailable to prevent creating recurring tasks for providers missing CLI/API keys.src/components/Settings/constants.ts-77-91 (1)
77-91:⚠️ Potential issue | 🟠 MajorAdd
piEnabledto the shared settings defaults.
CLIPathsSectionnow reads and saves this flag, butDEFAULT_APP_SETTINGSnever initializes it. Fresh settings objects therefore start with an implicitundefinedstate, and the UI has to cast around the missing field instead of reading a typed default. Mirror the field in the shared settings type as well.Suggested fix
opencodeEnabled: false, + piEnabled: false, opencodeDefaultModel: '',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/constants.ts` around lines 77 - 91, DEFAULT_APP_SETTINGS is missing the piEnabled boolean default which causes fresh settings to have undefined for that flag; update DEFAULT_APP_SETTINGS to include piEnabled: false and ensure the shared settings type (e.g., the AppSettings/shared settings interface used by CLIPathsSection) includes piEnabled: boolean so the initializer and type definitions match; locate DEFAULT_APP_SETTINGS in src/components/Settings/constants.ts and add the piEnabled entry alongside other provider flags and update the shared settings type declaration to include piEnabled with type boolean.src/components/Settings/CLIPathsSection.tsx-198-207 (1)
198-207:⚠️ Potential issue | 🟠 MajorTest the path the user is editing, not a different shell command.
The OpenCode button always runs
opencode --version, and the Pi button readsappSettings.cliPaths?.piinstead oflocalPaths.piwhile interpolating that path directly into a shell string. Users cannot validate unsaved edits, and paths with spaces or quotes will fail unexpectedly. Use the current input value and invoke the binary with args instead of shell concatenation.Also applies to: 242-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/CLIPathsSection.tsx` around lines 198 - 207, The test buttons are running a hardcoded shell invocation and/or reading saved appSettings instead of the current input state and are building shell strings that break on spaces/quotes; update the handlers (the OpenCode handler which uses setTestingOpencode and setOpencodeResult and the Pi handler that reads appSettings.cliPaths?.pi) to read the current input state (localPaths.opencode and localPaths.pi or the component state that backs the inputs) and call window.electronAPI.shell.exec with a command+args payload (e.g. { command: localPaths.opencode, args: ['--version'] }) rather than interpolating into one shell string; also keep the same success/error handling but ensure you pass the raw path value (fallback-guard it) and avoid using appSettings.cliPaths?.pi so unsaved edits can be tested.electron/services/api-routes/agent-routes.ts-234-244 (1)
234-244:⚠️ Potential issue | 🟠 MajorIgnore exit callbacks from superseded PTYs.
/startnow kills the previous PTY before spawning a new one, and/messagecan also replace a PTY after reconnect. Both delayedonExithandlers still mutateagent.statusunconditionally, so the old PTY can flip the new run toerror/completed~1.5s later.Suggested fix
ptyProcess.onExit(({ exitCode }) => { // Delay status change to let hooks (on-stop.sh, task-completed.sh) finish // capturing output before wait_for_agent resolves. setTimeout(() => { + if (agent.ptyId !== ptyId) return; if (agent.status === 'running') { agent.status = exitCode === 0 ? 'completed' : 'error'; } else if (agent.status === 'waiting') { // PTY exited while agent was waiting for input — the claude process // crashed. Mark as error so /wait is unblocked and the orchestrator // can retry rather than hanging until timeout. agent.status = 'error'; } if (exitCode !== 0) { agent.error = `Process exited with code ${exitCode}`; } agent.lastActivity = new Date().toISOString(); + agent.ptyId = undefined; + agent.ptyCwd = undefined; ptyProcesses.delete(ptyId); saveAgents(); ctx.agentStatusEmitter.emit(`status:${agent.id}`); }, 1500); });Apply the same guard/cleanup to the
reconnectPtyIdexit handler below as well.Also applies to: 297-301, 432-445
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/services/api-routes/agent-routes.ts` around lines 234 - 244, Delayed onExit handlers for superseded PTYs (the block that checks agent.ptyId and ptyProcesses) can still run and mutate agent.status for a newer PTY; update the exit handlers that reference reconnectPtyId (and any other PTY onExit callbacks around lines with reconnectPtyId and the 432-445 block) to early-return unless the PTY id that triggered the callback matches the current agent.ptyId (or reconnectPtyId as intended), and remove/cleanup the listener when killing/replacing a PTY; in practice, add a guard at the start of the onExit callbacks (compare the captured ptyId to agent.ptyId/reconnectPtyId) and only update agent.status or perform cleanup when they match, and ensure you delete the old pty from ptyProcesses when you kill it so delayed handlers can detect it was superseded.electron/handlers/scheduler-handlers.ts-881-891 (1)
881-891:⚠️ Potential issue | 🟠 MajorDon't let task edits strip the saved skills list.
createTask()now storesskills, but the update/regenerate path never readsmetadata[taskId].skillsback intobuildScheduledScript()or the recreated launchd/cron job. Any later edit will silently drop the skill preamble from future runs.Suggested fix
// Resolve provider from metadata const meta = loadSchedulerMetadata(); const taskProvider: AgentProvider = meta[taskId]?.provider || resolveDefaultProvider(deps); + const taskSkills = meta[taskId]?.skills; // Always regenerate the shell script using provider const claudePath = await getCLIPath(taskProvider); @@ const scriptContent = cliProvider.buildScheduledScript({ binaryPath: claudePath, binaryDir: claudeDir, projectPath, prompt: escapedPrompt, autonomous, mcpConfigPath, logPath, homeDir, + skills: taskSkills, }); @@ - await createLaunchdJob(taskId, schedule, projectPath, prompt, autonomous, taskProvider); + await createLaunchdJob(taskId, schedule, projectPath, prompt, autonomous, taskProvider, taskSkills); @@ - await createCronJob(taskId, schedule, projectPath, prompt, autonomous, taskProvider); + await createCronJob(taskId, schedule, projectPath, prompt, autonomous, taskProvider, taskSkills);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/handlers/scheduler-handlers.ts` around lines 881 - 891, The update/regenerate path is not preserving the saved skills because the code that recreates jobs doesn't read metadata[taskId].skills; update the regenerate flow to load the scheduler metadata (from where saveSchedulerMetadata writes), read metadata[taskId].skills (falling back to config.skills or empty array), and pass that skills array into buildScheduledScript and the job-creation calls (createLaunchdJob and createCronJob) so the recreated job and script include the original skill preamble.electron/handlers/scheduler-handlers.ts-869-872 (1)
869-872:⚠️ Potential issue | 🟠 MajorKeep legacy scheduled tasks on Claude when metadata is missing.
Now that new tasks persist
provider, the update flow below still falls back toresolveDefaultProvider(deps)for older tasks with no stored provider. Editing a pre-provider task after changing Settings can silently switch its execution/auth provider.Suggested fix
- const taskProvider: AgentProvider = meta[taskId]?.provider || resolveDefaultProvider(deps); + const taskProvider: AgentProvider = meta[taskId]?.provider || 'claude';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/handlers/scheduler-handlers.ts` around lines 869 - 872, The current fallback uses resolveDefaultProvider(deps) which causes legacy tasks lacking a stored provider to unintentionally pick up the current app default; update the provider resolution in the task creation/update path so it checks config.provider, then deps.agents.get(config.agentId)?.provider, then any stored provider on the task (e.g., config.metadata?.provider or the legacy field that held provider), and only if none of those exist fall back to the literal 'claude' (keep references to AgentProvider, taskProvider, config.provider, deps.agents.get, and resolveDefaultProvider when locating the code to change).electron/services/api-routes/agent-routes.ts-417-422 (1)
417-422:⚠️ Potential issue | 🟠 MajorReset stale transcript/error state before auto-respawn.
This branch starts a fresh one-shot session, but it keeps
agent.output,lastCleanOutput, anderrorfrom the previous crashed run. A successful retry can therefore still surface the old error and interleave transcripts.Suggested fix
ptyProcesses.set(reconnectPtyId, reconnectPty); agent.ptyId = reconnectPtyId; agent.ptyCwd = rawWorkingDir; agent.status = 'running'; agent.currentTask = message.slice(0, 100); + agent.output = []; + agent.lastCleanOutput = undefined; + agent.error = undefined; agent.lastActivity = new Date().toISOString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/services/api-routes/agent-routes.ts` around lines 417 - 422, Reset the agent's previous run state before starting the one-shot respawn: clear agent.output and agent.lastCleanOutput (set to empty strings) and clear agent.error (set to null/undefined) in the same branch where you set agent.ptyId, agent.ptyCwd, agent.status, and agent.currentTask so stale transcripts or errors from the previous crashed run cannot surface or interleave with the new session.src/types/electron.d.ts-352-365 (1)
352-365:⚠️ Potential issue | 🟠 MajorMirror the new provider fields in
appSettings.save().
appSettings.get()now exposesopencode*/openRouter*/deepSeek*/mimo*/moonshot*/qwen*/zhipu*, but thesave()signature below still rejects them. The new Settings UI will otherwise need type escapes to persist its own fields.Suggested fix
save: (settings: { notificationsEnabled?: boolean; notifyOnWaiting?: boolean; notifyOnComplete?: boolean; notifyOnStop?: boolean; notifyOnError?: boolean; telegramEnabled?: boolean; telegramBotToken?: string; telegramChatId?: string; telegramAuthToken?: string; telegramAuthorizedChatIds?: string[]; telegramRequireMention?: boolean; slackEnabled?: boolean; slackBotToken?: string; slackAppToken?: string; slackSigningSecret?: string; slackChannelId?: string; jiraEnabled?: boolean; jiraDomain?: string; jiraEmail?: string; jiraApiToken?: string; socialDataEnabled?: boolean; socialDataApiKey?: string; tasmaniaEnabled?: boolean; tasmaniaServerPath?: string; defaultProvider?: string; + opencodeEnabled?: boolean; + opencodeDefaultModel?: string; + openRouterEnabled?: boolean; + openRouterApiKey?: string; + deepSeekEnabled?: boolean; + deepSeekApiKey?: string; + mimoEnabled?: boolean; + mimoApiKey?: string; + moonshotEnabled?: boolean; + moonshotApiKey?: string; + qwenEnabled?: boolean; + qwenApiKey?: string; + zhipuEnabled?: boolean; + zhipuApiKey?: string; notificationSounds?: { waiting?: string; complete?: string; stop?: string; error?: string; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/electron.d.ts` around lines 352 - 365, The appSettings.save() signature does not include the new provider fields added to the Settings type (opencodeEnabled, opencodeDefaultModel, openRouterEnabled, openRouterApiKey, deepSeekEnabled, deepSeekApiKey, mimoEnabled, mimoApiKey, moonshotEnabled, moonshotApiKey, qwenEnabled, qwenApiKey, zhipuEnabled, zhipuApiKey), causing saves from the new Settings UI to be rejected; update the parameter type (or SaveSettings/interface used by the appSettings.save method) to include these optional properties with the same types as declared in electron.d.ts so appSettings.save accepts and persists those fields, then ensure any runtime serialization logic inside the save implementation handles/passes through these new fields unchanged.
🟡 Minor comments (12)
mcp-orchestrator/src/tools/agents.ts-181-181 (1)
181-181:⚠️ Potential issue | 🟡 MinorExtract the duplicated model description to a shared constant.
The identical model description at lines 181 and 472 should be stored as a constant to prevent drift when Claude models change. However, verify that the aliases 'opusplan' and 'sonnet[1m]' are current—the April 2026 API docs confirm 'sonnet', 'opus', 'haiku', and the listed full IDs (claude-sonnet-4-6, claude-opus-4-6, claude-haiku-4-5-20251001) are valid, but 'opusplan' and the '[1m]' notation do not appear in official Anthropic documentation. Also note that other files in this codebase define model enums as
'sonnet' | 'opus' | 'haiku'without 'opusplan', which suggests possible inconsistency across the tool definitions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-orchestrator/src/tools/agents.ts` at line 181, The model description string used in the zod schema (the value passed to model: z.string().optional().describe(...)) is duplicated (at the current occurrences around the model field in agents.ts and again later) and should be extracted to a single exported constant (e.g., CLAUDE_MODEL_DESCRIPTION) so both schema calls reference the same value; update the z.string().optional().describe(...) invocations in the functions/objects that reference that field to use the constant. While extracting, reconcile the alias list to match the codebase canonical enum ('sonnet' | 'opus' | 'haiku') by removing or flagging nonstandard entries ('opusplan' and 'sonnet[1m]') unless you confirm they’re intentionally supported, and ensure the constant’s text matches the authoritative API docs so future changes only need one edit.src/app/recurring-tasks/components/TaskCard.tsx-90-100 (1)
90-100:⚠️ Potential issue | 🟡 MinorKeep a visible fallback when provider lookup misses.
if (!def) return nullhides the provider entirely. With new providers and rename churn in this PR, that makes registry drift much harder to spot in the task list. Falling back totask.provideras plain text keeps the UI debuggable.💡 Suggested fix
{task.provider && task.provider !== 'claude' && (() => { const def = getProviderDef(task.provider); - if (!def) return null; + if (!def) { + return <div className="flex items-center gap-1">{task.provider}</div>; + } return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/recurring-tasks/components/TaskCard.tsx` around lines 90 - 100, The provider lookup in TaskCard (getProviderDef(task.provider)) returns null and currently causes the UI to hide the provider (`if (!def) return null`); change this to render a visible fallback that displays task.provider as plain text (e.g., a small label or span) so missing/renamed providers remain visible; keep the existing rendering logic for when def exists (icon branches: def.icon.type === 'image' | 'svg-gemini' | 'cpu' | 'text') and only use the fallback when def is falsy, referencing getProviderDef, task.provider, and the local def variable to locate the code to modify.src/components/AgentList/AgentCard.tsx-104-106 (1)
104-106:⚠️ Potential issue | 🟡 MinorDon't badge local agents as Claude.
When
agent.provider === 'local', this forces the header badge toclaude. The card then shows a Claude icon for a local-model agent, which is misleading. Passlocalthrough if the registry supports it, or omit the provider badge for local agents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/AgentList/AgentCard.tsx` around lines 104 - 106, The ProviderIcon call in AgentCard currently maps agent.provider === 'local' to 'claude', causing local agents to show a Claude badge; update the ProviderIcon invocation to pass agent.provider through unchanged (i.e., pass 'local' when agent.provider === 'local') or pass undefined/null to omit the badge for local agents instead of forcing 'claude'—change the ProviderIcon prop usage in AgentCard (the ProviderIcon component invocation) so it receives the real agent.provider value or nothing for local agents.src/components/ProviderBadge.tsx-19-28 (1)
19-28:⚠️ Potential issue | 🟡 Minor
ProviderBadgeis already out of sync with the multi-provider registry.This local map still only covers
claude,codex,gemini,opencode, andpi, so anyopenrouter/deepseek/moonshot/mimo/qwen/zaibadge routed through this component will still hitif (!config) return nullat Line 37. Reusing the shared provider registry here would keep badge coverage in sync with the rest of the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ProviderBadge.tsx` around lines 19 - 28, The component defines a local PROVIDER_CONFIG map that is missing many providers and causes ProviderBadge to return null for providers like openrouter/deepseek/moonshot/mimo/qwen/zai; remove or stop using the local PROVIDER_CONFIG inside ProviderBadge and instead import and use the shared multi-provider registry (the central provider registry used by the app) to look up provider metadata inside ProviderBadge (use the same lookup call the rest of the UI uses), ensuring the lookup covers icon and label types expected by ProviderBadge and fall back gracefully if a provider entry lacks an icon; update references to PROVIDER_CONFIG in ProviderBadge to use the shared registry lookup so badge coverage stays in sync.src/components/AgentList/AgentCard.tsx-39-44 (1)
39-44:⚠️ Potential issue | 🟡 MinorThis fallback drops the actual SVG assets.
For any provider icon that isn't
image,svg-gemini,cpu, ortext, the card now renderslabel.slice(0, 2)instead of the real icon. That means the new SVG provider logos added in this PR won't actually appear on agent cards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/AgentList/AgentCard.tsx` around lines 39 - 44, The current fallback unconditionally renders label.slice(0, 2) which drops any other SVG assets; update the AgentCard icon rendering to first attempt to render the provider's actual icon/SVG (e.g., check for a provider.icon / provider.svg / iconType not equal to 'image'|'svg-gemini'|'cpu'|'text' and render that asset using the existing SVG/image component or <img> wrapper), and only use the compact badge (the label.slice(0, 2> span with className="shrink-0 inline-flex font-bold text-[10px] text-text-muted") when no icon asset is available; modify the return in AgentCard to conditionally render the real SVG/image asset before falling back to the two-letter label.src/components/Settings/AIProvidersSection.tsx-213-227 (1)
213-227:⚠️ Potential issue | 🟡 MinorThe routing explainer contradicts the cards above it.
This footer says every provider routes through OpenRouter and that direct routing is future work, but the individual cards describe several providers as direct endpoints today. Users will get the wrong setup/debugging guidance from this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/AIProvidersSection.tsx` around lines 213 - 227, The "How routing works" footer text is incorrect and contradicts the provider cards; update the copy in the <div> labeled "How routing works" (the JSX block containing the ANTHROPIC_BASE_URL and ANTHROPIC_API_KEY <code> elements) so it accurately reflects current behavior: state which providers use direct API endpoints today versus which route via OpenRouter, explain that provider-specific keys are used when present and OpenRouter key is the fallback, and remove the blanket statement that "All providers use Claude CLI... and direct API routing is future work." Cross-check the provider cards above in AIProvidersSection to ensure the footer matches those descriptions.electron/providers/moonshot-provider.ts-67-73 (1)
67-73:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
Same issue as
zhipu-provider.ts— the--modelargument is unquoted.Proposed fix
- if (params.model && params.model !== 'default') command += ` --model ${params.model}`; + if (params.model && params.model !== 'default') command += ` --model '${params.model}'`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/moonshot-provider.ts` around lines 67 - 73, The --model argument in buildOneShotCommand is currently appended unquoted; update buildOneShotCommand so when params.model && params.model !== 'default' you escape any single quotes in params.model (like other fields use .replace(/'/g, "'\\''")) and wrap it in single quotes before adding ` --model ...`, ensuring params.binaryPath, params.prompt handling remains unchanged; locate the buildOneShotCommand function and change the model concatenation to use the same quoting/escaping pattern as params.binaryPath and params.prompt.electron/providers/openrouter-provider.ts-124-137 (1)
124-137:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
Proposed fix
- if (params.model && params.model !== 'default') { - command += ` --model ${params.model}`; - } + if (params.model && params.model !== 'default') { + command += ` --model '${params.model}'`; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/openrouter-provider.ts` around lines 124 - 137, In buildOneShotCommand the --model value is appended unquoted which can break shells or allow injection; change the construction so that when params.model is used you escape any single quotes in params.model (like params.model.replace(/'/g, "'\\''")) and wrap it in single quotes when appending (i.e. ` --model 'escapedModel'`), similar to how params.binaryPath and params.prompt are escaped, ensuring params.model is safely quoted before insertion.electron/providers/deepseek-provider.ts-91-97 (1)
91-97:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
Proposed fix
- if (params.model && params.model !== 'default') command += ` --model ${params.model}`; + if (params.model && params.model !== 'default') command += ` --model '${params.model}'`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/deepseek-provider.ts` around lines 91 - 97, The model value in buildOneShotCommand is interpolated unquoted into the shell command—escape and quote it like binaryPath and prompt to avoid shell injection. In buildOneShotCommand (OneShotCommandParams), when adding `--model ${params.model}` replace it with a safely escaped-and-quoted version (e.g. build a quotedModel = `'${params.model.replace(/'/g, "'\\''")}'` and use `--model ${quotedModel}`) so the model argument is always shell-safe.electron/providers/qwen-provider.ts-69-75 (1)
69-75:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
Proposed fix
- if (params.model && params.model !== 'default') command += ` --model ${params.model}`; + if (params.model && params.model !== 'default') command += ` --model '${params.model}'`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/qwen-provider.ts` around lines 69 - 75, buildOneShotCommand builds a shell command but inserts params.model without quoting, which is unsafe; update buildOneShotCommand so when adding `--model ${params.model}` it instead quotes and escapes the model value the same way as binaryPath and prompt (e.g., replace any single quotes in params.model with "'\\''" and wrap the result in single quotes) and only append the quoted `--model 'escapedModel'` when params.model exists and is not 'default'.electron/providers/mimo-provider.ts-67-73 (1)
67-73:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
Proposed fix
- if (params.model && params.model !== 'default') command += ` --model ${params.model}`; + if (params.model && params.model !== 'default') command += ` --model '${params.model}'`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/mimo-provider.ts` around lines 67 - 73, In buildOneShotCommand, the --model value is added unquoted which is unsafe for shell injection; change the branch that appends ` --model ${params.model}` to quote and escape the model exactly like the binaryPath and prompt (e.g., use params.model.replace(/'/g, "'\\''") and wrap in single quotes) so the command becomes ` --model 'escapedModel'`; keep the existing check for params.model && params.model !== 'default' and otherwise leave behavior unchanged.electron/providers/zhipu-provider.ts-69-75 (1)
69-75:⚠️ Potential issue | 🟡 MinorModel argument should be quoted for shell safety.
The
--modelargument is not quoted, unlike inbuildInteractiveCommand(Line 43). While model IDs fromgetModels()appear safe, user-selected or custom models could contain characters that break the command.Proposed fix
buildOneShotCommand(params: OneShotCommandParams): string { let command = `'${params.binaryPath.replace(/'/g, "'\\''")}'`; command += ' -p'; - if (params.model && params.model !== 'default') command += ` --model ${params.model}`; + if (params.model && params.model !== 'default') command += ` --model '${params.model}'`; command += ` '${params.prompt.replace(/'/g, "'\\''")}'`; return command; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/providers/zhipu-provider.ts` around lines 69 - 75, The buildOneShotCommand function currently appends --model without quoting/escaping the model value, which can break the shell command; update buildOneShotCommand to quote and escape params.model the same way buildInteractiveCommand does (escape single quotes and wrap the model value in single quotes when adding `--model`), so the constructed command uses `--model 'escaped-model'` whenever params.model is set and not 'default'.
| buildScheduledScript(params: { | ||
| binaryPath: string; binaryDir: string; projectPath: string; prompt: string; | ||
| autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; | ||
| skills?: string[]; | ||
| }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
| return `#!/bin/bash | ||
| export HOME="${params.homeDir}" | ||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | ||
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | ||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
Same unescaped prompt issue as other providers.
🐛 Proposed fix
buildScheduledScript(params: {
binaryPath: string; binaryDir: string; projectPath: string; prompt: string;
autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string;
skills?: string[];
}): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
export HOME="${params.homeDir}"
...
unset CLAUDECODE
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
echo "=== Task completed at $(date) ===" >> "${params.logPath}"
`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/deepseek-provider.ts` around lines 185 - 205, The
generated script in buildScheduledScript embeds promptWithSkills directly into a
single-quoted -p argument, allowing shell injection; instead write the prompt
safely to a temp file and pass it to the binary (or pass via a decoded base64
file) rather than inlining the raw string. Update buildScheduledScript to:
create a uniquely-named temp file (e.g., using mktemp) inside the returned
script, write a safely-escaped or base64-encoded promptWithSkills into that
file, pass the file contents to the binary (or use a --prompt-file option if
supported), and remove the temp file afterwards; reference buildScheduledScript
and the promptWithSkills variable when making the change.
| HookConfig, | ||
| } from './cli-provider'; | ||
|
|
||
| const MIMO_BASE_URL = 'https://api.mimo.com/v1'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the official API endpoint URL for Xiaomi MiMo AI models?
💡 Result:
The official API endpoint URL for Xiaomi MiMo AI models is https://api.xiaomimimo.com/v1. This is the OpenAI-compatible base URL for accessing models like MiMo-V2-Pro, MiMo-V2-Omni, MiMo-V2-Flash, and MiMo-V2-TTS. The official platform is at https://platform.xiaomimimo.com/.
Citations:
- 1: https://platform.xiaomimimo.com/
- 2: http://mimo-v2.org/
- 3: [Feature]: Add support for Xiaomi MiMo API and MiMo-V2-Flash model CherryHQ/cherry-studio#11959
- 4: https://www.mimo-v2.com/docs
- 5: https://docs.litellm.ai/docs/providers/xiaomi_mimo
- 6: https://mimo-v2.org/
Correct the MiMo API base URL to the official endpoint.
The URL https://api.mimo.com/v1 is incorrect. The official API endpoint for Xiaomi's MiMo AI models is https://api.xiaomimimo.com/v1. Update the constant accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/mimo-provider.ts` at line 14, Update the MIMO_BASE_URL
constant to use the official Xiaomi MiMo API endpoint: replace the current value
of MIMO_BASE_URL (const MIMO_BASE_URL = 'https://api.mimo.com/v1') with
'https://api.xiaomimimo.com/v1' so all requests from the mimo-provider.ts module
use the correct base URL.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
| return `#!/bin/bash | ||
| export HOME="${params.homeDir}" | ||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | ||
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | ||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
Same unescaped prompt issue as in other providers.
🐛 Proposed fix
buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
export HOME="${params.homeDir}"
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
...
`;
}📝 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.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } | |
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''"); | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/mimo-provider.ts` around lines 132 - 148, The
buildScheduledScript function embeds promptWithSkills directly into a
single-quoted shell argument, which allows shell injection; instead, write
promptWithSkills to a temporary file inside the generated script (e.g., create a
TMP prompt file under ${params.homeDir} or use mktemp), use a safe write (printf
'%s' to the temp file) and pass the prompt to the binary by reading the file
(e.g., -p "$(cat "$TMPFILE")"), then remove the temp file; update
buildScheduledScript to perform those steps so promptWithSkills is never
interpolated into the command line.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
| return `#!/bin/bash | ||
| export HOME="${params.homeDir}" | ||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | ||
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | ||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
Same issue as in zhipu-provider.ts. The prompt is embedded without escaping single quotes, which can break the script or allow injection.
🐛 Proposed fix
buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
export HOME="${params.homeDir}"
if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi
if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi
export PATH="${params.binaryDir}:$PATH"
cd "${params.projectPath}"
echo "=== Task started at $(date) ===" >> "${params.logPath}"
unset CLAUDECODE
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
echo "=== Task completed at $(date) ===" >> "${params.logPath}"
`;
}📝 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.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } | |
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''"); | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/moonshot-provider.ts` around lines 148 - 164, The
buildScheduledScript function embeds promptWithSkills directly into a
single-quoted bash argument, creating a shell injection/breakage risk; update
buildScheduledScript to safely quote/escape the prompt before embedding (e.g.,
transform single quotes in promptWithSkills to the safe bash sequence '"'"' or
switch to a quoted heredoc token such as <<'EOF' to pass the raw prompt) so that
any single quotes or shell metacharacters in params.prompt or params.skills
cannot terminate the surrounding quotes or inject commands; ensure the
escaped/quoted value is used where '-p '${promptWithSkills}'' appears.
| buildScheduledScript(params: { | ||
| binaryPath: string; | ||
| binaryDir: string; | ||
| projectPath: string; | ||
| prompt: string; | ||
| autonomous: boolean; | ||
| mcpConfigPath: string; | ||
| logPath: string; | ||
| homeDir: string; | ||
| skills?: string[]; | ||
| }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
|
|
||
| return `#!/bin/bash | ||
|
|
||
| export HOME="${params.homeDir}" | ||
|
|
||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then | ||
| source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true | ||
| fi | ||
|
|
||
| if [ -f "${params.homeDir}/.zshrc" ]; then | ||
| source "${params.homeDir}/.zshrc" 2>/dev/null || true | ||
| elif [ -f "${params.homeDir}/.bash_profile" ]; then | ||
| source "${params.homeDir}/.bash_profile" 2>/dev/null || true | ||
| fi | ||
|
|
||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
Same issue as other providers — the prompt needs single-quote escaping before embedding in the bash script.
🐛 Proposed fix
buildScheduledScript(params: {
...
}): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
...
unset CLAUDECODE
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
...
`;
}📝 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.
| buildScheduledScript(params: { | |
| binaryPath: string; | |
| binaryDir: string; | |
| projectPath: string; | |
| prompt: string; | |
| autonomous: boolean; | |
| mcpConfigPath: string; | |
| logPath: string; | |
| homeDir: string; | |
| skills?: string[]; | |
| }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then | |
| source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true | |
| fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then | |
| source "${params.homeDir}/.zshrc" 2>/dev/null || true | |
| elif [ -f "${params.homeDir}/.bash_profile" ]; then | |
| source "${params.homeDir}/.bash_profile" 2>/dev/null || true | |
| fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } | |
| buildScheduledScript(params: { | |
| binaryPath: string; | |
| binaryDir: string; | |
| projectPath: string; | |
| prompt: string; | |
| autonomous: boolean; | |
| mcpConfigPath: string; | |
| logPath: string; | |
| homeDir: string; | |
| skills?: string[]; | |
| }): string { | |
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | |
| const promptWithSkills = (params.skills && params.skills.length > 0) | |
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | |
| : params.prompt; | |
| const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''"); | |
| return `#!/bin/bash | |
| export HOME="${params.homeDir}" | |
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then | |
| source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true | |
| fi | |
| if [ -f "${params.homeDir}/.zshrc" ]; then | |
| source "${params.homeDir}/.zshrc" 2>/dev/null || true | |
| elif [ -f "${params.homeDir}/.bash_profile" ]; then | |
| source "${params.homeDir}/.bash_profile" 2>/dev/null || true | |
| fi | |
| export PATH="${params.binaryDir}:$PATH" | |
| cd "${params.projectPath}" | |
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | |
| unset CLAUDECODE | |
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1 | |
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | |
| `; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/openrouter-provider.ts` around lines 261 - 298, The
generated bash script embeds promptWithSkills directly into a single-quoted
argument, creating a shell injection risk; update buildScheduledScript to escape
the prompt before embedding (e.g., transform single quotes in promptWithSkills
into a safe shell-escaped sequence and also handle backslashes/newlines as
needed) and use that escaped string in the returned script instead of
promptWithSkills so the produced -p '...' argument cannot break out of the
single quotes.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
| return `#!/bin/bash | ||
| export HOME="${params.homeDir}" | ||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | ||
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | ||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
Same unescaped prompt issue as other providers.
🐛 Proposed fix
buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
...
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
...
`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/qwen-provider.ts` around lines 134 - 150, The generated
script embeds promptWithSkills directly in a single-quoted -p argument, which
allows shell injection; in buildScheduledScript, stop interpolating the raw
prompt into the command and instead either (A) write promptWithSkills to a
temporary file (via mktemp) and pass that file to the binary (e.g.,
--prompt-file or cat it into stdin) or (B) fully shell-escape single quotes in
promptWithSkills before embedding (replace every ' with '\''), or use a
single-quoted here-doc (<<'EOF') to safely supply the prompt; update the script
generation to create the temp file or here-doc, reference params.binaryPath and
params.logPath appropriately, and ensure the temp file is removed afterwards.
| buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string { | ||
| const flags = params.autonomous ? '--dangerously-skip-permissions' : ''; | ||
| const promptWithSkills = (params.skills && params.skills.length > 0) | ||
| ? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}` | ||
| : params.prompt; | ||
| return `#!/bin/bash | ||
| export HOME="${params.homeDir}" | ||
| if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi | ||
| if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi | ||
| export PATH="${params.binaryDir}:$PATH" | ||
| cd "${params.projectPath}" | ||
| echo "=== Task started at $(date) ===" >> "${params.logPath}" | ||
| unset CLAUDECODE | ||
| "${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1 | ||
| echo "=== Task completed at $(date) ===" >> "${params.logPath}" | ||
| `; | ||
| } |
There was a problem hiding this comment.
Shell injection risk: promptWithSkills is not escaped in the generated bash script.
The promptWithSkills variable is embedded directly into the bash script using single quotes, but single quotes within the prompt are not escaped. If params.prompt or skill names contain ', the generated script will either break or potentially allow shell injection.
Compare with buildScheduledCommand (Line 65) which correctly escapes:
command += ` -p '${params.prompt.replace(/'/g, "'\\''")}'`;🐛 Proposed fix: escape single quotes in promptWithSkills
buildScheduledScript(params: { binaryPath: string; binaryDir: string; projectPath: string; prompt: string; autonomous: boolean; mcpConfigPath: string; logPath: string; homeDir: string; skills?: string[]; }): string {
const flags = params.autonomous ? '--dangerously-skip-permissions' : '';
const promptWithSkills = (params.skills && params.skills.length > 0)
? `[IMPORTANT: Use these skills for this session: ${params.skills.join(', ')}. Invoke them with /<skill-name> when relevant to the task.] ${params.prompt}`
: params.prompt;
+ const escapedPrompt = promptWithSkills.replace(/'/g, "'\\''");
return `#!/bin/bash
export HOME="${params.homeDir}"
if [ -s "${params.homeDir}/.nvm/nvm.sh" ]; then source "${params.homeDir}/.nvm/nvm.sh" 2>/dev/null || true; fi
if [ -f "${params.homeDir}/.zshrc" ]; then source "${params.homeDir}/.zshrc" 2>/dev/null || true; fi
export PATH="${params.binaryDir}:$PATH"
cd "${params.projectPath}"
echo "=== Task started at $(date) ===" >> "${params.logPath}"
unset CLAUDECODE
-"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${promptWithSkills}' >> "${params.logPath}" 2>&1
+"${params.binaryPath}" ${flags} --output-format stream-json --verbose --mcp-config "${params.mcpConfigPath}" --add-dir "${params.homeDir}/.dorothy" -p '${escapedPrompt}' >> "${params.logPath}" 2>&1
echo "=== Task completed at $(date) ===" >> "${params.logPath}"
`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@electron/providers/zhipu-provider.ts` around lines 138 - 154, The generated
bash script in buildScheduledScript inserts promptWithSkills directly into a
single-quoted -p argument which breaks or allows injection if the prompt or
skill names contain single quotes; fix this by escaping single quotes in
promptWithSkills (or when embedding it) the same way buildScheduledCommand does
(replace all ' with '\'' ) before constructing the script so the resulting -p
'...escaped...' is safe and robust.
| function ProviderIcon({ icon, selected, accent }: { icon: ProviderIconDef; selected: boolean; accent: string }) { | ||
| const colorClass = selected ? `text-${accent}` : 'text-text-muted'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
tailwind_files="$(fd -HI --glob 'tailwind.config.*' . || true)"
if [ -n "$tailwind_files" ]; then
while IFS= read -r file; do
echo "== $file =="
sed -n '1,220p' "$file"
done <<< "$tailwind_files"
fi
echo "== Dynamic class call sites =="
rg -n --fixed-strings 'text-${accent}' src/components/NewChatModal/StepModel.tsx
rg -n --fixed-strings 'border-${accent}' src/components/NewChatModal/StepModel.tsx
rg -n --fixed-strings 'bg-${accent}/10' src/components/NewChatModal/StepModel.tsx
echo "== Existing safelist / explicit class references =="
rg -n 'safelist|accent-blue|accent-green|accent-purple|accent-cyan|amber-500|sky-500|violet-500|orange-500|blue-500|indigo-500' .Repository: Charlie85270/Dorothy
Length of output: 40495
🏁 Script executed:
# Check common tailwind config locations and next.js config
find . -maxdepth 2 -type f \( -name "next.config.*" -o -name "tailwind.config.*" -o -name "postcss.config.*" \) | head -20
# Check package.json for tailwind config
grep -A 20 '"tailwind"' package.json 2>/dev/null || echo "No tailwind config in package.json"
# Check if postcss.config has tailwind setup
cat postcss.config.js 2>/dev/null || echo "No postcss.config.js found"Repository: Charlie85270/Dorothy
Length of output: 217
🏁 Script executed:
# Read StepModel.tsx to see the actual implementation
wc -l src/components/NewChatModal/StepModel.tsx
head -50 src/components/NewChatModal/StepModel.tsx
sed -n '35,55p' src/components/NewChatModal/StepModel.tsx
sed -n '190,210p' src/components/NewChatModal/StepModel.tsxRepository: Charlie85270/Dorothy
Length of output: 3707
🏁 Script executed:
# Check providers.ts to understand accent value structure
sed -n '30,120p' src/lib/providers.tsRepository: Charlie85270/Dorothy
Length of output: 3554
🏁 Script executed:
# Check the next.config.ts for tailwind safelist
cat next.config.ts
# Check postcss.config.mjs
cat postcss.config.mjsRepository: Charlie85270/Dorothy
Length of output: 824
🏁 Script executed:
# Search for safelist configuration in the codebase
rg -i 'safelist|content.*safelist' --type ts --type js --type mjsRepository: Charlie85270/Dorothy
Length of output: 91
🏁 Script executed:
# Verify the exact lines in StepModel.tsx where dynamic classes are used
sed -n '40,50p' src/components/NewChatModal/StepModel.tsx
sed -n '193,205p' src/components/NewChatModal/StepModel.tsxRepository: Charlie85270/Dorothy
Length of output: 1261
🏁 Script executed:
# Check if there's a tailwind.config in a different location
find . -name "tailwind.config.*" -o -name "tailwind.js"
# Check app/globals.css for any `@apply` or other class definitions
head -100 src/app/globals.css
# Search for where badgeClass is used (this stores full class names)
rg -n 'badgeClass' src/components/NewChatModal/ | head -20Repository: Charlie85270/Dorothy
Length of output: 2818
🏁 Script executed:
# Look for tailwind.css (v4 config file)
find . -name "tailwind.css" -type f
# Check the full postcss.config.mjs
cat postcss.config.mjs 2>/dev/null || cat postcss.config.js 2>/dev/null || cat postcss.config.ts 2>/dev/null
# Find where badgeClass is actually used in the template
rg -A 3 'badgeClass' src/components/NewChatModal/StepModel.tsxRepository: Charlie85270/Dorothy
Length of output: 157
🏁 Script executed:
# Check if there's a content/safelist config anywhere in the codebase
rg -i '"content".*\[|content.*glob|safelist' --type ts --type js -B 2 -A 2Repository: Charlie85270/Dorothy
Length of output: 46
🏁 Script executed:
# Check for tailwind.css in the src directory (Tailwind v4 config location)
find src -name "tailwind.css" -o -name "tailwind.config.*"
# Check the main tailwind.css file if it exists in src/app
ls -la src/app/ | grep -i tailwind
# Look at how Tailwind v4 is configured - check if there's a `@source` directive in globals.css
grep -n '@source\|@config\|content' src/app/globals.cssRepository: Charlie85270/Dorothy
Length of output: 46
🏁 Script executed:
# Verify the actual accent values being used and if they're ever fully specified in class names
rg 'accent-blue|accent-green|accent-purple|accent-cyan|amber-500|sky-500|violet-500|orange-500|blue-500|indigo-500' src/lib/providers.ts | head -20Repository: Charlie85270/Dorothy
Length of output: 904
🏁 Script executed:
# Check if badgeClass (which stores full class names) is actually used or if it's dead code
rg -n 'badgeClass' src/components/NewChatModal/StepModel.tsx -A 2 -B 2Repository: Charlie85270/Dorothy
Length of output: 46
🏁 Script executed:
# Check if globals.css has a complete theme extension for accent colors
sed -n '250,350p' src/app/globals.css
# Check how Tailwind v4 is processing content - look for any extend or theme config
rg -A 10 'theme|extend|colors' src/app/globals.css | head -30Repository: Charlie85270/Dorothy
Length of output: 4437
🏁 Script executed:
# Verify if accent-* classes are actually rendered as utilities or just CSS vars
grep -n 'accent-blue\|accent-green\|accent-purple' src/app/globals.css | head -20Repository: Charlie85270/Dorothy
Length of output: 777
🏁 Script executed:
# Check build/dist output to see if these classes actually make it through
find . -name "*.css" -path "*/dist/*" -o -path "*/.next/*" 2>/dev/null | head -5Repository: Charlie85270/Dorothy
Length of output: 46
Dynamic accent classes will not be compiled by Tailwind and will break styling in production.
text-${accent}, border-${accent}, and bg-${accent}/10 compose class names at runtime. Tailwind v4 uses static content analysis and cannot extract these patterns—they won't appear in the compiled CSS. Only static class names like 'text-text-muted' (hardcoded fallback) are detected and compiled.
The codebase already defines badgeClass in the provider registry with full, pre-composed class names (e.g., 'bg-blue-500/15 text-blue-600 dark:text-blue-400'), but this field is not used in StepModel.tsx. Use the pre-composed class names from the registry instead of composing fragments at runtime.
Also applies to: 193-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/NewChatModal/StepModel.tsx` around lines 42 - 43, The dynamic
Tailwind classes in ProviderIcon (e.g., `text-${accent}`) and elsewhere are not
compiled; switch to using the provider registry's pre-composed `badgeClass`
instead of composing `text-${accent}`, `border-${accent}`, or `bg-${accent}/10`
at runtime. Update the `ProviderIcon` component to accept/use the provider's
`badgeClass` (from the provider registry entry) for its className logic and
remove runtime template strings; likewise replace similar dynamic class
composition in the other block mentioned (lines ~193-199) to use the registry
`badgeClass` so Tailwind can statically include the styles.
Summary
.worktrees/paths from project listsTest plan
npx tsc --noEmit)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX