Skip to content

Commit da8825f

Browse files
A bare backend name was read as a model name, and the CLI's failures said nothing (#56)
Two defects that hid each other. Both found by installing 0.1.2 from PyPI into a clean venv and driving it against a real `claude` v2.1.220. **`split_spec` only consulted BACKENDS when the spec contained a slash.** A bare backend name fell through to "assume the default backend, keep the whole string as the model", so: split_spec("claude-cli") -> ("claude-cli", "claude-cli") split_spec("mock") -> ("claude-cli", "mock") `--model claude-cli` therefore shelled out to `claude -p --model claude-cli` and was refused by the CLI on every call (3/3), while `grapharc models --check` went on reporting the backend `usable` and `grapharc models claude-cli` printed `model: claude-cli` without complaint — two commands whose job is to say whether a spec will work, both saying yes about one that never did. `--model mock` was the worse half: it named the *paid* subscription backend and spawned the real binary, so the double `models --check` describes as "scripted test double; never reaches a provider" reached for one. It happened to fail before billing only because `mock` is not a model name. This is the same "silently folded into a model name … fails much later with a confusing error" failure `split_spec` already refuses for a mistyped backend *with* a slash; it just could not see the case without one. A bare backend name now resolves to that backend. `claude-cli` takes its own default model and `mock` takes the scripted double (which ignores the model segment entirely). `openrouter`, `openai` and `ollama` front catalogues rather than a model, so a bare name there is refused with a spelling that works rather than a guess about what to bill someone for. Slash forms and bare *model* names are untouched. **The gateway read the wrong stream.** `claude -p` fails with a non-zero exit, an empty stderr, and its whole explanation in the JSON envelope on stdout: {"is_error": true, "result": "There's an issue with the selected model (claude-cli). It may not exist or you may not have access to it."} `_invoke_cli` reported `proc.stderr`, so the error was `claude -p exited 1: ` — a sentence that stops at the colon. That is how the bug above presented itself: as no message at all. The one string that would have diagnosed it in seconds was captured, held in `proc.stdout`, and discarded. Note the code already parses this shape a few lines below, but only on the `returncode == 0` path, and the CLI sets `is_error` *and* exits non-zero, so it took the branch that ignores stdout. stdout is read first now, falling back to stderr when it is not the documented envelope. The recovered text also feeds `_cli_failure`, which classifies transient-vs-deterministic and was previously classifying from `""`. Tests: every new test was confirmed to fail against the old code and pass against the new — including the mock one, which asserts no subprocess is *created* rather than just checking the returned type, since the type was what was wrong and a refactor could fix the type and still shell out. `BARE_BACKEND_MODEL["claude-cli"]` is a second copy of the model class's default so the registry need not import a backend to split a string, and a test pins the two together the way CI already pins `__version__` to the packaged version. Verified: `grapharc demo stage1 --model claude-cli` now completes (8 nodes, target_met, exit 0) where it previously died with an empty error; `--model mock` resolves to the double with no subprocess; `--model openrouter` exits 2 with an example. Full suite green on 3.12 and 3.13; ruff clean. Co-authored-by: Claude <noreply@anthropic.com>
1 parent a68d957 commit da8825f

4 files changed

Lines changed: 226 additions & 3 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,8 @@ Re-derived on 2026-07-28 by running each item, not by reading the commit log.
529529
- **A planning round is an envelope, not a measurement.** A `round` event used to carry the planner's `tokens` and the round's `duration_ms`, both of which `metrics`, `cost` and `replay` add on top of node totals — and the planner's spend was already reported by its own `plan` event, so it was counted twice, and a round's duration encloses the plan plus every node it ran. Neither is on the event now; both are on its `state_delta` as `round_tokens` / `round_iterations` / `round_duration_ms`, where no reader sums them. `RoundRecord.iterations` also holds a figure now rather than always `0`.
530530
- **The Claude CLI backend is completion-only.** Tool calling and structured output need one of the OpenAI-wire backends: `openrouter`, `openai`, or a local `ollama`.
531531
- **A session turn is synchronous**, and a runner claim is a claim rather than a lease — nothing reclaims a session whose runner died holding it.
532-
- **A bare model spec resolves to the paid `claude-cli` backend.** `--model mock` does not reach the scripted double; it becomes the model name `mock` on the subscription backend. Only the slash form (`mock/anything`) reaches the double. A mistyped backend *with* a slash is rejected properly, exit 2.
532+
- *Closed:* a **bare backend name was read as a model name**, because `split_spec` only consulted the backend list when the spec contained a slash. `--model claude-cli` — the backend `models --check` reports as `usable` — shelled out to `claude -p --model claude-cli` and was refused by the CLI on *every* call, and `--model mock` named the paid subscription backend and spawned the real binary, so the double documented as "never reaches a provider" reached for one. A bare backend name now resolves to that backend (`claude-cli` to its own default model, `mock` to the scripted double, which ignores the model segment anyway); `openrouter`, `openai` and `ollama` front catalogues rather than a model, so those are refused with an example spelling instead of a guess about what to bill you for. The slash forms and bare *model* names are unchanged.
533+
- *Closed:* a failing `claude -p` **reported no reason at all**. The CLI exits non-zero with an empty stderr and puts its explanation in the JSON envelope on stdout; the gateway read only stderr, so the error was `claude -p exited 1: ` — a sentence that stops at the colon. Since the wrong-model bug above presented itself exactly that way, the two hid each other. stdout is read first now, and the recovered text also feeds the transient-vs-deterministic classifier, which was previously deciding from `""`.
533534
- **`.env` is found by walking up parent directories; `grapharc.toml` is not.** The config layer refuses an upward search on purpose — a run must not be governed by a file you did not know about. The credential loader predates that decision and still searches upward, so the thing that *spends money* is discovered more eagerly than the thing that *constrains* it.
534535
- **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.
535536

