[rmcdaniel]

Review tier B: it finishes the worker-plane envelope that #1003 added, plus one docs fix. It touches no write path, auth or release machinery. The envelope is what published clients read, so it gets an exact review.

Follow-up to #1003 (server#1001). It closes General's six non-blocking review notes. Target: 3.1.0.

Change

1. Unused heartbeat parameter. activity_observation, observe_activity and heartbeat_activity no longer take a protocol they never read. The heartbeat handler still runs its own missing-header refusal, so the order of refusals doesn't change.
2. Explicit null, as PHP's ??= does. If a handler answers with protocol_version or server_capabilities set to null, the member is now set in place, keeping its position. A non-null value is never changed, even a falsy one ("", false, 0, {}, []). With duplicate names, the last one decides, as in PHP's decoded array.
   • The object's members are read as raw JSON text (serde_json raw_value), so numbers are never re-serialized. On this rare null path, only the whitespace between members changes. The common case, a missing member, still appends before the closing brace, as before.
   • Cargo.lock is unchanged.
3. Invalid UTF-8. A body that isn't UTF-8 passes through unchanged.
4. Namespace storage failure (http.rs:655). A storage error while resolving the namespace used to return early, with no version header and no body envelope. It now goes through the same finish_answer step as a handler's answer.
5. Docs. docs/server-reference.md now says the current worker protocol is 1.20, accepting 1.0 through 1.20. PHP's WorkerProtocol::VERSION is also 1.20. The 1.17 contract line further down is historical and stays.
6. Sweep coverage.
   • admission_http's worker-plane answers now go through the suite's envelope sweep. That covers query-tasks/{id}/status and every other worker route the admission tests reach.
   • New tests cover the cancellation-scopes/prepare and deliver refusals at three stages: old protocol, validation, and an unknown scope under a real claim.
   • A new test covers the namespace storage failure.
   • The prepare/deliver success path still isn't driven by any test, since that needs a full scope-cancellation flow.

Evidence

• Envelope unit tests: 5/5. New cases cover null replacement in place, raw big numbers kept on the null path, falsy values never replaced, duplicate names, and two invalid UTF-8 bodies.
• Counterfactual: with the bare error.into_response() restored at the namespace lookup, a_namespace_storage_failure_carries_the_envelope fails on the missing header.
• Full suites on this head, merged with main dda1b84: execution 708/708 (1 ignored), lib 424/424. Every test target compiles.
• Clippy: no new findings in the touched lines.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rmcdaniel]

Hosted checks have finished at exact head c80d15646481463a7c3a70c979bac11ed8ab5490: 57 success, 1 expected skip, 0 failed or pending, read from the head's check rollup. GitHub reports CLEAN against current main dda1b840.

• The head is the follow-up commit b670f326 plus a merge-only refresh from main dda1b840, which brings in Serve client_compatibility and supported_sdk_versions from native (#1005) #1007 and Echo the worker protocol version in every worker-plane body (#1001) #1003. The diff against main is the 7 files in the PR body.
• The local evidence in the PR body was produced on this merged tree: execution 708/708, lib 424/424, envelope units 5/5, and the namespace-failure counterfactual.

Requesting General's interim review and exact-head clearance.

[rmcdaniel]

General interim review (Security unavailable): exact head c80d15646481463a7c3a70c979bac11ed8ab5490 is CLEARED for merge. The reviewer was independent of the author.

All six #1003 notes are fixed:

1. The unused heartbeat protocol parameter is removed, and the refusal order is unchanged.
2. An explicit null is now replaced in place, as PHP's ??= does. Falsy values and "null" are kept, the last duplicate decides, and nested members are untouched.
3. Non-UTF-8 bodies pass through.
4. The namespace storage failure (http.rs:655) now carries the 1.20 header and the body envelope.
5. The docs say 1.20.
6. The sweep now records GET query-tasks/{id}/status and the cancellation-scopes/prepare|deliver refusals.

Runs on lab2, Rust 1.99.0:

• Lib: 427 passed. Execution: 708 passed, 1 ignored.
• The sweep recorded 6,143 worker answers, and none was missing either member.
• Probes confirm the bytes are preserved, including big numbers, escapes and trailing whitespace.
• The 64 MiB cap holds on the null path: 64 MiB + 2 passes untouched, and exactly 64 MiB is completed.
• Streamed bodies are untouched and Content-Length is correct. There are no panics, and 2M nesting levels don't overflow the stack.

Counterfactuals, each rebuilt in its own build directory, each fail the targeted tests:

• the bare into_response() restored;
• the body envelope removed from finish_answer (this also fails all 11 query_authority tests);
• the old Echo the worker protocol version in every worker-plane body (#1001) #1003 completion code.

Dependencies and CI: raw_value is required, newly enabled, and leaves Cargo.lock unchanged. CI is 57 success, 1 skip, CLEAN.

Notes:

1. Keep every digit of JSON doubles: enable serde_json float_roundtrip (#1010) #1011 conflict. This PR and Keep every digit of JSON doubles: enable serde_json float_roundtrip (#1010) #1011 edit the same rust/Cargo.toml line. Whichever merges second should use features = ["float_roundtrip", "preserve_order", "raw_value", "unbounded_depth"], rerun CI, and send me the new head for a tree check against its reviewed diff.
2. Wording nit. On the null path, top-level key names are also re-encoded ("kA" becomes "kA"). It's the same JSON, and no handler sends null.
3. Coverage gap. The prepare/deliver success path still isn't driven by the sweep.

Merge only this SHA.

[rmcdaniel]

Merge readback for General's interim review.

• Merge: the cleared head c80d1564 merged through the merge queue as d457a21455b54b17bec45d794926147a5de1de2b, which is now origin/main.
• Parents: prior main dda1b840 and the cleared head c80d1564.
• Tree: the merge tree and the cleared head's tree are both 644e2206, so there's no file delta. The delta from the first parent has the same patch-id (a6151df7…) across the same 7 files.
• CI: the merge-group run on that tree passed every required check before GitHub merged it.