fix: validate bounded JSONRPC frames and envelopes (Fixes #532)

[Karthik Nadig (karthiknadig)]

Parse bounded multi-header JSONRPC frames and validate request envelopes before dispatch, while preserving request IDs, notifications, and legacy method-level error behavior.

• Read headers through the separator; accept case-insensitive names, optional Content-Type, LF/CRLF, fragmented bytes, and consecutive frames.
• Enforce exact 8 KiB header and 16 MiB payload limits before allocation; reject duplicate, missing, invalid, and overflowing lengths.
• Return explicit errors for invalid envelopes/parameter containers and recover after complete malformed JSON/UTF-8 frames.
• Add deterministic fragmentation, native wire, and rejection-to-valid-dispatch regression coverage; document the supported contract.

Validation: 51 library and 19 native cases; workspace formatting/Clippy and independent code review pass. #559 is merged and main integrated; only the five protocol files remain in this PR. Exact-main Linux/Windows line and function coverage now improve, with no budget or exclusion changes. Current-head Copilot has no code findings and all CodeQL analyses are clean. The independently reviewed Windows matched-host control found no material regression; detailed platform snapshots, accepted tail outliers, and sampling limitations are recorded in the PR comments. All 35 automated checks now pass, including both native macOS jobs. Ready for required human approval and protected auto-merge.

Fixes #532

[github-actions]

Test Coverage Report (Linux)

Result: ✅ Within regression budget

Metric	PR	Baseline	Delta
Lines	85.535%	85.163%	+0.372pp
Functions	88.368%	88.144%	+0.224pp

Allowed numerical tolerance: 0.01 percentage points.

[github-actions]

Performance Report (Linux)

Result: ✅ Within regression budgets

Metric	PR	Baseline	Delta	Change	Blocking budget	Status
Server startup P50	1ms	1ms	+0ms	+0.0%	>5ms and >100%	➖
Server startup P95	1ms	1ms	+0ms	+0.0%	>50ms and >200%	➖
Discovery duration P50	56ms	65ms	-9ms	-13.8%	>25ms and >30%	✅
Discovery duration P95	58ms	68ms	-10ms	-14.7%	>50ms and >50%	✅
Startup-to-first environment P50	15ms	15ms	+0ms	+0.0%	>20ms and >100%	➖
Startup-to-first environment P95	19ms	16ms	+3ms	+18.8%	>25ms and >100%	🔺
Cold discovery duration P50	137ms	144ms	-7ms	-4.9%	>100ms and >50%	✅
Refresh round-trip P50	56ms	65ms	-9ms	-13.8%	>25ms and >30%	✅
Refresh round-trip P95	58ms	68ms	-10ms	-14.7%	>50ms and >50%	✅
Request-to-first environment P50	13ms	13ms	+0ms	+0.0%	>20ms and >100%	➖
Request-to-first environment P95	18ms	15ms	+3ms	+20.0%	>25ms and >100%	🔺
Cold refresh round-trip P50	138ms	144ms	-6ms	-4.2%	>100ms and >50%	✅

Workload	PR	Baseline
Environments	5	5
Managers	1	1

A regression must exceed both the documented absolute and relative budget. Environment and manager inventories must match exactly within the same inventory schema.

[github-actions]

Performance Report (Windows)

Result: ✅ Within regression budgets

Metric	PR	Baseline	Delta	Change	Blocking budget	Status
Server startup P50	9ms	9ms	+0ms	+0.0%	>10ms and >50%	➖
Server startup P95	13ms	13ms	+0ms	+0.0%	>50ms and >100%	➖
Discovery duration P50	158ms	155ms	+3ms	+1.9%	>150ms and >50%	🔺
Discovery duration P95	177ms	158ms	+19ms	+12.0%	>250ms and >100%	🔺
Startup-to-first environment P50	23ms	23ms	+0ms	+0.0%	>25ms and >50%	➖
Startup-to-first environment P95	26ms	34ms	-8ms	-23.5%	>100ms and >100%	✅
Cold discovery duration P50	156ms	155ms	+1ms	+0.6%	>150ms and >50%	🔺
Refresh round-trip P50	158ms	156ms	+2ms	+1.3%	>150ms and >50%	🔺
Refresh round-trip P95	177ms	159ms	+18ms	+11.3%	>250ms and >100%	🔺
Request-to-first environment P50	13ms	13ms	+0ms	+0.0%	>25ms and >50%	➖
Request-to-first environment P95	16ms	23ms	-7ms	-30.4%	>100ms and >100%	✅
Cold refresh round-trip P50	156ms	156ms	+0ms	+0.0%	>150ms and >50%	➖

Workload	PR	Baseline
Environments	8	8
Managers	1	1

A regression must exceed both the documented absolute and relative budget. Environment and manager inventories must match exactly within the same inventory schema.

[github-actions]

Test Coverage Report (Windows)

Result: ✅ Within regression budget

Metric	PR	Baseline	Delta
Lines	83.083%	82.658%	+0.425pp
Functions	85.753%	85.464%	+0.289pp

Allowed numerical tolerance: 0.01 percentage points.

[github-actions]

Performance Report (macOS)

Result: ✅ Within regression budgets

Metric	PR	Baseline	Delta	Change	Blocking budget	Status
Server startup P50	79ms	105ms	-26ms	-24.8%	>100ms and >50%	✅
Server startup P95	536ms	847ms	-311ms	-36.7%	>750ms and >100%	✅
Discovery duration P50	104ms	184ms	-80ms	-43.5%	>100ms and >50%	✅
Discovery duration P95	121ms	221ms	-100ms	-45.2%	>300ms and >100%	✅
Startup-to-first environment P50	96ms	157ms	-61ms	-38.9%	>150ms and >50%	✅
Startup-to-first environment P95	123ms	189ms	-66ms	-34.9%	>250ms and >100%	✅
Cold discovery duration P50	258ms	379ms	-121ms	-31.9%	>250ms and >50%	✅
Refresh round-trip P50	105ms	185ms	-80ms	-43.2%	>250ms and >50%	✅
Refresh round-trip P95	122ms	222ms	-100ms	-45.0%	>300ms and >100%	✅
Request-to-first environment P50	33ms	43ms	-10ms	-23.3%	>50ms and >50%	✅
Request-to-first environment P95	51ms	58ms	-7ms	-12.1%	>100ms and >100%	✅
Cold refresh round-trip P50	259ms	380ms	-121ms	-31.8%	>600ms and >50%	✅

Workload	PR	Baseline
Environments	10	10
Managers	1	1

A regression must exceed both the documented absolute and relative budget. Environment and manager inventories must match exactly within the same inventory schema.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Cross-platform process ownership, concurrent output, and transport shutdown semantics warrant final human review.

Review effort: Balanced
Findings: None

What changed in this PR

Adds bounded JSONRPC framing and envelope validation while incorporating #559’s transport shutdown lifecycle.

Changes:

• Parses bounded multi-header frames and validates JSONRPC envelopes.
• Adds FIFO, size-limited output and supervised subprocess cancellation.
• Expands protocol, native-wire, fragmentation, and shutdown coverage.

File	Description
docs/​JSONRPC.md	Documents framing, validation, and shutdown contracts.
crates/​pet/​tests/​process_utils.rs	Adds bounded fixture shutdown helpers.
crates/​pet/​tests/​jsonrpc_server_test.rs	Adds native protocol and lifecycle regressions.
crates/​pet/​tests/​jsonrpc_client.rs	Supports isolated startup and graceful shutdown.
crates/​pet/​tests/​fixtures/​shutdown_probe.py	Provides descendant-cleanup fixture.
crates/​pet/​tests/​e2e_performance.rs	Gracefully closes performance fixtures.
crates/​pet/​src/​main.rs	Returns failure for server errors.
crates/​pet/​src/​jsonrpc.rs	Coordinates transport and probe cleanup.
crates/​pet-python-utils/​src/​process/​supervisor.rs	Supervises probe admission and cancellation.
crates/​pet-python-utils/​src/​process.rs	Integrates cancellable subprocess execution.
crates/​pet-python-utils/​src/​env.rs	Handles interpreter cancellation quietly.
crates/​pet-poetry/​src/​environment_locations_spawn.rs	Handles Poetry cancellation.
crates/​pet-jsonrpc/​src/​server.rs	Validates envelopes and dispatches framed input.
crates/​pet-jsonrpc/​src/​output.rs	Adds bounded FIFO protocol output.
crates/​pet-jsonrpc/​src/​lib.rs	Routes replies through the output subsystem.
crates/​pet-jsonrpc/​src/​framing.rs	Implements bounded frame parsing.
crates/​pet-conda/​src/​conda_info.rs	Handles Conda cancellation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Karthik Nadig (karthiknadig)]

