mirror of
https://github.com/Graphify-Labs/graphify.git
synced 2026-09-14 19:34:09 +08:00
fix(claude-cli): tolerate a diagnostic preamble before the JSON envelope
`claude -p --output-format json` shares stdout with the CLI's own
subsystems, so the envelope is not always the first thing on it. An
attached MCP server that advertises no tools makes every invocation emit
Client.listTools() called but server does not advertise tools
capability - returning empty list
ahead of the envelope, and `json.loads(stdout)` fails on the whole buffer.
The failure is raised after the model has already answered, so the chunk
is discarded with its tokens spent. On a real corpus this burned 149,582
input and 52,162 output tokens on chunk 1 of 3 and returned nothing, and
the error names the JSON rather than the preamble that caused it, so the
log points at the wrong thing. Anyone with an MCP server configured hits
it on every chunk.
`_envelope_after_preamble` scans for the first `[`/`{` that begins a
valid JSON document. `raw_decode` ignores trailing bytes, so a diagnostic
on either side is tolerated. The fast path is unchanged: clean stdout
still parses on the first `json.loads`, and stdout with no JSON at all
still returns None so the caller raises -- test_raises_on_garbage_envelope
holds.
(cherry picked from commit ede3a9e0a8)
This commit is contained in:
+40
-4
@@ -1524,6 +1524,40 @@ def _call_claude(api_key: str, model: str, user_message: str, max_tokens: int =
|
||||
return result
|
||||
|
||||
|
||||
def _envelope_after_preamble(stdout: str):
|
||||
"""Recover the envelope when `claude -p` prefixes it with a diagnostic line.
|
||||
|
||||
The CLI shares stdout with its own subsystems, so the JSON is not always the
|
||||
first thing on it. An attached MCP server that advertises no tools makes
|
||||
every invocation emit
|
||||
|
||||
Client.listTools() called but server does not advertise tools capability
|
||||
- returning empty list
|
||||
|
||||
ahead of the envelope, and `json.loads` then fails on the whole buffer.
|
||||
Because that failure is raised after the model has already answered, the
|
||||
chunk is discarded with its tokens spent -- on a mid-size corpus a run could
|
||||
burn the whole budget and return nothing, and the error names the JSON
|
||||
rather than the preamble that caused it, so the log points at the wrong
|
||||
thing. Any user with an MCP server configured hits this on every chunk.
|
||||
|
||||
Scans for the first `[`/`{` that begins a valid JSON document. `raw_decode`
|
||||
ignores trailing bytes, so a diagnostic on either side is tolerated, and
|
||||
stdout carrying no JSON at all still returns None for the caller to raise on.
|
||||
"""
|
||||
decoder = json.JSONDecoder()
|
||||
for idx, ch in enumerate(stdout):
|
||||
if ch not in "[{":
|
||||
continue
|
||||
try:
|
||||
value, _ = decoder.raw_decode(stdout, idx)
|
||||
except json.JSONDecodeError:
|
||||
continue
|
||||
if isinstance(value, (dict, list)):
|
||||
return value
|
||||
return None
|
||||
|
||||
|
||||
def _claude_cli_envelope(stdout: str) -> dict:
|
||||
"""Parse the JSON returned by `claude -p --output-format json`.
|
||||
|
||||
@@ -1536,10 +1570,12 @@ def _claude_cli_envelope(stdout: str) -> dict:
|
||||
try:
|
||||
envelope = json.loads(stdout)
|
||||
except json.JSONDecodeError as exc:
|
||||
raise RuntimeError(
|
||||
f"claude -p produced unparseable JSON envelope: {exc}; "
|
||||
f"first 500 chars of stdout: {stdout[:500]!r}"
|
||||
) from exc
|
||||
envelope = _envelope_after_preamble(stdout)
|
||||
if envelope is None:
|
||||
raise RuntimeError(
|
||||
f"claude -p produced unparseable JSON envelope: {exc}; "
|
||||
f"first 500 chars of stdout: {stdout[:500]!r}"
|
||||
) from exc
|
||||
if isinstance(envelope, list):
|
||||
result_events = [
|
||||
e for e in envelope
|
||||
|
||||
@@ -172,6 +172,43 @@ def test_call_llm_success_still_returns_result_text():
|
||||
assert llm._call_llm("dummy", backend="claude-cli") == "a fine label"
|
||||
|
||||
|
||||
_MCP_PREAMBLE = (
|
||||
"Client.listTools() called but server does not advertise tools capability "
|
||||
"- returning empty list\n"
|
||||
)
|
||||
|
||||
|
||||
def test_envelope_survives_a_diagnostic_preamble_on_stdout():
|
||||
"""An MCP server that advertises no tools must not kill the whole chunk.
|
||||
|
||||
The CLI writes that notice to stdout ahead of the envelope, so json.loads
|
||||
saw `Client.listTools()...` and raised. The failure lands after the model
|
||||
has answered, so the tokens are already spent and the chunk is dropped --
|
||||
every chunk, for anyone with an MCP server configured.
|
||||
"""
|
||||
completed = MagicMock(
|
||||
returncode=0, stdout=_MCP_PREAMBLE + json.dumps(_ENVELOPE), stderr=""
|
||||
)
|
||||
with patch("shutil.which", return_value="/fake/bin/claude"), \
|
||||
patch("subprocess.run", return_value=completed):
|
||||
result = llm._call_claude_cli("dummy", max_tokens=8192)
|
||||
assert [n["label"] for n in result["nodes"]] == ["Foo", "greet"]
|
||||
|
||||
|
||||
def test_envelope_survives_preamble_around_array_shaped_stream():
|
||||
"""Same, for the >= 2.1 array-of-events shape."""
|
||||
stream = [{"type": "system", "subtype": "init"}, _ENVELOPE]
|
||||
completed = MagicMock(
|
||||
returncode=0,
|
||||
stdout=_MCP_PREAMBLE + json.dumps(stream) + "\ntrailing noise\n",
|
||||
stderr="",
|
||||
)
|
||||
with patch("shutil.which", return_value="/fake/bin/claude"), \
|
||||
patch("subprocess.run", return_value=completed):
|
||||
result = llm._call_claude_cli("dummy", max_tokens=8192)
|
||||
assert [n["label"] for n in result["nodes"]] == ["Foo", "greet"]
|
||||
|
||||
|
||||
def test_raises_on_garbage_envelope():
|
||||
completed = MagicMock(returncode=0, stdout="not json", stderr="")
|
||||
with patch("shutil.which", return_value="/fake/bin/claude"), \
|
||||
|
||||
Reference in New Issue
Block a user