Skip to content

Commit 57aeb05

Browse files
committed
claude: merge a caller's --settings instead of clobbering ucode's
`ucode claude` prepends its own `--settings <ucode-file>` to every launch. Claude Code honors only ONE --settings flag, so a caller that also passes --settings (e.g. an integration injecting hooks) has exactly one of the two silently dropped — either ucode's gateway config or the caller's hooks. This broke omnigent's managed Claude sessions: its transcript/permission hooks were dropped and the web UI rendered empty. Compose instead of clobber: when a caller supplies --settings, deep-merge it with ucode's (ucode's gateway keys win; hooks from both are unioned) and hand Claude a single merged --settings (inline JSON). The merge is per-launch and is never written back to the shared ucode settings file, so concurrent launches cannot accumulate one another's hooks. No caller --settings → unchanged (pass ucode's settings file directly). Co-authored-by: Isaac
1 parent a0a0d1c commit 57aeb05

2 files changed

Lines changed: 203 additions & 1 deletion

File tree

src/ucode/agents/claude.py

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
from __future__ import annotations
44

5+
import copy
6+
import json
57
import os
68
import re
79
import shutil
@@ -495,12 +497,130 @@ def default_model(state: dict) -> str | None:
495497
return claude_models.get("opus") or claude_models.get("sonnet") or claude_models.get("haiku")
496498

497499

500+
def _extract_caller_settings(tool_args: list[str]) -> tuple[list[str], list[str]]:
501+
"""Split caller-supplied ``--settings`` values out of *tool_args*.
502+
503+
Returns ``(values, remaining_args)``, handling both ``--settings <value>``
504+
and ``--settings=<value>`` spellings. Each value is either a JSON string or
505+
a path to a settings file — Claude Code accepts either.
506+
"""
507+
values: list[str] = []
508+
remaining: list[str] = []
509+
i = 0
510+
while i < len(tool_args):
511+
arg = tool_args[i]
512+
if arg == "--settings" and i + 1 < len(tool_args):
513+
values.append(tool_args[i + 1])
514+
i += 2
515+
continue
516+
if arg.startswith("--settings="):
517+
values.append(arg[len("--settings=") :])
518+
i += 1
519+
continue
520+
remaining.append(arg)
521+
i += 1
522+
return values, remaining
523+
524+
525+
def _load_caller_settings(value: str) -> dict | None:
526+
"""Resolve a ``--settings`` value (inline JSON or file path) to a dict.
527+
528+
Returns ``None`` when it is neither parseable JSON nor an existing file, so
529+
the caller can pass such a value through untouched instead of dropping it.
530+
"""
531+
text = value.strip()
532+
if text.startswith("{"):
533+
try:
534+
parsed = json.loads(text)
535+
except json.JSONDecodeError:
536+
return None
537+
return parsed if isinstance(parsed, dict) else None
538+
path = Path(text)
539+
if path.exists():
540+
return read_json_safe(path)
541+
return None
542+
543+
544+
def _union_claude_hooks(base: dict, overlay: dict) -> dict:
545+
"""Union two Claude Code ``hooks`` maps.
546+
547+
``hooks`` is ``{event: [entry, ...]}``. Per event we concatenate the entry
548+
lists so hooks from BOTH settings sources fire, rather than one replacing
549+
the other (which is what a plain deep-merge does to lists). This is what
550+
lets ucode's tracing Stop hook and a caller's own hooks coexist.
551+
"""
552+
result: dict = {}
553+
for event in [*base, *(e for e in overlay if e not in base)]:
554+
entries: list = []
555+
for src in (base, overlay):
556+
val = src.get(event)
557+
if isinstance(val, list):
558+
entries.extend(val)
559+
result[event] = entries
560+
return result
561+
562+
563+
def _merge_claude_settings(base: dict, overlay: dict) -> dict:
564+
"""Deep-merge *overlay* onto *base* (overlay wins on conflicting leaves),
565+
but UNION the ``hooks`` so neither side's hooks are dropped. Inputs are not
566+
mutated.
567+
"""
568+
merged = deep_merge_dict(copy.deepcopy(base), overlay)
569+
base_hooks = base.get("hooks")
570+
overlay_hooks = overlay.get("hooks")
571+
if isinstance(base_hooks, dict) or isinstance(overlay_hooks, dict):
572+
merged["hooks"] = _union_claude_hooks(
573+
base_hooks if isinstance(base_hooks, dict) else {},
574+
overlay_hooks if isinstance(overlay_hooks, dict) else {},
575+
)
576+
return merged
577+
578+
579+
def _build_claude_argv(binary: str, tool_args: list[str]) -> list[str]:
580+
"""Build the ``claude`` argv, composing any caller ``--settings`` with
581+
ucode's managed settings.
582+
583+
ucode needs its own settings (gateway ``apiKeyHelper`` + env) to reach
584+
Claude, and normally passes ``--settings <ucode-file>``. But Claude Code
585+
honors only ONE ``--settings`` flag, so a caller that ALSO passes
586+
``--settings`` (e.g. an integration injecting hooks) would have exactly one
587+
of the two silently dropped. To let ucode compose with any prior command,
588+
we merge a caller-supplied ``--settings`` with ucode's — ucode's gateway
589+
keys win, hooks from both are unioned — and hand Claude a single merged
590+
``--settings`` (inline JSON). The merge is per-launch and is never written
591+
back to the shared ucode settings file, so concurrent launches cannot
592+
accumulate one another's hooks.
593+
"""
594+
caller_values, remaining = _extract_caller_settings(tool_args)
595+
if not caller_values:
596+
# No caller --settings: hand Claude ucode's settings file directly (the
597+
# common path; behavior unchanged).
598+
return [binary, "--settings", str(CLAUDE_SETTINGS_PATH), *tool_args]
599+
caller_settings: dict = {}
600+
unparsed: list[str] = []
601+
for value in caller_values:
602+
parsed = _load_caller_settings(value)
603+
if parsed is None:
604+
unparsed.append(value)
605+
continue
606+
caller_settings = _merge_claude_settings(caller_settings, parsed)
607+
# ucode wins over the caller for conflicting keys (protects gateway auth);
608+
# hooks from both sides survive.
609+
merged = _merge_claude_settings(caller_settings, read_json_safe(CLAUDE_SETTINGS_PATH))
610+
argv = [binary, "--settings", json.dumps(merged, separators=(",", ":")), *remaining]
611+
# Anything we could not parse (not JSON, not an existing file) is passed
612+
# through untouched rather than silently dropped.
613+
for value in unparsed:
614+
argv.extend(["--settings", value])
615+
return argv
616+
617+
498618
def launch(state: dict, tool_args: list[str]) -> None:
499619
binary = SPEC["binary"]
500620
workspace = state.get("workspace")
501621
if workspace:
502622
os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile"))
503-
exec_or_spawn([binary, "--settings", str(CLAUDE_SETTINGS_PATH), *tool_args])
623+
exec_or_spawn(_build_claude_argv(binary, tool_args))
504624

505625

506626
def validate_cmd(binary: str) -> list[str]:

tests/test_agent_claude.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import json
56
import os
67

78
from ucode.agents import claude
@@ -523,3 +524,84 @@ def test_prunes_stale_name_companion_keys_from_older_ucode(self, monkeypatch):
523524
assert "ANTHROPIC_DEFAULT_OPUS_MODEL_NAME" not in env
524525
assert "ANTHROPIC_DEFAULT_SONNET_MODEL_NAME" not in env
525526
assert "ANTHROPIC_DEFAULT_HAIKU_MODEL_NAME" not in env
527+
528+
529+
class TestBuildClaudeArgv:
530+
def test_no_caller_settings_uses_ucode_file(self, monkeypatch):
531+
monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
532+
argv = claude._build_claude_argv("claude", ["-p", "hi"])
533+
assert argv == ["claude", "--settings", str(claude.CLAUDE_SETTINGS_PATH), "-p", "hi"]
534+
535+
def test_inline_caller_settings_merged_into_single_flag(self, monkeypatch):
536+
ucode_settings = {
537+
"apiKeyHelper": "ucode-helper",
538+
"env": {"ANTHROPIC_BASE_URL": "https://gw"},
539+
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "ucode-stop"}]}]},
540+
}
541+
monkeypatch.setattr(claude, "read_json_safe", lambda p: ucode_settings)
542+
caller = json.dumps(
543+
{
544+
"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "caller-stop"}]}]},
545+
"statusLine": {"type": "command", "command": "sl"},
546+
}
547+
)
548+
argv = claude._build_claude_argv("claude", ["--settings", caller, "-p", "hi"])
549+
# Exactly one --settings reaches Claude, and the caller's raw flag is gone.
550+
assert argv.count("--settings") == 1
551+
assert argv[:2] == ["claude", "--settings"]
552+
assert argv[3:] == ["-p", "hi"]
553+
merged = json.loads(argv[2])
554+
# ucode's gateway config survives.
555+
assert merged["apiKeyHelper"] == "ucode-helper"
556+
assert merged["env"]["ANTHROPIC_BASE_URL"] == "https://gw"
557+
# The caller's own (non-hook) settings pass through.
558+
assert merged["statusLine"] == {"type": "command", "command": "sl"}
559+
# Hooks from BOTH sides fire (unioned, not clobbered).
560+
stop_cmds = [h["command"] for e in merged["hooks"]["Stop"] for h in e["hooks"]]
561+
assert "ucode-stop" in stop_cmds
562+
assert "caller-stop" in stop_cmds
563+
564+
def test_equals_form_is_handled(self, monkeypatch):
565+
monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
566+
caller = json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "c"}]}]}})
567+
argv = claude._build_claude_argv("claude", [f"--settings={caller}"])
568+
assert argv.count("--settings") == 1
569+
merged = json.loads(argv[2])
570+
assert merged["apiKeyHelper"] == "u"
571+
assert merged["hooks"]["Stop"][0]["hooks"][0]["command"] == "c"
572+
573+
def test_ucode_wins_on_conflicting_env(self, monkeypatch):
574+
monkeypatch.setattr(
575+
claude, "read_json_safe", lambda p: {"env": {"ANTHROPIC_BASE_URL": "https://ucode"}}
576+
)
577+
caller = json.dumps({"env": {"ANTHROPIC_BASE_URL": "https://caller", "FOO": "bar"}})
578+
argv = claude._build_claude_argv("claude", ["--settings", caller])
579+
merged = json.loads(argv[2])
580+
assert merged["env"]["ANTHROPIC_BASE_URL"] == "https://ucode" # ucode wins
581+
assert merged["env"]["FOO"] == "bar" # caller's non-conflicting key kept
582+
583+
def test_file_path_caller_settings(self, tmp_path, monkeypatch):
584+
caller_file = tmp_path / "caller.json"
585+
caller_file.write_text(
586+
json.dumps(
587+
{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "cs"}]}]}}
588+
)
589+
)
590+
591+
def fake_read(p):
592+
if str(p) == str(claude.CLAUDE_SETTINGS_PATH):
593+
return {"apiKeyHelper": "u"}
594+
return json.loads(p.read_text())
595+
596+
monkeypatch.setattr(claude, "read_json_safe", fake_read)
597+
argv = claude._build_claude_argv("claude", ["--settings", str(caller_file)])
598+
assert argv.count("--settings") == 1
599+
merged = json.loads(argv[2])
600+
assert merged["apiKeyHelper"] == "u"
601+
assert merged["hooks"]["SessionStart"][0]["hooks"][0]["command"] == "cs"
602+
603+
def test_unparseable_value_passed_through(self, monkeypatch):
604+
monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
605+
argv = claude._build_claude_argv("claude", ["--settings", "not-json-not-a-file"])
606+
# Could not parse → passed through untouched rather than dropped.
607+
assert "not-json-not-a-file" in argv

0 commit comments

Comments
 (0)