claude: fail loudly on an unresolvable --settings value

Per review on #189: a caller --settings value that ucode could not parse
was silently swallowed (returned None) and then passed through as a second
--settings flag. Claude Code honors only one --settings, so that collision
silently dropped either the caller's settings or ucode's gateway config
(breaking auth). Raise an actionable RuntimeError instead — malformed inline
JSON, a malformed/unreadable settings file, a nonexistent file path, or
non-object JSON — surfaced by the CLI as a clean error. Removes the
pass-through branch, which never helped (the colliding flag always loses).

Co-authored-by: Isaac

Author: Edwinhe03

src/ucode/agents/claude.py

@@ -522,23 +522,42 @@ def _extract_caller_settings(tool_args: list[str]) -> tuple[list[str], list[str]
     return values, remaining
 
 
-def _load_caller_settings(value: str) -> dict | None:
+def _load_caller_settings(value: str) -> dict:
     """Resolve a ``--settings`` value (inline JSON or file path) to a dict.
 
-    Returns ``None`` when it is neither parseable JSON nor an existing file, so
-    the caller can pass such a value through untouched instead of dropping it.
+    Claude Code accepts either inline JSON or a path to a JSON file. Raises
+    ``RuntimeError`` (surfaced by the CLI as an actionable error) when the value
+    is neither, rather than silently dropping it: a dropped value would also be
+    passed through as a second ``--settings`` flag, and Claude Code honors only
+    one — so either the caller's settings or ucode's gateway config would be
+    silently ignored. Failing loudly lets the caller fix their input.
     """
     text = value.strip()
     if text.startswith("{"):
+        source, malformed = text, "value is not valid JSON"
+    else:
+        path = Path(text)
+        if not path.exists():
+            raise RuntimeError(
+                f"--settings file not found: {value!r}. "
+                "Pass inline JSON or a path to an existing JSON file."
+            )
         try:
-            parsed = json.loads(text)
-        except json.JSONDecodeError:
-            return None
-        return parsed if isinstance(parsed, dict) else None
-    path = Path(text)
-    if path.exists():
-        return read_json_safe(path)
-    return None
+            source = path.read_text(encoding="utf-8")
+        except OSError as exc:
+            raise RuntimeError(f"--settings file could not be read: {value!r} ({exc}).") from exc
+        malformed = "file is not valid JSON"
+    try:
+        parsed = json.loads(source)
+    except json.JSONDecodeError as exc:
+        raise RuntimeError(
+            f"--settings {malformed} ({exc}): {value!r}. Pass inline JSON or a path to a JSON file."
+        ) from exc
+    if not isinstance(parsed, dict):
+        raise RuntimeError(
+            f"--settings must be a JSON object, got {type(parsed).__name__}: {value!r}."
+        )
+    return parsed
 
 
 def _union_claude_hooks(base: dict, overlay: dict) -> dict:
@@ -589,30 +608,22 @@ def _build_claude_argv(binary: str, tool_args: list[str]) -> list[str]:
     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.
+    accumulate one another's hooks. A caller ``--settings`` value ucode cannot
+    resolve raises (see :func:`_load_caller_settings`) rather than being passed
+    through as a second, colliding flag.
     """
     caller_values, remaining = _extract_caller_settings(tool_args)
     if not caller_values:
         # No caller --settings: hand Claude ucode's settings file directly (the
         # common path; behavior unchanged).
         return [binary, "--settings", str(CLAUDE_SETTINGS_PATH), *tool_args]
     caller_settings: dict = {}
-    unparsed: list[str] = []
     for value in caller_values:
-        parsed = _load_caller_settings(value)
-        if parsed is None:
-            unparsed.append(value)
-            continue
-        caller_settings = _merge_claude_settings(caller_settings, parsed)
+        caller_settings = _merge_claude_settings(caller_settings, _load_caller_settings(value))
     # ucode wins over the caller for conflicting keys (protects gateway auth);
     # hooks from both sides survive.
     merged = _merge_claude_settings(caller_settings, read_json_safe(CLAUDE_SETTINGS_PATH))
-    argv = [binary, "--settings", json.dumps(merged, separators=(",", ":")), *remaining]
-    # Anything we could not parse (not JSON, not an existing file) is passed
-    # through untouched rather than silently dropped.
-    for value in unparsed:
-        argv.extend(["--settings", value])
-    return argv
+    return [binary, "--settings", json.dumps(merged, separators=(",", ":")), *remaining]
 
 
 def launch(state: dict, tool_args: list[str]) -> None:

tests/test_agent_claude.py

@@ -5,6 +5,8 @@
 import json
 import os
 
+import pytest
+
 from ucode.agents import claude
 
 WS = "https://example.databricks.com"
@@ -587,21 +589,40 @@ def test_file_path_caller_settings(self, tmp_path, monkeypatch):
                 {"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "cs"}]}]}}
             )
         )
-
-        def fake_read(p):
-            if str(p) == str(claude.CLAUDE_SETTINGS_PATH):
-                return {"apiKeyHelper": "u"}
-            return json.loads(p.read_text())
-
-        monkeypatch.setattr(claude, "read_json_safe", fake_read)
+        # The caller file is read directly; read_json_safe is only used for
+        # ucode's own settings file.
+        monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
         argv = claude._build_claude_argv("claude", ["--settings", str(caller_file)])
         assert argv.count("--settings") == 1
         merged = json.loads(argv[2])
         assert merged["apiKeyHelper"] == "u"
         assert merged["hooks"]["SessionStart"][0]["hooks"][0]["command"] == "cs"
 
-    def test_unparseable_value_passed_through(self, monkeypatch):
+    def test_malformed_inline_json_raises(self, monkeypatch):
+        monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
+        # Clearly-intended-as-JSON but broken: fail loudly rather than pass it
+        # through as a second, colliding --settings flag.
+        with pytest.raises(RuntimeError, match="not valid JSON"):
+            claude._build_claude_argv("claude", ["--settings", '{"hooks": '])
+
+    def test_nonexistent_file_raises(self, monkeypatch):
+        monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
+        with pytest.raises(RuntimeError, match="file not found"):
+            claude._build_claude_argv("claude", ["--settings", "/no/such/settings.json"])
+
+    def test_non_object_file_json_raises(self, tmp_path, monkeypatch):
+        # A --settings file whose JSON is not an object (e.g. an array) can't be
+        # merged; fail loudly. (An inline value only enters the JSON branch when
+        # it starts with "{", so the non-object case is reachable via a file.)
+        bad_file = tmp_path / "bad.json"
+        bad_file.write_text("[1, 2, 3]")
+        monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
+        with pytest.raises(RuntimeError, match="must be a JSON object"):
+            claude._build_claude_argv("claude", ["--settings", str(bad_file)])
+
+    def test_malformed_file_json_raises(self, tmp_path, monkeypatch):
+        bad_file = tmp_path / "bad.json"
+        bad_file.write_text('{"hooks": ')
         monkeypatch.setattr(claude, "read_json_safe", lambda p: {"apiKeyHelper": "u"})
-        argv = claude._build_claude_argv("claude", ["--settings", "not-json-not-a-file"])
-        # Could not parse → passed through untouched rather than dropped.
-        assert "not-json-not-a-file" in argv
+        with pytest.raises(RuntimeError, match="not valid JSON"):
+            claude._build_claude_argv("claude", ["--settings", str(bad_file)])