Merged the complete reviewed parent fixes from #559 through 83df603 without rewriting history (f668ed5). This retains all four native protocol cases and fixes the real-pipe test setup/reader-inheritance race with bounded cleanup; the measured one-second shutdown deadline is unchanged. Windows and Linux native protocol suites, mandatory formatting/Clippy, and independent merge-context review passed. Current-head Copilot review and hosted checks are running. This PR still needs to land after #559; main will be integrated after that merge so the final diff contains only the protocol work.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The stacked transport, concurrency, and cross-platform shutdown changes warrant final human validation.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

It combines protocol parsing, asynchronous output, and cross-platform process-lifecycle changes that warrant final human validation.

Review effort: Balanced
Findings: None

[Karthik Nadig (karthiknadig)]

Final review/CI update for 6209cbc: current-head Copilot review has no findings; there are no unresolved threads; all GitHub Actions checks pass. All three CodeQL analyses have zero results/errors/warnings. Coverage improves on Linux and Windows.

The exact combined-protocol macOS control completed: https://github.com/microsoft/python-environment-tools/actions/runs/36194233841. Eight counterbalanced passes, verified exact base/head binaries and identical full inventories (10 environments, 1 manager). Medians of per-pass percentiles (not pooled percentiles):

Metric	Base P50/P95	Head P50/P95
Refresh RTT	117.5 / 130.5 ms	116 / 153.5 ms
Cold refresh RTT	275 / 310 ms	267 / 361 ms
Request to first environment	22.5 / 38.5 ms	32 / 50 ms
Startup to first environment	89 / 107.5 ms	100 / 120.5 ms

