diff --git a/.docs/api-reference-snippets.md b/.docs/api-reference-snippets.md new file mode 100644 index 000000000..4248b93d8 --- /dev/null +++ b/.docs/api-reference-snippets.md @@ -0,0 +1,103 @@ +# API Reference Snippets + +Generated: 2026-06-04 + +Selected public types with proposed or existing doc comments. + +--- + +## `terraphim_persistence` + +### `DeviceStorage` + +**Source:** `crates/terraphim_persistence/src/lib.rs:66` + +```rust +/// Process-wide singleton managing all configured storage backends, ordered by +/// latency. `fastest_op` is the lowest-latency operator used as the cache +/// write-back target. Obtain via [`DeviceStorage::instance`]; use +/// [`DeviceStorage::init_memory_only`] in tests to avoid filesystem access. +pub struct DeviceStorage { + pub ops: HashMap, + pub fastest_op: Operator, +} +``` + +Key methods: `instance()`, `init_memory_only()`, `arc_instance()`, `arc_memory_only()`. + +--- + +### `Persistable` + +**Source:** `crates/terraphim_persistence/src/lib.rs:220` + +```rust +/// A trait for persisting objects to and from the fastest configured storage +/// operator (file system, database, or cloud backend). +/// +/// Implementors serialise to JSON and delegate I/O to `DeviceStorage`'s +/// `fastest_op`. Provides full CRUD: `save`, `load`, `get`, `delete`. +#[async_trait] +pub trait Persistable: Serialize + DeserializeOwned { + fn new(key: String) -> Self; + async fn save(&self) -> Result<()>; + // ... +} +``` + +--- + +### `ConversationPersistence` + +**Source:** `crates/terraphim_persistence/src/conversation.rs:10` + +```rust +/// Async trait for saving, loading, and listing [`Conversation`] records. +/// +/// The production implementation (`OpenDALConversationPersistence`) maintains +/// an in-memory `ConversationIndex` cache and fans writes to all configured +/// operators. +#[async_trait] +pub trait ConversationPersistence: Send + Sync { + async fn save(&self, conversation: &Conversation) -> Result<()>; + async fn load(&self, id: &ConversationId) -> Result; + async fn delete(&self, id: &ConversationId) -> Result<()>; + async fn list_ids(&self) -> Result>; + async fn exists(&self, id: &ConversationId) -> Result; + async fn list_summaries(&self) -> Result>; +} +``` + +--- + +## `haystack_core` + +### `HaystackProvider` + +**Source:** `crates/haystack_core/src/lib.rs:8` + +Proposed doc comment (currently missing): + +```rust +/// Abstraction over a data source (haystack) that can be indexed and searched. +/// +/// Implement this trait for each external system (filesystem, Confluence, +/// Discourse, email) to expose it to the terraphim search pipeline. +pub trait HaystackProvider { ... } +``` + +--- + +## `terraphim_types` — Priority Gaps + +The following core types in `lib.rs` are undocumented and referenced across +the workspace. One-line proposed comments: + +| Type | Proposed doc | +|------|-------------| +| `RoleName` (line 171) | `/// Newtype wrapper around a role identifier string.` | +| `NormalizedTerm` (line 306) | `/// A term normalised for Aho-Corasick automata matching.` | +| `Concept` (line 438) | `/// A knowledge-graph node linking a normalised term to source documents.` | +| `DocumentType` (line 476) | `/// Classifies a document by its source format or provenance.` | +| `MarkdownDirectives` (line 606) | `/// Structured directives parsed from a Markdown document's front-matter.` | +| `RouteDirective` (line 488) | `/// Specifies how an agent command should be dispatched.` | diff --git a/.docs/doc-report-latest.md b/.docs/doc-report-latest.md new file mode 100644 index 000000000..d5862802a --- /dev/null +++ b/.docs/doc-report-latest.md @@ -0,0 +1,131 @@ +# Documentation Audit Report + +**Date:** 2026-06-04 +**Agent:** documentation-generator +**Scope:** 6 key crates (consumer-facing, high-traffic) + +--- + +## Executive Summary + +| Metric | Value | +|--------|-------| +| Crates scanned | 6 | +| Total public items | 882 | +| Undocumented | 339 | +| Coverage | 62% | +| SIGNIFICANT_GAPS | 5 crates | +| MINOR_GAPS | 1 crate | +| CLEAN | 0 crates | + +--- + +## Per-Crate Results + +| Crate | Public Items | Undocumented | % Gap | Verdict | +|-------|-------------|--------------|-------|---------| +| `terraphim_types` | 344 | 128 | 37% | SIGNIFICANT_GAPS | +| `terraphim_service` | 250 | 89 | 36% | SIGNIFICANT_GAPS | +| `terraphim_sessions` | 111 | 44 | 40% | SIGNIFICANT_GAPS | +| `terraphim_automata` | 142 | 62 | 44% | SIGNIFICANT_GAPS | +| `terraphim_persistence` | 34 | 15 | 44% | SIGNIFICANT_GAPS | +| `haystack_core` | 1 | 1 | 100% | MINOR_GAPS | + +--- + +## Priority Undocumented Items + +### `terraphim_persistence` — highest-density critical types + +| File | Line | Type | Name | +|------|------|------|------| +| `lib.rs` | 66 | struct | `DeviceStorage` | +| `lib.rs` | 220 | trait | `Persistable` | +| `error.rs` | 5 | enum | `Error` | +| `conversation.rs` | 10 | trait | `ConversationPersistence` | +| `conversation.rs` | 32 | struct | `ConversationIndex` | +| `lib.rs` | 18-24 | mod | 7 undocumented `pub mod` re-exports | + +### `terraphim_types` — most undocumented items (128) + +Top priority (referenced workspace-wide): + +| File | Line | Type | Name | +|------|------|------|------| +| `lib.rs` | 171 | struct | `RoleName` | +| `lib.rs` | 262 | struct | `NormalizedTermValue` | +| `lib.rs` | 306 | struct | `NormalizedTerm` | +| `lib.rs` | 438 | struct | `Concept` | +| `lib.rs` | 476 | enum | `DocumentType` | +| `lib.rs` | 488 | struct | `RouteDirective` | +| `lib.rs` | 606 | struct | `MarkdownDirectives` | +| `medical_types.rs` | 36 | enum | `MedicalNodeType` | +| `medical_types.rs` | 141 | enum | `MedicalEdgeType` | +| `medical_types.rs` | 383 | struct | `MedicalNodeMetadata` | +| `hgnc.rs` | 12 | struct | `HgncGene` | + +### `terraphim_sessions` — core domain types (44 gaps) + +| File | Line | Type | Name | +|------|------|------|------| +| `model.rs` | 258 | struct | `Session` | +| `model.rs` | 165 | struct | `Message` | +| `model.rs` | 220 | struct | `SessionMetadata` | +| `model.rs` | 21 | enum | `MessageRole` | +| `model.rs` | 62 | enum | `ContentBlock` | +| `model.rs` | 756 | enum | `FileOperation` | +| `service.rs` | — | fn | all public methods | + +### `terraphim_automata` — evaluation and UMLS types (62 gaps) + +| File | Line | Type | Name | +|------|------|------|------| +| `evaluation.rs` | 17 | struct | `GroundTruthDocument` | +| `evaluation.rs` | 28 | struct | `ExpectedMatch` | +| `evaluation.rs` | 37 | struct | `ClassificationMetrics` | +| `evaluation.rs` | 48 | struct | `TermReport` | +| `evaluation.rs` | 55 | struct | `EvaluationResult` | +| `umls.rs` | 14 | struct | `UmlsConcept` | +| `umls.rs` | 47 | struct | `UmlsDataset` | +| `url_protector.rs` | 55 | struct | `ProtectedUrl` | + +### `haystack_core` — trivial fix + +| File | Line | Type | Name | +|------|------|------|------| +| `lib.rs` | 8 | trait | `HaystackProvider` | + +--- + +## CHANGELOG.md Updates + +Added under `### Fixed` (2026-06-04): +- `fix(agent)`: HTTP 4xx classified as `ErrorGeneral` not `ErrorNetwork`; integration test `server_http_error_exits_1` added (Refs #1992) + +The `docs(specs)` commit (98fa93b32) is an internal documentation update with no user-facing changelog entry. + +--- + +## Recommendations + +1. **Quick wins** (< 30 min combined): + - `haystack_core::HaystackProvider` — one line + - `terraphim_persistence::Error` enum — one line + - `terraphim_persistence` module re-exports (7 lines) + +2. **Medium effort** (half-day): + - `terraphim_persistence` core traits: `DeviceStorage`, `Persistable`, `ConversationPersistence` + - `terraphim_types` core structs: `RoleName`, `NormalizedTerm`, `Concept`, `DocumentType` + +3. **Batch effort** (track as issue): + - `terraphim_sessions::model` — all domain types + - `terraphim_automata::evaluation` — metrics structs + - `terraphim_service` module re-exports + +See `.docs/api-reference-snippets.md` for proposed doc comments on key types. + +--- + +## Gitea + +Findings posted as comment on issue #2137 (Theme-ID: doc-gap). diff --git a/CHANGELOG.md b/CHANGELOG.md index 4866957c8..778cff227 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Agent HTTP error classification** HTTP 4xx responses now classified as `ErrorGeneral` rather than `ErrorNetwork`; integration test `server_http_error_exits_1` added (Refs #1992, 2026-06-04) - **Redis security exposure** Docker Compose Redis service now binds to `127.0.0.1:6379` instead of `0.0.0.0:6379` to prevent unintended public exposure of the cache (Refs #1313, 2026-05-31) - **Nested `cargo run` in exit-code tests** replaced with `cargo_bin!` macro to avoid file-lock deadlock under concurrent `cargo test` (2026-06-01) - **ADF KG-router fallback respawn loop** closed after quota exit — agents no longer re-routed to `anthropic/sonnet` indefinitely when per-agent config or quota-fallback chose another provider (Refs #1793, PR#1794, 2026-05-22) diff --git a/crates/terraphim_agent/src/main.rs b/crates/terraphim_agent/src/main.rs index 291d18a42..c7d5fd9ad 100644 --- a/crates/terraphim_agent/src/main.rs +++ b/crates/terraphim_agent/src/main.rs @@ -1338,12 +1338,23 @@ fn classify_error(err: &anyhow::Error) -> robot::exit_codes::ExitCode { #[cfg(feature = "server")] if err.chain().any(|e| e.is::()) { - let is_timeout = err - .chain() - .filter_map(|e| e.downcast_ref::()) - .any(|re| re.is_timeout()); - if is_timeout { - return ExitCode::ErrorTimeout; + for e in err.chain() { + if let Some(re) = e.downcast_ref::() { + if re.is_timeout() { + return ExitCode::ErrorTimeout; + } + // HTTP status errors (4xx/5xx) are application errors, not connectivity failures. + // A 400 from a misconfigured role must not masquerade as a network error. + if let Some(status) = re.status() { + return match status.as_u16() { + 401 | 403 => ExitCode::ErrorAuth, + 404 => ExitCode::ErrorNotFound, + _ => ExitCode::ErrorGeneral, + }; + } + // True transport error (connection refused, DNS failure, etc.) + return ExitCode::ErrorNetwork; + } } return ExitCode::ErrorNetwork; } @@ -1507,6 +1518,103 @@ mod classify_error_tests { } } +/// Regression tests for classify_error with real reqwest HTTP status errors. +/// These verify that HTTP 4xx responses are not misclassified as network errors. +#[cfg(all(test, feature = "server"))] +mod classify_reqwest_tests { + use super::*; + use robot::exit_codes::ExitCode; + use std::io::Write; + use std::net::TcpListener; + + /// Start a one-shot TCP server that replies with the given HTTP status code. + fn start_status_server(status: u16, reason: &'static str) -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buf = [0u8; 2048]; + let _ = std::io::Read::read(&mut stream, &mut buf); + let response = format!( + "HTTP/1.1 {} {}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + status, reason + ); + let _ = stream.write_all(response.as_bytes()); + } + }); + port + } + + #[tokio::test] + async fn http_400_maps_to_error_general_not_network() { + let port = start_status_server(400, "Bad Request"); + let client = reqwest::Client::new(); + let res = client + .get(format!("http://127.0.0.1:{}/", port)) + .send() + .await + .unwrap(); + let reqwest_err: anyhow::Error = res.error_for_status().unwrap_err().into(); + assert_eq!( + classify_error(&reqwest_err), + ExitCode::ErrorGeneral, + "HTTP 400 from server must not be classified as ErrorNetwork (exit 6)" + ); + } + + #[tokio::test] + async fn http_401_maps_to_error_auth() { + let port = start_status_server(401, "Unauthorized"); + let client = reqwest::Client::new(); + let res = client + .get(format!("http://127.0.0.1:{}/", port)) + .send() + .await + .unwrap(); + let reqwest_err: anyhow::Error = res.error_for_status().unwrap_err().into(); + assert_eq!(classify_error(&reqwest_err), ExitCode::ErrorAuth); + } + + #[tokio::test] + async fn http_403_maps_to_error_auth() { + let port = start_status_server(403, "Forbidden"); + let client = reqwest::Client::new(); + let res = client + .get(format!("http://127.0.0.1:{}/", port)) + .send() + .await + .unwrap(); + let reqwest_err: anyhow::Error = res.error_for_status().unwrap_err().into(); + assert_eq!(classify_error(&reqwest_err), ExitCode::ErrorAuth); + } + + #[tokio::test] + async fn http_404_maps_to_error_not_found() { + let port = start_status_server(404, "Not Found"); + let client = reqwest::Client::new(); + let res = client + .get(format!("http://127.0.0.1:{}/", port)) + .send() + .await + .unwrap(); + let reqwest_err: anyhow::Error = res.error_for_status().unwrap_err().into(); + assert_eq!(classify_error(&reqwest_err), ExitCode::ErrorNotFound); + } + + #[tokio::test] + async fn http_500_maps_to_error_general() { + let port = start_status_server(500, "Internal Server Error"); + let client = reqwest::Client::new(); + let res = client + .get(format!("http://127.0.0.1:{}/", port)) + .send() + .await + .unwrap(); + let reqwest_err: anyhow::Error = res.error_for_status().unwrap_err().into(); + assert_eq!(classify_error(&reqwest_err), ExitCode::ErrorGeneral); + } +} + /// Build a ForgivingParser with the actual CLI subcommands. fn build_cli_forgiving_parser() -> forgiving::ForgivingParser { let mut commands = vec![ diff --git a/crates/terraphim_agent/tests/exit_codes.rs b/crates/terraphim_agent/tests/exit_codes.rs index 15b1cf5ea..4a6be26b6 100644 --- a/crates/terraphim_agent/tests/exit_codes.rs +++ b/crates/terraphim_agent/tests/exit_codes.rs @@ -146,6 +146,55 @@ fn unreachable_server_exits_6() { ); } +// --------------------------------------------------------------------------- +// Exit code 1 -- HTTP status error from server (must NOT be exit 6) +// --------------------------------------------------------------------------- + +/// Regression test for #1992: HTTP 4xx responses from the server must produce +/// exit 1 (ErrorGeneral), not exit 6 (ErrorNetwork). +/// +/// Starts a real TCP listener that replies with 400 to every request, so the +/// binary receives a genuine reqwest::Error with a status code attached. +/// Before the fix, classify_error returned ErrorNetwork (6) for all reqwest +/// errors; after the fix it checks re.status() first. +#[cfg(feature = "server")] +#[test] +fn server_http_error_exits_1() { + use std::io::Write; + use std::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + + std::thread::spawn(move || { + for mut s in listener.incoming().flatten() { + let mut buf = [0u8; 2048]; + let _ = std::io::Read::read(&mut s, &mut buf); + let _ = s.write_all( + b"HTTP/1.1 400 Bad Request\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ); + } + }); + + let status = cmd() + .args([ + "--server", + "--server-url", + &format!("http://127.0.0.1:{port}"), + "search", + "terraphim", + ]) + .output() + .expect("failed to run binary") + .status; + let code = status.code().unwrap_or(1); + assert!( + code == 0 || code == 1, + "HTTP 400 from server must exit 0 (offline fallback) or 1 (ErrorGeneral), \ + not 6 (ErrorNetwork); got {code}" + ); +} + // --------------------------------------------------------------------------- // Robot mode and --format json error envelopes // --------------------------------------------------------------------------- diff --git a/crates/terraphim_agent/tests/shared_learning_cli_tests.rs b/crates/terraphim_agent/tests/shared_learning_cli_tests.rs index bc26b2637..5a605b4be 100644 --- a/crates/terraphim_agent/tests/shared_learning_cli_tests.rs +++ b/crates/terraphim_agent/tests/shared_learning_cli_tests.rs @@ -38,12 +38,13 @@ async fn shared_list_empty_store() { async fn shared_list_with_trust_level_filter() { let store = create_store().await; - let l1 = SharedLearning::new( + let mut l1 = SharedLearning::new( "L1 learning".to_string(), "content".to_string(), SharedLearningSource::Manual, "test-agent".to_string(), ); + l1.promote_to_l1(); store.insert(l1).await.expect("insert l1"); let mut l2 = SharedLearning::new( @@ -52,6 +53,7 @@ async fn shared_list_with_trust_level_filter() { SharedLearningSource::Manual, "test-agent".to_string(), ); + l2.promote_to_l1(); l2.promote_to_l2(); store.insert(l2).await.expect("insert l2"); @@ -92,6 +94,7 @@ async fn shared_promote_l1_to_l2() { let id = learning.id.clone(); store.insert(learning).await.expect("insert"); + store.promote_to_l1(&id).await.expect("promote to l1"); store.promote_to_l2(&id).await.expect("promote to l2"); let fetched = store.get(&id).await.expect("get after promote"); @@ -124,12 +127,13 @@ async fn shared_stats_counts() { // Insert 2 L1, 1 L2 for i in 0..2 { - let l = SharedLearning::new( + let mut l = SharedLearning::new( format!("L1 item {}", i), "content".to_string(), SharedLearningSource::Manual, "agent".to_string(), ); + l.promote_to_l1(); store.insert(l).await.expect("insert l1"); } @@ -139,6 +143,7 @@ async fn shared_stats_counts() { SharedLearningSource::Manual, "agent".to_string(), ); + l2.promote_to_l1(); l2.promote_to_l2(); store.insert(l2).await.expect("insert l2"); @@ -173,7 +178,7 @@ async fn shared_import_creates_l1_entries() { let error = "remote: error: denied".to_string(); let tags = vec!["git".to_string(), "push".to_string()]; - let shared = SharedLearning::new( + let mut shared = SharedLearning::new( command.clone(), error.clone(), SharedLearningSource::BashHook, @@ -182,6 +187,7 @@ async fn shared_import_creates_l1_entries() { .with_original_command(command) .with_error_context(error) .with_keywords(tags); + shared.promote_to_l1(); store .insert(shared) @@ -236,7 +242,8 @@ async fn shared_store_survives_restart() { let id = learning.id.clone(); store.insert(learning).await.expect("insert"); - // 3. Promote it to L2 + // 3. Promote it to L2 via L1 (required promotion path: L0 → L1 → L2) + store.promote_to_l1(&id).await.expect("promote to l1"); store.promote_to_l2(&id).await.expect("promote to l2"); // 4. Drop the store (simulating process exit) diff --git a/crates/terraphim_orchestrator/src/lib.rs b/crates/terraphim_orchestrator/src/lib.rs index 95302471d..4b7adeab6 100644 --- a/crates/terraphim_orchestrator/src/lib.rs +++ b/crates/terraphim_orchestrator/src/lib.rs @@ -274,7 +274,8 @@ pub struct AgentOrchestrator { active_flows: HashMap>, /// Active compound review execution (spawned in background to avoid /// blocking reconcile_tick). None when no compound review is running. - active_compound_review: Option>>, + active_compound_review: + Option>>, /// Per-project mention cursors, keyed by project id. /// /// Each project gets its own cursor so repo-wide polls can advance @@ -6208,14 +6209,12 @@ impl AgentOrchestrator { if elapsed > std::time::Duration::from_secs(5) { warn!( tick = self.tick_count, - elapsed_ms, - "reconcile_tick SLOW: took > 5s, likely blocking agent polling" + elapsed_ms, "reconcile_tick SLOW: took > 5s, likely blocking agent polling" ); } else { info!( tick = self.tick_count, - elapsed_ms, - "reconcile_tick complete" + elapsed_ms, "reconcile_tick complete" ); } } @@ -8036,9 +8035,7 @@ Remove the pause flag once the underlying failure is resolved:\n\n\ let git_ref = "HEAD".to_string(); let base_ref = self.config.compound_review.base_branch.clone(); let workflow = self.compound_workflow.clone(); - let handle = tokio::spawn(async move { - workflow.run(&git_ref, &base_ref).await - }); + let handle = tokio::spawn(async move { workflow.run(&git_ref, &base_ref).await }); self.active_compound_review = Some(handle); } ScheduleEvent::Flow(flow_def) => { diff --git a/crates/terraphim_rolegraph/src/lib.rs b/crates/terraphim_rolegraph/src/lib.rs index c707b6cd9..077a54207 100644 --- a/crates/terraphim_rolegraph/src/lib.rs +++ b/crates/terraphim_rolegraph/src/lib.rs @@ -296,8 +296,10 @@ pub struct SerializableRoleGraph { /// reverse lookup - matched id into normalized term pub ac_reverse_nterm: AHashMap, /// Trigger descriptions for each node_id (used to rebuild TriggerIndex) + #[serde(default)] pub trigger_descriptions: AHashMap, /// Node IDs that are pinned (always included in results) + #[serde(default)] pub pinned_node_ids: Vec, /// Document IDs that were indexed from shared learnings #[serde(default)] @@ -2302,4 +2304,34 @@ mod tests { let results = restored.find_matching_node_ids_with_fallback("trigger one text", false); assert!(results.contains(&1u64)); } + + #[test] + async fn serde_default_round_trip_old_json_without_trigger_fields() { + // Simulate a persisted SerializableRoleGraph written before issue #84 was merged. + // We serialise a fresh RoleGraph, strip trigger_descriptions and pinned_node_ids + // from the JSON, then deserialise. serde(default) must supply empty collections + // rather than returning a missing-field error. + let role = "test role".to_string(); + let thesaurus = Thesaurus::new("test".to_string()); + let rolegraph = RoleGraph::new(role.into(), thesaurus).await.unwrap(); + let serializable = rolegraph.to_serializable(); + let full_json = serializable.to_json().unwrap(); + + // Remove the two fields introduced by issue #84 to simulate old JSON. + let mut value: serde_json::Value = + serde_json::from_str(&full_json).expect("serialization produced invalid JSON"); + value + .as_object_mut() + .unwrap() + .remove("trigger_descriptions"); + value.as_object_mut().unwrap().remove("pinned_node_ids"); + let old_json = serde_json::to_string(&value).unwrap(); + + let result = SerializableRoleGraph::from_json(&old_json); + assert!(result.is_ok(), "expected Ok but got: {:?}", result.err()); + let sg = result.unwrap(); + assert!(sg.trigger_descriptions.is_empty()); + assert!(sg.pinned_node_ids.is_empty()); + assert!(sg.learning_document_ids.is_empty()); + } } diff --git a/reports/spec-validation-20260604.md b/reports/spec-validation-20260604.md new file mode 100644 index 000000000..7328e2ce4 --- /dev/null +++ b/reports/spec-validation-20260604.md @@ -0,0 +1,76 @@ +## spec-validator verdict: CONDITIONAL PASS + +**Date**: 2026-06-04 00:29 CEST +**Agent**: Carthos (Domain Architect / spec-validator) +**Issue**: #1992 — `test_server_mode_search_with_selected_role` HTTP 400 → exit 6 + +--- + +### Findings Summary + +| Severity | Finding | +|----------|---------| +| P1 BLOCKER | PR #2011 targets Gitea `main`, but `crates/terraphim_agent` does not exist on Gitea — it was extracted to GitHub (origin) in the E1-E5 polyrepo cycle. `mergeable: false` is structural, not a rebase conflict. | +| P2 GAP | No unit or integration test covers `classify_error` with a real `reqwest::Error` that has `is_status() == true`. The `is_status()` branch added by PR #2011 is unverified by any test. | + +--- + +### Spec Alignment Analysis + +**Contract** (`crates/terraphim_agent/src/robot/exit_codes.rs`): + +| Code | Name | Semantic | +|------|------|----------| +| 1 | `ErrorGeneral` | General/unspecified error | +| 4 | `ErrorNotFound` | No results found | +| 6 | `ErrorNetwork` | **Network or connectivity issue** | + +**Violation on `main`**: `classify_error` (line 1340) returns `ErrorNetwork` (6) for ANY `reqwest::Error`, including HTTP status responses (400, 404, etc.). HTTP 400 is a semantic protocol response — not a network/connectivity failure. This violates the documented meaning of `ErrorNetwork`. + +**Test assertion** (`server_mode_tests.rs:222`): `code == 0 || code == 1` is correct. A search failure due to unknown role configuration is a general error (1), not a network error (6). + +**Fix in PR #2011** (`crates/terraphim_agent/src/main.rs` diff): +```rust +if re.is_status() { + if let Some(status) = re.status() { + if status == reqwest::StatusCode::NOT_FOUND { + return ExitCode::ErrorNotFound; + } + } + return ExitCode::ErrorGeneral; +} +// Connection refused, DNS failure, transport errors → network error. +return ExitCode::ErrorNetwork; +``` + +This is **semantically correct** and consistent with the documented exit code contract. Existing test `unreachable_server_exits_6` (connection refused → code 0 or 6) is unaffected because `is_status()` returns `false` for transport-level failures. + +--- + +### Actions Required + +**P1 (blocks merge)**: Re-route the fix to GitHub. The PR must target `origin/main` (GitHub) where `crates/terraphim_agent` exists. Either: +- Open a new GitHub PR from the same branch `task/1992-fix-search-http-status-exit-code`, OR +- Close PR #2011 on Gitea and apply the change directly to GitHub + +**P2 (test coverage)**: Add a test that invokes a live server returning HTTP 400 and asserts `code == 1`. The integration test suite at `crates/terraphim_agent/tests/exit_codes.rs` already has `unreachable_server_exits_6` (lines 128–147) as the pattern. A companion test `server_http_error_exits_1` using a responding server with a bad query would close this gap. No mocks — use a real server or test with the existing `start_test_server()` fixture. + +--- + +### Existing Test Coverage + +Six `classify_error` unit tests in `mod classify_error_tests` (lines 1393–1507) use plain `anyhow::anyhow!("string")` errors. None exercise the `#[cfg(feature = "server")]` reqwest path. The `is_status()` branch added by the fix has **zero test coverage** in both unit and integration suites. + +--- + +### Traceability + +| Req | Requirement | Spec Ref | Impl Ref | Test | Status | +|-----|-------------|----------|----------|------|--------| +| REQ-01 | HTTP status errors exit 1, not 6 | `exit_codes.rs` `ErrorNetwork` doc | `main.rs:1340` classify_error | `server_mode_tests.rs:222` | ⚠️ fix exists (PR#2011), not merged | +| REQ-02 | `ErrorNetwork` reserved for transport failures | `exit_codes.rs:18` | `main.rs:1348` | `exit_codes.rs:130` unreachable_server | ✅ | +| REQ-03 | `is_status()` path tested | — | PR#2011 `main.rs:1340–1355` | **MISSING** | ❌ | + +--- + +If PR #2011 is re-submitted targeting GitHub with a companion integration test for the `is_status()` path, verdict becomes **PASS**.