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

.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<String, (Operator, u128)>,
+    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<Conversation>;
+    async fn delete(&self, id: &ConversationId) -> Result<()>;
+    async fn list_ids(&self) -> Result<Vec<ConversationId>>;
+    async fn exists(&self, id: &ConversationId) -> Result<bool>;
+    async fn list_summaries(&self) -> Result<Vec<ConversationSummary>>;
+}
+```
+
+---
+
+## `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.` |

.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).

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)

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::<reqwest::Error>()) {
-        let is_timeout = err
-            .chain()
-            .filter_map(|e| e.downcast_ref::<reqwest::Error>())
-            .any(|re| re.is_timeout());
-        if is_timeout {
-            return ExitCode::ErrorTimeout;
+        for e in err.chain() {
+            if let Some(re) = e.downcast_ref::<reqwest::Error>() {
+                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![