fix(providers): replace Claude SDK embed with explicit binary-path resolver#1217
Merged
fix(providers): replace Claude SDK embed with explicit binary-path resolver#1217
Conversation
…esolver Drop `@anthropic-ai/claude-agent-sdk/embed` and resolve Claude Code via CLAUDE_BIN_PATH env → assistants.claude.claudeBinaryPath config → throw with install instructions. The embed's silent failure modes on macOS (#1210) and Windows (#1087) become actionable errors with a documented recovery path. Dev mode (bun run) remains auto-resolved via node_modules. The setup wizard auto-detects Claude Code by probing the native installer path (~/.local/bin/claude), npm global cli.js, and PATH, then writes CLAUDE_BIN_PATH to ~/.archon/.env. Dockerfile pre-sets CLAUDE_BIN_PATH so extenders using the compiled binary keep working. Release workflow gets negative and positive resolver smoke tests. Docs, CHANGELOG, README, .env.example, CLAUDE.md, test-release and archon skills all updated to reflect the curl-first install story. Retires #1210, #1087, #1091 (never merged, now obsolete). Implements #1176.
…Node `--no-env-file` is a Bun flag that prevents Bun from auto-loading `.env` from the subprocess cwd. It is only meaningful when the Claude Code executable is a `cli.js` file — in which case the SDK spawns it via `bun`/`node` and the flag reaches the runtime. When `CLAUDE_BIN_PATH` points at a native compiled Claude binary (e.g. `~/.local/bin/claude` from the curl installer, which is Anthropic's recommended default), the SDK executes the binary directly. Passing `--no-env-file` then goes straight to the native binary, which rejects it with `error: unknown option '--no-env-file'` and the subprocess exits code 1. Emit `executableArgs` only when the target is a `.js` file (dev mode or explicit cli.js path). Caught by end-to-end smoke testing against the curl-installed native Claude binary.
Verified end-to-end with sentinel `.env` and `.env.local` files in a workflow CWD that the native Claude binary (curl installer) does not auto-load `.env` files. With Archon's full spawn pathway and parent env stripped, the subprocess saw both sentinels as UNSET. The first-layer protection in `@archon/paths` (#1067) handles the inheritance leak; `--no-env-file` only matters for the Bun-spawned cli.js path, where it is still emitted.
📝 WalkthroughWalkthroughRemoves embedded Claude Code SDK from compiled binaries and adds explicit binary-path resolution. Implements a Claude binary resolver, threads optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Setup as "archon setup"
participant Detector as "Detect (probeNpmRoot / which / file)"
participant Config as "Config (~/.archon/config.yaml)"
participant Provider as "Claude Provider"
participant Resolver as "resolveClaudeBinaryPath"
participant CLI as "Claude Code CLI"
User->>Setup: run setup
Setup->>Detector: detect Claude CLI path
Detector-->>Detector: check platform default\ncheck npm root\ncheck which/where
Detector-->>Setup: return path or null
Setup->>User: confirm or input path
Setup->>Config: write CLAUDE_BIN_PATH or claudeBinaryPath
User->>Provider: send Claude query
Provider->>Resolver: resolveClaudeBinaryPath(configClaudeBinaryPath)
Resolver-->>Resolver: check BUNDLED_IS_BINARY\ncheck CLAUDE_BIN_PATH\ncheck config path\nvalidate fileExists
Resolver-->>Provider: return path or throw error
Provider->>CLI: spawn subprocess (with path or let SDK resolve + conditional --no-env-file)
CLI-->>Provider: response or error
Provider-->>User: result or remediation message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
Author
PR Review Summary (5 agents)Ran Critical IssuesNone. Core resolver (env → config → throw) is correct, fail-fast, well-scoped, end-to-end validated. Important Issues
Suggestions
Strengths
Documentation IssuesTwo fixes inside files the PR already touched ( Verdict: READY TO MERGE WITH MINOR POLISHNo correctness defects. No silent failures. The two Important test gaps and two doc nits are worth addressing in this PR for long-term maintainability, but none block merge. Recommended Actions
|
Final-sweep cleanup tied to the binary-resolver PR: - Mirror Codex's package surface for the new Claude resolver: add `./claude/binary-resolver` subpath export and re-export `resolveClaudeBinaryPath` + `claudeFileExists` from the package index. Renames the previously single `fileExists` re-export to `codexFileExists` for symmetry; nothing outside the providers package was importing it. - Add a "Claude Code not found" entry to the troubleshooting reference doc with platform-specific install snippets and pointers to the AI Assistants binary-path section. - Reframe the example claudeBinaryPath in reference/configuration.md away from cli.js-only language; it accepts either the native binary or cli.js.
Two test gaps and one doc nit from the PR review (#1217): - Extract the `--no-env-file` decision into a pure exported helper `shouldPassNoEnvFile(cliPath)` so the native-binary branch is unit testable without mocking `BUNDLED_IS_BINARY` or running the full sendQuery pathway. Six new tests cover undefined, cli.js, native binary (Linux + Windows), Homebrew symlink, and suffix-only matching. Also adds a `claude.subprocess_env_file_flag` debug log so the security-adjacent decision is auditable. - Extract the three install-location probes in setup.ts into exported wrappers (`probeFileExists`, `probeNpmRoot`, `probeWhichClaude`) and export `detectClaudeExecutablePath` itself, so the probe order can be spied on. Six new tests cover each tier winning, fall-through ordering, npm-tier skip when not installed, and the which-resolved-but-stale-path edge case. - CLAUDE.md `claudeBinaryPath` placeholder updated to reflect that the field accepts either the native binary or cli.js (the example value was previously `/absolute/path/to/cli.js`, slightly misleading now that the curl-installer native binary is the default). Skipped from the review by deliberate scope decision: - `resolveClaudeBinaryPath` async-with-no-await: matches Codex's resolver signature exactly. Changing only Claude breaks symmetry; if pursued, do both providers in a separate cleanup PR. - `isAbsolute()` validation in parseClaudeConfig: Codex doesn't do it either. Resolver throws on non-existence already. - Atomic `.env` writes in setup wizard: pre-existing pattern this PR touched only adjacently. File as separate issue if needed. - classifyError branch in dag-executor for setup errors: scope creep. - `.env.example` "missing #" claim: false positive (verified all CLAUDE_BIN_PATH lines have proper comment prefixes).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/archon/guides/setup.md:
- Line 122: Update the Note about CLAUDE_BIN_PATH in the Archon setup guide:
clarify that CLAUDE_BIN_PATH can point to either the Claude Code SDK's cli.js or
a native claude binary, and that the archon setup wizard (Step 4 in `archon
setup`) performs broader detection (not only `npm root -g`) and resolves common
global and system paths before writing the resolved path to `~/.archon/.env`;
retain the remark that source installs (e.g., `bun run`) do not require setting
CLAUDE_BIN_PATH because the SDK finds `cli.js` via `node_modules`.
In @.claude/skills/test-release/SKILL.md:
- Line 251: Update the pass criteria text in SKILL.md to match the actual error
string emitted by INSTALL_INSTRUCTIONS in binary-resolver.ts: change "Claude
Code CLI not found" to "Claude Code not found" (or alternatively update
INSTALL_INSTRUCTIONS to include "CLI" if you prefer consistency the other way);
locate the INSTALL_INSTRUCTIONS symbol in binary-resolver.ts and the pass
criteria sentence in SKILL.md and make the text identical across both files.
In `@Dockerfile`:
- Around line 111-117: The Dockerfile sets ENV CLAUDE_BIN_PATH to the wrong
package path; change the environment variable CLAUDE_BIN_PATH so it points to
the Claude Code package's cli.js (i.e.,
/app/node_modules/@anthropic-ai/claude-code/cli.js) instead of
/app/node_modules/@anthropic-ai/claude-agent-sdk/cli.js, ensuring the Docker
image and any compiled Archon binary will resolve the correct executable
referenced by binary-resolver.ts and the CHANGELOG.
In `@packages/docs-web/src/content/docs/deployment/docker.md`:
- Around line 16-17: Update the CLAUDE_BIN_PATH guidance to mention that it can
point to either a mounted cli.js or a native Claude executable (not just
cli.js); modify the sentence referencing CLAUDE_BIN_PATH in the
docs/deployment/docker.md content so it explains both supported resolver binary
types and links to the existing Binary path configuration section, ensuring
CLAUDE_BIN_PATH is described as accepting the path to the appropriate
binary/executable for the resolver.
In `@packages/docs-web/src/content/docs/reference/configuration.md`:
- Around line 63-65: The doc currently restricts claudeBinaryPath to cli.js;
update the wording for the claudeBinaryPath configuration entry to indicate it
accepts an executable path (either the cli.js or a native binary) rather than
“cli.js”-only phrasing, and adjust the example/comment text to reflect
“executable path to the Claude Code SDK binary or cli.js” so users know native
binary paths are supported; edit the lines describing claudeBinaryPath in the
configuration.md file to make this change.
In `@packages/providers/src/claude/binary-resolver.test.ts`:
- Around line 79-90: The test currently reuses the same promise returned by
resolver.resolveClaudeBinaryPath() across multiple await expect(...).rejects
assertions (in test 'throws with install instructions when nothing configured'),
which is an anti-pattern in Bun; instead call resolver.resolveClaudeBinaryPath()
once, await it inside a try/catch (or capture the rejection via const err =
await resolver.resolveClaudeBinaryPath().catch(e => e)), then run multiple
synchronous assertions against err.message to check it contains 'Claude Code not
found', 'CLAUDE_BIN_PATH', 'https://claude.ai/install.sh', 'npm install -g
`@anthropic-ai/claude-code`', and 'claudeBinaryPath' so you only consume the
promise once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d28a2bd7-e805-4a62-8a04-31555ae53d55
📒 Files selected for processing (25)
.claude/skills/archon/guides/setup.md.claude/skills/test-release/SKILL.md.env.example.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdDockerfileREADME.mdpackages/cli/src/commands/setup.test.tspackages/cli/src/commands/setup.tspackages/docs-web/src/content/docs/deployment/docker.mdpackages/docs-web/src/content/docs/deployment/local.mdpackages/docs-web/src/content/docs/getting-started/ai-assistants.mdpackages/docs-web/src/content/docs/getting-started/configuration.mdpackages/docs-web/src/content/docs/getting-started/installation.mdpackages/docs-web/src/content/docs/getting-started/overview.mdpackages/docs-web/src/content/docs/getting-started/quick-start.mdpackages/docs-web/src/content/docs/reference/configuration.mdpackages/providers/package.jsonpackages/providers/src/claude/binary-resolver-dev.test.tspackages/providers/src/claude/binary-resolver.test.tspackages/providers/src/claude/binary-resolver.tspackages/providers/src/claude/config.tspackages/providers/src/claude/provider.tspackages/providers/src/types.ts
| 3. Verify: `archon version` | ||
| 4. Check Claude is installed: `which claude`, then `claude /login` if needed | ||
|
|
||
| > **Note — Claude Code binary path.** Archon does not bundle Claude Code. In compiled Archon binaries (quick install, Homebrew), the Claude Code SDK needs `CLAUDE_BIN_PATH` set to the absolute path of its `cli.js`. The `archon setup` wizard in Step 4 auto-detects this via `npm root -g` and writes it to `~/.archon/.env` — no manual action needed in the typical case. Source installs (`bun run`) don't need this; the SDK finds `cli.js` via `node_modules` automatically. |
There was a problem hiding this comment.
Update setup note to reflect full resolver/detection behavior.
Line 122 still frames CLAUDE_BIN_PATH as cli.js-only and cites only npm root -g. The current flow supports native claude binary paths too and setup detection is broader.
Suggested wording update
-> **Note — Claude Code binary path.** Archon does not bundle Claude Code. In compiled Archon binaries (quick install, Homebrew), the Claude Code SDK needs `CLAUDE_BIN_PATH` set to the absolute path of its `cli.js`. The `archon setup` wizard in Step 4 auto-detects this via `npm root -g` and writes it to `~/.archon/.env` — no manual action needed in the typical case. Source installs (`bun run`) don't need this; the SDK finds `cli.js` via `node_modules` automatically.
+> **Note — Claude Code binary path.** Archon does not bundle Claude Code. In compiled Archon binaries (quick install, Homebrew), set `CLAUDE_BIN_PATH` to the Claude executable path (native `claude` binary or npm `cli.js`). The `archon setup` wizard in Step 4 auto-detects common locations and writes it to `~/.archon/.env` in typical setups. Source installs (`bun run`) usually don't need this because resolution can use `node_modules`.📝 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.
Suggested change
| > **Note — Claude Code binary path.** Archon does not bundle Claude Code. In compiled Archon binaries (quick install, Homebrew), the Claude Code SDK needs `CLAUDE_BIN_PATH` set to the absolute path of its `cli.js`. The `archon setup` wizard in Step 4 auto-detects this via `npm root -g` and writes it to `~/.archon/.env` — no manual action needed in the typical case. Source installs (`bun run`) don't need this; the SDK finds `cli.js` via `node_modules` automatically. | |
| > **Note — Claude Code binary path.** Archon does not bundle Claude Code. In compiled Archon binaries (quick install, Homebrew), set `CLAUDE_BIN_PATH` to the Claude executable path (native `claude` binary or npm `cli.js`). The `archon setup` wizard in Step 4 auto-detects common locations and writes it to `~/.archon/.env` in typical setups. Source installs (`bun run`) usually don't need this because resolution can use `node_modules`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/archon/guides/setup.md at line 122, Update the Note about
CLAUDE_BIN_PATH in the Archon setup guide: clarify that CLAUDE_BIN_PATH can
point to either the Claude Code SDK's cli.js or a native claude binary, and that
the archon setup wizard (Step 4 in `archon setup`) performs broader detection
(not only `npm root -g`) and resolves common global and system paths before
writing the resolved path to `~/.archon/.env`; retain the remark that source
installs (e.g., `bun run`) do not require setting CLAUDE_BIN_PATH because the
SDK finds `cli.js` via `node_modules`.
|
|
||
| - Exit code 0 | ||
| - The Claude subprocess spawns successfully (no `spawn EACCES`, `ENOENT`, or `process exited with code 1` in the early output) | ||
| - No `Claude Code CLI not found` error (that means the resolver rejected the configured path — verify the cli.js actually exists) |
There was a problem hiding this comment.
Minor text inconsistency with actual error message.
The pass criteria references "Claude Code CLI not found" but the actual error message from INSTALL_INSTRUCTIONS in binary-resolver.ts is "Claude Code not found" (without "CLI"). Consider aligning for consistency.
📝 Suggested fix
-- No `Claude Code CLI not found` error (that means the resolver rejected the configured path — verify the cli.js actually exists)
+- No `Claude Code not found` error (that means the resolver rejected the configured path — verify the cli.js actually exists)📝 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.
Suggested change
| - No `Claude Code CLI not found` error (that means the resolver rejected the configured path — verify the cli.js actually exists) | |
| - No `Claude Code not found` error (that means the resolver rejected the configured path — verify the cli.js actually exists) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/test-release/SKILL.md at line 251, Update the pass criteria
text in SKILL.md to match the actual error string emitted by
INSTALL_INSTRUCTIONS in binary-resolver.ts: change "Claude Code CLI not found"
to "Claude Code not found" (or alternatively update INSTALL_INSTRUCTIONS to
include "CLI" if you prefer consistency the other way); locate the
INSTALL_INSTRUCTIONS symbol in binary-resolver.ts and the pass criteria sentence
in SKILL.md and make the text identical across both files.
Comment on lines
+111
to
+117
| # Pre-configure the Claude Code SDK cli.js path for any consumer that runs | ||
| # a compiled Archon binary inside (or extending) this image. In source mode | ||
| # (the default `bun run start` ENTRYPOINT), BUNDLED_IS_BINARY is false and | ||
| # this variable is ignored — the SDK resolves cli.js via node_modules. Kept | ||
| # here so extenders don't need to rediscover the path. | ||
| # Path matches the hoisted layout produced by `bun install --linker=hoisted`. | ||
| ENV CLAUDE_BIN_PATH=/app/node_modules/@anthropic-ai/claude-agent-sdk/cli.js |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which Claude package is actually installed and contains cli.js
echo "=== Checking package.json dependencies ==="
rg -n "claude" package.json packages/providers/package.json
echo ""
echo "=== Looking for cli.js references in codebase ==="
rg -n "claude.*cli\.js" --type tsRepository: coleam00/Archon
Length of output: 2302
🏁 Script executed:
# Let's read the binary-resolver.ts file to understand the context
head -60 packages/providers/src/claude/binary-resolver.tsRepository: coleam00/Archon
Length of output: 2643
🏁 Script executed:
# Also check the Dockerfile context around the line in question
head -120 Dockerfile | tail -20Repository: coleam00/Archon
Length of output: 1048
🏁 Script executed:
# Search for any package.json or documentation that mentions both package names
rg -n "claude-code|claude-agent-sdk" --type json --type md | head -30Repository: coleam00/Archon
Length of output: 4716
Fix Dockerfile: CLAUDE_BIN_PATH must point to @anthropic-ai/claude-code, not @anthropic-ai/claude-agent-sdk.
The Dockerfile sets ENV CLAUDE_BIN_PATH=/app/node_modules/@anthropic-ai/claude-agent-sdk/cli.js, but this is incorrect. Per the CHANGELOG and source code, @anthropic-ai/claude-agent-sdk is the SDK package that Archon uses; it internally spawns a separate Claude Code executable. The cli.js file belongs to @anthropic-ai/claude-code, not the SDK.
All documentation, install instructions in binary-resolver.ts, and the CHANGELOG consistently reference @anthropic-ai/claude-code/cli.js as the correct path. Update line 117 to:
ENV CLAUDE_BIN_PATH=/app/node_modules/@anthropic-ai/claude-code/cli.js
Note: @anthropic-ai/claude-code is not currently listed in the Dockerfile's or root package.json dependencies, but it will be installed as a peer dependency or must be installed separately in compiled binary deployments (as documented in binary-resolver.ts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 111 - 117, The Dockerfile sets ENV CLAUDE_BIN_PATH
to the wrong package path; change the environment variable CLAUDE_BIN_PATH so it
points to the Claude Code package's cli.js (i.e.,
/app/node_modules/@anthropic-ai/claude-code/cli.js) instead of
/app/node_modules/@anthropic-ai/claude-agent-sdk/cli.js, ensuring the Docker
image and any compiled Archon binary will resolve the correct executable
referenced by binary-resolver.ts and the CHANGELOG.
Comment on lines
+16
to
+17
| > required. If you build a custom image that omits the npm install, set `CLAUDE_BIN_PATH` yourself | ||
| > to point at a mounted `cli.js` (see [AI Assistants → Binary path configuration](/getting-started/ai-assistants/#binary-path-configuration-compiled-binaries-only)). |
There was a problem hiding this comment.
Broaden CLAUDE_BIN_PATH guidance beyond cli.js.
Line 16-Line 17 currently implies custom images must point to a mounted cli.js. The resolver now supports native Claude executables too, so this wording is too restrictive and may mislead users.
Suggested doc tweak
-> required. If you build a custom image that omits the npm install, set `CLAUDE_BIN_PATH` yourself
-> to point at a mounted `cli.js` (see [AI Assistants → Binary path configuration](/getting-started/ai-assistants/#binary-path-configuration-compiled-binaries-only)).
+> required. If you build a custom image that omits the npm install, set `CLAUDE_BIN_PATH` yourself
+> to the Claude executable path (native `claude` binary or npm `cli.js`; see [AI Assistants → Binary path configuration](/getting-started/ai-assistants/#binary-path-configuration-compiled-binaries-only)).📝 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.
Suggested change
| > required. If you build a custom image that omits the npm install, set `CLAUDE_BIN_PATH` yourself | |
| > to point at a mounted `cli.js` (see [AI Assistants → Binary path configuration](/getting-started/ai-assistants/#binary-path-configuration-compiled-binaries-only)). | |
| > required. If you build a custom image that omits the npm install, set `CLAUDE_BIN_PATH` yourself | |
| > to the Claude executable path (native `claude` binary or npm `cli.js`; see [AI Assistants → Binary path configuration](/getting-started/ai-assistants/#binary-path-configuration-compiled-binaries-only)). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs-web/src/content/docs/deployment/docker.md` around lines 16 -
17, Update the CLAUDE_BIN_PATH guidance to mention that it can point to either a
mounted cli.js or a native Claude executable (not just cli.js); modify the
sentence referencing CLAUDE_BIN_PATH in the docs/deployment/docker.md content so
it explains both supported resolver binary types and links to the existing
Binary path configuration section, ensuring CLAUDE_BIN_PATH is described as
accepting the path to the appropriate binary/executable for the resolver.
Comment on lines
+79
to
+90
| test('throws with install instructions when nothing configured', async () => { | ||
| fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false); | ||
|
|
||
| const promise = resolver.resolveClaudeBinaryPath(); | ||
| await expect(promise).rejects.toThrow('Claude Code not found'); | ||
| await expect(promise).rejects.toThrow('CLAUDE_BIN_PATH'); | ||
| // Native curl installer is Anthropic's primary recommendation. | ||
| await expect(promise).rejects.toThrow('https://claude.ai/install.sh'); | ||
| // npm path is still documented as an alternative. | ||
| await expect(promise).rejects.toThrow('npm install -g @anthropic-ai/claude-code'); | ||
| await expect(promise).rejects.toThrow('claudeBinaryPath'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how Bun handles multiple await expect().rejects on the same promise
cd packages/providers
bun test src/claude/binary-resolver.test.ts --timeout 30000 2>&1 | head -50Repository: coleam00/Archon
Length of output: 100
🏁 Script executed:
cat -n packages/providers/src/claude/binary-resolver.test.tsRepository: coleam00/Archon
Length of output: 4266
🏁 Script executed:
# Search for how similar patterns are used in other test files
fd -e test.ts -o -e test.tsx | head -20 | xargs -I {} sh -c 'grep -l "await expect.*rejects" {} 2>/dev/null' | head -5Repository: coleam00/Archon
Length of output: 175
🌐 Web query:
Bun test expect().rejects.toThrow() multiple calls same promise
💡 Result:
In Bun test (as of 2026-04-14), you cannot reliably make multiple expect.rejects.toThrow calls on the same promise. The expect(promise).rejects matcher awaits and consumes the promise synchronously during the first assertion, so subsequent calls on the same promise will fail because the promise is already settled. Official Bun documentation shows examples like expect(Promise.reject(new Error)).rejects.toBeInstanceOf(Error) (single matcher) and supports chaining one matcher after .rejects or .resolves, such as .rejects.toBe, .rejects.toThrow, etc. However, no examples or documentation support multiple separate expect chains on the same promise instance. Bun's implementation differs from Jest: Bun's promise matchers block synchronously until settlement (see issue discussions), preventing reuse. Workarounds for multiple assertions on the rejection reason: 1. Use expect.assertions(n) with try-catch: test('multiple checks', async => { expect.assertions(2); try { await rejectingPromise; } catch (err) { expect(err).toBeInstanceOf(Error); expect(err.message).toMatch(/expected/); } }); 2. Single chain if possible, e.g., expect(promise).rejects.toSatisfy(err => { expect(err.message).toBe('foo'); return true; }) (but Bun may pass promise instead of value in some matchers like toSatisfy). 3. Create separate promises or use test.each for multiple cases. Some .rejects.toThrow combinations have historical bugs (e.g., issues #5602, #4909), but basic .rejects.toBe(errorValue) works per docs and PR #3318. For Jest compatibility, avoid reusing the same promise across multiple expects.
Citations:
- 1: https://bun.sh/docs/test/writing-tests
- 2: https://bun.com/reference/bun/test/Expect
- 3:
expect().resolvesandexpect().rejectsoven-sh/bun#3318 - 4: Fix
expect().rejects.toThrow()oven-sh/bun#5602 - 5: bun:test expect is sync and doesn't work with .rejects oven-sh/bun#4909
- 6:
expect(...).resolves.toSatisfy(...)passes in the promise directly oven-sh/bun#15428
Rewrite multiple await expect() calls to avoid promise reuse anti-pattern in Bun tests.
Bun's expect(promise).rejects matcher consumes the promise on the first assertion, making subsequent calls on the same promise instance unreliable. The test should capture the error once and assert on the message:
Suggested fix
test('throws with install instructions when nothing configured', async () => {
- fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false);
-
- const promise = resolver.resolveClaudeBinaryPath();
- await expect(promise).rejects.toThrow('Claude Code not found');
- await expect(promise).rejects.toThrow('CLAUDE_BIN_PATH');
- // Native curl installer is Anthropic's primary recommendation.
- await expect(promise).rejects.toThrow('https://claude.ai/install.sh');
- // npm path is still documented as an alternative.
- await expect(promise).rejects.toThrow('npm install -g `@anthropic-ai/claude-code`');
- await expect(promise).rejects.toThrow('claudeBinaryPath');
+ fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false);
+
+ let error: Error | undefined;
+ try {
+ await resolver.resolveClaudeBinaryPath();
+ } catch (e) {
+ error = e as Error;
+ }
+ expect(error).toBeDefined();
+ expect(error!.message).toContain('Claude Code not found');
+ expect(error!.message).toContain('CLAUDE_BIN_PATH');
+ // Native curl installer is Anthropic's primary recommendation.
+ expect(error!.message).toContain('https://claude.ai/install.sh');
+ // npm path is still documented as an alternative.
+ expect(error!.message).toContain('npm install -g `@anthropic-ai/claude-code`');
+ expect(error!.message).toContain('claudeBinaryPath');
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/claude/binary-resolver.test.ts` around lines 79 - 90,
The test currently reuses the same promise returned by
resolver.resolveClaudeBinaryPath() across multiple await expect(...).rejects
assertions (in test 'throws with install instructions when nothing configured'),
which is an anti-pattern in Bun; instead call resolver.resolveClaudeBinaryPath()
once, await it inside a try/catch (or capture the rejection via const err =
await resolver.resolveClaudeBinaryPath().catch(e => e)), then run multiple
synchronous assertions against err.message to check it contains 'Claude Code not
found', 'CLAUDE_BIN_PATH', 'https://claude.ai/install.sh', 'npm install -g
`@anthropic-ai/claude-code`', and 'claudeBinaryPath' so you only consume the
promise once.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/docs-web/src/content/docs/reference/troubleshooting.md (1)
287-290: Add language specifier to fenced code block.The code block showing the error message is missing a language identifier, which triggers a markdownlint warning (MD040). Use
textorplaintextfor non-code output.📝 Proposed fix
-``` +```text Claude Code not found. Archon requires the Claude Code executable to be reachable at a configured path in compiled builds.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/docs-web/src/content/docs/reference/troubleshooting.mdaround lines
287 - 290, The fenced code block containing the error message "Claude Code not
found. Archon requires the Claude Code executable to be reachable at a
configured path in compiled builds." is missing a language specifier and
triggers markdownlint MD040; update that fenced block in
packages/docs-web/src/content/docs/reference/troubleshooting.md to use a
plain-text specifier (e.g.,text orplaintext) so the block is marked as
non-code output and the lint warning is resolved.</details> </blockquote></details> <details> <summary>packages/cli/src/commands/setup.ts (1)</summary><blockquote> `473-491`: **Consider validating that user-provided path is absolute.** The prompt asks for an "Absolute path" but doesn't validate that the input is actually absolute. Relative paths would be resolved against the current working directory at runtime, which may differ from where setup was run. <details> <summary>🛡️ Proposed validation</summary> ```diff +import { isAbsolute } from 'path'; + const trimmed = (customPath ?? '').trim(); if (!trimmed) return undefined; + if (!isAbsolute(trimmed)) { + log.warning( + `Path "${trimmed}" is not absolute. Please provide a full path starting from root.` + ); + return undefined; + } + if (!existsSync(trimmed)) { log.warning( ``` </details> Note: `isAbsolute` is already imported from `path` at line 26, so the import addition is not needed. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/setup.ts` around lines 473 - 491, The prompt accepts a "Absolute path" but doesn't validate it; update the logic after computing trimmed (from customPath) to check path.isAbsolute(trimmed) and handle non-absolute input: if !isAbsolute(trimmed) log a clear warning/error (using log.warning or similar) telling the user the path must be absolute and return undefined (or re-prompt if desired) instead of accepting a relative path; keep the existing existsSync(trimmed) check for existence after the absolute check so the code that references trimmed only gets absolute paths. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@packages/cli/src/commands/setup.ts:
- Around line 473-491: The prompt accepts a "Absolute path" but doesn't validate
it; update the logic after computing trimmed (from customPath) to check
path.isAbsolute(trimmed) and handle non-absolute input: if !isAbsolute(trimmed)
log a clear warning/error (using log.warning or similar) telling the user the
path must be absolute and return undefined (or re-prompt if desired) instead of
accepting a relative path; keep the existing existsSync(trimmed) check for
existence after the absolute check so the code that references trimmed only gets
absolute paths.In
@packages/docs-web/src/content/docs/reference/troubleshooting.md:
- Around line 287-290: The fenced code block containing the error message
"Claude Code not found. Archon requires the Claude Code executable to be
reachable at a configured path in compiled builds." is missing a language
specifier and triggers markdownlint MD040; update that fenced block in
packages/docs-web/src/content/docs/reference/troubleshooting.md to use a
plain-text specifier (e.g.,text orplaintext) so the block is marked as
non-code output and the lint warning is resolved.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `20c1c99d-0a1a-4622-a7b9-ea95b98f02f7` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 357d8cc695d51f81381bf0a43a94b6093dfaef00 and 44ec6e3023b20e1d2abaa6efbe34fc6a056b200d. </details> <details> <summary>📒 Files selected for processing (9)</summary> * `CLAUDE.md` * `packages/cli/src/commands/setup.test.ts` * `packages/cli/src/commands/setup.ts` * `packages/docs-web/src/content/docs/reference/configuration.md` * `packages/docs-web/src/content/docs/reference/troubleshooting.md` * `packages/providers/package.json` * `packages/providers/src/claude/provider.test.ts` * `packages/providers/src/claude/provider.ts` * `packages/providers/src/index.ts` </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * CLAUDE.md * packages/docs-web/src/content/docs/reference/configuration.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * packages/providers/package.json * packages/cli/src/commands/setup.test.ts </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
The "tier 2 wins (npm cli.js)" test hardcoded forward-slash path comparisons, but `path.join` produces backslashes on Windows. Caused the Windows CI leg of the test suite to fail while macOS and Linux passed. Use `path.join` for both the mock return value and the expectation so the separator matches whatever the platform produces.
This was referenced Apr 20, 2026
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…solver (coleam00#1217) * feat(providers): replace Claude SDK embed with explicit binary-path resolver Drop `@anthropic-ai/claude-agent-sdk/embed` and resolve Claude Code via CLAUDE_BIN_PATH env → assistants.claude.claudeBinaryPath config → throw with install instructions. The embed's silent failure modes on macOS (coleam00#1210) and Windows (coleam00#1087) become actionable errors with a documented recovery path. Dev mode (bun run) remains auto-resolved via node_modules. The setup wizard auto-detects Claude Code by probing the native installer path (~/.local/bin/claude), npm global cli.js, and PATH, then writes CLAUDE_BIN_PATH to ~/.archon/.env. Dockerfile pre-sets CLAUDE_BIN_PATH so extenders using the compiled binary keep working. Release workflow gets negative and positive resolver smoke tests. Docs, CHANGELOG, README, .env.example, CLAUDE.md, test-release and archon skills all updated to reflect the curl-first install story. Retires coleam00#1210, coleam00#1087, coleam00#1091 (never merged, now obsolete). Implements coleam00#1176. * fix(providers): only pass --no-env-file when spawning Claude via Bun/Node `--no-env-file` is a Bun flag that prevents Bun from auto-loading `.env` from the subprocess cwd. It is only meaningful when the Claude Code executable is a `cli.js` file — in which case the SDK spawns it via `bun`/`node` and the flag reaches the runtime. When `CLAUDE_BIN_PATH` points at a native compiled Claude binary (e.g. `~/.local/bin/claude` from the curl installer, which is Anthropic's recommended default), the SDK executes the binary directly. Passing `--no-env-file` then goes straight to the native binary, which rejects it with `error: unknown option '--no-env-file'` and the subprocess exits code 1. Emit `executableArgs` only when the target is a `.js` file (dev mode or explicit cli.js path). Caught by end-to-end smoke testing against the curl-installed native Claude binary. * docs: record env-leak validation result in provider comment Verified end-to-end with sentinel `.env` and `.env.local` files in a workflow CWD that the native Claude binary (curl installer) does not auto-load `.env` files. With Archon's full spawn pathway and parent env stripped, the subprocess saw both sentinels as UNSET. The first-layer protection in `@archon/paths` (coleam00#1067) handles the inheritance leak; `--no-env-file` only matters for the Bun-spawned cli.js path, where it is still emitted. * chore(providers): cleanup pass — exports, docs, troubleshooting Final-sweep cleanup tied to the binary-resolver PR: - Mirror Codex's package surface for the new Claude resolver: add `./claude/binary-resolver` subpath export and re-export `resolveClaudeBinaryPath` + `claudeFileExists` from the package index. Renames the previously single `fileExists` re-export to `codexFileExists` for symmetry; nothing outside the providers package was importing it. - Add a "Claude Code not found" entry to the troubleshooting reference doc with platform-specific install snippets and pointers to the AI Assistants binary-path section. - Reframe the example claudeBinaryPath in reference/configuration.md away from cli.js-only language; it accepts either the native binary or cli.js. * test+refactor(providers, cli): address PR review feedback Two test gaps and one doc nit from the PR review (coleam00#1217): - Extract the `--no-env-file` decision into a pure exported helper `shouldPassNoEnvFile(cliPath)` so the native-binary branch is unit testable without mocking `BUNDLED_IS_BINARY` or running the full sendQuery pathway. Six new tests cover undefined, cli.js, native binary (Linux + Windows), Homebrew symlink, and suffix-only matching. Also adds a `claude.subprocess_env_file_flag` debug log so the security-adjacent decision is auditable. - Extract the three install-location probes in setup.ts into exported wrappers (`probeFileExists`, `probeNpmRoot`, `probeWhichClaude`) and export `detectClaudeExecutablePath` itself, so the probe order can be spied on. Six new tests cover each tier winning, fall-through ordering, npm-tier skip when not installed, and the which-resolved-but-stale-path edge case. - CLAUDE.md `claudeBinaryPath` placeholder updated to reflect that the field accepts either the native binary or cli.js (the example value was previously `/absolute/path/to/cli.js`, slightly misleading now that the curl-installer native binary is the default). Skipped from the review by deliberate scope decision: - `resolveClaudeBinaryPath` async-with-no-await: matches Codex's resolver signature exactly. Changing only Claude breaks symmetry; if pursued, do both providers in a separate cleanup PR. - `isAbsolute()` validation in parseClaudeConfig: Codex doesn't do it either. Resolver throws on non-existence already. - Atomic `.env` writes in setup wizard: pre-existing pattern this PR touched only adjacently. File as separate issue if needed. - classifyError branch in dag-executor for setup errors: scope creep. - `.env.example` "missing #" claim: false positive (verified all CLAUDE_BIN_PATH lines have proper comment prefixes). * fix(test): use path.join in Windows-compatible probe-order test The "tier 2 wins (npm cli.js)" test hardcoded forward-slash path comparisons, but `path.join` produces backslashes on Windows. Caused the Windows CI leg of the test suite to fail while macOS and Linux passed. Use `path.join` for both the mock return value and the expectation so the separator matches whatever the platform produces.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cli.jsvia@anthropic-ai/claude-agent-sdk/embed, which depends on Bun'swith { type: 'file' }asset import. The pipeline silently failed on macOS (bug: compiled binary cannot invoke Claude Code — SDK cli.js path hardcoded to CI runner filesystem #1210) and Windows (Claude Code process exited with code 1 via webui #1087) when Bun's bundler did not produce a usable$bunfspath — the SDK passed a build-host filesystem path to spawn, and users got a cryptic "Module not found /Users/runner/..." error with no recovery handle./embedimport. Resolve Claude Code via two tiers:CLAUDE_BIN_PATHenv →assistants.claude.claudeBinaryPathconfig → throw with install instructions. Setup wizard auto-detects via probing~/.local/bin/claude(Anthropic's curl-installer default),$(npm root -g)/@anthropic-ai/claude-code/cli.js, then PATH. Dockerfile pre-setsCLAUDE_BIN_PATHsodocker runkeeps working. Release workflow gets negative + positive resolver smoke tests.bun run) — SDK still resolves vianode_modules. The first-layer env-leak defense (stripCwdEnv()from 2 issues: v0.3.5: CLI workflow run silently hangs — dotenv loads .env from CWD instead of ~/.archon/.env,, + rchon serve hardcodes skipPlatformAdapters:true — Telegram/Discord/Slack adapters are unreachable #1067) is unchanged. Codex provider behavior is unchanged. No database migration. No new dependencies.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
await resolveClaudeBinaryPath()insendQueryparseClaudeConfigclaudeBinaryPathfieldCLAUDE_BIN_PATHlineENV CLAUDE_BIN_PATH=...Label Snapshot
risk: medium(breaking change for compiled binary users; well-documented migration; resolver tests + end-to-end smoke verified)size: M(25 files, 751 insertions / 17 deletions; bulk is docs)core, providers, cli, ci, docs, dependenciesproviders:claude,cli:setup,ci:releaseChange Metadata
bug(primary: fixes bug: compiled binary cannot invoke Claude Code — SDK cli.js path hardcoded to CI runner filesystem #1210, Claude Code process exited with code 1 via webui #1087) +feature(secondary: implements feat: add claudeBinaryPath config option for custom Claude Code install paths #1176)core(via providers package)Linked Issue
claudeBinaryPathconfig option~BUNextraction fix. This PR takes a different approach (drop the embed entirely); the draft becomes obsolete once this lands but remains open for the author to close.Validation Evidence (required)
End-to-end smoke testing (against the actual compiled binary):
versionfrom compiled binaryBuild: binary— correctworkflow listfrom compiled binarye2e-claude-smoke(3 nodes via SDK)4,{"category":"math"},`@archon/providers`)CLAUDE_BIN_PATH)CLAUDE_BIN_PATH=~/.local/bin/claude)/api/health,/api/providers,/api/commands.env+.env.localin workflow CWD)UNSETin the spawned bash tool — env protection intactSmoke testing surfaced one real bug in the initial implementation (
--no-env-filewas rejected by the native Claude binary) — fixed in commit0fdf1402.Security Impact (required)
stripCwdEnv()first-layer protection from 2 issues: v0.3.5: CLI workflow run silently hangs — dotenv loads .env from CWD instead of ~/.archon/.env,, + rchon serve hardcodes skipPlatformAdapters:true — Telegram/Discord/Slack adapters are unreachable #1067 unchanged. The second-layer Bun--no-env-fileflag is still passed for cli.js paths; native binaries don't have Bun's auto-load behavior (verified end-to-end with sentinel.envfiles — both arrived UNSET in the spawned bash tool).Compatibility / Migration
bun run) are unaffected.CLAUDE_BIN_PATHenv var andassistants.claude.claudeBinaryPathconfig option.Upgrade steps for compiled-binary users:
archon setupauto-detects and writesCLAUDE_BIN_PATHfor users who run the wizard. Docker users are unaffected (image pre-sets the env var). npm-installed users can point at$(npm root -g)/@anthropic-ai/claude-code/cli.js.Human Verification (required)
What was personally validated beyond CI:
archon-darwin-arm64binary on macOS 26.3 (built viascripts/build-binaries.sh TARGET=bun-darwin-arm64 …)~/.local/bin/claudev2.1.107) → workflow completed with correct outpute2e-claude-smoke— 3 nodes including structured output and tool use) returned correct resultsbun run cli) still works without any config.env+.env.localin workflow CWD via Archon's full spawn pathway) → sentinels arrivedUNSET--no-env-fileflag is now conditional oncliPath?.endsWith('.js')— caught a real bug where the native binary rejected the unknown flagThe release-workflow smoke test added in this PR exercises the resolver on
bun-linux-x64in CI, covering the platforms I couldn't execute locally.Side Effects / Blast Radius (required)
CLAUDE_BIN_PATH. Migration is one env var or one YAML line; the error message walks them through it.--no-env-fileconditional change could in theory leave some Bun-runtime use case uncovered. Mitigated by: (a)cliPath?.endsWith('.js')covers dev mode (cliPath undefined) and npm cli.js, (b) end-to-end env-leak probe verified no leak via the production spawn pathway, (c) the parent-processstripCwdEnv()is the load-bearing protection regardless.db.orphaned_workflow_runs_failedlog event would surface server-side issues (separate bug, see bug: server startup marks actively-running workflows as failed via failOrphanedRuns() #1216)--no-env-filereasoning and verification result for future maintainersRollback Plan (required)
git revert 33a242e2 0fdf1402 357d8cc6ondev. Three commits, atomic, fully reversible. No DB changes to roll back.Claude Code not foundon first run after upgrade if user hasn't configuredCLAUDE_BIN_PATH(expected, documented behavior)--no-env-filecan be made unconditional again with a one-line revert of0fdf1402plus accepting that native-binary users will see the "unknown option" errorRisks and Mitigations
~/.local/bin/claude.which claudeis the third probe tier, so any future install path that putsclaudeon PATH still works. If Anthropic moves away from PATH entirely, that's a much larger problem affecting all SDK consumers.--no-env-fileconditional change could leak env if a future Bun version's auto-load behavior changes for native binaries.stripCwdEnv()defense from 2 issues: v0.3.5: CLI workflow run silently hangs — dotenv loads .env from CWD instead of ~/.archon/.env,, + rchon serve hardcodes skipPlatformAdapters:true — Telegram/Discord/Slack adapters are unreachable #1067 still strips CWD env keys from the parent before spawn, so the subprocess inherits a clean env regardless of whether the second-layer Bun flag is in effect. End-to-end probe with sentinel.envfiles verified the protection holds today; comment in provider.ts documents the assumption for future maintainers.Summary by CodeRabbit
Breaking Changes
New Features
CLAUDE_BIN_PATHenv var andassistants.claude.claudeBinaryPathconfig for compiled binaries.Documentation
Bug Fixes