You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The unsafe raw-pointer aliasing (#21) and the panic! on stream I/O errors (#22) were already removed on main by the async runtime refactor (b3a9add): StreamTurnStream borrows its reader safely, and the read_line error arm returns ProviderError::StreamError and latches the stream as done. What was still missing were the regression tests those issues asked for — this PR adds them:
Genericize StreamTurnStream over R: AsyncBufRead + Unpin + Send so the stream can be driven by in-memory readers in tests (no behavior change in production, which still uses BufReader<ChildStdout>).
Add regression tests for EOF → SessionCrashed then done-latch, broken-pipe I/O error → StreamError then done-latch (via a FailingReader test double), and caller-level preservation of partial transcripts when the provider stream fails mid-step (execute_steps_provider_stream_io_error).
Step-level context for stream errors (raised in #22) is provided by the executor, which records each failure as a per-step StepResult::ProviderFailed, so the error message itself only needs to identify the provider stream.
Test evidence
RUSTFLAGS="-D warnings" cargo test — 247 tests passed.
…t test assertion
Bounds belong on the impl where they're used; the broken-pipe test now
asserts only on our own error prefix since io::Error's Display text is
platform-dependent.
Changelog entries for the init/doctor, layered config, per-test command
overrides, cross-platform release, provider color, update-check deadline,
subprocess lifecycle, stream safety, and tokio migration PRs (#54-#61).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The unsafe raw-pointer aliasing (#21) and the
panic!on stream I/O errors (#22) were already removed onmainby the async runtime refactor (b3a9add):StreamTurnStreamborrows its reader safely, and theread_lineerror arm returnsProviderError::StreamErrorand latches the stream as done. What was still missing were the regression tests those issues asked for — this PR adds them:StreamTurnStreamoverR: AsyncBufRead + Unpin + Sendso the stream can be driven by in-memory readers in tests (no behavior change in production, which still usesBufReader<ChildStdout>).SessionCrashedthen done-latch, broken-pipe I/O error →StreamErrorthen done-latch (via aFailingReadertest double), and caller-level preservation of partial transcripts when the provider stream fails mid-step (execute_steps_provider_stream_io_error).Step-level context for stream errors (raised in #22) is provided by the executor, which records each failure as a per-step
StepResult::ProviderFailed, so the error message itself only needs to identify the provider stream.Test evidence
RUSTFLAGS="-D warnings" cargo test— 247 tests passed.cargo clippy --all-targets -- -D warnings— clean.Closes#21
Closes#22