Skip to content

fix(agent): classify HTTP status errors as ErrorGeneral not ErrorNetwork Refs terraphim/terraphim-ai#1992 (Gitea)#899

Open
AlexMikhalev wants to merge 7 commits into
mainfrom
task/1992-http-status-exit-codes
Open

fix(agent): classify HTTP status errors as ErrorGeneral not ErrorNetwork Refs terraphim/terraphim-ai#1992 (Gitea)#899
AlexMikhalev wants to merge 7 commits into
mainfrom
task/1992-http-status-exit-codes

Conversation

@AlexMikhalev
Copy link
Copy Markdown
Contributor

Summary

Re-submission of Gitea PR #2011 (which had a GO review) as a GitHub PR.

  • classify_error now distinguishes HTTP status errors from true transport failures:
    • 401/403 → ErrorAuth (exit 5)
    • 404 → ErrorNotFound (exit 4)
    • other 4xx/5xx → ErrorGeneral (exit 1)
    • connection refused / DNS failure → ErrorNetwork (exit 6)
  • Adds five unit-level regression tests using a real one-shot TCP server (no mocks)
  • Adds server_http_error_exits_1 integration test in tests/exit_codes.rs that runs the real binary against a 400-responding server and asserts exit 0 or 1, never 6

Test plan

  • cargo test -p terraphim_agent --features server classify_reqwest — unit tests covering 400, 401, 403, 404, 500
  • cargo test -p terraphim_agent --features server -- server_http_error_exits_1 — integration test
  • cargo test -p terraphim_agent --features server -- unreachable_server_exits_6 — existing test still passes

Manual verification

cargo build --features server -p terraphim_agent
# Point at a server returning 400
echo 'HTTP/1.1 400\r\n\r\n' | nc -l 9999 &
./target/debug/terraphim-agent --server --server-url http://127.0.0.1:9999 search terraphim; echo $?
# Expected: 1 (not 6)

Refs #1992

🤖 Generated with Terraphim AI

Test User and others added 7 commits June 4, 2026 00:14
…1992

reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.

Now check re.status() before falling back to ErrorNetwork:
- 401 / 403  → ErrorAuth (5)
- 404        → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status  → ErrorNetwork (6, true connectivity failure)

Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039

Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.

Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133

Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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