Skip to content

fix(windows): resolve Claude binary path and apiKeyHelper shell syntax on Windows #137

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

Open
yakup-ozturk wants to merge 1 commit into
base: main
Choose a base branch
from
+55 −2
Open

fix(windows): resolve Claude binary path and apiKeyHelper shell syntax on Windows #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 31 additions & 1 deletion src/ucode/agents/claude.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,31 @@

CLAUDE_CONFIG_DIR = Path.home() / ".claude"
CLAUDE_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "ucode-settings.json"
CLAUDE_DEFAULT_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "settings.json"
CLAUDE_BACKUP_PATH = APP_DIR / "claude-ucode-settings.backup.json"
CLAUDE_DEFAULT_BACKUP_PATH = APP_DIR / "claude-settings.backup.json"

# On Windows, subprocess can fail to locate 'claude' via PATH because npm
# wrappers are .cmd files that Python's subprocess doesn't resolve the same
# way as cmd.exe. Resolve the actual .exe inside the npm package tree so
# the subprocess call is unambiguous on all platforms.
_claude_wrapper = shutil.which("claude.cmd") or shutil.which("claude") or shutil.which("claude.bat")
if _claude_wrapper:
_claude_root = Path(_claude_wrapper).parent
_claude_exe = (
_claude_root
/ "node_modules"
/ "@anthropic-ai"
/ "claude-code"
/ "bin"
/ "claude.exe"
Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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.

)
CLAUDE_BINARY = str(_claude_exe) if _claude_exe.exists() else _claude_wrapper
else:
CLAUDE_BINARY = "claude"

SPEC: ToolSpec = {
"binary": "claude",
"binary": CLAUDE_BINARY,
Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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.

"package": "@anthropic-ai/claude-code",
"display": "Claude Code",
"config_path": CLAUDE_SETTINGS_PATH,
Expand Down Expand Up @@ -215,6 +236,7 @@ def _unregister_web_search_mcp() -> None:

def write_tool_config(state: dict, model: str) -> dict:
backup_existing_file(CLAUDE_SETTINGS_PATH, CLAUDE_BACKUP_PATH)
backup_existing_file(CLAUDE_DEFAULT_SETTINGS_PATH, CLAUDE_DEFAULT_BACKUP_PATH)
web_search_model = _resolve_web_search_model(state)
overlay, managed_keys = render_overlay(
state["workspace"],
Expand Down Expand Up @@ -251,6 +273,14 @@ def write_tool_config(state: dict, model: str) -> dict:
_remove_tracing_stop_hook(merged)
write_json_file(CLAUDE_SETTINGS_PATH, merged)

# Mirror the auth/env overlay into ~/.claude/settings.json so Claude Code
# 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)
Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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.

merged_default = deep_merge_dict(existing_default, overlay)
write_json_file(CLAUDE_DEFAULT_SETTINGS_PATH, merged_default)

if web_search_model:
_register_web_search_mcp(state["workspace"], web_search_model, state.get("profile"))

Expand Down
25 changes: 24 additions & 1 deletion src/ucode/databricks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import shlex
import shutil
import subprocess
import sys
from pathlib import Path
from typing import Literal, cast, overload
from urllib import error as urllib_error
Expand Down Expand Up @@ -956,7 +957,29 @@ def list_databricks_apps(workspace: str, profile: str | None = None) -> list[dic


def build_auth_shell_command(workspace: str, profile: str | None = None) -> str:
workspace_arg = shlex.quote(workspace.rstrip("/"))
workspace_clean = workspace.rstrip("/")
Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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.


if os.name == "nt":
# On Windows, Claude Code runs apiKeyHelper via cmd.exe, which does not
# understand POSIX syntax ([ -n "$VAR" ], env -u, jq pipes). Instead we
# delegate to the Python interpreter that is running ucode — it is always
# available, handles DATABRICKS_BEARER short-circuit, and parses the JSON
# token response without requiring jq on PATH.
python_exe = sys.executable
databricks_exe = shutil.which("databricks") or "databricks"
profile_part = f", '--profile', {profile!r}" if profile else ""
helper_code = (
"import json, os, subprocess, sys; "
"bearer=os.environ.get('DATABRICKS_BEARER'); "
"sys.exit(print(bearer) or 0) if bearer else None; "
f"cmd=[{databricks_exe!r},'auth','token','--host',{workspace_clean!r}"
f"{profile_part},'--force-refresh','--output','json']; "
"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}'
Copy link
Copy Markdown
Collaborator

@rohita5l rohita5l Jun 5, 2026

Choose a reason for hiding this comment

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

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?


workspace_arg = shlex.quote(workspace_clean)
if profile:
profile_arg = shlex.quote(profile)
cli_command = (
Expand Down