Skip to content

fix(core): parse cc provider subprocess errors into clean messages#44

Open
anhermon wants to merge 1 commit into
devfrom
fix/anga-573-cc-error-parsing
Open

fix(core): parse cc provider subprocess errors into clean messages#44
anhermon wants to merge 1 commit into
devfrom
fix/anga-573-cc-error-parsing

Conversation

@anhermon

@anhermon anhermon commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Thinking Path

The claude CLI can return JSON with is_error: true when there's an account/model access error. The existing code blindly extracted the result field as success text, meaning error messages like "Your account does not have access..." were silently returned as if they were valid assistant responses.

What Changed

  • New parse_json_response() helper on ClaudeCodeProvider — parses the JSON output from claude --output-format json, checks is_error before extracting result, and returns a clean HarnessError::Provider on error responses.
  • Refactored run_subprocess() to delegate JSON parsing to parse_json_response() instead of inline parsing.
  • Added is_error check in stream() fallback path — the fallback that reads the result field from stream-json output now also checks for error responses before pushing a chunk.
  • 3 unit tests covering success, error detection, and missing-result scenarios via the extracted helper.

Verification

  • cargo test --workspace — all 82 tests pass
  • cargo clippy -- -D warnings — clean, no warnings

Risks

  • Low risk. The parse_json_response helper is a pure function extracted from existing logic with an added guard clause. The stream fallback change mirrors the same pattern.
  • No changes to the happy path behavior; only error responses are now caught earlier.

Checklist

  • cargo test --workspace passes
  • cargo clippy -- -D warnings passes
  • Conventional commit format
  • Unit tests for new logic

Closes ANGA-573

When the `claude` CLI returns a JSON response with `is_error: true`,
the provider now detects this and returns a clean `HarnessError::Provider`
message instead of silently treating the error text as a success response.

- Extract `parse_json_response()` helper for testability
- Refactor `run_subprocess()` to delegate to `parse_json_response()`
- Add `is_error` check in `stream()` fallback path
- Add unit tests for success, error, and missing-result cases

Closes ANGA-573

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@anhermon anhermon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

CTO Review — Approve

Clean fix. parse_json_response extraction is well-scoped, 3 focused tests cover success/error/missing-field. Clippy clean.

Minor: em dash in error prefix (cc provider —) — confirm consistency with rest of codebase. Consider extracting shared check_error_response helper to reduce duplication between parse_json_response and stream() fallback path.

No blocking issues. ✅

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.

1 participant