Skip to content

fix(providers/claude): reject directory paths and expand npm package dirs (#1723) #1737

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.

Sign up for GitHub

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

Merged
Wirasm merged 3 commits into from
May 21, 2026
Merged

fix(providers/claude): reject directory paths and expand npm package dirs (#1723) #1737

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e8e4b7
fix(providers/claude): reject directory paths and expand npm package …
Wirasm May 21, 2026
4e6727e
fix(providers/claude): address self-review — broken-symlink test + co…
Wirasm May 21, 2026
f762e48
fix(providers/claude): address review — autodetect parity, EACCES bre…
Wirasm May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/providers/src/claude/binary-resolver-dev.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import * as resolver from './binary-resolver';

describe('resolveClaudeBinaryPath (dev mode)', () => {
const originalEnv = process.env.CLAUDE_BIN_PATH;
let fileExistsSpy: ReturnType<typeof spyOn> | undefined;
let pathKindSpy: ReturnType<typeof spyOn> | undefined;

beforeEach(() => {
delete process.env.CLAUDE_BIN_PATH;
fileExistsSpy?.mockRestore();
fileExistsSpy = undefined;
pathKindSpy?.mockRestore();
pathKindSpy = undefined;
});

afterAll(() => {
Expand All @@ -35,7 +35,7 @@ describe('resolveClaudeBinaryPath (dev mode)', () => {
} else {
delete process.env.CLAUDE_BIN_PATH;
}
fileExistsSpy?.mockRestore();
pathKindSpy?.mockRestore();
});

test('returns undefined when nothing is configured', async () => {
Expand All @@ -50,15 +50,15 @@ describe('resolveClaudeBinaryPath (dev mode)', () => {

test('honors CLAUDE_BIN_PATH env var when file exists', async () => {
process.env.CLAUDE_BIN_PATH = '/usr/local/bin/claude';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath();
expect(result).toBe('/usr/local/bin/claude');
});

test('throws when CLAUDE_BIN_PATH is set but file does not exist', async () => {
process.env.CLAUDE_BIN_PATH = '/nonexistent/claude';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('missing');

await expect(resolver.resolveClaudeBinaryPath()).rejects.toThrow(
'CLAUDE_BIN_PATH is set to "/nonexistent/claude" but the file does not exist'
Expand All @@ -67,7 +67,7 @@ describe('resolveClaudeBinaryPath (dev mode)', () => {

test('env var wins over config path in dev mode', async () => {
process.env.CLAUDE_BIN_PATH = '/env/claude';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath('/config/claude');
expect(result).toBe('/env/claude');
Expand Down
99 changes: 92 additions & 7 deletions packages/providers/src/claude/binary-resolver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ import * as resolver from './binary-resolver';
describe('resolveClaudeBinaryPath (binary mode)', () => {
const originalEnv = process.env.CLAUDE_BIN_PATH;
let fileExistsSpy: ReturnType<typeof spyOn>;
let pathKindSpy: ReturnType<typeof spyOn>;

beforeEach(() => {
delete process.env.CLAUDE_BIN_PATH;
fileExistsSpy?.mockRestore();
pathKindSpy?.mockRestore();
mockLogger.info.mockClear();
});

Expand All @@ -36,34 +38,35 @@ describe('resolveClaudeBinaryPath (binary mode)', () => {
delete process.env.CLAUDE_BIN_PATH;
}
fileExistsSpy?.mockRestore();
pathKindSpy?.mockRestore();
});

test('uses CLAUDE_BIN_PATH env var when set and file exists', async () => {
process.env.CLAUDE_BIN_PATH = '/usr/local/lib/node_modules/@anthropic-ai/claude-code/cli.js';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath();
expect(result).toBe('/usr/local/lib/node_modules/@anthropic-ai/claude-code/cli.js');
});

test('throws when CLAUDE_BIN_PATH is set but file does not exist', async () => {
process.env.CLAUDE_BIN_PATH = '/nonexistent/cli.js';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('missing');

await expect(resolver.resolveClaudeBinaryPath()).rejects.toThrow(
'CLAUDE_BIN_PATH is set to "/nonexistent/cli.js" but the file does not exist'
);
});

test('uses config claudeBinaryPath when file exists', async () => {
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath('/custom/claude/cli.js');
expect(result).toBe('/custom/claude/cli.js');
});

test('throws when config claudeBinaryPath file does not exist', async () => {
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(false);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('missing');

await expect(resolver.resolveClaudeBinaryPath('/nonexistent/cli.js')).rejects.toThrow(
'assistants.claude.claudeBinaryPath is set to "/nonexistent/cli.js" but the file does not exist'
Expand All @@ -72,7 +75,7 @@ describe('resolveClaudeBinaryPath (binary mode)', () => {

test('env var takes precedence over config path', async () => {
process.env.CLAUDE_BIN_PATH = '/env/cli.js';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath('/config/cli.js');
expect(result).toBe('/env/cli.js');
Expand Down Expand Up @@ -104,7 +107,7 @@ describe('resolveClaudeBinaryPath (binary mode)', () => {

test('env var takes precedence over autodetect when both would match', async () => {
process.env.CLAUDE_BIN_PATH = '/custom/env/claude';
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath();
expect(result).toBe('/custom/env/claude');
Expand All @@ -115,7 +118,7 @@ describe('resolveClaudeBinaryPath (binary mode)', () => {
});

test('config takes precedence over autodetect when both would match', async () => {
fileExistsSpy = spyOn(resolver, 'fileExists').mockReturnValue(true);
pathKindSpy = spyOn(resolver, 'pathKind').mockReturnValue('file');

const result = await resolver.resolveClaudeBinaryPath('/custom/config/claude');
expect(result).toBe('/custom/config/claude');
Expand All @@ -138,4 +141,86 @@ describe('resolveClaudeBinaryPath (binary mode)', () => {
await expect(promise).rejects.toThrow('npm install -g @anthropic-ai/claude-code');
await expect(promise).rejects.toThrow('claudeBinaryPath');
});

// ─── Directory expansion (issue #1723) ──────────────────────────────────
// The npm-distributed Claude Code package nests the native binary inside
// a platform-specific directory (`@anthropic-ai/claude-code-<platform>`).
// Users on Windows naturally configure that directory as
// `claudeBinaryPath`; the resolver must transparently expand it to the
// contained executable so the SDK's spawn doesn't ENOENT on a directory.
// ─────────────────────────────────────────────────────────────────────────

test('expands a configured directory to claude/claude.exe when the binary is present (config path)', async () => {
const dir = '/opt/claude-code-package';
const expectedFile = join(dir, process.platform === 'win32' ? 'claude.exe' : 'claude');
pathKindSpy = spyOn(resolver, 'pathKind').mockImplementation((p: string) => {
if (p === dir) return 'directory';
if (p === expectedFile) return 'file';
return 'missing';
});

const result = await resolver.resolveClaudeBinaryPath(dir);
expect(result).toBe(expectedFile);
// Log must show the expanded executable path, not the user's directory —
// operators triaging spawn issues need the actual path the SDK will use.
expect(mockLogger.info).toHaveBeenCalledWith(
{ binaryPath: expectedFile, source: 'config' },
'claude.binary_resolved'
);
});

test('expands a configured directory passed via CLAUDE_BIN_PATH', async () => {
const dir = '/opt/claude-code-package';
const expectedFile = join(dir, process.platform === 'win32' ? 'claude.exe' : 'claude');
process.env.CLAUDE_BIN_PATH = dir;
pathKindSpy = spyOn(resolver, 'pathKind').mockImplementation((p: string) => {
if (p === dir) return 'directory';
if (p === expectedFile) return 'file';
return 'missing';
});

const result = await resolver.resolveClaudeBinaryPath();
expect(result).toBe(expectedFile);
expect(mockLogger.info).toHaveBeenCalledWith(
{ binaryPath: expectedFile, source: 'env' },
'claude.binary_resolved'
);
});

test('throws a directory-specific error when config path is a directory missing the expected executable', async () => {
const dir = '/some/empty/dir';
pathKindSpy = spyOn(resolver, 'pathKind').mockImplementation((p: string) =>
p === dir ? 'directory' : 'missing'
);
const expected = process.platform === 'win32' ? 'claude.exe' : 'claude';

const promise = resolver.resolveClaudeBinaryPath(dir);
await expect(promise).rejects.toThrow('assistants.claude.claudeBinaryPath');
await expect(promise).rejects.toThrow('which is a directory');
await expect(promise).rejects.toThrow(`does not contain ${expected}`);
});

test('throws a directory-specific error when CLAUDE_BIN_PATH is a directory missing the expected executable', async () => {
const dir = '/some/empty/dir';
process.env.CLAUDE_BIN_PATH = dir;
pathKindSpy = spyOn(resolver, 'pathKind').mockImplementation((p: string) =>
p === dir ? 'directory' : 'missing'
);

const promise = resolver.resolveClaudeBinaryPath();
await expect(promise).rejects.toThrow('CLAUDE_BIN_PATH');
await expect(promise).rejects.toThrow('which is a directory');
});
});

describe('pathKind', () => {
test('returns "missing" for nonexistent paths', () => {
expect(resolver.pathKind('/definitely/does/not/exist/anywhere/12345')).toBe('missing');
});

// `pathKind` is a thin wrapper around `statSync`; the file/directory
// discrimination itself is tested via the resolver-level tests above
// (which spy on pathKind). The integration concern that *can't* be tested
// there — that the wrapper actually catches ENOENT instead of throwing —
// is asserted here.
});
87 changes: 67 additions & 20 deletions packages/providers/src/claude/binary-resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* undefined so the caller omits `pathToClaudeCodeExecutable` entirely and
* the SDK resolves via its normal node_modules lookup.
*/
import { existsSync as _existsSync } from 'node:fs';
import { existsSync as _existsSync, statSync as _statSync } from 'node:fs';
import { homedir } from 'node:os';
import { join } from 'node:path';
import { BUNDLED_IS_BINARY, createLogger } from '@archon/paths';
Expand All @@ -28,6 +28,64 @@ export function fileExists(path: string): boolean {
return _existsSync(path);
}

/**
* Classify a configured path. The Claude Agent SDK requires a spawnable file:
* a directory passes `existsSync` but fails downstream as ENOENT inside the
* SDK's `child_process.spawn`, surfaced as the misleading "native binary not
* found" error. Wrapped for spyOn parity with `fileExists`.
*
* Non-file, non-directory entries (sockets, FIFOs, etc.) are reported as
* 'missing' so the caller's "set to X but unusable" error path fires.
*/
export function pathKind(path: string): 'file' | 'directory' | 'missing' {
try {
const stat = _statSync(path);
if (stat.isFile()) return 'file';
if (stat.isDirectory()) return 'directory';
return 'missing';
} catch {
return 'missing';
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="packages/providers/src/claude/binary-resolver.ts"
echo "== Lines around pathKind() =="
nl -ba "$FILE" | sed -n '1,140p'

echo "== Search for validateAndExpand() usage and error messages =="
rg -n "validateAndExpand|pathKind\\(" "$FILE"

echo "== Lines around validateAndExpand() =="
nl -ba "$FILE" | sed -n '140,260p'

Repository: coleam00/Archon

Length of output: 150

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="packages/providers/src/claude/binary-resolver.ts"

echo "== Lines around pathKind() =="
sed -n '1,160p' "$FILE" | cat -n

echo "== Occurrences of validateAndExpand() and pathKind() in file =="
rg -n "validateAndExpand|pathKind\\(" "$FILE" || true

echo "== Lines around validateAndExpand() =="
# Print a wider range to ensure we capture function definition
sed -n '160,320p' "$FILE" | cat -n

Repository: coleam00/Archon

Length of output: 9362

Don’t collapse all statSync failures into "missing"

packages/providers/src/claude/binary-resolver.ts pathKind() catches all _statSync errors and returns 'missing', so validateAndExpand() can incorrectly report permission/access failures (e.g., EACCES/EPERM) as “file does not exist”.

Suggested fix 
 export function pathKind(path: string): 'file' | 'directory' | 'missing' {
   try {
     const stat = _statSync(path);
     if (stat.isFile()) return 'file';
     if (stat.isDirectory()) return 'directory';
     return 'missing';
-  } catch {
-    return 'missing';
+  } catch (error: unknown) {
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code === 'ENOENT' || code === 'ENOTDIR') return 'missing';
+    throw error;
   }
 }
📝 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
export function pathKind(path: string): 'file' | 'directory' | 'missing' {
try {
const stat = _statSync(path);
if (stat.isFile()) return 'file';
if (stat.isDirectory()) return 'directory';
return 'missing';
} catch {
return 'missing';
}
export function pathKind(path: string): 'file' | 'directory' | 'missing' {
try {
const stat = _statSync(path);
if (stat.isFile()) return 'file';
if (stat.isDirectory()) return 'directory';
return 'missing';
} catch (error: unknown) {
const code = (error as NodeJS.ErrnoException).code;
if (code === 'ENOENT' || code === 'ENOTDIR') return 'missing';
throw error;
}
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/providers/src/claude/binary-resolver.ts` around lines 40 - 48,
pathKind currently swallows all errors from _statSync and returns 'missing',
masking permission/access errors (EACCES/EPERM) so validateAndExpand misreports
them; update pathKind(path) to only treat ENOENT (and maybe ENOTDIR) as
'missing' and rethrow any other errors from _statSync (or return a distinct
result that validateAndExpand handles) so callers like validateAndExpand can
surface permission failures—update the function around the _statSync call and
the catch block accordingly, referencing pathKind and its use by
validateAndExpand.

}

/**
* If a configured path is a directory, expand to the platform-appropriate
* child executable (`claude.exe` on Windows, `claude` on Unix). Common when
* users point at the npm platform-package directory
* (`@anthropic-ai/claude-code-<platform>`), which contains the binary inside.
* Returns the expanded file path if present, otherwise undefined.
*/
function expandDirectoryToExecutable(dir: string): string | undefined {
const candidate = join(dir, process.platform === 'win32' ? 'claude.exe' : 'claude');
return pathKind(candidate) === 'file' ? candidate : undefined;
}

/**
* Validate a user-supplied path and, if a directory is given, expand to the
* platform-appropriate child executable. Distinguishes missing paths from
* directories-without-the-expected-binary so the error message tells the user
* what to fix.
*/
function validateAndExpand(rawPath: string, sourceLabel: string): string {
const kind = pathKind(rawPath);
if (kind === 'file') return rawPath;
if (kind === 'directory') {
const expanded = expandDirectoryToExecutable(rawPath);
if (expanded) return expanded;
const expected = process.platform === 'win32' ? 'claude.exe' : 'claude';
throw new Error(
`${sourceLabel} is set to "${rawPath}", which is a directory, but it does not contain ${expected}.\n` +
'Please point this setting at the Claude Code executable itself (native binary\n' +
'from the curl/PowerShell installer, or cli.js from an npm global install).'
);
}
throw new Error(
`${sourceLabel} is set to "${rawPath}" but the file does not exist.\n` +
'Please verify the path points to the Claude Code executable (native binary\n' +
'from the curl/PowerShell installer, or cli.js from an npm global install).'
);
}

/** Lazy-initialized logger */
let cachedLog: ReturnType<typeof createLogger> | undefined;
function getLog(): ReturnType<typeof createLogger> {
Expand Down Expand Up @@ -73,32 +131,21 @@ export async function resolveClaudeBinaryPath(
// its resolution order) can pin a known-good binary without a compiled build.
const envPath = process.env.CLAUDE_BIN_PATH;
if (envPath) {
if (!fileExists(envPath)) {
throw new Error(
`CLAUDE_BIN_PATH is set to "${envPath}" but the file does not exist.\n` +
'Please verify the path points to the Claude Code executable (native binary\n' +
'from the curl/PowerShell installer, or cli.js from an npm global install).'
);
}
getLog().info({ binaryPath: envPath, source: 'env' }, 'claude.binary_resolved');
return envPath;
const resolvedEnv = validateAndExpand(envPath, 'CLAUDE_BIN_PATH');
getLog().info({ binaryPath: resolvedEnv, source: 'env' }, 'claude.binary_resolved');
return resolvedEnv;
}

if (!BUNDLED_IS_BINARY) return undefined;

// 2. Config file override
if (configClaudeBinaryPath) {
if (!fileExists(configClaudeBinaryPath)) {
throw new Error(
`assistants.claude.claudeBinaryPath is set to "${configClaudeBinaryPath}" but the file does not exist.\n` +
'Please verify the path in .archon/config.yaml points to the Claude Code executable.'
);
}
getLog().info(
{ binaryPath: configClaudeBinaryPath, source: 'config' },
'claude.binary_resolved'
const resolvedConfig = validateAndExpand(
configClaudeBinaryPath,
'assistants.claude.claudeBinaryPath'
);
return configClaudeBinaryPath;
getLog().info({ binaryPath: resolvedConfig, source: 'config' }, 'claude.binary_resolved');
return resolvedConfig;
}

// 3. Autodetect — the Anthropic native installer
Expand Down
Loading