Skip to content

fix(acp): serialize stdin writes through a single actor - #6675

Open
moneypartners wants to merge 1 commit into
block:mainfrom
moneypartners:codex/fix-acp-cancel-safe-stdin-writer
Open

fix(acp): serialize stdin writes through a single actor#6675
moneypartners wants to merge 1 commit into
block:mainfrom
moneypartners:codex/fix-acp-cancel-safe-stdin-writer

Conversation

@moneypartners

Copy link
Copy Markdown

Fixes #6671.

What changed

  • Move ownership of the ACP agent's stdin into one writer task.
  • Submit complete serialized NDJSON frames through a bounded FIFO channel.
  • Return write/flush results through a oneshot while preserving the existing 30-second timeout and error mapping.
  • Add a regression test using tokio::io::duplex and LinesCodec that cancels a >256 KiB prompt write before sending session/cancel.

Why

write_ndjson previously performed write_all(line), newline, and flush inside caller futures. The prompt path is cancellable through tokio::select!; dropping it after a partial pipe write left those bytes behind. The cleanup path then appended session/cancel, creating one invalid NDJSON line.

The production capture behind #6671 was a 65,773-byte malformed frame ending in a partial prompt immediately followed by a complete cancel notification. The raw capture includes channel payloads, so the issue records its checksum and sanitized boundary instead of publishing the payload.

With the actor, cancelling a caller can only leave a complete frame enqueued or no frame enqueued. The actual write_all + flush is never owned by the cancellable caller.

Verification

Ported onto upstream main at 0720f5380ce8 rather than pushing the original desktop-v0.5.4-based branch.

cargo fmt --all -- --check                                  PASS
cargo test -p buzz-acp                                      PASS (802 tests)
cargo clippy -p buzz-acp --all-targets -- -D warnings       PASS

The new regression test passes against the production StdinWriter and reads two independently valid JSON lines through the same codec used by AcpClient.

Route complete NDJSON frames through one writer task so cancelling a prompt future cannot leave partial bytes for session/cancel to append to. Add a duplex-based regression test for the cancellation race.

Refs block#6671

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Money Partners <moneypartners.tools@gmail.com>
@moneypartners
moneypartners requested a review from a team as a code owner August 24, 2026 06:44
@moneypartners

Copy link
Copy Markdown
Author

Full repository gate verified on commit dc2492c: just ci completed successfully (exit 0), including formatting, workspace clippy, Rust/unit tests, desktop checks and production build, Tauri workspace tests, and Flutter analyze/tests.

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.

buzz-acp can merge NDJSON frames when a prompt write is cancelled

1 participant