fix: resolve Windows Bun virtual FS paths for Claude CLI subprocess

[coleam00]

Summary

• On Windows, the Archon binary sets pathToClaudeCodeExecutable to a Bun virtual FS path (B:/~BUN/root/cli-xxx.js) that child processes cannot access, causing exit code 1 and "Module not found" errors
• The SDK's extractFromBunfs utility only detects $bunfs (Linux/Mac) paths and silently passes Windows B:/~BUN/root/ paths through unchanged
• Adds resolveWindowsBunfsCliPath() to packages/core/src/clients/claude.ts that detects Windows Bun FS paths by the ~BUN marker and extracts them to a real temp file using the same atomic write pattern as extractFromBunfs

Changes

• packages/core/src/clients/claude.ts: Added resolveWindowsBunfsCliPath() function and resolvedCliPath constant; pathToClaudeCodeExecutable now uses resolvedCliPath instead of raw cliPath; updated embed import comment to document the Windows workaround
• packages/core/src/clients/claude.test.ts: Added tests for resolveWindowsBunfsCliPath covering pass-through for non-Windows paths, Windows path extraction, and fallback behavior on extraction failure

Validation

All checks passed on first run:

Check	Result
bun run type-check	✅ 0 errors (all 9 packages)
bun run lint	✅ 0 errors, 0 warnings
bun run format:check	✅
bun run test	✅ 58+ tests passed, 0 failed

Root Cause

The Archon binary cross-compiles on Linux for Windows (--target bun-windows-x64). At compile time, import cliPath from '@anthropic-ai/claude-agent-sdk/embed' embeds cli.js into the binary's virtual FS. On Linux/Mac binaries the embedded path format is $bunfs/...; on Windows binaries the format is B:/~BUN/root/.... The SDK's extractFromBunfs.js hardcodes a $bunfs check and returns Windows virtual FS paths unchanged — child processes then fail to find the module since they cannot access the parent's virtual FS.

Fixes #1087

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17691ff9-2b0f-4f18-ac75-6c1ad15c9e38

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch archon/task-fix-issue-1087

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coleam00]

🔍 Comprehensive PR Review

PR: #1091 — fix: resolve Windows Bun virtual FS paths for Claude CLI subprocess
Reviewed by: 4 specialized agents (code-review, error-handling, test-coverage, comment-quality)
Date: 2026-04-11

---

Summary

This is a well-implemented, focused Windows compatibility fix that mirrors the upstream SDK logic faithfully. The implementation is correct, all 4 logical branches are tested, and scope discipline is excellent (zero unrelated changes). No critical or high-severity issues were found. Two quick MEDIUM fixes are recommended before marking ready.

Verdict: APPROVE (2 quick MEDIUM fixes recommended)

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	2
🟢 LOW	5

---

🟡 Medium Issues (Recommend Fixing — Quick Wins)

1. console.warn message names the wrong virtual-FS prefix

📍 packages/core/src/clients/claude.ts:84

The warning says "Windows $bunfs" but $bunfs is the Linux/macOS prefix. The Windows prefix is ~BUN (what the guard on line 65 actually checks). A developer triaging a failed Claude subprocess on Windows will search for $bunfs, find the Linux/macOS code path, and investigate the wrong place.

View fix

// Current (misleading):
`[archon] Failed to extract Claude CLI from Windows $bunfs: ${(err as Error).message}. ...`

// Fixed:
`[archon] Failed to extract Claude CLI from Windows ~BUN virtual FS: ${(err as Error).message}. ...`

---

2. console.warn instead of structured Pino logger

📍 packages/core/src/clients/claude.ts:83-85

The rest of claude.ts uses the lazy-initialized getLog() structured logger throughout. The console.warn here bypasses Pino's log-level filtering and loses structured context (err, embeddedPath). The lazy getLog() pattern already exists in this file to allow safe calls before test mocks initialize — it's safe to use here.

View fix (also resolves Issue 1)

} catch (err) {
  // Log warning but fall back to original path — will fail but that was the
  // pre-fix behavior. Fail-open here since we can't throw at module init time.
  getLog().warn(
    { err, embeddedPath },
    'claude.windows_bunfs_extraction_failed'
  );
  return embeddedPath;
}

Switching to getLog().warn(...) resolves both MEDIUM issues — the structured event name and context are unambiguous.

---

🟢 Low Issues

View 5 low-priority suggestions

Issue	Location	Agent	Suggestion
Node built-in imports placed after project imports	claude.ts:46-49	code-review	Move fs, path, os, crypto to top of import block (before SDK/@archon imports)
Test uses require() (CJS) instead of ESM import	claude.test.ts:1124-1126	code-review	Add top-level ESM import for fs, os, path at file top
(err as Error).message loses stack trace	claude.ts:84	error-handling	Pass full err object; if keeping console.warn, use String(err) defensively
No assertion that console.warn fires in fallback test	claude.test.ts	test-coverage	Add spyOn(console, 'warn') assertion for regression-proofing the diagnostic
Inline guard comment slightly overstates extractFromBunfs guarantee	claude.ts:62-64	comment-quality	Change "are already resolved" → "are expected to have been resolved already"

---

✅ What's Good

• Mirrors upstream SDK faithfully — implementation is nearly identical to extractFromBunfs.js, minimizing divergence risk and making intent immediately clear to reviewers
• Atomic write pattern — writeFileSync → chmodSync → renameSync prevents partial-read races, same as upstream
• Content-hash temp dir — reuses the same extracted file for the same binary version, no temp file accumulation across restarts
• All 4 branches tested — pass-through (non-Windows), $bunfs pass-through, successful extraction with content integrity verified, and error fallback
• Real FS tests are justified — Bun's module binding means spyOn(fs, 'readFileSync') can't intercept named imports; real FS tests are more reliable here (documented in scope)
• Fallback is intentional and documented — catch block comment explains why throwing is impossible at module init time, matching CLAUDE.md's requirement to document intentional fail-open
• Correct test batching — no new mock.module() calls; slots into existing claude.test.ts batch without package.json changes

