fix(windows): resolve Claude binary path and apiKeyHelper shell syntax on Windows#137
fix(windows): resolve Claude binary path and apiKeyHelper shell syntax on Windows#137yakup-ozturk wants to merge 1 commit into
Conversation
…x on Windows Three changes that make `ucode claude` work on Windows without manual workarounds: 1. **WinError 2 — Claude binary not found** (`agents/claude.py`) Python's `subprocess` resolves PATH differently from cmd.exe and cannot find npm `.cmd` wrappers by name. Resolve `CLAUDE_BINARY` by walking from the `claude.cmd` wrapper to the actual `claude.exe` inside the npm package tree. Falls back to the wrapper path (or the bare `"claude"` string) if the .exe is absent, so non-Windows installs and non-standard npm layouts are unaffected. 2. **apiKeyHelper shell syntax error on Windows** (`databricks.py`) Claude Code runs the `apiKeyHelper` command via `cmd.exe` on Windows, which rejects the POSIX `[ -n "$VAR" ]` / `env -u` / `jq` syntax currently emitted by `build_auth_shell_command`. On `os.name == 'nt'` the function now returns a `python -c "..."` one-liner that handles the `DATABRICKS_BEARER` short- circuit and calls the Databricks CLI with `subprocess`, avoiding both the bash syntax and the `jq` dependency. POSIX behaviour is unchanged. 3. **Settings not visible to the Claude desktop / IDE extension** (`agents/claude.py`) `ucode claude` writes config to `~/.claude/ucode-settings.json` and passes `--settings` when launching the CLI, but the Claude desktop app and IDE extensions read `~/.claude/settings.json` by default. After writing the ucode-managed file, also merge the auth/env overlay into `settings.json` so the `apiKeyHelper` and `ANTHROPIC_*` vars are available in every launch path. A backup of the pre-existing `settings.json` is created alongside the existing ucode-settings backup.
rohita5l
left a comment
There was a problem hiding this comment.
Thanks for fixing the Windows path here. I think the intent is right, but it requires some changes
The PR currently mixes three separate behavior changes:
- Windows auth command syntax.
- Windows Claude binary resolution.
- Writing ucode’s Claude overlay into the user’s default
~/.claude/settings.json.
The first two are Windows fixes. The third changes default behavior for all users and should either be opt-in or split into a separate PR. We definitely dont want as the default behavior and prefer to keep ucode settings separate.
My preferred direction:
- Keep
SPEC["binary"] = "claude"stable. - Resolve a Windows-specific Claude executable only at launch/validation time.
- Keep writing
~/.claude/ucode-settings.jsonby default. - Make merging into
~/.claude/settings.jsonexplicit opt-in - Add focused tests for the Windows auth command and binary resolution behavior.
|
|
||
| SPEC: ToolSpec = { | ||
| "binary": "claude", | ||
| "binary": CLAUDE_BINARY, |
There was a problem hiding this comment.
I’m not convinced we should resolve CLAUDE_BINARY at import time or on all platforms.
This makes SPEC["binary"] environment-dependent and changes macOS/Linux behavior too. In my local focused test run on this PR, tests/test_agent_claude.py failed because SPEC["binary"] became my absolute
~/.nvm/.../bin/claude path instead of "claude".
Could we scope this to Windows launch/validation only, preferably behind a helper like _resolve_claude_binary()? That would keep the default spec stable and avoid doing PATH/package-layout probing during module import.
| / "@anthropic-ai" | ||
| / "claude-code" | ||
| / "bin" | ||
| / "claude.exe" |
There was a problem hiding this comment.
Can we verify this actually fixes the Windows launch failure?
My concern is that npm usually installs .cmd / .ps1 / shim wrappers, and I’m not sure @anthropic-ai/claude-code actually ships bin/claude.exe. If _claude_exe.exists() is false, this falls back to the
.cmd wrapper path. Passing a .cmd path to subprocess with shell=False can still fail on Windows because CreateProcess does not execute batch files directly.
It would be safer to either invoke the .cmd through cmd /c in the Windows path, or resolve this at the call site in a way that matches how validate_tool / launch actually execute the binary.
| # finds the apiKeyHelper and ANTHROPIC_* env vars regardless of whether it | ||
| # is launched via `ucode claude` (which passes --settings) or directly from | ||
| # the Claude desktop app / IDE extension (which reads settings.json by default). | ||
| existing_default = read_json_safe(CLAUDE_DEFAULT_SETTINGS_PATH) |
There was a problem hiding this comment.
Can we make writing to the default ~/.claude/settings.json opt-in?
ucode claude already launches with --settings ~/.claude/ucode-settings.json, so this PR broadens the behavior from “manage ucode’s Claude config” to “also mutate the user’s main Claude config.” That has a larger blast radius, especially because direct claude usage would start inheriting the Databricks gateway apiKeyHelper / ANTHROPIC_* settings.
A simpler shape might be: keep writing ucode-settings.json by default, and add an explicit option for users who want this global behavior
There was a problem hiding this comment.
If we do write to ~/.claude/settings.json, we should also wire it into cleanup/rollback.
Right now the managed config state and validation rollback are centered on SPEC["config_path"] / SPEC["backup_path"], which still point at ucode-settings.json. This adds a second managed file, but I don’t see matching restore behavior for it.
|
|
||
| def build_auth_shell_command(workspace: str, profile: str | None = None) -> str: | ||
| workspace_arg = shlex.quote(workspace.rstrip("/")) | ||
| workspace_clean = workspace.rstrip("/") |
There was a problem hiding this comment.
Could we split the platform-specific command construction into small helpers instead of building both shell dialects inline here?
The early return is not the main issue for me; the function now mixes POSIX shell, Windows cmd.exe, Python -c, profile handling, and DATABRICKS_BEARER fallback in one place, which makes the quoting hard to
audit.
For example:
def build_auth_shell_command(workspace: str, profile: str | None = None) -> str:
workspace_clean = workspace.rstrip("/")
if os.name == "nt":
return _build_windows_auth_command(workspace_clean, profile)
return _build_posix_auth_command(workspace_clean, profile)That keeps the public API simple while isolating the two command syntaxes. It would also make it easier to add focused tests for Windows quoting separately from the existing POSIX sh behavior.
| "p=subprocess.run(cmd,capture_output=True,text=True,check=True,timeout=30); " | ||
| "print(json.loads(p.stdout)['access_token'])" | ||
| ) | ||
| return f'"{python_exe}" -c {helper_code!r}' |
There was a problem hiding this comment.
This python -c command is hard to audit because it nests Python repr inside another repr, then hands the result to cmd.exe.
Values containing quotes or % can behave differently under cmd.exe, and this is exactly the kind of code that tends to regress silently. Could we either move the Windows helper into a small dedicated helper
function with explicit quoting/tests, or avoid the -c blob by invoking a stable ucode subcommand/helper instead?
Something like: Then in launch/validation: and validation:
For Windows, I’d avoid a complicated inline python -c if possible. |
fix(windows): resolve Claude binary path and apiKeyHelper syntax on Windows:
Three changes that make
ucode claudework on Windows without manual workarounds:WinError 2 — Claude binary not found (
agents/claude.py) Python'ssubprocessresolves PATH differently from cmd.exe and cannot find npm.cmdwrappers by name. ResolveCLAUDE_BINARYby walking from theclaude.cmdwrapper to the actualclaude.exeinside the npm package tree. Falls back to the wrapper path (or the bare"claude"string) if the .exe is absent, so non-Windows installs and non-standard npm layouts are unaffected.apiKeyHelper shell syntax error on Windows (
databricks.py) Claude Code runs theapiKeyHelpercommand viacmd.exeon Windows, which rejects the POSIX[ -n "$VAR" ]/env -u/jqsyntax currently emitted bybuild_auth_shell_command. Onos.name == 'nt'the function now returns apython -c "..."one-liner that handles theDATABRICKS_BEARERshort- circuit and calls the Databricks CLI withsubprocess, avoiding both the bash syntax and thejqdependency. POSIX behaviour is unchanged.Settings not visible to the Claude desktop / IDE extension (
agents/claude.py)ucode claudewrites config to~/.claude/ucode-settings.jsonand passes--settingswhen launching the CLI, but the Claude desktop app and IDE extensions read~/.claude/settings.jsonby default. After writing the ucode-managed file, also merge the auth/env overlay intosettings.jsonso theapiKeyHelperandANTHROPIC_*vars are available in every launch path. A backup of the pre-existingsettings.jsonis created alongside the existing ucode-settings backup.Reference: https://medium.com/@Yakup-Ozturk/getting-claude-code-to-work-with-the-databricks-ai-gateway-on-windows-three-fixes-i-wish-i-knew-f6e71ece06a9