Add Zod worker protocol conformance tests#63
Merged
Conversation
Introduce a JSON worker launch spec and route custom launches through the same server-managed stdin, stdio, sandbox, and sideband setup as built-in workers. Add the standalone zod-worker development binary with a worker_ready handshake, deterministic echo behavior, and basic protocol events. Add a public MCP integration test that launches Zod through --worker-spec and verifies server-normalized stdin plus the worker-supplied prompt. Validation: - cargo check - cargo test --test zod_protocol zod_worker_echoes_input_and_returns_worker_prompt - cargo +nightly fmt
Track the active stdin bytes for protocol workers and drain them only from worker-reported readline_input/readline_discard events. A worker prompt now completes a custom-worker turn only after the active byte queue is empty, and protocol errors propagate instead of looking like IPC disconnects. Expand the Zod public MCP coverage for prompt-shaped output, empty prompts, timeout polling, and busy follow-up rejection. Validation: - cargo check - cargo test --test zod_protocol - cargo +nightly fmt
Keep the existing WorkerManager::new(Backend, ...) constructor for built-in tests and add new_with_launch for custom workers. Reject explicit --worker-spec plus --interpreter combinations, preserve server-owned environment over custom worker env overlays, and reject sideband streams whose first structural message is not startup metadata. Validation: - cargo check - cargo test parse_cli_args_rejects_worker_spec_with_interpreter - cargo test --test zod_protocol - cargo +nightly fmt
Move R and Python startup onto the worker_ready handshake while keeping the server able to accept legacy backend_info from older workers. The handshake now carries protocol version, descriptive worker identity, image capability, and the worker-provided graceful shutdown stdin text. Validation: - cargo check - cargo test --test python_backend python_smoke - cargo test --test server_smoke sends_input_to_r_console_smoke - cargo test --test zod_protocol - cargo +nightly fmt
Add Zod test commands that intentionally violate output base64 and readline_input accounting, and cover both through public MCP tests. These cases exercise the strict protocol error path added for custom workers. Validation: - cargo check - cargo test --test zod_protocol - cargo +nightly fmt
Add Zod command coverage for sideband-ordered stdout/stderr output and deterministic image emission through the public MCP surface. Validation: - cargo check - cargo test --test zod_protocol - cargo +nightly fmt
Add a public MCP test that times out an interruptible Zod turn, sends Ctrl-C with tail input, and verifies the tail runs only after the worker recovers to an unsatisfied prompt. Validation: - cargo check - cargo test --test zod_protocol - cargo +nightly fmt
Remove needless returns from sideband transport setup so all-target clippy passes with warnings denied. Validation: - cargo +nightly fmt - cargo clippy --all-targets --all-features -- -D warnings
Keep strict startup-message validation on real server IPC streams, but mark test-only connection pairs as already past startup so unit fixtures can send focused synthetic sideband events. Update malformed-message and image-ID protocol tests to use the current contract.
Treat session_end as terminal on the worker-to-server sideband and surface a late terminality violation after request-completion settling. Add a Zod public protocol test that intentionally emits output after session_end.
Add a Zod command that writes prompt-shaped raw stdout before delaying completion, then verify raw output remains visible but does not complete the active turn. Add explicit coverage that input already ending in a newline is not double-normalized.
Fix review findings: [P2] Honor client_waiting before sealing custom turns — /Users/tomasz/github/posit-dev/mcp-repl/src/ipc.rs:390-394 When a protocol worker sends `readline_start` with `client_waiting: false` after accounting for all bytes of the active payload, this treats an empty `active_stdin` queue as sufficient to mark the prompt unsatisfied. That can finalize the reply while the runtime is about to consume worker-internal buffered input rather than waiting for the next MCP input; require `client_waiting` in addition to the active queue being empty. Addressed by requiring `client_waiting` before marking a drained active-stdin prompt as unsatisfied, with public Zod coverage for a client-busy prompt that only completes after a later client-waiting prompt. [P2] Preserve fatal protocol errors across requests — /Users/tomasz/github/posit-dev/mcp-repl/src/ipc.rs:636-636 Clearing `protocol_error` at request start can hide a fatal sideband error that arrived while the worker was idle or just after the previous reply's settle window; the reader thread has already exited in those cases, so the next request can hang or time out instead of reporting the real protocol failure. Protocol errors should remain latched until the worker is reset or the error is surfaced. Addressed by keeping protocol errors latched across request start and surfacing them before new stdin writes, with public Zod coverage for a delayed idle protocol error reported on the next request. Validation: - cargo +nightly fmt - cargo check - cargo build - cargo clippy --all-targets --all-features -- -D warnings - cargo test
Fix review finding: [P2] Account stdin consumed by documented custom workers — /Users/tomasz/github/posit-dev/mcp-repl/src/ipc.rs:641-641 When a custom worker follows the currently documented sideband contract and only sends `readline_result` after consuming stdin, this queue is never drained because the reader only calls `account_active_stdin` for `readline_input`/`readline_discard`. The following `readline_start` is therefore treated as already satisfied and `wait_for_request_completion` can time out even though the worker is idle; either document/require the new accounting messages for `--worker-spec` workers or account `readline_result.line` for protocol-compatible workers. Addressed by restoring the original worker contract shape: `readline_start` carries only prompt text, while active-turn completion is derived from required `readline_input` and `readline_discard` accounting facts. The public sideband docs now document those accounting messages, and the Zod conformance worker no longer emits a prompt wait hint. Added regression coverage that serialized `readline_start` has no extra field and that a prompt with remaining active stdin does not complete until the buffered input is accounted for. Validation: - cargo +nightly fmt - cargo check - cargo build - cargo clippy --all-targets --all-features -- -D warnings - cargo test
Move the Zod protocol worker from a package binary target to an example fixture so default Cargo installs only expose mcp-repl. The zod protocol tests now build the example fixture explicitly before spawning it, and the install tests assert Cargo's default binary target surface. Validation: - cargo +nightly fmt - cargo check - cargo build - cargo clippy --all-targets --all-features -- -D warnings - cargo test Review finding: [P2] Keep the test worker out of installed binaries — /Users/tomasz/github/posit-dev/mcp-repl/Cargo.toml:14-16 With this extra `[[bin]]` target, `cargo install --git ... --locked` installs both `mcp-repl` and `zod-worker`. That makes the internal protocol test fixture part of the user-facing install surface and can even block installation when a user already has a `zod-worker` executable in Cargo's bin dir; gate it behind a non-default feature or move it out of package bin targets. Response: Moved zod-worker to an explicit Cargo example fixture at tests/fixtures/zod-worker.rs, so default package binary targets only include mcp-repl. Updated zod protocol tests to build the example fixture and added a cargo metadata regression for the installable binary surface.
Latch protocol errors observed while probing timed-out requests so the next pending poll or busy follow-up reports the real worker protocol failure instead of consuming it and reporting only busy state. Validation: - cargo +nightly fmt - cargo check - cargo build - cargo clippy --all-targets --all-features -- -D warnings - cargo test Review finding: [P2] Propagate protocol errors while resolving timeouts — /Users/tomasz/github/posit-dev/mcp-repl/src/worker_process.rs:4387-4387 If a sideband protocol error arrives after a request has timed out, this timeout-marker probe consumes it from `wait_for_request_completion` and then ignores it unless the worker has exited. A non-empty follow-up runs this path before the busy poll, so inputs like delayed invalid JSON/base64 can lose the actual protocol error and leave the request reported only as busy; return or re-latch the protocol error instead of folding it into the timeout/disconnect case. Response: Added a settled pending-error slot for timed-out requests. Timeout-marker resolution now latches protocol errors instead of folding them into timeout/disconnect handling, and pending polls or busy follow-ups return that error before probing completion again. Added a zod protocol regression that delays invalid output until after the first timeout and verifies the follow-up reports the base64 protocol error.
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
1 participant
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 adds an internal Zod conformance worker and custom worker launch path for testing the worker/server protocol independently of R and Python.
It tightens sideband IPC handling around worker readiness, stdin accounting, request completion, session-end ordering, interrupts, timeout follow-ups, and protocol-error reporting.
Public-facing changes
cargo installcontinues to expose only themcp-replbinary; the Zod worker is an internal test fixture.Internal-only changes
tests/fixtures/zod-worker.rsas a deterministic protocol worker.Diff composition
Measured against
origin/main, this PR is1912insertions and153deletions across14files.src/:+763/-118(42.7%of churn)src/:+39/-29(3.3%of churn)tests/:+1085/-0(52.5%of churn)+21/-6(1.3%of churn)+4/-0(0.2%of churn)Largest files:
tests/zod_protocol.rs:+629/-0tests/fixtures/zod-worker.rs:+407/-0src/ipc.rs:+303/-63src/worker_process.rs:+315/-44src/backend.rs:+96/-0Testing
cargo checkcargo buildcargo clippy --all-targets --all-features -- -D warningscargo testcargo +nightly fmt