---

📋 Suggested Follow-up Issues

Issue Title	Priority
Add console.warn assertion to Windows bunfs fallback test	P3
Normalize import ordering in claude.ts	P3

---

Next Steps

1. ⚡ Fix the 2 MEDIUM issues (ideally by switching console.warn → getLog().warn(...) which resolves both)
2. 📝 LOW issues are optional — consider deferring to follow-up
3. 🎯 Mark ready and merge when CI is green

---

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/cca84316788e97e0481c92609c58fad2/review/

[coleam00]

Fix Report: PR #1091

Date: 2026-04-11T00:00:00Z
Status: COMPLETE
Commit: 42f322c
Philosophy: Aggressive fix — lean towards fixing everything

---

Summary

All 7 review findings were addressed (2 MEDIUM + 5 LOW). The primary fix switches the catch block from console.warn to getLog().warn() with structured context and a correct event name, which simultaneously resolves the wrong virtual-FS prefix label ($bunfs → event name claude.windows_bunfs_extraction_failed) and the logging inconsistency. Additional polish fixes cover import ordering, comment accuracy, a new module-level constant comment, and ESM-style test imports with a logger-assertion in the fallback test.

---

Fixes Applied

Severity	Finding	Location	What Was Done
MEDIUM	console.warn uses wrong virtual-FS prefix ($bunfs instead of ~BUN)	claude.ts:84	Switched to getLog().warn({ err, embeddedPath }, 'claude.windows_bunfs_extraction_failed') — event name is unambiguous and prefix-correct
MEDIUM	console.warn instead of structured Pino logger	claude.ts:83-85	Same fix as above — getLog().warn() with structured { err, embeddedPath } context
LOW	Node built-in imports placed after project imports	claude.ts:46-49	Moved fs, path, os, crypto imports to top of file (before SDK and @archon/* imports)
LOW	Inline guard comment overstates extractFromBunfs guarantee	claude.ts:62-64	Changed "are already resolved" to "are expected to have been resolved already"
LOW	resolvedCliPath constant has no comment	claude.ts:90	Added one-liner explaining once-at-init-time caching rationale
LOW	Test uses require() (CJS) instead of ESM imports	claude.test.ts:1124-1126	Added top-level import { mkdirSync, writeFileSync, readFileSync, rmSync } from 'fs' etc.; removed inline require() calls
LOW	No assertion that logger fires on fallback	claude.test.ts:1147	Added mockLogger.warn call count and event name assertions in the fallback test

---

Validation

Check	Status
Type check	✅
Lint	✅
Format	✅
Tests	✅ all 9 packages passed (0 failures)

[coleam00]

Archon PR Validation Report

Verdict: REQUEST_CHANGES

Summary

The fix correctly solves the Windows Bun virtual FS path issue with a clean, minimal approach mirroring the SDK's own extraction pattern. One blocking issue: a Temporal Dead Zone (TDZ) ordering bug would crash the module on Windows when extraction fails. Simple fix — reorder declarations.

Bug Confirmation

Claim	Main	Feature
pathToClaudeCodeExecutable passes raw virtual FS path	Confirmed	Fixed — uses resolvedCliPath
No Windows virtual FS handling exists	Confirmed	Fixed — resolveWindowsBunfsCliPath() added
SDK only handles $bunfs paths	Confirmed	Worked around

Issues

Must fix (HIGH): const resolvedCliPath = resolveWindowsBunfsCliPath(cliPath) is initialized BEFORE cachedLog/getLog() are declared. If extraction fails, getLog() hits a TDZ → ReferenceError crash instead of graceful fallback. Fix: move resolvedCliPath init after getLog() declaration.

Nice to have (LOW): Add existsSync check to skip re-extraction when temp file already exists (matches SDK pattern).

What's Done Well

• Mirrors SDK's atomic-write extraction pattern
• Hash-based idempotent temp paths
• Well-structured tests with cleanup
• Minimal, narrowly scoped change

---

Validated by archon-validate-pr workflow

[Wirasm]

Filing context: #1217 is landing as a different approach to the same class of bug this draft targets.

Instead of extracting cli.js from Windows ~BUN virtual FS, #1217 drops the SDK embed entirely (import cliPath from '@anthropic-ai/claude-agent-sdk/embed') and requires compiled-binary users to point Archon at their Claude Code install via CLAUDE_BIN_PATH env or assistants.claude.claudeBinaryPath config. That closes off all platform-specific extraction code paths in one shot — Windows ~BUN, macOS /Users/runner/…, and any future Bun bundler quirks — and also implements #1176 (the user-facing override option).

The net effect is that #1217 fixes #1087 (which this draft was targeting) and #1210 (the macOS manifestation that surfaced after this draft) without needing the Windows-only extractor.

Leaving this PR open for you to close at your convenience — not trying to auto-close on merge.

[coleam00]

Closing — the underlying bug was real (#1087), but PR #1217 (merged April 14) already resolved this with a better approach: it removed the embed import entirely and switched to binary-resolver.ts which requires compiled-binary users to point to their own Claude Code installation via CLAUDE_BIN_PATH or assistants.claude.claudeBinaryPath.

Since there's no remaining code path using import cliPath from '@anthropic-ai/claude-agent-sdk/embed', the Windows virtual FS extraction workaround in this PR is no longer needed. The file it targets (packages/core/src/clients/claude.ts) has also been moved to packages/providers/src/claude/provider.ts.

Thanks for the solid investigation and fix — the approach was correct, just superseded by #1217.