From cf22e70dee4be7e9819d33a28f1e35b2901f4241 Mon Sep 17 00:00:00 2001 From: kragent66-glitch Date: Sat, 17 Oct 2026 12:30:32 +0530 Subject: [PATCH 1/2] fix(retry): retry a 408 Request Timeout, which is not a request defect A 408 says the server gave up reading the request before the body arrived -- it is a statement about the clock, not about the bytes we built. Every other permanent 4xx means the request itself is wrong and rebuilding it produces the same wrong bytes, so re-sending is pointless. A 408 is the exception: the identical request is worth sending again unchanged. This is the follow-up the maintainer asked for on #589, which fixed the opposite case (permanent 4xx being retried three times). - provider.rs: 408 joins 5xx and 429 in `is_retryable`, with the reasoning written where the policy lives, plus a unit test pinning it. - planner.rs: `a_408_request_timeout_goes_out_twice` -- a stub that times out on the first attempt and answers on the second, asserting the request reached the provider exactly twice and that the retry re-sent it unchanged (no correction appended, unlike the retryable 400 tool-use case). - test baseline: 1,991 -> 1,993 Rust tests, updated in README.md, docs/introduction.md, docs/distro-support.md and tests/evidence/. --- README.md | 2 +- crates/sysknife-brain/src/planner.rs | 69 +++++++++++++++++++++++++++ crates/sysknife-brain/src/provider.rs | 37 +++++++++++--- docs/distro-support.md | 2 +- docs/introduction.md | 2 +- tests/evidence/workspace-tests.json | 2 +- 6 files changed, 104 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9d8906a4..a4024ece 100644 --- a/README.md +++ b/README.md @@ -351,7 +351,7 @@ milestone. | **Every Ubuntu LTS validated** โ€” 22.04, 24.04 and 26.04 all at 79/79, each with a replay twin that reproduces it | โœ… | | Telegram approval interface | ๐Ÿ“‹ roadmap | -**1,991 Rust tests and 72 frontend tests** form the current deterministic +**1,993 Rust tests and 72 frontend tests** form the current deterministic release baseline. ## Configure your LLM diff --git a/crates/sysknife-brain/src/planner.rs b/crates/sysknife-brain/src/planner.rs index 42532fb8..23d23623 100644 --- a/crates/sysknife-brain/src/planner.rs +++ b/crates/sysknife-brain/src/planner.rs @@ -1742,6 +1742,75 @@ mod tests { ); } + /// A 408 Request Timeout is the second 4xx worth retrying, and for a + /// different reason than the tool-use 400: the status says the *server* did + /// not see the whole request in time, so nothing about the bytes we built is + /// known to be wrong. The identical request is re-sent, unchanged, and must + /// reach the provider a second time. + /// + /// The stub times out on the first attempt and answers on the second, so the + /// request count is exactly two โ€” "a 408 goes out twice", not "a 408 goes out + /// until the retry budget is spent". + #[tokio::test] + async fn a_408_request_timeout_goes_out_twice() { + struct TimesOutOnceThenAnswers { + calls: Arc, + second: Arc>>, + } + + #[async_trait::async_trait] + impl LlmProvider for TimesOutOnceThenAnswers { + async fn complete( + &self, + _system: &str, + messages: &[Message], + _tools: &[ToolDefinition], + _max_tokens: u32, + ) -> Result { + if self.calls.fetch_add(1, Ordering::SeqCst) == 0 { + return Err(rig_status( + http::StatusCode::REQUEST_TIMEOUT, + "request timed out", + )); + } + *self.second.lock().await = messages.to_vec(); + Ok(Completion { + content: vec![ContentBlock::Text { + text: "recovered".into(), + }], + stop_reason: StopReason::EndTurn, + }) + } + } + + let calls = Arc::new(AtomicUsize::new(0)); + let second = Arc::new(tokio::sync::Mutex::new(Vec::new())); + let planner = LlmPlanner::new( + Box::new(TimesOutOnceThenAnswers { + calls: Arc::clone(&calls), + second: Arc::clone(&second), + }), + Box::new(UnusedState), + 1, + ); + planner + .complete_with_retry("SYS", &[Message::user_text("hi")], &[]) + .await + .expect("the second attempt succeeds"); + + assert_eq!( + calls.load(Ordering::SeqCst), + 2, + "a 408 must be re-sent exactly once, so it goes out twice" + ); + let sent = second.lock().await; + assert_eq!( + sent.len(), + 1, + "a 408 retry re-sends the request unchanged โ€” no correction is appended" + ); + } + /// A 400 that *is* worth retrying: the retry appends a correction, so the /// bytes change. It must reach the provider a second time. #[tokio::test] diff --git a/crates/sysknife-brain/src/provider.rs b/crates/sysknife-brain/src/provider.rs index f233f096..347ee365 100644 --- a/crates/sysknife-brain/src/provider.rs +++ b/crates/sysknife-brain/src/provider.rs @@ -165,8 +165,8 @@ pub enum ProviderError { /// `providers::classify_status`; the `Auth`/`RateLimit` classes become their /// own variants, and `StatusClass::Other` โ€” every 4xx and 5xx that is not /// 401, 403, or 429 โ€” becomes this variant, keeping the status intact. - /// [`is_retryable`](Self::is_retryable) then applies the real policy: 5xx and - /// 429 deserve a backoff, a permanent 4xx does not. + /// [`is_retryable`](Self::is_retryable) then applies the real policy: 5xx, + /// 429, and 408 deserve a backoff; a permanent 4xx does not. /// /// This is the only *status-bearing* variant, and it is deliberately the one /// the policy was written for. [`Request`](Self::Request) is now reserved for @@ -226,11 +226,19 @@ impl ProviderError { // 5xx is the provider's own fault and usually momentary; 429 means // "later", which is exactly what a backoff provides. Every other 4xx // is a defect in the request just built, and building it again - // produces the same bytes โ€” except a tool-use rejection, where the - // retry appends a correction and so does send different bytes. That - // is the one 4xx worth re-asking about. + // produces the same bytes โ€” except two cases. + // + // A tool-use rejection is one: the retry appends a correction, so it + // does send different bytes, and without this arm the correction + // would never reach the model. + // + // 408 Request Timeout is the other, and it is not a statement about + // the request at all โ€” the server gave up *reading* it before the + // body arrived. The bytes were fine; the read did not win its race + // with the clock. Re-sending the identical request is the whole fix, + // so it retries even though it is a 4xx. ProviderError::Http { status, .. } => { - *status >= 500 || *status == 429 || self.is_invalid_tool_call() + *status >= 500 || *status == 429 || *status == 408 || self.is_invalid_tool_call() } ProviderError::RateLimit(_) => true, ProviderError::Auth(_) => false, @@ -331,6 +339,23 @@ mod retryability_tests { ); } + /// The other 4xx worth retrying, and for a different reason than a tool-use + /// rejection. A 408 Request Timeout says the *server* did not receive the + /// whole request in time โ€” nothing about the bytes we sent is known to be + /// wrong, so re-sending the identical request is the right move rather than a + /// request defect. + #[test] + fn a_request_timeout_is_retryable_despite_being_a_4xx() { + assert!( + ProviderError::Http { + status: 408, + body: "request timed out".into() + } + .is_retryable(), + "408 means the server timed out reading the request, not that our request was wrong" + ); + } + #[test] fn auth_failure_is_not_retryable() { assert!(!ProviderError::Auth("invalid api key".into()).is_retryable()); diff --git a/docs/distro-support.md b/docs/distro-support.md index 920d84d9..8944190b 100644 --- a/docs/distro-support.md +++ b/docs/distro-support.md @@ -91,7 +91,7 @@ family and the atomic story family are implemented and covered by the workspace suite. What is missing is a way to put the helpers somewhere the daemon's own grants already point. -The deterministic workspace baseline is 1,991 Rust tests plus 72 frontend +The deterministic workspace baseline is 1,993 Rust tests plus 72 frontend tests. Those tests verify action construction, policy, approval, storage, and UI behavior, but they do not replace a real distribution VM run. diff --git a/docs/introduction.md b/docs/introduction.md index fb9a188b..724166a1 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -144,7 +144,7 @@ flow. ## Status -192 typed actions ยท 1,991 Rust tests + 72 frontend tests ยท MIT +192 typed actions ยท 1,993 Rust tests + 72 frontend tests ยท MIT SysKnife is the reference implementation of the [LACS specification](https://github.com/lacs-project/specification) โ€” a diff --git a/tests/evidence/workspace-tests.json b/tests/evidence/workspace-tests.json index 46fd23d8..b3e4e3eb 100644 --- a/tests/evidence/workspace-tests.json +++ b/tests/evidence/workspace-tests.json @@ -4,6 +4,6 @@ "tests": "cargo nextest run --workspace --locked" }, "frontend_tests": 72, - "tests": 1991, + "tests": 1993, "version": 2 } From 8fc2aff8677100f2538a239c64a6d62a057ffa6f Mon Sep 17 00:00:00 2001 From: kragent66-glitch Date: Sat, 17 Oct 2026 13:24:54 +0530 Subject: [PATCH 2/2] chore(deps): lock rustls 0.23.46, clearing four RustSec advisories cargo audit refuses rustls 0.23.45 with four vulnerabilities published on 2026-10-14 -- RUSTSEC-2026-0346 through -0349 (ECH rejection buffering, Brotli certificate decompression, a ServerHello key-exchange downgrade, and TLS secrets exposed through Debug impls). All four declare `patched = ">= 0.23.46"`, so the single-package lockfile bump clears them. This is not caused by the 408 change: security-audit fails on main for the same reason (the advisories landed after main's last green run). It is included here because the gate cannot pass otherwise, and a lockfile-only dependency bump is maintenance rather than development. Cargo.lock: rustls 0.23.45 -> 0.23.46 (version + checksum, no other crate moved). Verified: cargo nextest run --workspace --locked -- 1993 passed, 6 skipped. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 89a9896a..d78bcc71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4005,9 +4005,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.45" +version = "0.23.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" +checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" dependencies = [ "aws-lc-rs", "once_cell",