-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(providers): replace Claude SDK embed with explicit binary-path resolver #1217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
33a242e
0fdf140
357d8cc
73b012b
44ec6e3
556879b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -222,7 +222,23 @@ git commit -q --allow-empty -m init | |||||
|
|
||||||
| ### Test 3 — SDK path works (assist workflow) | ||||||
|
|
||||||
| In the same `$TESTREPO`: | ||||||
| **Prerequisite.** Compiled binaries require Claude Code installed on the host and a configured binary path. Before running this test, ensure one of: | ||||||
|
|
||||||
| ```bash | ||||||
| # Option A — env var (easy for ad-hoc testing) | ||||||
| # After the native installer (Anthropic's default): | ||||||
| export CLAUDE_BIN_PATH="$HOME/.local/bin/claude" | ||||||
| # Or after npm global install: | ||||||
| export CLAUDE_BIN_PATH="$(npm root -g)/@anthropic-ai/claude-code/cli.js" | ||||||
|
|
||||||
| # Option B — config file (persistent) | ||||||
| # Add to ~/.archon/config.yaml: | ||||||
| # assistants: | ||||||
| # claude: | ||||||
| # claudeBinaryPath: /absolute/path/to/claude | ||||||
| ``` | ||||||
|
|
||||||
| Then in the same `$TESTREPO`: | ||||||
|
|
||||||
| ```bash | ||||||
| "$BINARY" workflow run assist "say hello and nothing else" 2>&1 | tee /tmp/archon-test-assist.log | ||||||
|
|
@@ -232,15 +248,34 @@ In the same `$TESTREPO`: | |||||
|
|
||||||
| - 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor text inconsistency with actual error message. The pass criteria references "Claude Code CLI not found" but the actual error message from 📝 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
Suggested change
🤖 Prompt for AI Agents |
||||||
| - A response is produced (any response — even just "hello" — proves the SDK round-trip works) | ||||||
|
|
||||||
| **Common failures:** | ||||||
|
|
||||||
| - `Claude Code not found` → `CLAUDE_BIN_PATH` / `claudeBinaryPath` is unset or points at a non-existent file. Fix the path and re-run. | ||||||
| - `Module not found "/Users/runner/..."` → regression of #1210: the resolver was bypassed and the SDK's `import.meta.url` fallback leaked a build-host path. Investigate `packages/providers/src/claude/provider.ts` and the resolver. | ||||||
| - `Credit balance is too low` → auth is pointing at an exhausted API key (check `CLAUDE_USE_GLOBAL_AUTH` and `~/.archon/.env`) | ||||||
| - `unable to determine transport target for "pino-pretty"` → #960 regression, binary crashes on TTY | ||||||
| - `package.json not found (bad installation?)` → #961 regression, `isBinaryBuild` detection broken | ||||||
| - Process exits before producing output → generic spawn failure, capture stderr | ||||||
|
|
||||||
| ### Test 3b — Resolver error path (run without `CLAUDE_BIN_PATH`) | ||||||
|
|
||||||
| Quickly verify the resolver fails loud when nothing is configured: | ||||||
|
|
||||||
| ```bash | ||||||
| (unset CLAUDE_BIN_PATH; "$BINARY" workflow run assist "hello" 2>&1 | tee /tmp/archon-test-no-path.log) | ||||||
| ``` | ||||||
|
|
||||||
| **Pass criteria (when no `~/.archon/config.yaml` configures `claudeBinaryPath`):** | ||||||
|
|
||||||
| - Error message contains `Claude Code not found` | ||||||
| - Error message mentions both `CLAUDE_BIN_PATH` and `claudeBinaryPath` as remediation options | ||||||
| - No `Module not found` stack traces referencing the CI filesystem | ||||||
|
|
||||||
| If you *do* have `claudeBinaryPath` set globally, skip this test or temporarily rename `~/.archon/config.yaml`. | ||||||
|
|
||||||
| ### Test 4 — Env-leak gate refuses a leaky .env (optional, for releases including #1036/#1038/#983) | ||||||
|
|
||||||
| Create a second throwaway repo with a fake sensitive key: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,6 +108,14 @@ RUN apt-get update && apt-get install -y --no-install-recommends nodejs npm \ | |
| # Point agent-browser to system Chromium (avoids ~400MB Chrome for Testing download) | ||
| ENV AGENT_BROWSER_EXECUTABLE_PATH=/usr/bin/chromium | ||
|
|
||
| # 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 | ||
|
Comment on lines
+111
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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: The Dockerfile sets All documentation, install instructions in Note: 🤖 Prompt for AI Agents |
||
|
|
||
| # Create non-root user for running Claude Code | ||
| # Claude Code refuses to run with --dangerously-skip-permissions as root for security | ||
| RUN useradd -m -u 1001 -s /bin/bash appuser \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update setup note to reflect full resolver/detection behavior.
Line 122 still frames
CLAUDE_BIN_PATHascli.js-only and cites onlynpm root -g. The current flow supports nativeclaudebinary paths too and setup detection is broader.Suggested wording update
📝 Committable suggestion
🤖 Prompt for AI Agents