‎grapharc/gateway/claude_cli.py‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,34 @@ def _canonical_model(model: str) -> str:
121121
return model.split("/", 1)[1] if model.startswith("anthropic/") else model
122122

123123

124+
def _payload_error(stdout: str) -> str:
125+
"""The CLI's own explanation of a failure, which it writes to *stdout*.
126+
127+
`claude -p` reports a refused model, an expired login and their siblings
128+
with a non-zero exit code, an **empty stderr**, and the whole reason in the
129+
JSON envelope on stdout::
130+
131+
{"is_error": true, "result": "There's an issue with the selected model
132+
(claude-cli). It may not exist or you may not have access to it."}
133+
134+
Reading only stderr therefore produced ``claude -p exited 1: `` — a
135+
sentence that stops at the colon — and discarded the one string that said
136+
what was wrong. It matters for classification too: `_cli_failure` decides
137+
transient-vs-deterministic from this text, and it was deciding from "".
138+
139+
Returns "" when stdout is not the documented envelope, so the caller falls
140+
back to stderr rather than inventing a reason.
141+
"""
142+
try:
143+
payload = json.loads(stdout)
144+
except (TypeError, ValueError): # JSONDecodeError is a ValueError
145+
return ""
146+
if not isinstance(payload, dict):
147+
return ""
148+
result = payload.get("result")
149+
return str(result).strip() if result else ""
150+
151+
124152
def _cli_failure(message: str, evidence: str) -> GatewayError:
125153
"""Build the right error class for a CLI failure from its own text.
126154
@@ -219,8 +247,10 @@ def _invoke_cli(self, argv: list[str], prompt: str, cwd: str) -> dict[str, Any]:
219247
) from exc
220248

221249
if proc.returncode != 0:
222-
stderr = proc.stderr.strip()
223-
raise _cli_failure(f"claude -p exited {proc.returncode}: {stderr[:500]}", stderr)
250+
# stdout first: the CLI puts its reason in the JSON envelope there
251+
# and leaves stderr empty. See `_payload_error`.
252+
detail = _payload_error(proc.stdout) or proc.stderr.strip()
253+
raise _cli_failure(f"claude -p exited {proc.returncode}: {detail[:500]}", detail)
224254

225255
try:
226256
payload = json.loads(proc.stdout)

‎grapharc/gateway/registry.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,31 @@ class UnknownBackendError(Exception):
4949
"mock": "mock",
5050
}
5151

52+
#: What a *bare* backend name resolves to. A backend belongs here only when it
53+
#: can pick a model without being told: `claude-cli` has a documented default,
54+
#: and the `mock` double ignores the model segment entirely (`get_model` builds
55+
#: a `ScriptedChatModel` with no model argument at all). The remote catalogues
56+
#: are deliberately absent — `openrouter` alone fronts thousands of models and
57+
#: no default would be defensible, so a bare `openrouter` is an error rather
58+
#: than a guess about which model to bill you for.
59+
#:
60+
#: The `claude-cli` value is a second copy of `ClaudeCodeCLIChatModel.model`'s
61+
#: own default, kept here so this module does not import a backend just to
62+
#: split a string. `test_the_bare_claude_cli_default_matches_the_model_class`
63+
#: fails if the two ever drift, the same way CI already pins `__version__` to
64+
#: the packaged version.
65+
BARE_BACKEND_MODEL = {
66+
"claude-cli": "claude-sonnet-5",
67+
"mock": "mock",
68+
}
69+
70+
#: A concrete spec to show someone who typed a bare backend that needs a model.
71+
_BARE_BACKEND_EXAMPLE = {
72+
"openrouter": "openrouter/anthropic/claude-sonnet-4.5",
73+
"openai": "openai/gpt-4o-mini",
74+
"ollama": "ollama/llama3.1",
75+
}
76+
5277

5378
def split_spec(spec: str) -> tuple[str, str]:
5479
"""Split `backend/model` — bare names get the default backend.
@@ -66,10 +91,30 @@ def split_spec(spec: str) -> tuple[str, str]:
6691
someone typing it means. Reaching the same model through the broker is
6792
still `openrouter/openai/gpt-4o-mini`, because only the first segment is
6893
ever read as a backend.
94+
95+
A **bare backend name** is read as a backend too, which it was not before:
96+
the slash test above meant `claude-cli` fell through to the last line and
97+
became the *model* `claude-cli`, so `--model claude-cli` shelled out to
98+
`claude -p --model claude-cli` and was refused by the CLI on every call —
99+
while `models --check` went on reporting the backend `usable`. `--model
100+
mock` was worse than useless: it named the paid subscription backend and
101+
spawned the real binary, so the double documented as "never reaches a
102+
provider" reached for one. That is the same "silently folded into a model
103+
name … fails much later with a confusing error" failure this function
104+
already refuses for a mistyped backend *with* a slash; it just could not
105+
see the case without one.
69106
"""
70107
head, sep, rest = spec.partition("/")
71108
if sep and head in BACKENDS:
72109
return head, rest
110+
if not sep and spec in BACKENDS:
111+
if spec in BARE_BACKEND_MODEL:
112+
return spec, BARE_BACKEND_MODEL[spec]
113+
raise UnknownBackendError(
114+
f"{spec!r} names a backend, not a model, and it has no default "
115+
f"model to fall back on — write {spec}/<model>, for example "
116+
f"{_BARE_BACKEND_EXAMPLE[spec]!r}"
117+
)
73118
if sep and head not in KNOWN_AUTHORS:
74119
raise UnknownBackendError(
75120
f"unknown backend {head!r} in spec {spec!r}; expected one of: "

‎tests/test_gateway.py‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,150 @@ def test_live_cli_completes_a_prompt():
8181
assert "pong" in msg.content.lower()
8282
assert model.last_usage is not None
8383
assert model.last_usage["total_tokens"] > 0
84+
85+
86+
# ---- a bare backend name is a backend, not a model ---------------------------
87+
#
88+
# `split_spec` only consulted `BACKENDS` when the spec contained a slash, so a
89+
# bare backend name fell through to "assume the default backend, keep the whole
90+
# string as the model". Two live failures came out of that, and both are gates
91+
# below rather than prose.
92+
93+
94+
@pytest.mark.parametrize(
95+
"spec, expected",
96+
[
97+
("claude-cli", ("claude-cli", "claude-sonnet-5")),
98+
("mock", ("mock", "mock")),
99+
# The slash forms and the bare-model form must be untouched by the fix.
100+
("claude-cli/claude-sonnet-5", ("claude-cli", "claude-sonnet-5")),
101+
("mock/whatever", ("mock", "whatever")),
102+
("claude-sonnet-5", ("claude-cli", "claude-sonnet-5")),
103+
("anthropic/claude-haiku-4.5", ("claude-cli", "anthropic/claude-haiku-4.5")),
104+
],
105+
)
106+
def test_a_bare_backend_name_resolves_to_that_backend(spec, expected):
107+
"""`--model claude-cli` used to mean "the model called claude-cli".
108+
109+
It shelled out to `claude -p --model claude-cli`, which the CLI refuses on
110+
every call — `There's an issue with the selected model (claude-cli)` —
111+
while `grapharc models --check` went on reporting the backend `usable` and
112+
`grapharc models claude-cli` printed `model: claude-cli` without complaint.
113+
Both commands exist to say whether a spec will work, and both said yes
114+
about one that never did.
115+
"""
116+
from grapharc.gateway.registry import split_spec
117+
118+
assert split_spec(spec) == expected
119+
120+
121+
def test_the_mock_double_never_reaches_a_provider():
122+
"""`grapharc models --check` calls mock a "scripted test double; never
123+
reaches a provider". Bare `mock` used to resolve to the *paid* Claude CLI
124+
backend and spawn the real binary — the one guarantee a double exists to
125+
make, broken in the direction that costs money.
126+
127+
Asserted on the behaviour (no subprocess is created), not just on the
128+
returned type, because the type is what was wrong and a future refactor
129+
could get the type right and still shell out.
130+
"""
131+
import subprocess
132+
133+
from grapharc.gateway import get_model
134+
from grapharc.testing import ScriptedChatModel
135+
136+
spawned: list = []
137+
real_run, real_popen = subprocess.run, subprocess.Popen
138+
subprocess.run = lambda *a, **k: spawned.append(a[0]) or real_run(*a, **k)
139+
subprocess.Popen = lambda *a, **k: spawned.append(a[0]) or real_popen(*a, **k)
140+
try:
141+
model = get_model("mock", responses=["hi"])
142+
assert isinstance(model, ScriptedChatModel)
143+
assert model.invoke("anything").content == "hi"
144+
finally:
145+
subprocess.run, subprocess.Popen = real_run, real_popen
146+
147+
assert spawned == [], f"the mock double spawned a subprocess: {spawned}"
148+
149+
150+
@pytest.mark.parametrize("backend", ["openrouter", "openai", "ollama"])
151+
def test_a_bare_backend_with_no_default_model_is_refused_with_an_example(backend):
152+
"""These front catalogues, not a model. Guessing one would be guessing what
153+
to bill someone for, so the spec is refused — and the message has to carry
154+
a spelling that works, because "that is wrong" without "this is right" is
155+
what sent people to `--model openrouter` in the first place.
156+
"""
157+
from grapharc.gateway.registry import UnknownBackendError, split_spec
158+
159+
with pytest.raises(UnknownBackendError) as caught:
160+
split_spec(backend)
161+
message = str(caught.value)
162+
assert "names a backend, not a model" in message
163+
assert f"{backend}/" in message
164+
165+
166+
def test_the_bare_claude_cli_default_matches_the_model_class():
167+
"""`BARE_BACKEND_MODEL` holds a second copy of the model class's own default
168+
so the registry does not import a backend just to split a string. Two
169+
declarations of one value drift; this is the check that they have not, in
170+
the same spirit as CI pinning `__version__` to the packaged version.
171+
"""
172+
from grapharc.gateway.registry import BARE_BACKEND_MODEL
173+
174+
assert (
175+
BARE_BACKEND_MODEL["claude-cli"]
176+
== ClaudeCodeCLIChatModel.model_fields["model"].default
177+
)
178+
179+
180+
# ---- the CLI's failures explain themselves ----------------------------------
181+
182+
183+
def test_a_failure_reports_the_reason_the_cli_wrote_to_stdout():
184+
"""`claude -p` fails with a non-zero exit, an *empty stderr*, and the whole
185+
reason in its JSON envelope on stdout. Reading only stderr produced
186+
`claude -p exited 1: ` — a sentence that stops at the colon — and threw
187+
away the one string that said what was wrong. That is how a wrong model
188+
spec presented itself: as no message at all.
189+
"""
190+
import json
191+
192+
from grapharc.gateway.claude_cli import _payload_error
193+
194+
# The apostrophe in "There's" is the reason this is built with `json.dumps`
195+
# and not an f-string with quotes swapped.
196+
reason = "There's an issue with the selected model (claude-cli)."
197+
stdout = json.dumps({"is_error": True, "result": reason})
198+
199+
assert _payload_error(stdout) == reason
200+
# Not the envelope -> "" so the caller falls back to stderr rather than
201+
# inventing a reason from whatever happened to be on stdout.
202+
assert _payload_error("boom, not json") == ""
203+
assert _payload_error("[1, 2]") == ""
204+
assert _payload_error('{"is_error": true}') == ""
205+
206+
207+
def test_a_failure_with_a_stdout_reason_beats_an_empty_stderr(monkeypatch):
208+
"""End to end through `_invoke_cli`, because the bug was in which stream it
209+
read, not in parsing: the payload reader above can be perfect and the error
210+
still be blank if the call site keeps reaching for `proc.stderr`.
211+
"""
212+
import subprocess
213+
214+
from grapharc.gateway.errors import GatewayError
215+
216+
reason = "There's an issue with the selected model (nope)."
217+
218+
class _Proc:
219+
returncode = 1
220+
stdout = __import__("json").dumps({"is_error": True, "result": reason})
221+
stderr = ""
222+
223+
monkeypatch.setattr(subprocess, "run", lambda *a, **k: _Proc())
224+
model = ClaudeCodeCLIChatModel(model="nope")
225+
226+
with pytest.raises(GatewayError) as caught:
227+
model._invoke_cli(model._build_argv(None), "hi", ".")
228+
229+
assert reason in str(caught.value)
230+
assert not str(caught.value).endswith(": "), "the message stopped at the colon again"

0 commit comments

Comments
 (0)