CmdPal JS/TS Extensions - Phase 2: JSON-RPC transport + manifest - #49323
Open
michaeljolley wants to merge 12 commits into
Open
CmdPal JS/TS Extensions - Phase 2: JSON-RPC transport + manifest#49323michaeljolley wants to merge 12 commits into
michaeljolley wants to merge 12 commits into
Conversation
Adds low-level C# host primitives for JavaScript/TypeScript CmdPal extensions: - JSON-RPC 2.0 message model (request/response/notification/error) using System.Text.Json source-generated contexts. - JsonRpcConnection: an LSP-style Content-Length framed transport over stdio streams with byte-accurate framing, an async read loop handling partial and coalesced reads, request/response correlation with a 10 second timeout, fire-and-forget notifications, inbound request/notification dispatch, serialized writes, out-of-band stderr logging, and disposal that cancels pending requests. - JSExtensionManifest: parses package.json (npm fields plus the cmdpal section) and validates it as a CmdPal extension via a TryParse/TryParseFile API that returns success plus a failure reason. This is additive library code only; nothing is wired into DI or the UI yet (that is a later phase). Unit tests cover framing round-trips including multi-byte UTF-8 and partial/coalesced reads, request correlation and timeout, notification dispatch, and each manifest validation rule using in-memory pipes. Refs #48707 Co-authored-by: Copilot App <[email protected]> Copilot-Session: 2a035fc1-177d-4e3f-8c8d-88614dd3bb52
github-actions
Bot
added
the
Product-Command Palette
…k' into dev/mjolley/phase-2-cmdpal-jsonrpc-transport
… points - JsonRpcConnection: reject an inbound Content-Length that exceeds a 32 MB maximum before allocating the body buffer, so a malformed extension cannot force a multi-gigabyte allocation. The oversized header surfaces as an InvalidDataException via the read loop and Error event. - JSExtensionManifest: require the entry point to be a relative path that stays within the extension directory. Absolute/rooted paths and '..' traversal that escapes the directory are now rejected with clear reasons, matching the spec's relative-path requirement. Adds unit tests for the oversized Content-Length rejection and for absolute and directory-escaping entry points. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 2a035fc1-177d-4e3f-8c8d-88614dd3bb52
…o phase 2 Co-authored-by: Copilot App <[email protected]> Copilot-Session: 762976e0-2453-40ca-8034-978a04a19d1e
Address six review findings in the Command Palette JS extension transport and manifest layers: - Partial-write framing: acquire the write lock cancellably, then emit the header, body, and flush under a non-cancellable token so a frame is never half-written; any write failure moves the connection to a terminal closed state so it is never reused. - Reader-exit terminal state: flip an atomic connection state when the read loop exits and re-check it after a pending request is registered, so sends after EOF fail fast instead of waiting for the timeout. - Bounded stderr: replace unbounded ReadLineAsync with a new BoundedStderrReader that truncates oversized lines, rate limits per window, and caps total forwarded volume. - Decoupled notifications: enqueue inbound notifications onto a bounded channel drained by a dedicated consumer task, so slow or reentrant handlers never block frame reading or response correlation. Full queue drops the oldest entry with a rate limited warning. - Disposal safety: enter the terminal state first, complete the queue, drain in-flight writers, await the read loop, error pump, and notification consumer, and dispose the write lock last so an active writer never triggers ObjectDisposedException. - Manifest hardening: ignore unknown fields, surface missing or malformed values through the parse result, restrict entry points to .js, .mjs, and .cjs, and reject reparse-point (junction or symlink) containment escapes. Expose a normalized NameKey for a future discovery-level uniqueness check. Add unit tests covering each area. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 3f0875f9-177f-40db-888c-248a044d2bb9
Propagates the stale-test and golden-fixture corrections from dev/mjolley/phase-1-cmdpal-ts-sdk so the SDK typecheck stays green at this layer. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
… 2, phase 2) Fixes three round-2 phase-2 findings in the Command Palette JSON-RPC transport: - Non-cancellable writes: frame emission (WriteAsync/FlushAsync) no longer runs under CancellationToken.None. It is now bounded by a dedicated write timeout and linked to disposal, so a child that stops draining stdin can no longer hang a write (or the write lock) forever. On timeout or disposal the write is abandoned, the connection enters its terminal closed state (raising Disconnected so the owner tears the child down), and the caller fails fast with a transport error. Caller cancellation still never corrupts a live, reusable connection. - Malformed-body logging: the parse-failure log path now truncates the offending payload to a small cap with a marker instead of logging the full body up to the 32 MB frame cap. - Inbound request fan-out: inbound requests are drained by a fixed pool of workers through a bounded queue, replacing the untracked fire-and-forget tasks. This caps concurrency, applies backpressure to the peer when saturated, and lets disposal await the in-flight handlers cleanly. Adds unit tests covering the write timeout and disposal teardown of a stuck write, truncated logging of an oversized malformed body, and bounded inbound concurrency with clean disposal. Updates the disposal-during-write test to the new contract. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Bring the phase-1 TypeScript SDK round-2 fixes down onto phase-2. Clean merge (ts-sdk vs C# transport, no overlap). Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
… flood, and slow dispose Round 3 review remediation for the Phase 2 JSON-RPC transport (PR #49323). r3-p2-01 (reader-admission deadlock): the read loop no longer awaits bounded work-queue admission. Dispatch is now synchronous: response and error frames are correlated and completed immediately on the reader, while requests and notifications are admitted with non-blocking TryWrite. An over-budget request is rejected with a JSON-RPC server-busy error and notifications are dropped, so the reader always stays free to route responses that in-flight handlers await. r3-p2-02 (count-only bounds): the notification and request queues now enforce aggregate byte budgets in addition to their item counts, so frames up to the 32 MiB cap can no longer retain tens of gigabytes. Over-budget requests are rejected and notifications dropped rather than admitted. r3-p2-03 (protocol-error log flood): protocol-error logging is routed through a new bounded RateLimitedProtocolLog that emits at most a fixed number of entries per window and reports a suppressed-count summary, so a peer that streams malformed frames cannot flood the log. The limiter holds only fixed counters. r3-p2-04 (slow dispose): disposal now drains the write lock and every background task under a single aggregate Stopwatch deadline instead of a separate per-task timeout, bounding total shutdown time regardless of worker count. Adds MSTest coverage for each behavior: an in-flight handler completing while the request queue is saturated, byte-budget rejection of requests and dropping of notifications, rate-limited protocol logging, and bounded disposal with many blocked workers. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
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.
Warning
This PR is one in a series of PRs focused on adding TypeScript/JavaScript extension support to Command Palette.
This PR should not be merged until PR #49324 is merged into it.
Note
To test the final result, run the branch associated with PR #49364.
Summary
Phase 2 of the effort to add JavaScript/TypeScript extensions to Command Palette (CmdPal). It adds the low-level C# host primitives only: the JSON-RPC transport and the extension manifest parser. This is additive library code under
Microsoft.CmdPal.UI.ViewModels. Nothing is wired into DI, the runner, or the UI yet; that comes in a later phase.Tracking issue: #48707
Stacking
This PR stacks on Phase 1 (the TypeScript SDK, PR #49321) and targets
dev/mjolley/phase-1-cmdpal-ts-sdk, notmain. Please keep it as a draft until Phase 1 is reviewed and merged. Phase 3 (adapters/proxies) will stack on top of this branch.What is included
JSON-RPC 2.0 message model (
Services/JsonRpc/)System.Text.Jsonsource-generated contexts (AOT friendly).JsonRpcException.JsonRpcConnection transport (
Services/JsonRpc/JsonRpcConnection.cs)Content-Lengthframing over a pair of byte streams (typically a child process stdout/stdin), so it is unit-testable with in-memory pipes.SendRequestAsyncwith id correlation and a 10 second request timeout; a generic overload that deserializes the result via aJsonTypeInfo<T>.SendNotificationAsync.JSExtensionManifest parser (
Models/JSExtensionManifest.cs)cmdpalsection (displayName, icon, publisher, capabilities, debug, debugPort, main override).TryParse/TryParseFilereturn success plus a failure reason. Recognized as a CmdPal extension iff it has acmdpalobject AND a non-empty name AND eithercmdpal.mainor top-levelmainresolves to an existing file.Tests
Added MSTest coverage in
Microsoft.CmdPal.UI.ViewModels.UnitTestsusing in-memory pipes (no real Node process):All 24 new tests pass; the full CmdPal solution filter build is clean (exit code 0) with no new analyzer or StyleCop warnings.