Skip to content

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

Open
acamarata wants to merge 1 commit into
mainfrom
refactor/stream-guards
Open

acamarata wants to merge 1 commit into
mainfrom
refactor/stream-guards

Conversation

@acamarata

Copy link
Copy Markdown
Contributor

The Codex and Cursor runners each applied the same two rules to every line of provider output — notice a rate-limit message, and stop accumulating once the capture would pass the 1 MB cap — written out inline and identically in both files.

Because they sat inside an async method reading a spawned child process, the only way to reach them was to spawn a real child. So in practice they were never exercised, and every mutant the gate generated for them survived, in both files.

This moves them into session/stream_guards.rs as two pure functions with tests. One definition instead of two, and the rules are now testable without a child process.

Mutants killed

All 11 mutations of the extracted code were hand-applied and the suite confirmed to fail:

mutation killed
accumulated_len + line_len*, - ✅ ✅
line_len + 1*, - ✅ ✅
> cap<, ==, >= ✅ ✅ ✅
removal of each of the 4 rate-limit markers ✅ ✅ ✅ ✅

Two fixtures are needed to pin the arithmetic, because no single one can do it: 10 + 5 + 1 = 16 exceeds a cap of 15 but fits a cap of 16, while the * mutation gives 51 and exceeds both. One test takes each side.

The cap is pinned as an inclusive ceiling — landing exactly on it is allowed — which is what separates > from >= and ==. An off-by-one there silently truncates output that fit.

Two decisions worth flagging

The arithmetic is deliberately not saturating. accumulated_len.saturating_add(line_len) would read as more defensive, but it removes the + operators the gate mutates — which deletes those mutants from the metric instead of killing them. That would show up as an improved score without any improvement. Plain + keeps them in scope, and the tests kill them.

One intentional behaviour change. cursor.rs matched with to_ascii_lowercase() while codex.rs used to_lowercase(). The shared helper uses to_lowercase(). Every marker is ASCII, so no current input is affected; the difference only shows on non-ASCII provider output, where the shared version is the more correct of the two. Flagging it because it is a real, if narrow, change rather than a pure move.

Still not covered

The pid != 0 guards in pause/resume/stop and the surrounding async runner bodies are untouched here. They need a live child process, and the pid guards carry a genuine hazard: mutating != 0 to == 0 makes the code call libc::kill(0, ...), which signals the entire process group — so they cannot be verified by hand-applying mutants in-process the way everything above was. They need an isolated harness, which is separate work.

cargo fmt --check, cargo clippy --all-targets -- -D warnings and the full 815-test lib suite are clean.

The Codex and Cursor runners each applied the same two rules to every line
of provider output: notice a rate-limit message, and stop accumulating once
the capture would pass the 1 MB cap. Both rules were written out inline and
identically in both files.

Because they sat inside an async method that reads a spawned child process,
the only way to reach them was to spawn a real child -- so in practice they
were never exercised, and every mutant the gate generated for them survived
in both files.

This moves them into session/stream_guards.rs as two pure functions and
gives them tests. One definition now instead of two, and the rules can be
tested without a child process.

Behaviour is unchanged with one deliberate exception: cursor.rs matched with
to_ascii_lowercase() while codex.rs used to_lowercase(). The shared helper
uses to_lowercase(). Every marker is ASCII so no current input is affected;
the difference only shows on non-ASCII output, where the shared version is
the more correct of the two.

The arithmetic is left as plain `accumulated_len + line_len + 1 > cap`
rather than saturating. Saturating addition would read as more defensive,
but it would remove the `+` operators the gate mutates, which deletes those
mutants from the metric instead of killing them. The point is to kill them.

All 11 mutations of the extracted code were hand-applied and the suite
confirmed to fail: both `+` operators against `*` and `-`, the `>`
comparison against `<`, `==` and `>=`, and the removal of each of the four
rate-limit markers in turn.

Two fixtures pin the arithmetic from both sides, because no single one can:
10 + 5 + 1 = 16 exceeds a cap of 15 but fits a cap of 16, while the `*`
mutation gives 51 and exceeds both. The cap itself is pinned as an inclusive
ceiling -- landing exactly on it is allowed -- which is what separates `>`
from `>=` and `==`.

Still not covered, and untouched here: the `pid != 0` guards in
pause/resume/stop and the surrounding async runner bodies. Those need a live
child process, and the pid guards carry a hazard -- mutating `!= 0` to
`== 0` makes the code call libc::kill(0, ...), which signals the entire
process group. They need an isolated harness.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant