Repository navigation
test(conformance): wire MCP fixtures, reverse-attach negatives, parity checklist - #62
Merged
Merged
Conversation
…y checklist (refs #24) Adds the open test items from #24. No Sources/ or production Rust changes. New files - conformance/mcp_wire_vectors.json: 12 JSON-RPC cases (initialize, ping, tools/list shape, tools/call errors, notification, parse and batch errors). Expectations are the Swift MCPServer result. 7 Rust divergences are listed in known_drift with the output Rust gives today. - Tests/InstanceMCPCoreTests/WireConformanceTests.swift: runs the fixture against MCPServer.parse and handle. - poc/reverse-attach-linux/src/wire_conformance.rs: runs the fixture against mcp::answer; fails if a known_drift output changes or a drift is fixed. - Tests/InstanceMCPCoreTests/ReverseAttachNegativeTests.swift: a plain TCP runtime answers 401; the client must dial once, send "Bearer right" and end in handshakeRejected(401). Also a canary check: the attach secret is absent from POST, GET list and GET one bodies. - poc/reverse-attach-linux/src/attach/client_tests.rs: 401 on wrong, same-length wrong, empty and missing bearer (one dial each); right bearer sends the expected headers; wss:// to a plaintext listener does not succeed and sends no HTTP. - poc/reverse-attach-linux/src/attach/grant_secrecy_tests.rs: grant_json never contains the secret. - docs/parity-checklist.md: the Phase 3 gate, with wire, HTTP, reverse-attach and per-tool rows. Profile rows are a placeholder for #59. Changed - conformance/auth_vectors.json: 3 cases appended (same-length byte change, case change, trailing space). The old last object gained a trailing comma. - conformance/README.md: rows and a link to the checklist. - main.rs, attach/mod.rs, attach/client.rs: one cfg(test) mod line each. Verification (commands run on macmini; nothing built on the laptop) - Baseline on untouched origin/main de5e211 (git archive): swift test 136 passed, 0 failed; cargo test --release 18 passed. - swift test on this commit: 139 passed, 0 failed (136 + 3 new suites). Includes ConformanceVectorTests (auth vectors, 19 cases). - cargo test --release --locked in rust:latest (rustc 1.99.1, Linux aarch64, container on macmini via OrbStack): 25 passed, 0 failed (18 + 7 new). - cargo fmt --check: pass (after rustfmt on the three new Rust files). - cargo clippy --release --locked -- -D warnings: pass (CI command). - cargo test --release on native macOS: FAILS AT LINK (not a test failure). The crate is Linux-only (seat.rs calls prctl). A test that calls answer() with tools/call reaches tool_key and so links seat.rs. The baseline never reached that path from tests, so it linked by dead-stripping. CI runs the Rust job on Linux and is the authoritative Rust run. - json.tool: both fixtures valid. - Not run: bash smoke.sh and markdownlint (see follow-ups). Finding (pre-existing, not fixed here) - On Linux, tungstenite connect() with a wss:// URL panics: rustls 0.23 has no process-level CryptoProvider in this build. A wss:// runtime would panic the dial thread and leave the grant in "dialing". The wss test accepts that panic as "dial did not succeed". Fix in a follow-up. Still open for #24 after this PR - 1: 7 known_drift Rust fixes (wire layer). HTTP-layer gaps (session 404, 415, DELETE semantics, 400 vs 202) are in the checklist. - 2a: successful wss:// handshake (needs TLS fixture); Swift and Rust. - 2b: sha256 verifier (runtime side, openab-pty; not in this repo). - 2c: pod stores only the verifier (runtime side). - 2d: per-connection profile enforcement at dispatch (#59). - 2e: origin policy, certificate verification. - 3 and 4 are done by this PR.
…#24) Review (CHANGES_REQUESTED) on 7a55843 asked for two fixes. B1: the Rust refused-upgrade tests called tungstenite::connect directly, so they did not exercise the production dialer. They now run dial_loop against a runtime that answers 401 and keeps accepting for 2.5 s. The grant deadline is 3 s away, so a redial would land inside the window and be counted. Tests: - same_length_wrong_bearer_is_refused_once_and_stops - empty_bearer_is_refused_once_and_stops Both assert state "ended", reason "handshake_rejected_401", and exactly one connection. The missing-Authorization test is documented as a runtime-side refusal check. build_ws_request always sends Authorization, so it does not test the dialer. The wss test is documented as tungstenite-level. Mutation check (macmini copy only, not committed): changing dial_loop to redial on 401 fails both dial_loop tests above. B2: the verification the plan requires is now recorded below. Also changed: - right_bearer_upgrade_sends_websocket_headers is split out of the right-bearer test, so the header check has its own test. - docs/parity-checklist.md: the "refused upgrade" row names the dial_loop test. The "GET /mcp 405" row is "open (not tested)", matching the legend. The Swift wss cell says "not tested" instead of "covered by switchboard tests", because SwitchboardTests only checks URL validation. Verification (macmini, arm64 macOS; nothing built on the laptop) - swift test: 139 passed, 0 failed, 0 unexpected. - cargo test --release, native macOS: fails at link with undefined _prctl. The crate is Linux-only (seat.rs calls prctl). This is an environment limit, not a test result. - cargo test --release --locked in rust:latest on macmini (rustc 1.99.1, linux aarch64, OrbStack docker): 26 passed, 0 failed. Baseline was 18 on de5e211; this branch adds 8 (7 in 7a55843, plus the split header test). - cargo test --release --locked wire_conformance: 1 passed. - cargo test --release --locked attach::client::tests: 6 passed. - cargo test --release --locked grant_secrecy: 1 passed. - cargo fmt --check: pass (after rustfmt on client_tests.rs). - cargo clippy --release --locked -- -D warnings: pass (CI command). - cargo build --release --locked: pass. - bash smoke.sh in the same Linux container (python3, procps, curl installed there): RESULT 48 passed, 0 failed. - markdownlint-cli2@0.17.2 "docs/**/*.md" (the version the pinned CI action uses): 0 errors in 11 files. Newer markdownlint versions report MD060 on existing docs too; CI does not use them. - json.tool on conformance/mcp_wire_vectors.json and auth_vectors.json: valid. Still open for #24 after this PR (unchanged from 7a55843) - 1: 7 known_drift Rust fixes in mcp.rs (wire layer). HTTP-layer gaps are in docs/parity-checklist.md section 3. - 2a: successful wss:// handshake (needs a TLS fixture). Also: wss on Linux panics without a process-level rustls CryptoProvider (pre-existing). - 2b: sha256 verifier (runtime side, openab-pty, not in this repo). - 2c: pod stores only the verifier (runtime side). - 2d: per-connection profile enforcement at dispatch (#59). - 2e: origin policy and certificate verification. - 3 and 4 are done by this PR.
…checklist (refs #24) Review finding 1: the checklist claimed a Swift pass for the bearer and upgrade-header check on the WS dial. FakeRuntime accepts any bearer and never reads Authorization, Upgrade, or Connection, so no Swift test asserts them. The bearer is only seen on the refused-upgrade (401) path, covered by ReverseAttachRefusedUpgradeTests. The Swift cell now says "not asserted" and the status is "pass (Rust); Swift open". Verification on macmini, run on the synced tree at this commit: - swift test: 139 executed, 0 failures (WireConformanceTests 1/1 pass, ReverseAttachNegativeTests pass, ConformanceVectorTests pass). - cargo test: native macOS does not link (undefined _prctl from platform/linux/seat.rs, which this PR does not touch; pre-existing). Ran in rust:latest Linux container via OrbStack on macmini with --locked: cargo fmt --check OK, clippy -D warnings OK, cargo build --release OK, cargo test --release: 26 passed, 0 failed. - python3 -m json.tool on conformance/mcp_wire_vectors.json and conformance/auth_vectors.json: OK. - markdownlint-cli2@0.18.1 "docs/**/*.md" (matches the markdownlint-cli2-action v19 pin): 0 errors. Newer markdownlint reports MD060 on existing docs, which is not from this PR. - smoke.sh: not run in this pass.
- parity-checklist §5: Rust does serve browser_* tools (mcp.rs Upstream via MCP_UPSTREAM, DESKTOP_BROWSER_TOOLS, upstream_tool_allowed); fix the cell and name that code among the section sources. - parity-checklist §4: rename the "constant-time" row to byte-exact compare (vectors check outcomes, not timing); note on the wss row that the Linux dial panics (no rustls CryptoProvider) and the test accepts it; link follow-up #66. - AttachCredentialSecrecyTests: assert 200 on GET /attach and GET /attach/<id>, and that the grant id appears in the list body. - client_tests.rs: rename right_bearer_upgrades_with_the_expected_headers to right_bearer_is_sent_and_upgrades; widen the dial_loop grant deadline to now+4 s so it clearly outlasts the 2.5 s linger. - conformance/README.md: the no-Authorization case is a runtime-side check the dialer never sends, not a dial-side "missing bearer" negative. Correction to 3718049's message: the pinned markdownlint-cli2-action sha (v19.1.0) bundles markdownlint-cli2 0.17.2, not 0.18.1 (as c083da6 said). Verified on macmini: swift test 139 pass, 0 fail. In rust:1 container (rustc 1.99.1): cargo fmt --check ok; cargo clippy --release --locked --all-targets -D warnings ok; cargo test --release --locked 26 pass, 0 fail. markdownlint-cli2@0.17.2 on docs/** and conformance/*.md: 0 errors.
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.
Refs #24. Test-only PR (no Sources/ or production Rust changes).
Added
conformance/mcp_wire_vectors.json: 12 JSON-RPC wire cases, Swift result as oracle. 7 Rust divergences pinned inknown_drift.Tests/InstanceMCPCoreTests/WireConformanceTests.swift,poc/reverse-attach-linux/src/wire_conformance.rs: runners for the fixture.ReverseAttachNegativeTests.swift: refused upgrade (401) stops after one dial; attach secret absent from grant bodies.attach/client_tests.rs: wrong, same-length, empty and missing bearer refused once; right bearer headers; wss to plaintext does not fall back.attach/grant_secrecy_tests.rs:grant_jsonnever carries the secret.docs/parity-checklist.md: Phase 3 gate. Profile rows are a placeholder for Collapse tool profiles to observe and owner; browser belongs to owner #59.conformance/auth_vectors.json: 3 same-length, case and trailing-space bearer negatives.Verified on macmini
Finding (not fixed here)
On Linux, a wss:// dial panics in tungstenite because rustls has no process-level CryptoProvider. A wss:// runtime would panic the dial thread. The wss test accepts that as a failed dial. Follow-up: #66.
Still open in #24
Items 3 and 4 are done by this PR.
Note on commit history
Commit 3718049 says markdownlint-cli2 0.18.1 matches the CI action pin. That is wrong: the pinned markdownlint-cli2-action sha (v19.1.0) bundles markdownlint-cli2 0.17.2, as c083da6 says.