fix: use system claude binary in packaged Electron app#1188
fix: use system claude binary in packaged Electron app#1188JustYannicc wants to merge 10 commits intopingdotgg:codething/648ca884-claudefrom
Conversation
- Auto-start synthetic turns for assistant messages arriving without an active turnState (fixes invisible background agent/subagent responses that arrive between user-initiated turns) - Auto-close stale synthetic turns in sendTurn to prevent session deadlock when a background response left an orphaned turn open - Clear inFlightTools between turns in completeTurn to prevent stale JSON fragments from corrupting the next turn's tool inputs - Fix missing Fiber import in ProviderHealth Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The clear() was placed before the loop that emits item.completed events for remaining in-flight tools, causing the loop to never execute. Move it after the loop so tools get properly finalized before cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Updates all remaining references from "claudeCode" to "claudeAgent" to match the Anthropic branding guidelines applied upstream. Includes provider kind literals, object property keys, test payloads, and component comparisons across server, web, and shared packages. Also fixes synthetic turn state to match upstream's refactored ClaudeTurnState interface (assistantTextBlocks map instead of the old assistantItemId/messageCompleted/emittedTextDelta fields). Also fixes inFlightTools.clear() placement: moved after the loop that emits item.completed events for remaining in-flight tools, so tools are properly finalized before cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The claude-agent-sdk bundles its own cli.js inside node_modules. In the packaged Electron app this file lives inside the asar archive, where child_process.spawn cannot execute it — causing the Claude Code process to exit with code 1 immediately after session start. Default pathToClaudeCodeExecutable to "claude" (resolved via PATH) so the adapter uses the system-installed binary, matching how the Codex adapter already defaults to "codex". Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
github-actions
bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }); | ||
| }); | ||
|
|
||
| it("forwards claude effort options through session start and turn send", async () => { |
There was a problem hiding this comment.
🟢 Low Layers/ProviderCommandReactor.test.ts:354
The test "forwards claude effort options through session start and turn send" (lines 354-402) dispatches provider: "claudeAgent" with model: "claude-sonnet-4-6" on thread-1, but that thread was created with model: "gpt-5-codex" which infers provider codex. The ensureSessionForThread logic rejects provider switches with a ProviderAdapterRequestError, so startSession is never called and waitFor(() => harness.startSession.mock.calls.length === 1) will time out. The test appears to expect success, but the existing test at line 442 confirms this exact rejection behavior for provider mismatches. Consider adjusting the test to use a thread with a matching Claude model, or update assertions to expect the rejection.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts around line 354:
The test "forwards claude effort options through session start and turn send" (lines 354-402) dispatches `provider: "claudeAgent"` with `model: "claude-sonnet-4-6"` on thread-1, but that thread was created with `model: "gpt-5-codex"` which infers provider `codex`. The `ensureSessionForThread` logic rejects provider switches with a `ProviderAdapterRequestError`, so `startSession` is never called and `waitFor(() => harness.startSession.mock.calls.length === 1)` will time out. The test appears to expect success, but the existing test at line 442 confirms this exact rejection behavior for provider mismatches. Consider adjusting the test to use a thread with a matching Claude model, or update assertions to expect the rejection.
Evidence trail:
1. apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts:234 - `createHarness()` creates thread-1 with `model: "gpt-5-codex"`
2. apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts:354-402 - Test dispatches turn with `provider: "claudeAgent"`, `model: "claude-sonnet-4-6"` and expects `startSession` to be called
3. packages/shared/src/model.ts:70-85 - `inferProviderForModel` function that determines provider from model
4. packages/shared/src/model.test.ts:107 - Confirms `inferProviderForModel("gpt-5.3-codex")` returns `"codex"`
5. apps/server/src/orchestration/Layers/ProviderCommandReactor.ts:220-226 - Provider mismatch check that returns `ProviderAdapterRequestError`
6. apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts:442-489 - Existing test confirming provider mismatch rejection behavior
There was a problem hiding this comment.
🟡 Medium
In recoverSessionForThread, the recovered session's modelOptions and providerOptions are passed to adapter.startSession (lines 235–236) but not to upsertSessionBinding at line 247. This causes the binding stored in directory to lose those options, so any subsequent recovery attempts will fail to restore them. Consider passing persistedModelOptions and persistedProviderOptions to upsertSessionBinding to preserve the options across recoveries.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ProviderService.ts around line 247:
In `recoverSessionForThread`, the recovered session's `modelOptions` and `providerOptions` are passed to `adapter.startSession` (lines 235–236) but not to `upsertSessionBinding` at line 247. This causes the binding stored in `directory` to lose those options, so any subsequent recovery attempts will fail to restore them. Consider passing `persistedModelOptions` and `persistedProviderOptions` to `upsertSessionBinding` to preserve the options across recoveries.
Evidence trail:
apps/server/src/provider/Layers/ProviderService.ts lines 162-174 (upsertSessionBinding definition with optional extra param), lines 89-101 (toRuntimePayloadFromSession only includes modelOptions/providerOptions if extra param is provided), lines 228-229 (persistedModelOptions and persistedProviderOptions are read), lines 235-236 (options passed to adapter.startSession), line 247 (upsertSessionBinding called WITHOUT the options), lines 308-311 (correct usage example with extra param in startSession)
| props.provider === "claudeAgent" ? "text-[#d97757]" : "text-muted-foreground/70", | ||
| props.provider === "claudeAgent" && props.ultrathinkActive | ||
| ? "ultrathink-chroma" | ||
| : undefined, |
There was a problem hiding this comment.
🟢 Low chat/ProviderModelPicker.tsx:137
When lockedProvider differs from provider, the provider icon is determined by activeProvider (which uses lockedProvider when set) but the styling conditions on lines 137-139 check props.provider. This causes the Claude icon to render without the expected text-[#d97757] color and ultrathink-chroma animation when lockedProvider is claudeAgent but provider is something else. Change the conditions to use activeProvider to match the icon selection logic.
- props.provider === "claudeAgent" ? "text-[#d97757]" : "text-muted-foreground/70",
- props.provider === "claudeAgent" && props.ultrathinkActive
+ activeProvider === "claudeAgent" ? "text-[#d97757]" : "text-muted-foreground/70",
+ activeProvider === "claudeAgent" && props.ultrathinkActive
? "ultrathink-chroma"
: undefined,🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/chat/ProviderModelPicker.tsx around lines 137-140:
When `lockedProvider` differs from `provider`, the provider icon is determined by `activeProvider` (which uses `lockedProvider` when set) but the styling conditions on lines 137-139 check `props.provider`. This causes the Claude icon to render without the expected `text-[#d97757]` color and `ultrathink-chroma` animation when `lockedProvider` is claudeAgent but `provider` is something else. Change the conditions to use `activeProvider` to match the icon selection logic.
Evidence trail:
apps/web/src/components/chat/ProviderModelPicker.tsx:
- Line 87: `const activeProvider = props.lockedProvider ?? props.provider;`
- Line 91: `const ProviderIcon = PROVIDER_ICON_BY_PROVIDER[activeProvider];`
- Lines 137-140: Styling conditions use `props.provider` instead of `activeProvider`
Commit: REVIEWED_COMMIT
1 participant
Summary
The
claude-agent-sdkbundles its owncli.jsinsidenode_modules. In the packaged Electron app this file lives inside the asar archive, wherechild_process.spawncannot execute it — causing the Claude Code process to exit with code 1 immediately after session start.This defaults
pathToClaudeCodeExecutableto"claude"(resolved via PATH) so the adapter uses the system-installed binary, matching how the Codex adapter already defaults to"codex".Stack
Validation
query()works correctly when pointed at the systemclaudebinaryclaudeAgentprovider no longer crashes on session startbun typecheckNote
Fix Claude binary path to use system
claudeexecutable in packaged Electron apppathToClaudeCodeExecutableto'claude'(system PATH) when nobinaryPathis provided in provider options, fixing resolution in packaged Electron builds.ClaudeCodeEffortandProviderReasoningEfforttypes, maps'ultrathink'to'max'at runtime, and prefixes prompts with'Ultrathink:'when that effort is selected.modelOptions(including effort) through session start, turn dispatch, session persistence, and stale-session recovery.sendTurninstead of returning a validation error.ProviderTraitsPicker, and effort options surfaced for Claude in the compact controls menu.📊 Macroscope summarized 87cad84. 21 files reviewed, 7 issues evaluated, 2 issues filtered, 3 comments posted
🗂️ Filtered Issues
apps/server/src/provider/Layers/ClaudeAdapter.ts — 0 comments posted, 1 evaluated, 1 filtered
pathToClaudeCodeExecutableto"claude"as a fallback whenproviderOptions?.binaryPathis undefined. Previously, whenbinaryPathwasn't provided, the option was omitted entirely, allowing the SDK to use its built-in default (acli.jsrelative to the SDK package). Now, setting it to"claude"assumes the binary is available in the system PATH, which will causeENOENTfailures in environments where Claude Code isn't installed globally but the SDK's bundled CLI would have been used. The SDK's fallback logic inquery()only triggers whenpathToClaudeCodeExecutableis falsy—providing"claude"bypasses this entirely. [ Out of scope (triage) ]apps/server/src/provider/Layers/ProviderService.ts — 1 comment posted, 2 evaluated, 1 filtered
upsertSessionBindingis called without preservingmodelOptionsandproviderOptionsfrominput.binding.runtimePayload. When an existing active session is adopted, the binding is overwritten without these persisted options, losing them for any future recovery attempts. [ Out of scope ]