refactor(daemon): extract the provider stream guards and cover them

apps/daemon/src/session/codex.rs

@@ -8,6 +8,7 @@
 //!   `codex --approval-mode full-auto -q "<content>"`
 
 use super::runner::Runner;
+use super::stream_guards::{is_rate_limit_notice, would_exceed_cap};
 use crate::{ipc::event::EventBroadcaster, storage::Storage};
 use anyhow::{Context, Result};
 use async_trait::async_trait;
@@ -98,13 +99,7 @@ impl CodexRunner {
             let mut lines = BufReader::new(stderr).lines();
             while let Ok(Some(line)) = lines.next_line().await {
                 debug!(target: "codex_stderr", "{}", line);
-                // Detect rate-limit patterns: "rate limit", "too many requests", "429".
-                let lower = line.to_lowercase();
-                if lower.contains("rate limit")
-                    || lower.contains("rate_limit")
-                    || lower.contains("too many requests")
-                    || lower.contains("429")
-                {
+                if is_rate_limit_notice(&line) {
                     broadcaster_err.broadcast(
                         "session.statusChanged",
                         json!({
@@ -167,7 +162,9 @@ impl CodexRunner {
             trace!(session = %self.session_id, line = %line, "codex output");
 
             // Cap accumulated output at 1 MB to prevent OOM on runaway Codex output.
-            if !truncated && accumulated.len() + line.len() + 1 > Self::MAX_ACCUMULATED_BYTES {
+            if !truncated
+                && would_exceed_cap(accumulated.len(), line.len(), Self::MAX_ACCUMULATED_BYTES)
+            {
                 warn!(
                     session = %self.session_id,
                     bytes = accumulated.len(),

apps/daemon/src/session/cursor.rs

@@ -25,6 +25,7 @@
 //! via the `CURSOR_TOKEN` env var when spawning the subprocess.
 
 use super::runner::Runner;
+use super::stream_guards::{is_rate_limit_notice, would_exceed_cap};
 use crate::{ipc::event::EventBroadcaster, storage::Storage};
 use anyhow::{Context, Result};
 use async_trait::async_trait;
@@ -167,12 +168,7 @@ impl CursorRunner {
             let mut lines = BufReader::new(stderr).lines();
             while let Ok(Some(line)) = lines.next_line().await {
                 debug!(target: "cursor_stderr", "{}", line);
-                let lower = line.to_ascii_lowercase();
-                if lower.contains("rate limit")
-                    || lower.contains("rate_limit")
-                    || lower.contains("too many requests")
-                    || lower.contains("429")
-                {
+                if is_rate_limit_notice(&line) {
                     broadcaster_err.broadcast(
                         "session.statusChanged",
                         json!({
@@ -236,7 +232,9 @@ impl CursorRunner {
             trace!(session = %self.session_id, line = %line, "cursor output");
 
             // Cap accumulated output at 1 MB.
-            if !truncated && accumulated.len() + line.len() + 1 > Self::MAX_ACCUMULATED_BYTES {
+            if !truncated
+                && would_exceed_cap(accumulated.len(), line.len(), Self::MAX_ACCUMULATED_BYTES)
+            {
                 warn!(
                     session = %self.session_id,
                     bytes = accumulated.len(),

apps/daemon/src/session/mod.rs

@@ -5,6 +5,7 @@ pub mod cursor;
 pub mod events;
 pub mod router;
 pub mod runner;
+pub mod stream_guards;
 pub mod system_prompt;
 pub mod telemetry;
 pub mod worktree;

apps/daemon/src/session/stream_guards.rs

@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: MIT
+//! Guards applied to a provider's output stream as it is read.
+//!
+//! Both the Codex and Cursor runners read a child process's stdout and stderr
+//! line by line, and both apply the same two rules to every line: notice when
+//! the provider is telling us it has been rate limited, and stop accumulating
+//! once the captured output would pass a byte cap.
+//!
+//! The rules lived inline and identically in both runners, which meant they
+//! could only be exercised by spawning a real child process — so in practice
+//! they were not exercised at all.  Pulling them out here makes them ordinary
+//! pure functions with ordinary tests, and leaves one definition instead of
+//! two to keep in step.
+
+/// Substrings that mean a provider is refusing work because of a rate limit.
+///
+/// Matched case-insensitively against a single line of provider output.
+const RATE_LIMIT_MARKERS: [&str; 4] = ["rate limit", "rate_limit", "too many requests", "429"];
+
+/// Returns `true` if `line` looks like a rate-limit notice from a provider.
+///
+/// # Inputs
+/// * `line` — one raw line of provider stderr/stdout, any case.
+///
+/// # Outputs
+/// `true` when any known marker appears anywhere in the line.
+///
+/// # Constraints
+/// Matching is case-insensitive and substring-based, so it is deliberately
+/// permissive: a false positive costs one spurious `RATE_LIMITED` status
+/// event, whereas a false negative means a stalled session with no
+/// explanation.
+pub(crate) fn is_rate_limit_notice(line: &str) -> bool {
+    let lower = line.to_lowercase();
+    RATE_LIMIT_MARKERS
+        .iter()
+        .any(|marker| lower.contains(marker))
+}
+
+/// Returns `true` if appending `line_len` bytes to `accumulated_len` bytes
+/// would push the captured output past `cap`.
+///
+/// # Inputs
+/// * `accumulated_len` — bytes captured so far.
+/// * `line_len` — bytes in the line about to be appended.
+/// * `cap` — the hard ceiling, in bytes.
+///
+/// # Outputs
+/// `true` when the line must be refused.
+///
+/// # Constraints
+/// The `+ 1` accounts for the newline the caller appends after each line, so
+/// the check matches what is actually stored rather than what was read.  The
+/// comparison is strict: landing exactly on `cap` is allowed, since the cap is
+/// the largest permitted size and not the first forbidden one.
+pub(crate) fn would_exceed_cap(accumulated_len: usize, line_len: usize, cap: usize) -> bool {
+    accumulated_len + line_len + 1 > cap
+}
+
+#[cfg(test)]
+mod tests;

apps/daemon/src/session/stream_guards/tests.rs

@@ -0,0 +1,102 @@
+//! Tests for the provider output-stream guards.
+//!
+//! These two rules previously lived inline in the Codex and Cursor runners,
+//! where reaching them meant spawning a real child process — so every mutant
+//! the gate generated for them survived.  Each test below is pinned to a value
+//! that a specific mutation would change.
+
+use super::*;
+
+// ─── is_rate_limit_notice ────────────────────────────────────────────────────
+
+/// Every marker is pinned separately, so no single one can be dropped from the
+/// list unnoticed. Each string here matches exactly one marker: dropping that
+/// marker makes the line unrecognised and the session stalls with no status
+/// event to explain why.
+#[test]
+fn each_rate_limit_marker_is_recognised_on_its_own() {
+    assert!(is_rate_limit_notice("error: rate limit exceeded"));
+    assert!(is_rate_limit_notice("error: rate_limit_exceeded"));
+    assert!(is_rate_limit_notice("HTTP 400: too many requests"));
+    assert!(is_rate_limit_notice("server responded 429"));
+}
+
+/// Matching is case-insensitive — pins the `to_lowercase()` call, without
+/// which a provider shouting about a RATE LIMIT would be ignored.
+#[test]
+fn markers_are_matched_regardless_of_case() {
+    assert!(is_rate_limit_notice("RATE LIMIT EXCEEDED"));
+    assert!(is_rate_limit_notice("Too Many Requests"));
+    assert!(is_rate_limit_notice("Rate_Limit"));
+}
+
+/// The negative direction, so the assertions above cannot be satisfied by a
+/// function that returns `true` for everything.
+#[test]
+fn ordinary_output_is_not_a_rate_limit_notice() {
+    assert!(!is_rate_limit_notice(""));
+    assert!(!is_rate_limit_notice("compiling 12 crates"));
+    assert!(!is_rate_limit_notice("HTTP 200 OK"));
+    // Near misses: a different 4xx, and the words apart from each other.
+    assert!(!is_rate_limit_notice("server responded 42"));
+    assert!(!is_rate_limit_notice("HTTP 409: conflict"));
+    assert!(!is_rate_limit_notice("too many open files"));
+}
+
+// ─── would_exceed_cap ────────────────────────────────────────────────────────
+
+/// Kills both `+` -> `*` and `+` -> `-` mutations of the size sum.
+///
+/// The numbers are chosen so the true sum is distinguishable from every
+/// mutation of it: 10 + 5 + 1 = 16, against 10 * 5 + 1 = 51, 10 - 5 + 1 = 6,
+/// 10 + 5 * 1 = 15 and 10 + 5 - 1 = 14. With a cap of 15 the real answer is
+/// "yes, this line would exceed it", while `+ 5 * 1`, `+ 5 - 1` and `10 - 5`
+/// all say no, and `10 * 5` says yes for the wrong reason — so the companion
+/// test below pins a case where the products and the sum disagree the other
+/// way round.
+#[test]
+fn a_line_that_would_pass_the_cap_is_refused() {
+    assert!(would_exceed_cap(10, 5, 15));
+}
+
+/// The other side of the same sum: 10 + 5 + 1 = 16 fits under a cap of 16,
+/// but 10 * 5 + 1 = 51 does not. Together with the test above this pins every
+/// arithmetic mutation in both directions.
+#[test]
+fn a_line_that_exactly_fills_the_cap_is_accepted() {
+    assert!(!would_exceed_cap(10, 5, 16));
+}
+
+/// Kills `>` -> `>=`, `==` and `<`.
+///
+/// Landing exactly on the cap must be allowed, which separates `>` from `>=`
+/// and from `==`; being well under it must be allowed, which separates `>`
+/// from `<`. The cap is the largest permitted size, not the first forbidden
+/// one, and an off-by-one here silently truncates output that fit.
+#[test]
+fn the_cap_is_an_inclusive_ceiling() {
+    // Exactly at the cap: allowed.
+    assert!(!would_exceed_cap(100, 99, 200));
+    // One byte past it: refused.
+    assert!(would_exceed_cap(100, 100, 200));
+    // Far below it: allowed.
+    assert!(!would_exceed_cap(1, 1, 200));
+    // Far above it: refused.
+    assert!(would_exceed_cap(500, 1, 200));
+}
+
+/// The newline the caller appends is counted. Without the `+ 1` a line that
+/// exactly fills the cap would be accepted and then stored one byte over.
+#[test]
+fn the_trailing_newline_counts_against_the_cap() {
+    // 4 + 5 = 9 bytes of text, plus the newline, is 10 — one past a cap of 9.
+    assert!(would_exceed_cap(4, 5, 9));
+    assert!(!would_exceed_cap(4, 5, 10));
+}
+
+/// An empty line still costs its newline.
+#[test]
+fn an_empty_line_still_costs_one_byte() {
+    assert!(!would_exceed_cap(9, 0, 10));
+    assert!(would_exceed_cap(10, 0, 10));
+}