Skip to content

fix(claude-cli): prefer envelope error over stderr on chunk failure - #2733

Open
mdshzb04 wants to merge 2 commits into
Graphify-Labs:v8from
mdshzb04:fix/claude-cli-prefer-envelope-error
Open

fix(claude-cli): prefer envelope error over stderr on chunk failure#2733
mdshzb04 wants to merge 2 commits into
Graphify-Labs:v8from
mdshzb04:fix/claude-cli-prefer-envelope-error

Conversation

@mdshzb04

Copy link
Copy Markdown
Contributor

Summary

When a claude -p chunk failed under the claude-cli backend, the error message came from stderr first. If you have SessionEnd hooks (pretty common), stderr is mostly hook teardown noise and the real cause — rate limit, auth, etc. — sits in the stdout JSON envelope unread.

What changed

In _call_claude_cli and the claude-cli branch of _call_llm, non-zero exit now picks cli_error before stderr. Added two regression tests that fail on the old order.

Why

Fixes #2692. I hit this pattern on long extraction runs where every failed chunk reported "SessionEnd hook failed" instead of the actual API error.

Test plan

  • pytest tests/test_claude_cli_backend.py::test_nonzero_exit_prefers_envelope_error_over_stderr_hook_noise
  • pytest tests/test_claude_cli_backend.py::test_call_llm_nonzero_exit_prefers_envelope_error_over_stderr
  • existing envelope-when-stderr-empty tests still pass

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 Graphify-Labs#2692.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

This PR changes the error-detail precedence in the Claude CLI backend's non-zero exit handling. In both _call_claude_cli and _call_llm, when the process exits with a non-zero code, the code now prefers the error message parsed from the stdout JSON envelope over the raw stderr text (falling back to stderr, then to a placeholder). Two new tests are added to tests/test_claude_cli_backend.py that simulate a failure where stderr contains SessionEnd hook teardown noise while the stdout envelope holds the actual error, asserting the raised RuntimeError surfaces the envelope message and excludes the hook noise. The other listed symbols appear to be surrounding/unchanged context (rationale and helper functions) rather than functional edits in this diff.

Worth a look

  • claude-cli branch of _call_llm no longer surfaces stderr on nonzero exit with envelopegraphify/llm.py:2637 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 692 functions depend on the 211 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: _call_claude_cli() — 32 callers, 9 callees
  • worse: _call_llm() — 12 callers, 17 callees

Verification — 692 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 443 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify \_call\_claude\_cli.

The verifier did not have enough to check \_call\_claude\_cli, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly RuntimeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_call\_llm.

The verifier did not have enough to check \_call\_llm, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)

· 2 more finding(s) on lines outside this diff (see the check run).

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-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

This PR introduces a new helper _claude_cli_failure_detail in graphify/llm.py that constructs the error message for non-zero claude -p CLI exits, prioritizing the structured error from the stdout JSON envelope while appending a truncated copy of stderr when present. It replaces the inline error-detail logic in both _call_claude_cli and _call_llm with calls to this shared helper. The test file adds unit tests for the new helper and integration-style tests verifying the ordering of envelope error versus stderr hook noise in the raised exceptions.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 696 functions depend on the 215 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: _call_claude_cli() — 32 callers, 10 callees
  • worse: _call_llm() — 12 callers, 18 callees

Verification — 696 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 447 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify \_call\_claude\_cli.

The verifier did not have enough to check \_call\_claude\_cli, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly RuntimeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_call\_llm.

The verifier did not have enough to check \_call\_llm, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)

· 2 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[claude-cli] Chunk-failure detail prefers stderr over the JSON envelope error — hook teardown noise masks the real cause

1 participant