Independent inspection found no avoidable decoder/wakeup delay. The roughly 10 ms first-notification increase and +23/+51 ms warm/cold RTT tails are explicitly recorded, not dismissed as noise; the matched macOS result is acceptable with its single-host/Rosetta/sample-size limits.

Still holding draft/auto-merge:

1. fix: shut down JSONRPC cleanly on EOF (Fixes #529) #559 is ready with auto-merge enabled, but this dependent PR must land after it and then integrate main.
2. The final Windows snapshot improves complete refresh RTT (136/159 ms versus 155/164 ms) but worsens startup-to-first-environment (33/63 versus 23/36 ms) and request-to-first-environment (22/53 versus 14/27 ms). Earlier runs of the same production code were faster, which shows variability but does not prove the latest drift harmless. A Windows matched base/head control is needed before accepting this quality result. No budgets have been weakened.

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation matches the documented contract and includes comprehensive boundary, recovery, fragmentation, and integration coverage.

Review effort: Balanced
Findings: None

[Karthik Nadig (karthiknadig)]

#559 is merged and #529 is closed. bbdb481 integrates the exact merged main without rewriting history; the resulting tree is identical to the previously reviewed protocol head, and the PR now contains only the five protocol files. The current-head Copilot review recommends approval with no findings, and the independent merge-resolution review is clean. The outstanding Windows matched control is running against exact new-main de7611f and protocol 6209cbc (tree-identical to bbdb481): https://github.com/microsoft/python-environment-tools/actions/runs/36458287684. This is a separate manual-only diagnostic branch, not a workflow change proposed for merge. Keeping draft until the evidence and refreshed exact-main CI are inspected. The refreshed Windows coverage job has failed; investigating its logs before readiness.

[Karthik Nadig (karthiknadig)]

The new exact-main coverage baseline exposed a small function-coverage regression (Linux -0.103pp; Windows -0.032pp), despite increased line coverage. LCOV traced it to the new rejection tests registering callbacks that were deliberately never invoked. 3b2eec8 strengthens those tests with valid requests/notifications after rejection, proving real handler wiring, recovery, exact IDs/params, and no additional errors. All original negative assertions remain; production code and coverage budgets are unchanged. The independent reviewer is clean, 51 protocol tests pass, workspace formatting/Clippy pass, and local instrumented coverage confirms the seven previously unexecuted callbacks now run. Fresh hosted coverage and Copilot review are requested. The Windows matched control remains applicable because this commit changes only tests.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Windows performance validation and refreshed exact-base CI remain pending.

Review effort: Balanced
Findings: None

[Karthik Nadig (karthiknadig)]

The Windows matched control completed successfully: https://github.com/microsoft/python-environment-tools/actions/runs/36458287684. Independent quality review accepts the Windows performance prerequisite: no material regression reproduced in the matched control.

Verified exact merged main de7611f versus protocol 6209cbc; current 3b2eec8 changes only the test module. All eight B-H-H-B-H-B-B-H passes completed on one Windows host, with verified binary/harness hashes and identical full inventories (8 environments, 1 manager).

Medians of four per-pass percentiles per revision, not pooled percentiles:

Metric	Base P50/P95	Head P50/P95
Refresh RTT	98 / 104.5 ms	98.5 / 104.5 ms
Startup to first environment	15 / 18.5 ms	15 / 20.5 ms
Request to first environment	8 / 11.5 ms	8 / 12.5 ms
Cold refresh RTT	98 / 102 ms	97.5 / 104 ms
Cold startup to first environment	15 / 29.5 ms	16.5 / 27.5 ms
Cold request to first environment	8 / 15 ms	9 / 10.5 ms

Limitations: one host, four passes per revision, ten samples per pass. The first-base cold RTT P95 outlier (2203 ms) is retained, not discarded; other base passes are 100-104 ms versus head 98-109 ms. This is not a claim of universal improvement. The ordinary exact-base macOS snapshot retains +14 ms warm RTT P95 and +22 ms startup-first-environment P95; Linux first-environment tails increase by 1-2 ms. Those small tails and the previously accepted macOS tradeoff remain explicitly recorded.

Current-head Copilot has no code findings and all three CodeQL analyses report zero results/errors/warnings. Still draft until the refreshed hosted coverage and remaining CI checks complete. No budgets or exclusions were changed.

[Karthik Nadig (karthiknadig)]

Final-head (3b2eec8) coverage and performance inspection:

• Linux coverage: lines 85.535% (+0.372pp), functions 88.368% (+0.224pp) versus exact merged main.
• Windows coverage: lines 83.083% (+0.425pp), functions 85.753% (+0.289pp). Both hosted coverage jobs pass; no budgets/exclusions changed.
• Final macOS warm RTT P50/P95 is 105/122 ms versus 185/222; startup-first-environment 96/123 versus 157/189. Linux RTT is 56/58 versus 65/68, with first-environment P95 +3 ms. Windows warm RTT is 158/177 versus 156/159, startup-first-environment 23/26 versus 23/34, request-first-environment 13/16 versus 13/23. Inventories match.
• Retained Windows cold outlier: first measured cold iteration took 11,854 ms, producing cold RTT P95 6,595 ms versus baseline 1,898 ms; first request-to-environment was 188 ms. The other nine cold iterations were 147-167 ms (first environment 5-18 ms), and all ten additional diagnostic cold iterations were 148-161 ms. The same production code's preceding run had cold P95 357 ms. Together with the exact matched control, independent final review accepts this isolated sample as non-actionable measurement noise. Its underlying cause is not proven; the raw failure-shaped measurement is preserved, not rerun away or excluded. The historical macOS +14/+22 ms tails remain part of the record.

Independent review finds no remaining code or quality blockers. Current-head Copilot reports no code findings; all three current-head CodeQL analyses have zero results/errors/warnings. Awaiting the final two native macOS jobs before marking ready and enabling protected auto-merge.

[Karthik Nadig (karthiknadig)]

All 35 automated checks now pass at 3b2eec8, including both remaining native macOS jobs. The current-head Copilot review has no findings and there are no unresolved threads. The CI/performance prerequisites mentioned in that review are now complete, with actual coverage gains, matched Windows evidence, and the explicitly accepted isolated cold-tail outlier recorded above. Independent code and final quality reviews are clean. Marking ready and enabling protected squash auto-merge; required human approval and repository policy remain enforced.