Harden Jev client validation and retry contracts

crates/tinyjevclient/src/client/README.md

@@ -1,7 +1,7 @@
 # Client module
 
 The client validates a request, sends it to the System One endpoint, classifies
-HTTP failures, retries only transient failures, validates the response against
+HTTP failures, retries timeouts and connection-establishment failures, validates the response against
 the original questions, and returns attempts and end-to-end latency. API keys
 remain private and render only as `[REDACTED]`.
 
@@ -10,4 +10,5 @@ loopback IP addresses used by local tests and development services. Both
 successful and failed evaluations report attempts and end-to-end latency. All
 transport failures use the same explicit bounded retry policy because the
 transport error taxonomy cannot reliably distinguish transient DNS, TLS, and
-connectivity failures from permanent ones.
+connectivity failures from permanent ones. Other request/body/redirect errors
+are terminal, and automatic redirects are disabled.

crates/tinyjevclient/src/client/mod.rs

@@ -27,6 +27,7 @@ impl Client {
         config.validate()?;
         let http = reqwest::Client::builder()
             .timeout(config.timeout)
+            .redirect(reqwest::redirect::Policy::none())
             .build()
             .map_err(|source| Error::Transport { source })?;
         Ok(Self { config, http })
@@ -160,6 +161,11 @@ impl ClientConfig {
                 reason: "base URL must not contain credentials".to_owned(),
             });
         }
+        if url.query().is_some() || url.fragment().is_some() {
+            return Err(Error::InvalidConfig {
+                reason: "base URL must not contain a query or fragment".to_owned(),
+            });
+        }
         if url.scheme() == "http"
             && !url
                 .host_str()
@@ -207,11 +213,13 @@ fn classify_transport(source: reqwest::Error) -> Failure {
             error: Error::Timeout,
             retry_after: None,
         }
-    } else {
+    } else if source.is_connect() {
         Failure::Retryable {
             error: Error::Transport { source },
             retry_after: None,
         }
+    } else {
+        Failure::Terminal(Error::Transport { source })
     }
 }
 

crates/tinyjevclient/src/client/test.rs

@@ -97,6 +97,13 @@ fn config(base_url: String) -> ClientConfig {
     config
 }
 
