Skip to content

Fix Node SDK strict TypeScript consumer compatibility - #2639

Closed
patschmittdev wants to merge 1 commit into
github:mainfrom
patschmittdev:patschmittdev-ship-sdk-strict-types
Closed

Fix Node SDK strict TypeScript consumer compatibility#2639
patschmittdev wants to merge 1 commit into
github:mainfrom
patschmittdev:patschmittdev-ship-sdk-strict-types

Conversation

@patschmittdev

Copy link
Copy Markdown

Summary

Fix strict public-package consumption without weakening lifecycle event variants or using skipLibCheck or a dependency override.

  • Remove metadata only from the private SessionLifecycleEventBase. All five public variants remain unchanged: created/updated/foreground/background require metadata, and deleted keeps metadata?: undefined, matching the runtime's own property with an undefined value.
  • Migrate the actual vscode-jsonrpc dependency from 8.2.1 to 9.0.2 and consistently use its supported vscode-jsonrpc/node export. Update the TypeScript generator and regenerate against the current CLI pin; the generated diff is only that import. Preserve current RPC, session-event, message-source, and FFI changes.
  • Use export-aware build resolution and add strict packed-package fixtures, second declaration-consumer checks, and deterministic JSON-RPC 8 peer coverage. The 8.2.1 alias is development-only compatibility coverage.

Package version 0.0.0-dev, CLI pin 1.0.84-5, and koffi requirement ^3.2.1 are unchanged. No release, tag, or package publication is part of this PR. The changelog is release-generated, so no release entry or version bump is added.

Context and reproduction

The lifecycle union originated in #1357. Related historical type/export reports are #133 and #891; these references provide context, not requests to reopen or close those items.

A public SDK consumer with strict: true, exactOptionalPropertyTypes: true, and skipLibCheck: false encounters TS2430 because SessionDeletedEvent.metadata?: undefined cannot extend the old private base's metadata?: SessionLifecycleEventMetadata.

The packed consumer now compiles. Restoring only the old private-base property inside an isolated installed declaration reproduces TS2430, demonstrating regression sensitivity. Fixtures also assert discriminant narrowing, required metadata for the four non-deleted variants, deleted metadata rejection, public extension inference, and emitted declaration reuse.

Compatibility considerations

This is an explicit JSON-RPC major dependency migration, not a blanket drop-in compatibility claim. Consumers need export-aware module resolution (NodeNext, Node16, or bundler), and JSON-RPC 9 declarations require TypeScript 5.4 or newer. Public MessageConnection typing follows JSON-RPC 9. Existing applications using older compiler/resolver configurations or JSON-RPC 8-specific types may require changes.

Bidirectional JSON-RPC 8/9 requests, notifications, structured errors, cancellation, and default async-notification scheduling are covered. Wire interoperability does not establish TypeScript source compatibility for every previous setup.

Validation

Performed on this current-main-based shipping change:

  • SDK build, typecheck, and ESLint passed on Node 22.23.2. ESLint reports five existing warnings, with no errors.
  • 392 targeted tests passed on each of Node 22.23.2 and Node 24.15.0, including client, extension, send-and-wait, factory, CJS, TypeScript codegen, JSON-RPC compatibility, message-source, and FFI runtime-host tests.
  • 69 shared harness tests and all five extension environment-access tests passed. Missing harness dependencies were restored before the successful rerun.
  • All four combinations of Node 22/24 with @types/node 22.20.2/24.13.4 passed isolated packed-consumer checks using TypeScript 7.0.2: strict declarations, second declaration consumer, public ESM/CJS exports, and a real child extension against a deterministic JSON-RPC 8 peer.
  • All nine current-version release tarballs passed the repository packaging verifier. Fresh installed Windows optional-runtime consumers passed public SDK start/getStatus/stop on both Node versions and reported CLI 1.0.84-5 without a CLI-path override.
  • Configured Prettier passed all 121 staged LF source/test blobs without an EOL override. The direct Windows working-copy check reports 120 CRLF files; the content check with automatic EOL detection also passes. No unrelated formatting changes were made.
  • Focused read-only review found no actionable issues. No model prompts were issued. Node 20 and the full cross-platform E2E matrix were not run locally and remain outside this local evidence.

