Skip to content

test(v2/conversion): add round-trip coverage for high-blast-radius methods - #13

Draft
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/missing-test-coverage-3354
Draft

test(v2/conversion): add round-trip coverage for high-blast-radius methods#13
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/missing-test-coverage-3354

Conversation

@cursor

@cursor cursor Bot commented Jun 6, 2026

Copy link
Copy Markdown

Risky behavior now covered

The src/v2/conversion.rs module performs ~520 hand-written IntoV1/IntoV2 field-by-field moves so future v2 shape changes have obvious edit points. Until this PR the test surface was 13 cases against that module, covering InitializeRequest, NewSessionRequest, PromptRequest, ToolCall, SessionNotification, RequestPermissionResponse, Error::invalid_params, and one v2-side smoke test. The methods most users actually call against the protocol — authenticate, logout, every session/* lifecycle method, every fs/* method, every terminal/* method, and session/cancel — had no coverage at all, even though several of these were stabilized in the last few weeks (logout in agentclientprotocol#1273, terminal/* earlier in the v0.13 cycle).

This PR adds 23 new tests that, for each high-traffic type:

  1. Round-trip a v1 value through v2 and back, asserting structural equality (assert_v1_round_trip), which catches a dropped field or a missing match arm in either direction.
  2. Assert JSON-shape equality across the conversion (assert_json_eq_after_v1_to_v2), which catches a field rename on either side as soon as v1 and v2 diverge.
  3. Seed a non-empty _meta map so a silently-dropped _meta fails loudly — the prior single-field tests would pass on None == None.

The surfaces now covered:

  • authenticate / logout (recently stabilized) — request, response, and the AgentAuthCapabilities discriminator that signals logout support.
  • session lifecyclesession/load, session/resume, session/close, session/list, session/set_mode, and the session/cancel notification.
  • file systemfs/read_text_file with and without the line/limit window (so omitted optionals don't get collapsed to Some(0)), and fs/write_text_file.
  • terminalterminal/create with all optional fields populated, terminal/output in both "still running" and "finished" shapes, terminal/wait_for_exit, terminal/release, terminal/kill, and TerminalExitStatus across clean-exit / non-zero / signal / both / unknown.
  • promptPromptResponse for every StopReason variant (EndTurn, MaxTokens, MaxTurnRequests, Refusal, Cancelled), so a missing match arm couldn't collapse two semantically different end states.
  • permissionRequestPermissionRequest with every PermissionOptionKind so an AllowOnce -> AllowAlways drift can't sneak through.
  • errorsError with ErrorCode::Other(i32) (so the raw integer survives) and every well-known code.
  • cancellation (unstable_cancel_request)CancelRequestNotification with every RequestId variant (Null, i64, string) and the enclosing ProtocolLevelNotification enum dispatch (the latter via direct JSON equality because it intentionally doesn't derive PartialEq).

Test files added/updated

  • src/v2/conversion.rs — 23 new tests added inside the existing mod tests, sharing the existing assert_v1_round_trip / assert_json_eq_after_v1_to_v2 helpers. A small sample_meta() helper is introduced so every test exercises the _meta field. No production code is changed.

Total: cargo test --all-features now reports 291 passed; 0 failed (was 268 before).

Why these tests materially reduce regression risk

src/v2/conversion.rs is the only place that promises the v1 and v2 namespaces stay byte-equal until v2 intentionally diverges. The conversion impls are mechanically field-by-field — every new field on a v1 type has to be added in two IntoV1/IntoV2 methods, and the type system can't prove they were both updated. The previous 13 tests covered the structural primitives but left the entire authentication, session lifecycle, file system, terminal, and cancellation surfaces unguarded. With this PR, a dropped field, a renamed field, a missing enum arm, or a dropped _meta on any of those methods will fail a deterministic test instead of slipping out to SDK users who already use the conversion layer to mediate between v1 SDKs and v2 wire formats.

Validation

  • cargo test --all-features — 291 passed
  • cargo test (default features) — 41 passed
  • cargo test --features unstable_protocol_v2 --lib conversion — 35 passed (confirms the unstable_cancel_request-gated test compiles out cleanly when that feature isn't enabled)
  • cargo clippy --all-features — clean
  • cargo fmt -- --check — clean
  • npm run spellcheck — clean

Note: npm run format:check flags a pre-existing prettier formatting issue in AGENTS.md that exists on main independent of this PR.

Open in Web View Automation 

Summary by cubic

Adds 23 round‑trip tests for high‑traffic v1↔v2 conversions in src/v2/conversion.rs, covering authenticate/logout, session load/resume/close/list/set_mode/cancel, fs read/write, terminal create/output/wait/release/kill and exit status, prompt stop reasons, permission option kinds, error codes, and cancel‑request IDs. Each test checks v1→v2→v1 structural equality and JSON shape equality and seeds a non‑empty _meta; no production code changed (291 tests total, was 268).

Written for commit 6b05760. Summary will update on new commits.

Review in cubic

Note

Add round-trip conversion tests for high-blast-radius v1/v2 methods in conversion.rs

Adds comprehensive round-trip tests to conversion.rs covering authenticate, logout, session lifecycle, terminal, prompt, permission, error, and cancel request/notification types. Each test verifies v1→v2→v1 equality and JSON equality after v1→v2 conversion, using a shared sample_meta helper to confirm _meta fields survive round trips. Feature-gated tests for unstable_cancel_request cover CancelRequestNotification and ProtocolLevelNotification variants.

Macroscope summarized 6b05760.

…thods

Adds 23 new tests in src/v2/conversion.rs covering protocol surfaces
that previously had no v1<->v2 conversion coverage. Every test
exercises both v1 -> v2 -> v1 round-tripping (catches dropped fields
and missing match arms) and JSON-shape equality across the conversion
(catches accidental field renames on either side).

Surfaces newly covered:

- authenticate / logout (recently stabilized in agentclientprotocol#1273)
- AgentAuthCapabilities (presence vs absence of LogoutCapabilities)
- session/load, session/resume, session/close, session/list,
  session/set_mode, session/cancel
- fs/read_text_file (with and without line/limit window) and
  fs/write_text_file
- terminal/create (args, env, cwd, output_byte_limit, _meta),
  terminal/output (with and without exit_status),
  terminal/wait_for_exit, terminal/release, terminal/kill
- TerminalExitStatus across all four shapes (clean exit, non-zero
  exit, signal termination, unknown)
- PromptResponse for every StopReason variant (EndTurn, MaxTokens,
  MaxTurnRequests, Refusal, Cancelled) so a missing match arm in
  either direction is caught
- RequestPermissionRequest with every PermissionOptionKind so an
  AllowOnce->AllowAlways drift can't sneak through
- Error with the ErrorCode::Other(i32) fallback and every well-known
  code so the raw integer payload is preserved
- CancelRequestNotification with each RequestId variant (Null, i64,
  string) and the enclosing ProtocolLevelNotification dispatch

Each test also seeds a non-empty _meta map on the relevant types so a
silently-dropped _meta would fail loudly, which the prior single-
field tests didn't catch.

No production code changed.

Co-authored-by: QuantuM <qumusai@proton.me>
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.

1 participant