Preserve raw worker stdin at the server boundary#72
Merged
Conversation
Teach the standalone Zod worker to emit readline_discard for buffered stdin after an interrupt so stale active-turn input is accounted for before a follow-up tail runs. Validation: cargo check; cargo build; python3 tests/run_integration_tests.py --binary target/debug/mcp-repl; cargo clippy --all-targets --all-features -- -D warnings; cargo test --quiet; cargo +nightly fmt.
Reject worker sideband session_end frames that carry unrecognized reasons, and validate optional session_end message_b64 payloads before accepting the frame as final. Add a Zod test hook and MCP-level regression coverage so malformed protocol-worker session_end data fails fast instead of being treated as a clean session shutdown. Validation: cargo test --quiet --test zod_protocol zod_worker_invalid_session_end_reason_is_protocol_error; cargo test --quiet --test zod_protocol; cargo check; cargo build; python3 tests/run_integration_tests.py --binary target/debug/mcp-repl; cargo clippy --all-targets --all-features -- -D warnings; cargo test --quiet; cargo +nightly fmt
Add slow-shutdown and hang-shutdown commands to the standalone Zod worker so public protocol tests can exercise delayed graceful shutdown and pending shutdown states. Cover both through MCP-level tests and update the active protocol plan command list. Validation: - cargo test --quiet --test zod_protocol shutdown (red before fixture change) - cargo test --quiet --test zod_protocol - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Add an interrupt-report fixture command that records sideband interrupt notifications separately from OS interrupt delivery. Cover it through the public Zod protocol test and document the command in the active protocol plan. Validation: - cargo +nightly fmt - cargo check - cargo build - cargo test --test zod_protocol zod_worker_reports_sideband_and_os_interrupt_facts -- --nocapture - cargo test --test zod_protocol --quiet - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet
Consume only the leading Ctrl-C/Ctrl-D byte when splitting control-prefixed input so an immediate newline remains part of the tail payload. Add a public Zod MCP regression that proves the worker observes that newline before the tail command. Update existing tests that intend bare controls to send unterminated control bytes, and update transcript snapshots to show those bare inputs explicitly. Validation: - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Move CR/CRLF normalization out of the shared request path and keep it in the built-in R/Python drivers, so custom protocol workers receive the exact client stdin bytes plus the existing single appended LF rule. Add public Zod MCP coverage for supplied CRLF bytes and a trailing bare carriage return. Update the protocol plan to state the byte-preservation contract. Validation: - cargo test --test zod_protocol zod_worker_preserves_crlf_stdin_and_appended_newline -- --exact - cargo test --test write_stdin_edge_cases write_stdin_accepts_crlf_input -- --exact - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Snapshot pending output before reset shutdown so teardown-only worker output does not leak into the reset reply. Also classify Ctrl-C plus newline/CRLF as follow-up input for timeout bundle reuse; only bare Ctrl-C reuses the full timeout reply. Validation: - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Add a Zod command that blocks on a second stdin read and verify repl_reset closes the active stdin stream without sending interpreter shutdown text. Validation: - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Normalize CRLF and bare CR input before the built-in Unix Python backend
writes request bytes to its PTY. Keep custom protocol worker payloads
byte-preserving by applying the normalization only when the protocol driver is
serving the built-in Python bridge.
Add a Unix public regression test that sends a CRLF-formatted multiline Python
block through the MCP repl tool and asserts it executes without an injected
blank line or IndentationError.
Review finding:
- [P2] Normalize CRLF before Unix Python PTY writes — /Users/tomasz/github/posit-dev/mcp-repl/src/worker_process.rs:2624-2624
On Unix, the built-in Python backend uses `ProtocolBackendDriver`, whose `prepare_input_payload` now preserves stdin bytes. Since the previous normalization was removed before this call, CRLF input is written to the PTY as `\r\n`; the terminal maps `\r` to another newline, so a block like `if True:\r\n print("A")` reaches Python with a blank line and raises `IndentationError`. Custom protocol workers can preserve bytes, but built-in Python still needs newline normalization before writing to the PTY.
Response:
- Added `python_crlf_multiline_block_executes` to cover CRLF multiline Python input through the public MCP tool path.
- Updated `ProtocolBackendDriver::prepare_input_payload` so only the built-in Unix Python bridge normalizes CRLF/CR before PTY writes; custom protocol workers still preserve stdin bytes.
Validation:
- cargo test --test python_backend python_crlf_multiline_block_executes -- --nocapture (failed before fix, passed after fix)
- cargo check
- cargo build
- python3 tests/run_integration_tests.py --binary target/debug/mcp-repl
- cargo clippy --all-targets --all-features -- -D warnings
- cargo test --quiet
- cargo +nightly fmt
Move the shutdown-log path into CommandState so run_command stays within the clippy argument limit after the stdin-close reset coverage was adapted. Validation: - cargo test --test zod_protocol --quiet - cargo test --test python_backend --quiet crlf - cargo test --quiet timeout_bundle_reuse_treats_newline_ctrl_c_as_follow_up_input - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Keep server-side input payload preparation limited to the MCP UTF-8 string boundary and the explicit final-newline append rule. Backend/runtime-specific CRLF interpretation belongs to the worker instead of the server. This also removes the Unix Python CRLF multiline expectation, keeps CRLF acceptance scoped to Windows, and rewords the Zod protocol regression around raw client byte preservation. Validation: - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
Normalize CRLF input in the built-in R backend before writing bytes to R so Windows requests do not leave carriage returns in R source. Keep protocol worker byte preservation intact by leaving the protocol driver unchanged. Make the files-mode timeout poll regression deterministic by replacing a short sleep with an explicit file gate before releasing the request. Validation: - cargo check - cargo build - python3 tests/run_integration_tests.py --binary target/debug/mcp-repl - cargo clippy --all-targets --all-features -- -D warnings - cargo test --quiet - cargo +nightly fmt
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR tightens the worker stdin contract so the server no longer canonicalizes client-supplied line endings before handing input to the worker. Non-empty input still gets one trailing
\nwhen it is not already newline-terminated, but supplied\r\nand bare\rbytes are preserved.It also updates control-prefixed input handling so
Ctrl-C/Ctrl-Dconsume only the control byte. Any immediate newline remains part of the follow-up input, soCtrl-Dis a bare restart, whileCtrl-D\nis a restart followed by a blank input andCtrl-D\nfoois a restart followed by the leading blank line plusfoo.Protocol-worker shutdown handling is hardened by validating
session_end.reasonand optionalmessage_b64, and reset handling now snapshots output before graceful shutdown so teardown-only output does not leak into thenew session startedreply.Public Facing Changes
replstdin preserves supplied CRLF and bare-CR bytes at the server boundary, with only the final-newline append rule.Ctrl-C/Ctrl-Dfollowed by newline is treated as a control action with follow-up input, not as a bare control.session_endmetadata now fails as a protocol error instead of being accepted as a clean shutdown.Internal Changes
session_endframes.Ctrl-C/Ctrl-D.Diff Composition
Measured against
main, this PR is783insertions and100deletions across22files.src/:+93/-26(13.5%of churn)src/:+2/-2(0.5%of churn)tests/:+669/-61(82.7%of churn)+11/-3(1.6%of churn)+8/-8(1.8%of churn)Largest files:
tests/zod_protocol.rs:+343/-0tests/fixtures/zod-worker.rs:+186/-16src/worker_process.rs:+54/-19tests/sandbox_state_updates.rs:+41/-11tests/python_backend.rs:+32/-10