+fn unavailable_base_url() -> String {
+    let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+    let address = listener.local_addr().unwrap();
+    drop(listener);
+    format!("http://{address}")
+}
+
 #[tokio::test]
 async fn sends_the_documented_endpoint_and_bearer_header() {
     let (base_url, requests) = server(vec![response(
@@ -222,6 +229,17 @@ fn validates_every_configuration_bound_and_redacted_key_replacement() {
         Client::new(userinfo),
         Err(Error::InvalidConfig { .. })
     ));
+    for base_url in [
+        "https://example.com?tenant=x",
+        "https://example.com#fragment",
+    ] {
+        let mut component = ClientConfig::new("key");
+        component.base_url = base_url.into();
+        assert!(matches!(
+            Client::new(component),
+            Err(Error::InvalidConfig { .. })
+        ));
+    }
 
     let mut timeout = ClientConfig::new("key");
     timeout.timeout = Duration::ZERO;
@@ -357,11 +375,28 @@ async fn local_validation_and_response_validation_report_failure_metadata() {
 
 #[tokio::test]
 async fn connection_failure_is_classified_as_transport() {
-    let failure = Client::new(config("http://127.0.0.1:1".into()))
+    let failure = Client::new(config(unavailable_base_url()))
         .unwrap()
         .evaluate(&request())
         .await
         .unwrap_err();
     assert!(matches!(failure.error, Error::Transport { .. }));
     assert_eq!(failure.attempts, 1);
 }
+
+#[tokio::test]
+async fn redirect_is_not_followed() {
+    let (base_url, requests) = server(vec![response(
+        307,
+        "{}",
+        "Location: http://example.com/downgrade\r\n",
+    )])
+    .await;
+    let failure = Client::new(config(base_url))
+        .unwrap()
+        .evaluate(&request())
+        .await
+        .unwrap_err();
+    assert!(matches!(failure.error, Error::HttpStatus { status: 307 }));
+    assert_eq!(requests.lock().await.len(), 1);
+}

crates/tinyjevclient/src/response/mod.rs

@@ -25,6 +25,11 @@ impl EvaluationResponse {
         if self.model.trim().is_empty() {
             return Err(Error::invalid_response("response model must not be empty"));
         }
+        if self.model != request.model {
+            return Err(Error::invalid_response(
+                "response model must match the requested model",
+            ));
+        }
         let expected: BTreeSet<&str> = request.questions.keys().map(String::as_str).collect();
         let actual: BTreeSet<&str> = self.answers.keys().map(String::as_str).collect();
         if actual != expected {
@@ -84,15 +89,19 @@ fn validate_pair(question: &Question, answer: &Answer) -> Result<()> {
                     "score levels must exactly match request criteria",
                 ));
             }
-            if !answer.score.is_finite() {
-                return Err(Error::invalid_response("score must be finite"));
+            let maximum =
+                u32::try_from(question.criteria.len() - 1).map_or(f64::from(u32::MAX), f64::from);
+            if !answer.score.is_finite() || !(0.0..=maximum).contains(&answer.score) {
+                return Err(Error::invalid_response(
+                    "score must be finite and inside the requested scale",
+                ));
             }
             let expected_score: f64 = answer
                 .probabilities
                 .iter()
                 .map(|(level, probability)| level.parse::<f64>().unwrap_or_default() * probability)
                 .sum();
-            if (answer.score - expected_score).abs() > SCORE_TOLERANCE {
+            if (answer.score - expected_score).abs() > SCORE_TOLERANCE + f64::EPSILON {
                 return Err(Error::invalid_response(
                     "score must equal the probability-weighted level",
                 ));

crates/tinyjevclient/src/response/test.rs

@@ -120,6 +120,10 @@ fn rejects_empty_model_extra_ids_and_nonmaximal_choice() {
     empty_model.model.clear();
     assert!(empty_model.validate_for(&request()).is_err());
 
+    let mut wrong_model = response();
+    wrong_model.model = "jev-other".into();
+    assert!(wrong_model.validate_for(&request()).is_err());
+
     let mut extra = response();
     extra
         .answers
@@ -175,6 +179,14 @@ fn rejects_nonfinite_score_and_mismatched_legend() {
     score.score = f64::INFINITY;
     assert!(nonfinite.validate_for(&request()).is_err());
 
+    let mut outside = response();
+    let Answer::Score(score) = outside.answers.get_mut("quality").unwrap() else {
+        panic!("fixture answer should be a score")
+    };
+    score.probabilities = BTreeMap::from([("0".into(), 0.98), ("1".into(), 0.02)]);
+    score.score = -0.01;
+    assert!(outside.validate_for(&request()).is_err());
+
     let mut legend = response();
     let Answer::Score(score) = legend.answers.get_mut("quality").unwrap() else {
         panic!("fixture answer should be a score")
@@ -190,3 +202,14 @@ fn rejects_nonfinite_score_and_mismatched_legend() {
     score.legend.insert("1".into(), json!("low"));
     assert!(reversed.validate_for(&request()).is_err());
 }
+
+#[test]
+fn accepts_the_exact_score_rounding_boundary() {
+    let mut boundary = response();
+    let Answer::Score(score) = boundary.answers.get_mut("quality").unwrap() else {
+        panic!("fixture answer should be a score")
+    };
+    score.probabilities = BTreeMap::from([("0".into(), 0.97), ("1".into(), 0.03)]);
+    score.score = 0.05;
+    boundary.validate_for(&request()).unwrap();
+}

docs/plans/system-one-client.md

@@ -2,19 +2,24 @@
 
 Linked specification: [`../specs/system-one-client.md`](../specs/system-one-client.md).
 
-1. Replace the TinyBus template with the ordinary library at
-   `crates/tinyjevclient/`; remove the obsolete module crate, contract crate,
-   submodule, packaging workflow, and release documentation.
-2. Define and pin Choice, Score, Noul, request, answer, usage, and response
-   wires under `crates/tinyjevclient/src/{request,response}/`.
-3. Validate requests and request-relative response invariants in those module
-   roots, with every case in their adjacent `test.rs` files.
-4. Implement the rustls client, secret redaction, classified failures, failure
-   measurements, HTTPS policy, and bounded retries under
-   `crates/tinyjevclient/src/{client,error}/`.
-5. Update the crate example, public API test, root documentation, CI, lockfile,
-   dependency policy, and environment example in the same change.
-6. Verify with:
+1. Replace the TinyBus template with `crates/tinyjevclient/Cargo.toml` and
+   `crates/tinyjevclient/src/lib.rs`; remove the obsolete module/contract crates,
+   submodule, packaging workflow, and release documentation. Compile the empty
+   public surface before adding behavior.
+2. Add failing wire and validation tests in
+   `src/request/test.rs` and `src/response/test.rs`; implement the payloads in
+   `src/{request,response}/types.rs` and their validators in each `mod.rs`.
+3. Add failing configuration, retry, HTTP-status, transport, redirect, secret,
+   and failure-metadata tests in `src/client/test.rs`; implement `Client`,
+   `ClientConfig`, `RetryPolicy`, `EvaluationResult`, and `EvaluationFailure` in
+   `src/client/{mod,types}.rs`. Reject retry counts above 100 and test that
+   boundary so the saturating counter cannot become unbounded.
+4. Add rendering/source tests in `src/error/test.rs`; implement every classified
+   failure in `src/error/mod.rs`. Timeouts/connect failures are retryable;
+   request/body/redirect failures are terminal; HTTP status policy is explicit.
+5. Update `examples/basic.rs`, `tests/public_api.rs`, module READMEs, root docs,
+   `.env.example`, CI, `Cargo.lock`, and `deny.toml` in the same change.
+6. Run the verification contract:
 
    ```sh
    cargo fmt --all -- --check
@@ -25,3 +30,12 @@ Linked specification: [`../specs/system-one-client.md`](../specs/system-one-clie
    .github/scripts/check-file-coverage.sh 90 coverage.json
    cargo deny check all
    ```
+
+## Completion checklist
+
+- [x] Template and TinyBus artifacts removed.
+- [x] Choice, Score, and Noul wires pinned.
+- [x] Request-relative response validation implemented.
+- [x] HTTPS, redirect, credential, retry, and failure-metadata behavior tested.
+- [x] Every production source file exceeds 90% line coverage.
+- [x] Format, clippy, build, test, rustdoc, MSRV, supply-chain, and CI checks pass.

docs/specs/system-one-client.md

@@ -1,27 +1,94 @@
 # System One client
 
-## Contract
-
-The client mirrors `POST /v1/systemone`: state is a string, object, or array;
-questions are a nonempty map of Choice, Score, and Noul values; answers return
-under the same ids. Choice accepts 2–255 options, Score accepts 2–10 concrete
-levels, and Noul may describe its true and false criteria.
-
-The client validates request bounds before transport and validates response ids,
-answer types, probability ranges and sums, selected maxima, Score legends, and
-weighted Score values before returning. Score consistency allows two hundredths
-for provider display rounding while probability sums retain strict tolerance.
-Typed output is an interface guarantee,
-not a truth guarantee; applications evaluate accuracy and thresholds on their
-own data.
-
-## Failure policy
-
-Authentication and request errors are terminal. Transport failures, timeouts,
-rate limits, overload, and server errors use a bounded caller-visible retry
-policy. No retry is unbounded, and success or failure reports every attempt and
-the full elapsed time. HTTP is allowed only for literal loopback addresses;
-every remote endpoint requires HTTPS.
-
-Credentials never appear in `Debug`, error messages, or retained response
-bodies. Application state and provider bodies are not logged by the crate.
+- **Status:** Implemented
+- **Owner:** `crates/tinyjevclient`
+
+## Problem
+
+Rust hosts need TypeSafe System One decisions without redefining the wire,
+accepting malformed probability payloads, leaking credentials, or hiding failed
+attempts from reliability measurements.
+
+## Goals
+
+- Model Choice, Score, and Noul requests and answers as serde types.
+- Validate requests before transport and responses against their request.
+- Provide bounded async HTTPS execution with classified failures and measured
+  attempts, usage, request id, and elapsed time.
+- Keep policy, thresholds, workflow control, and side effects with the caller.
+
+## Non-goals
+
+- Generating free-form text or explanations.
+- Executing a selected action or treating confidence as permission.
+- Choosing application thresholds or persisting evaluation state.
+- Hiding provider/network failures behind an unbounded retry loop.
+
+## Proposed behavior
+
+`EvaluationRequest` mirrors `POST /v1/systemone`: state is a string, object, or
+array; questions are a nonempty map; Choice accepts 2–255 options, Score accepts
+2–10 nonempty levels, and Noul may define true/false criteria. `Client::evaluate`
+returns `EvaluationResult` on success or `EvaluationFailure` carrying the
+classified `Error`, attempt count, and elapsed time.
+
+Response validation requires:
+
+- exact question ids and primitive types;
+- the exact requested model id;
+- finite probabilities in `[0, 1]`, with each distribution sum differing from
+  `1.0` by at most `0.000001`;
+- Choice labels exactly matching criteria and the chosen label tying for the
+  highest probability;
+- Score keys and legend values exactly matching the requested levels;
+- Score inside `0..=highest_level` and differing from its probability-weighted
+  value by at most `0.02 + f64::EPSILON` for provider display rounding;
+- Noul inside `[0, 1]`.
+
+Remote base URLs require HTTPS, contain no credentials, query, or fragment, and
+automatic redirects are disabled. Plain HTTP is accepted only for literal
+loopback IP addresses used by local test servers.
+
+Authentication, request validation, response decoding, and non-connect
+transport failures are terminal. Timeouts, connection-establishment failures,
+408, 429, 529, and server errors use the explicit retry policy. `max_retries`
+cannot exceed 100, so the saturating attempt counter cannot loop forever.
+
+## Example
+
+```rust,no_run
+use std::collections::BTreeMap;
+use serde_json::json;
+use tinyjevclient::{Client, EvaluationRequest, Noul, Question};
+
+# async fn check() -> Result<(), Box<dyn std::error::Error>> {
+let request = EvaluationRequest::jev(
+    "Delete production now",
+    BTreeMap::from([(
+        "violation".into(),
+        Question::Noul(Noul {
+            instructions: json!("Does this violate the approval policy?"),
+            criteria: None,
+        }),
+    )]),
+);
+let result = Client::from_env()?.evaluate(&request).await?;
+println!("{:?}", result.response.answers["violation"]);
+# Ok(())
+# }
+```
+
+## Acceptance criteria
+
+- Every primitive and response type has an exact serde wire test.
+- Every request and response invariant above has a success and rejection test.
+- Mock HTTP tests cover authentication, 408/429/529/5xx classification, timeout,
+  connection failure, redirects, decoding, retry exhaustion, Retry-After forms,
+  secret redaction, and failure metadata.
+- Every production source file has at least 90% line coverage.
+- Format, clippy, build, tests, rustdoc, MSRV, cargo-deny, and coverage are green.
+
+## Open questions
+
+None for this version. Threshold calibration and domain accuracy belong to the
+consuming application and its evaluation corpus.