From b8c698714ae8a6e98587f7b8810d9356ff0b997f Mon Sep 17 00:00:00 2001 From: shazeb Date: Fri, 14 Aug 2026 10:18:23 +0530 Subject: [PATCH 1/2] fix(claude-cli): prefer envelope error over stderr on chunk failure When claude -p exits non-zero, hook teardown noise on stderr was masking the structured error in the stdout JSON envelope. Swap the fallback order so rate limits and auth failures surface correctly. Fixes #2692. --- graphify/llm.py | 4 ++-- tests/test_claude_cli_backend.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index 46a4785d3..00338c74a 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -1573,7 +1573,7 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo ) cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - detail = proc.stderr.strip() or cli_error or "(no stderr, no error envelope)" + detail = cli_error or proc.stderr.strip() or "(no stderr, no error envelope)" raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") if cli_error: raise RuntimeError(f"claude -p reported an error: {cli_error[:500]}") @@ -2634,7 +2634,7 @@ def _rec(inp, out) -> None: ) cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - detail = proc.stderr.strip() or cli_error or "(no stderr, no error envelope)" + detail = cli_error or proc.stderr.strip() or "(no stderr, no error envelope)" raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") if cli_error: # Without this the error text is returned as the model's reply and diff --git a/tests/test_claude_cli_backend.py b/tests/test_claude_cli_backend.py index 4f5187ab4..7fe531414 100644 --- a/tests/test_claude_cli_backend.py +++ b/tests/test_claude_cli_backend.py @@ -152,6 +152,36 @@ def test_call_llm_raises_on_error_envelope(): llm._call_llm("dummy", backend="claude-cli") +def test_nonzero_exit_prefers_envelope_error_over_stderr_hook_noise(): + # SessionEnd hook teardown often fills stderr on failure; the real cause + # (rate limit, auth, etc.) lives in the stdout JSON envelope (#2692). + hook_noise = "SessionEnd hook [notify]: failed: Hook cancelled" + completed = MagicMock( + returncode=1, + stdout=json.dumps(_ERROR_ENVELOPE), + stderr=hook_noise, + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached") as exc: + llm._call_claude_cli("dummy", max_tokens=8192) + assert hook_noise not in str(exc.value) + + +def test_call_llm_nonzero_exit_prefers_envelope_error_over_stderr(): + hook_noise = "SessionEnd hook [notify]: failed: Hook cancelled" + completed = MagicMock( + returncode=1, + stdout=json.dumps(_ERROR_ENVELOPE), + stderr=hook_noise, + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached") as exc: + llm._call_llm("dummy", backend="claude-cli") + assert hook_noise not in str(exc.value) + + def test_call_llm_nonzero_exit_surfaces_envelope_error(): completed = MagicMock( returncode=1, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", From f3231bd0c20aba8f21f1010aaabd8b8985248bb4 Mon Sep 17 00:00:00 2001 From: shazeb Date: Fri, 14 Aug 2026 10:32:48 +0530 Subject: [PATCH 2/2] fix(claude-cli): append stderr after envelope error on chunk failure Keep the stdout JSON envelope as the primary failure message while still surfacing a truncated stderr suffix when both are present, addressing the review note that stderr-only diagnostics were dropped entirely. --- graphify/llm.py | 18 ++++++++++++++++-- tests/test_claude_cli_backend.py | 21 +++++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index 00338c74a..64338472c 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -1406,6 +1406,20 @@ def _claude_cli_error(stdout: str) -> str: return "unspecified error" +def _claude_cli_failure_detail(cli_error: str, stderr: str) -> str: + """Build the error string for a non-zero ``claude -p`` exit. + + The stdout JSON envelope carries the CLI's structured failure (rate limits, + auth, etc.). stderr often holds hook teardown noise instead (#2692), so the + envelope leads — but when stderr also has content, append a truncated copy + so diagnostics are not lost entirely. + """ + stderr = stderr.strip() + if cli_error and stderr: + return f"{cli_error}; stderr: {stderr[:200]}" + return cli_error or stderr or "(no stderr, no error envelope)" + + # A JSON Schema pinning the top-level shape graphify consumes. Passed to # `claude -p --json-schema` (structured output) so the CLI CONSTRAINS the model # to emit the object directly instead of relying on it CHOOSING to honour a @@ -1573,7 +1587,7 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo ) cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - detail = cli_error or proc.stderr.strip() or "(no stderr, no error envelope)" + detail = _claude_cli_failure_detail(cli_error, proc.stderr) raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") if cli_error: raise RuntimeError(f"claude -p reported an error: {cli_error[:500]}") @@ -2634,7 +2648,7 @@ def _rec(inp, out) -> None: ) cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - detail = cli_error or proc.stderr.strip() or "(no stderr, no error envelope)" + detail = _claude_cli_failure_detail(cli_error, proc.stderr) raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") if cli_error: # Without this the error text is returned as the model's reply and diff --git a/tests/test_claude_cli_backend.py b/tests/test_claude_cli_backend.py index 7fe531414..b53672224 100644 --- a/tests/test_claude_cli_backend.py +++ b/tests/test_claude_cli_backend.py @@ -98,6 +98,19 @@ def test_raises_on_nonzero_exit(): } +def test_claude_cli_failure_detail_prefers_envelope_and_keeps_stderr(): + detail = llm._claude_cli_failure_detail( + "API Error: Rate limit reached", + "SessionEnd hook failed", + ) + assert detail.startswith("API Error: Rate limit reached; stderr:") + assert "SessionEnd hook failed" in detail + + +def test_claude_cli_failure_detail_stderr_only_when_no_envelope(): + assert llm._claude_cli_failure_detail("", "auth failed") == "auth failed" + + def test_nonzero_exit_surfaces_envelope_error_when_stderr_empty(): # The CLI reports API failures in the stdout JSON envelope, not on stderr. # Without reading it the user gets a bare "exited 1: " and no cause (#2554). @@ -165,7 +178,9 @@ def test_nonzero_exit_prefers_envelope_error_over_stderr_hook_noise(): patch("subprocess.run", return_value=completed): with pytest.raises(RuntimeError, match="Rate limit reached") as exc: llm._call_claude_cli("dummy", max_tokens=8192) - assert hook_noise not in str(exc.value) + msg = str(exc.value) + assert msg.index("Rate limit reached") < msg.index(hook_noise) + assert f"stderr: {hook_noise}" in msg def test_call_llm_nonzero_exit_prefers_envelope_error_over_stderr(): @@ -179,7 +194,9 @@ def test_call_llm_nonzero_exit_prefers_envelope_error_over_stderr(): patch("subprocess.run", return_value=completed): with pytest.raises(RuntimeError, match="Rate limit reached") as exc: llm._call_llm("dummy", backend="claude-cli") - assert hook_noise not in str(exc.value) + msg = str(exc.value) + assert msg.index("Rate limit reached") < msg.index(hook_noise) + assert f"stderr: {hook_noise}" in msg def test_call_llm_nonzero_exit_surfaces_envelope_error():