Intended target: github/copilot-sdk:main; head: patschmittdev/copilot-sdk:patschmittdev-ship-sdk-strict-types.

Preserve lifecycle event metadata variants while migrating JSON-RPC to its
supported Node export. Add strict packed public consumer checks and
deterministic compatibility coverage for JSON-RPC 8 peers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 12, 2026 19:51
@patschmittdev
patschmittdev requested a review from a team as a code owner September 12, 2026 19:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The samples lockfile is inconsistent with JSON-RPC 9, and the strict consumer verifier is not run by CI.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the Node SDK for strict TypeScript consumer compatibility and migrates runtime JSON-RPC usage to vscode-jsonrpc 9.

Changes:

  • Refines lifecycle typings and enables export-aware resolution.
  • Updates JSON-RPC imports, dependencies, and compatibility coverage.
  • Adds strict packed-consumer fixtures, verification tooling, and documentation.
  • Follow-ups required: regenerate the samples lockfile and run consumer verification in CI.
File summaries
File Summary
scripts/codegen/typescript.ts Updates generated RPC imports.
nodejs/tsconfig.test.json Includes compatibility test compilation.
nodejs/tsconfig.json Enables export-aware resolution.
nodejs/test/session-send-and-wait.test.ts Updates RPC imports.
nodejs/test/message-source.test.ts Updates RPC imports.
nodejs/test/jsonrpc-compat.test.ts Adds JSON-RPC 8/9 interoperability coverage.
nodejs/test/factory.test.ts Updates RPC imports.
nodejs/test/e2e/extension_env_access.e2e.test.ts Updates RPC imports.
nodejs/test/consumer-types/tsconfig.json Configures strict consumer compilation.
nodejs/test/consumer-types/tsconfig.consumer.json Configures declaration-consumer checks.
nodejs/test/consumer-types/runtime-extension.mjs Tests packed extension runtime behavior.
nodejs/test/consumer-types/lifecycle.mts Tests lifecycle narrowing and metadata requirements.
nodejs/test/consumer-types/extension.mts Tests extension typings.
nodejs/test/consumer-types/consumer.mts Tests emitted declaration reuse.
nodejs/test/client.test.ts Updates dynamic RPC imports.
nodejs/src/types.ts Fixes lifecycle event base typing.
nodejs/src/session.ts Uses the JSON-RPC 9 node export.
nodejs/src/generated/rpc.ts Regenerates the RPC import.
nodejs/src/client.ts Updates JSON-RPC imports.
nodejs/scripts/verify-consumer-types.mjs Adds packed-package verification.
nodejs/samples/package-lock.json Contains inconsistent JSON-RPC resolution metadata requiring regeneration.
nodejs/README.md Documents strict consumer validation.
nodejs/package.json Updates the dependency and verifier script.
nodejs/package-lock.json Locks JSON-RPC 9 and compatibility coverage.
Review details

Files not reviewed (2)

  • nodejs/package-lock.json: Generated file
  • nodejs/samples/package-lock.json: Generated file

Suppressed comments (1)

nodejs/package.json:43

  • The strict packed-consumer verifier is exposed as a script but is not invoked by .github/workflows/nodejs-sdk-tests.yml: CI runs typecheck, build/package verification, and the Vitest suite, but never test:consumer-types. The compatibility regression this PR targets can therefore return without failing CI; add a validation step that packs the SDK and runs this verifier (at least for one supported Node/types pair).
        "test:consumer-types": "node scripts/verify-consumer-types.mjs",
  • Files reviewed: 21/24 changed files
  • Comments generated: 1
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


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

"dependencies": {
"koffi": "^3.2.1",
"vscode-jsonrpc": "^8.2.1",
"vscode-jsonrpc": "^9.0.2",
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.

2 participants