TypeScript SDK API review fixes#1357
Merged
SteveSandersonMS merged 27 commits intoMay 21, 2026
Merged
Conversation
Mirrors C# PR #1343 Phase 4a renames: - onExitPlanMode -> onExitPlanModeRequest - onAutoModeSwitch -> onAutoModeSwitchRequest - createSessionFsHandler -> createSessionFsProvider - ResumeSessionConfig.disableResume -> suppressResumeEvent - ProviderConfig.maxInputTokens -> maxPromptTokens (drops the wire shim) - CopilotSession.getMessages() -> getEvents() - InputOptions -> UiInputOptions Wire RPC name 'session.getMessages' is unchanged (runtime contract). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove autoStart and autoRestart from CopilotClientOptions. The client now always starts on first createSession/resumeSession; users can still call client.start() explicitly for eager startup. - Make MCPServerConfigBase.tools optional (undefined = all, [] = none). - Fix streaming JSDoc block comment that wasn't attached due to single-star. Mirrors C# PR #1343 Phase 4c. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Split SessionLifecycleEvent into a discriminated union of SessionCreatedEvent / SessionDeletedEvent / SessionUpdatedEvent / SessionForegroundEvent / SessionBackgroundEvent. - Promote the metadata payload into a named SessionLifecycleEventMetadata interface; metadata is required on non-delete variants and absent on session.deleted. - Convert metadata.startTime and metadata.modifiedTime from string to Date, matching SessionMetadata. Parse on receipt in client.handleSessionLifecycleNotification. - Export the new variant types from index.ts. Mirrors C# PR #1343 Phase 4f + review §2.3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Change BaseHookInput.timestamp from number (Unix ms) to Date. - Parse incoming numeric timestamps into Date in handleHooksInvoke. - Update hooks_extended.e2e.test.ts assertion accordingly. Mirrors C# PR #1343 Phase 4g. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…equest union - Add optional feedback?: string field to PermissionRequestResult so consumers can return free-form text forwarded to the model with the decision. - Delete the hand-written narrow PermissionRequest interface in types.ts and re-export the generated discriminated union from session-events.ts instead. Handlers can now type-safely access per-kind fields (e.g. shell .commands, write .fileName / .diff, mcp .toolName / .args). Mirrors C# PR #1343 Phase 4g + review §2.9. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the fragile Pick<SessionConfig, '...30+ keys...'> definition for ResumeSessionConfig with a shared SessionConfigBase interface. SessionConfig and ResumeSessionConfig now both extend it: - SessionConfig adds sessionId? and cloud?. - ResumeSessionConfig adds suppressResumeEvent? and continuePendingWork?. SessionConfigBase is exported from index.ts for consumers that want to build shared helpers over both shapes. Mirrors C# PR #1343 Phase 5 + review §2.2. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Change defineTool from defineTool(name, config) to defineTool({ name, ...config })
so the call shape matches the Tool<T> interface. name remains mandatory and is
enforced by the Tool<T> type. Updates all samples, docs, tests, and the
CHANGELOG snippet.
Review §1.3.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 49da911.
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1357 · ● 1.3M
| * ``` | ||
| */ | ||
| async getMessages(): Promise<SessionEvent[]> { | ||
| async getEvents(): Promise<SessionEvent[]> { |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use the old name here:
- Python:
session.get_messages()→ should becomeget_events() - Go:
session.GetMessages()→ should becomeGetEvents()
Both still call the session.getMessages RPC under the hood (which is correct), but the public method names need to be aligned.
| * When provided, enables `exitPlanMode.request` callbacks. | ||
| */ | ||
| onExitPlanMode?: ExitPlanModeHandler; | ||
| onExitPlanModeRequest?: ExitPlanModeHandler; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use the old names:
- Python
on_exit_plan_modein bothSessionConfigandResumeSessionConfig→on_exit_plan_mode_request - Go
OnExitPlanModeinSessionConfigandResumeSessionConfig→OnExitPlanModeRequest
| * When provided, enables `autoModeSwitch.request` callbacks. | ||
| */ | ||
| onAutoModeSwitch?: AutoModeSwitchHandler; | ||
| onAutoModeSwitchRequest?: AutoModeSwitchHandler; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use the old names:
- Python
on_auto_mode_switchin bothSessionConfigandResumeSessionConfig→on_auto_mode_switch_request - Go
OnAutoModeSwitchinSessionConfigandResumeSessionConfig→OnAutoModeSwitchRequest
| * @default false | ||
| */ | ||
| disableResume?: boolean; | ||
| suppressResumeEvent?: boolean; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use the old name:
- Python
disable_resumeinResumeSessionConfig→suppress_resume_event - Go
DisableResumeinResumeSessionConfig→SuppressResumeEvent
Also note the Go wire serialization uses "disableResume" (go/types.go:1221) which should stay unchanged (it's an internal RPC field).
| | "onEvent" | ||
| | "createSessionFsHandler" | ||
| > & { | ||
| createSessionFsProvider?: (session: CopilotSession) => SessionFsProvider; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use the old name:
- Python
create_session_fs_handlerin bothSessionConfigandResumeSessionConfig→create_session_fs_provider - Go
CreateSessionFsHandlerin bothSessionConfigandResumeSessionConfig→CreateSessionFsProvider
| * exceed this limit. | ||
| */ | ||
| maxInputTokens?: number; | ||
| maxPromptTokens?: number; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use maxInputTokens:
- Python
max_input_tokensinProviderConfig→max_prompt_tokens - Go
MaxInputTokensinProviderConfig→MaxPromptTokens
Note: the Go field already serializes to the correct maxPromptTokens wire key, so only the public Go field name needs changing.
| * Options for the `input()` convenience method. | ||
| */ | ||
| export interface InputOptions { | ||
| export interface UiInputOptions { |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still use InputOptions:
- Python
InputOptions→UiInputOptions - Go
InputOptions→UiInputOptions
| export type PermissionRequestResult = ( | ||
| | PermissionDecisionRequest["result"] | ||
| | { kind: "no-result" } | ||
| ) & { feedback?: string }; |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: The feedback field on PermissionRequestResult is missing from Python and Go:
- Python's
PermissionRequestResultdataclass only haskind; needsfeedback: str | None = None - Go's
PermissionRequestResultstruct only hasKind; needsFeedback string \json:"feedback,omitempty"``
The .NET SDK already has this field (Feedback on PermissionDecisionResult).
| /** | ||
| * List of tools to include from this server. [] means none. "*" means all. | ||
| * List of tools to include from this server. | ||
| * `undefined` (the default) or `"*"` means include all tools. |
Contributor
There was a problem hiding this comment.
Cross-SDK parity: Python and Go still have required tools (non-optional):
- Python
tools: list[str]inMCPServerConfig→ should become optional, e.g.tools: list[str] | None = None - Go doesn't seem to have a
toolsfield on MCP config (may need to verify), but if it does, it should be optional
.NET already made this optional.
Replaces the flat connection-related fields on CopilotClientOptions
(cliPath, cliArgs, port, useStdio, cliUrl, tcpConnectionToken,
isChildProcess) with a single discriminated 'connection?: RuntimeConnection'
field. Construct values via factory functions:
RuntimeConnection.forStdio({ path?, args? }) // default
RuntimeConnection.forTcp({ port?, connectionToken?, path?, args? })
RuntimeConnection.forUri(url, { connectionToken? })
The mutually-exclusive combinations that used to be runtime errors are
now caught at compile time by the discriminated union. The previous
isChildProcess flag (only ever used by joinSession() in extension.ts) is
dropped from the public API surface; extension.ts now uses an @internal
_internalConnection hook to enter the parent-process stdio mode.
Other renames in this phase:
- CopilotClientOptions.copilotHome -> baseDirectory.
- Internal CopilotClient.actualPort field -> runtimePort.
All TS test files, scenario fixtures, samples, README, and docs updated
to the new shape. Mirrors C# PR #1343 Phase 9.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both methods now accept either a MessageOptions object or just a string
prompt. The string form is a shorthand for { prompt }:
await session.send('Hello');
await session.sendAndWait('Hello');
Mirrors C# PR #1343 Phase 7.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Enable stripInternal in tsconfig.json so @internal members no longer appear in the published .d.ts. Verified that CopilotSession constructor, the register*/clientSessionApis hooks, _handle* methods, NO_RESULT_PERMISSION_V2_ERROR, _internalConnection, and ParentProcessRuntimeConnection are all stripped from the public types. The MessageConnection import from vscode-jsonrpc no longer leaks either. - Tag NO_RESULT_PERMISSION_V2_ERROR with @internal explicitly. - Implement Symbol.asyncDispose on CopilotClient so it works inside 'await using' blocks, matching CopilotSession. - Tighten client.stop() so the Node process can exit cleanly without process.exit(): socket.destroy() in addition to socket.end(), explicit destroy() on the child process stdio streams, and cliProcess.unref(). Manually verified by running examples/basic-example.ts against a live runtime: the process exits within a few seconds of the await using block ending. - Remove the deprecated CopilotSession.destroy() alias. - Rewrite examples/basic-example.ts to import from '@github/copilot-sdk' (not '../src/index.js') and demonstrate the await using pattern with the new send/sendAndWait string overloads. Covers review §2.4, §2.5, §2.10, §3.1, §3.2, §4.1, §4.3 and the C# PR's Phase 8 docs/sample updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Twenty-one test/scenarios/**/typescript/src/index.ts files used 'githubToken: process.env.GITHUB_TOKEN' (lowercase h) instead of the correct 'gitHubToken'. They silently passed an unrecognized property and the runtime ignored the token. Fix in lock-step across all affected scenarios. The existing 'typecheck' npm script already runs 'tsc --noEmit -p tsconfig.test.json' in CI, so no further CI wiring is needed to prevent regressions: this typo would now be a compile error under the SDK's strict CopilotClientOptions shape. Other Phase L items (missing onPermissionRequest, invalid permission kinds, resumeSession scenarios without config) were either already caught by the runtime PR or do not apply to TS — no remaining work. Covers review §1.1, §2.1, §2.8, §2.11. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prettier check failed on Ubuntu CI because several files modified in earlier phases didn't get re-formatted after the bulk regex rewrites. Running 'npm run format' (prettier --write) normalizes them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- commands/multi-client/ui_elicitation: shorthand
'copilotClientOptions: { tcpConnectionToken }' wasn't matched by the
earlier batch rewrite, so client1 was still spawned in stdio mode
while client2 tried to connect by URI. Switch the harness call to
TCP + RuntimeConnection.forTcp({ connectionToken: tcpConnectionToken }).
- session_fs.e2e.test.ts: add the missing RuntimeConnection import.
- hooks_extended.e2e.test.ts: SessionStart and UserPromptSubmitted
timestamp assertions still used toBeGreaterThan(0) but
BaseHookInput.timestamp is now Date. Switch to toBeInstanceOf(Date).
- client.test.ts: delete the two obsolete 'allows *Session without
onPermissionRequest' unit tests. They asserted on the 'Client not
connected' error that only occurred when autoStart was false; with
Phase C removing autoStart, the client now auto-starts on the first
session call and those tests would need a real spawned runtime.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rmission tests The two client.test.ts unit tests deleted in the previous commit only asserted that createSession/resumeSession surface 'Client not connected' when called pre-start. The intent behind them was to confirm that omitting onPermissionRequest doesn't itself throw. With autoStart gone, the only meaningful version of that test is an E2E one that actually spawns a runtime. Port the equivalent C# coverage (ClientE2ETests.Should_Allow_*Session_Called_Without_PermissionHandler) to client.e2e.test.ts so we have parity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uest
Ports dotnet's Should_Allow_CreateSession_Called_Without_PermissionHandler
(Theory: stdio + tcp) and Should_Allow_ResumeSession_Called_Without_PermissionHandler
from dotnet/test/E2E/ClientE2ETests.cs into nodejs/test/e2e/session.e2e.test.ts.
These exercise the contract that {onPermissionRequest} is optional on both
SessionConfig and ResumeSessionConfig: when not provided, the runtime
leaves permission prompts pending for the consumer to resolve via the
low-level RPC. Without these tests, that contract was unprotected against
regression.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ting CLI path Earlier batch rewrite of createSdkTestContext meant that whenever a test passed copilotClientOptions.connection, the spread overrode the harness's own connection variant entirely - losing the COPILOT_CLI_PATH binding. Now merge by variant kind: if the caller asks for tcp/stdio without a path, the harness fills it in from COPILOT_CLI_PATH; explicit values from the caller win. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…client
In the 'should reject setProvider when sessions already exist' test, the
first client was still using the flat tcpConnectionToken property which
is no longer a valid CopilotClientOptions field. Switch to
RuntimeConnection.forTcp({ connectionToken }).
Verified locally: full session_fs.e2e.test.ts suite (9 tests) now passes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
normalizeHookInput lived in client.ts and inspected for a 'timestamp' property by name, which felt magical (brittle against any future hook-shaped wire payload that happens to contain a numeric 'timestamp'). Move the conversion into CopilotSession._handleHooksInvoke, renamed deserializeHookInput, right next to the GenericHandler cast that says 'this unknown is now a HookInput'. That's the only call site that actually knows the payload is a hook input, so it's the correct boundary for the schema transform. This is the TS equivalent of what C# does via UnixMillisecondsDateTimeOffsetConverter (attached per-property on each HookInput.Timestamp); TS just plumbs the same conversion through the hooks dispatcher instead of a per-type JSON converter. Verified 3/3 hooks_extended.e2e tests pass locally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
destroy() on the child's stdio pipes and the TCP socket is more aggressive than needed. If the child crashes with a useful message on stderr that our existing data listener hasn't drained yet, stderr.destroy() drops it. If there's an in-flight write to stdin, destroy() raises 'error' on the stream. Same trade-off for socket.destroy() short-circuiting the graceful FIN/ACK. unref() solves the actual problem (event loop staying alive after stop()) without disrupting late output. From the Node docs: 'unref will allow the program to exit if this is the only active socket in the event system. The socket does not lose any functionality' — error events still fire, late data still drains through registered listeners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the unref()-based fire-and-forget cleanup with deterministic awaiting: - Socket: end() + await 'close'. By the time stop() returns the FIN/ACK exchange has completed. - ChildProcess: kill() + await 'exit'. By the time stop() returns the child has truly exited, its stdio pipes are closed, and there are no lingering handles to keep the event loop alive. No SIGKILL escalation. If the child ignores SIGTERM, stop() blocks; callers that need a guaranteed-bounded shutdown should use forceStop() (which already sends SIGKILL). Replaces the previous unref() approach: that worked for clean exit but allowed late stderr output to surface after stop() resolved, which is exactly the timing window where consumers expect cleanup to be done. Verified locally: full client.test.ts (75 tests) passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous '& { feedback?: string }' incorrectly added feedback to every
variant of the union. In the runtime schema, feedback is reject-only —
it appears only on PermissionDecisionReject and is already typed by the
generated PermissionDecisionRequest['result'] union. No manual
augmentation needed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Matches the C# rename in PR #1357 Phase 4f (client.On -> client.OnLifecycle). client.onLifecycle is clearer than client.on at the call site because the two on() methods on CopilotClient and CopilotSession listen for completely different event families (lifecycle vs per-session). The receiver alone isn't always enough to disambiguate, especially in mixed code that holds both objects. session.on stays unchanged because that's where the bare 'on' verb belongs: session events are the primary stream for that object. Updates README + client_lifecycle.e2e.test.ts to the new name. Tests still pass (validated via tsc -p tsconfig.test.json). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ports the C# SDK API review decisions to the Node.js/TypeScript SDK, reshaping the public API (connection configuration, session/lifecycle APIs, timestamps, permissions) and updating tests, scenarios, and documentation to match.
Changes:
- Introduces a discriminated
RuntimeConnectionconfig (stdio/tcp/uri) and migrates client construction throughout tests/scenarios. - Renames/reshapes several session APIs (e.g.,
getMessages()→getEvents(), lifecycle subscriptions viaonLifecycle, hook timestamps toDate, resume suppression flag rename). - Updates docs, examples, snapshots, and adds TypeScript surface hygiene (e.g.,
stripInternal).
Show a summary per file
| File | Description |
|---|---|
| test/snapshots/multi_client/one_client_rejects_permission_and_both_see_the_result.yaml | Updates replay snapshot to match new event/tool behavior. |
| test/snapshots/hooks_extended/should_allow_pretooluse_to_return_modifiedargs_and_suppressoutput.yaml | Updates replay snapshot to match hook/tool output behavior. |
| test/scenarios/transport/tcp/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forUri. |
| test/scenarios/transport/stdio/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio + token casing changes. |
| test/scenarios/transport/reconnect/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forUri. |
| test/scenarios/tools/virtual-filesystem/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/tool-overrides/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/tool-filtering/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/skills/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/no-tools/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/mcp-servers/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/tools/custom-agents/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/sessions/streaming/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/sessions/session-resume/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/sessions/infinite-sessions/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/sessions/concurrent-sessions/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/prompts/system-message/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/prompts/reasoning-effort/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/prompts/attachments/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/modes/minimal/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/modes/default/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/callbacks/user-input/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/callbacks/permissions/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/callbacks/hooks/typescript/src/index.ts | Migrates scenario to RuntimeConnection.forStdio. |
| test/scenarios/bundling/fully-bundled/typescript/src/index.ts | Migrates bundling scenario to RuntimeConnection.forStdio. |
| test/scenarios/auth/gh-app/typescript/src/index.ts | Migrates auth scenario to RuntimeConnection.forStdio + token casing changes. |
| test/scenarios/auth/byok-openai/typescript/src/index.ts | Migrates BYOK OpenAI scenario to RuntimeConnection.forStdio. |
| test/scenarios/auth/byok-ollama/typescript/src/index.ts | Migrates BYOK Ollama scenario to RuntimeConnection.forStdio. |
| test/scenarios/auth/byok-azure/typescript/src/index.ts | Migrates BYOK Azure scenario to RuntimeConnection.forStdio. |
| test/scenarios/auth/byok-anthropic/typescript/src/index.ts | Migrates BYOK Anthropic scenario to RuntimeConnection.forStdio. |
| nodejs/tsconfig.json | Enables stripInternal for declaration generation. |
| nodejs/test/extension.test.ts | Updates join/resume config rename (disableResume → suppressResumeEvent). |
| nodejs/test/e2e/ui_elicitation.e2e.test.ts | Migrates multi-client TCP wiring to RuntimeConnection + rename changes. |
| nodejs/test/e2e/suspend.e2e.test.ts | Migrates TCP server/client construction to RuntimeConnection. |
| nodejs/test/e2e/streaming_fidelity.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/session.e2e.test.ts | Broad migration to new API names + adds coverage for optional permission handler behavior. |
| nodejs/test/e2e/session_lifecycle.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/session_fs.e2e.test.ts | Renames createSessionFsHandler → createSessionFsProvider and updates connection usage. |
| nodejs/test/e2e/session_fs_sqlite.e2e.test.ts | Renames createSessionFsHandler → createSessionFsProvider. |
| nodejs/test/e2e/rpc.e2e.test.ts | Updates default client construction semantics. |
| nodejs/test/e2e/rpc_shell_and_fleet.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/rpc_session_state.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/rpc_server.e2e.test.ts | Migrates CLI invocation to RuntimeConnection.forStdio. |
| nodejs/test/e2e/rpc_mcp_config.e2e.test.ts | Updates default client construction semantics. |
| nodejs/test/e2e/rpc_mcp_and_skills.e2e.test.ts | Migrates CLI args passing via RuntimeConnection.forStdio. |
| nodejs/test/e2e/rpc_event_side_effects.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/per_session_auth.e2e.test.ts | Migrates CLI invocation to RuntimeConnection.forStdio. |
| nodejs/test/e2e/pending_work_resume.e2e.test.ts | Migrates TCP wiring to RuntimeConnection + API renames. |
| nodejs/test/e2e/multi-client.e2e.test.ts | Migrates TCP wiring to RuntimeConnection + API renames. |
| nodejs/test/e2e/mode_handlers.e2e.test.ts | Renames mode handler config callbacks to *Request forms. |
| nodejs/test/e2e/hooks_extended.e2e.test.ts | Updates hook timestamp assertions to Date. |
| nodejs/test/e2e/harness/sdkTestHelper.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/harness/sdkTestContext.ts | Migrates harness to derive RuntimeConnection consistently. |
| nodejs/test/e2e/event_fidelity.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/error_resilience.e2e.test.ts | Updates API rename (getMessages → getEvents). |
| nodejs/test/e2e/connection_token.test.ts | Migrates token passing to RuntimeConnection variants. |
| nodejs/test/e2e/commands.e2e.test.ts | Migrates TCP wiring to RuntimeConnection + resume suppression rename. |
| nodejs/test/e2e/client.e2e.test.ts | Migrates to new connection model + adds coverage for optional permission handler. |
| nodejs/test/e2e/client_options.e2e.test.ts | Updates options semantics and renames (copilotHome → baseDirectory, uri auth constraints). |
| nodejs/test/e2e/client_lifecycle.e2e.test.ts | Renames lifecycle subscription API (on → onLifecycle). |
| nodejs/test/client.test.ts | Updates unit tests for renamed options/APIs and connection parsing changes. |
| nodejs/test/cjs-compat.test.ts | Adjusts client construction for changed options model. |
| nodejs/src/types.ts | Defines RuntimeConnection, refactors config types, updates permissions + lifecycle event shapes, and renames public types. |
| nodejs/src/session.ts | Adds string overloads for send/sendAndWait, renames getMessages → getEvents, deserializes hook timestamps to Date. |
| nodejs/src/index.ts | Re-exports RuntimeConnection and new/renamed public types. |
| nodejs/src/extension.ts | Updates joinSession to use internal connection hook + resume suppression rename. |
| nodejs/src/client.ts | Implements new connection model, lifecycle event parsing, and updated session creation/resume behavior. |
| nodejs/README.md | Updates documentation to the new API surface and naming. |
| nodejs/examples/basic-example.ts | Updates example to new package import + await using and new convenience overloads. |
| docs/troubleshooting/compatibility.md | Updates docs to new getEvents() naming. |
Copilot's findings
- Files reviewed: 70/70 changed files
- Comments generated: 30
Comment on lines
+5
to
+6
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| gitHubToken: process.env.GITHUB_TOKEN, |
Comment on lines
13
to
15
| const client = new CopilotClient({ | ||
| ...(process.env.COPILOT_CLI_PATH && { cliPath: process.env.COPILOT_CLI_PATH }), | ||
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| }); |
Comment on lines
5
to
8
| const client = new CopilotClient({ | ||
| ...(process.env.COPILOT_CLI_PATH && { cliPath: process.env.COPILOT_CLI_PATH }), | ||
| githubToken: process.env.GITHUB_TOKEN, | ||
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| gitHubToken: process.env.GITHUB_TOKEN, | ||
| }); |
Comment on lines
9
to
12
| async function main() { | ||
| const client = new CopilotClient({ | ||
| ...(process.env.COPILOT_CLI_PATH && { cliPath: process.env.COPILOT_CLI_PATH }), | ||
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| }); |
Comment on lines
12
to
15
|
|
||
| const client = new CopilotClient({ | ||
| ...(process.env.COPILOT_CLI_PATH && { cliPath: process.env.COPILOT_CLI_PATH }), | ||
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| }); |
Comment on lines
12
to
+15
| async function main() { | ||
| const client = new CopilotClient({ | ||
| ...(process.env.COPILOT_CLI_PATH && { cliPath: process.env.COPILOT_CLI_PATH }), | ||
| githubToken: process.env.GITHUB_TOKEN, | ||
| connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH }), | ||
| gitHubToken: process.env.GITHUB_TOKEN, |
Comment on lines
+75
to
+81
| connection = RuntimeConnection.forTcp({ | ||
| ...userConn, | ||
| path: userConn.path ?? process.env.COPILOT_CLI_PATH, | ||
| }); | ||
| } else if (userConn.kind === "stdio") { | ||
| connection = RuntimeConnection.forStdio({ | ||
| ...userConn, |
| if (!this.options.cliPath) { | ||
| if (!this.resolvedCliPath) { | ||
| throw new Error( | ||
| "Path to Copilot CLI is required. Please provide it via the cliPath option, or use cliUrl to rely on a remote CLI." |
| * `[]` means include none. | ||
| */ | ||
| tools: string[]; | ||
| tools?: string[]; |
| - `RuntimeConnection.forUri(url, { connectionToken? })` — connect to an already-running runtime (mutually exclusive with `gitHubToken`/`useLoggedInUser`). | ||
| - `cwd?: string` - Working directory for the runtime process (default: current process cwd). | ||
| - `baseDirectory?: string` - Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned runtime. When not set, the runtime defaults to `~/.copilot`. Ignored when connecting via `RuntimeConnection.forUri`. | ||
| - `logLevel?: string` - Log level (default: "info"). |
This comment has been minimized.
This comment has been minimized.
- Add missing RuntimeConnection imports to 26 test/scenarios TypeScript
fixtures. The earlier batch rewrite added the .forStdio/.forUri call
sites but missed the corresponding import statement, so each scenario
failed to compile until now.
- nodejs/test/e2e/harness/sdkTestContext.ts: strip the 'kind' property
before spreading a user-supplied RuntimeConnection back through the
forStdio/forTcp factory opts. The factory opt types don't accept
'kind' so the spread was producing excess-property type errors.
- nodejs/src/client.ts: rewrite the 'Path to Copilot CLI is required'
error message to point at the new connection options
(RuntimeConnection.forStdio({ path }), forTcp({ path }), forUri(...),
or the COPILOT_CLI_PATH environment variable). The old message
referenced removed cliPath / cliUrl options.
- nodejs/src/client.ts: change the default logLevel from 'debug' to
'info'. 'debug' was a TS-only outlier; Python and Rust default to
'info', and the README has always claimed 'info'. Go and .NET don't
pass --log-level at all when omitted (CLI defaults to info anyway),
so 'info' is consistent with every other SDK's effective default.
- nodejs/src/types.ts: fix MCPServerConfigBase.tools doc comment to
spell the all-tools sentinel as ['*'] (the actual type is string[],
so a bare '*' string can't be passed).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of defaulting to 'info' on the SDK side, omit the --log-level
flag entirely when the caller didn't set one and let the runtime use
its own default. Matches dotnet/Client.cs and go/client.go, which both
only pass --log-level when explicitly provided.
CopilotClientOptions.logLevel JSDoc and README updated to describe this
('When omitted, the runtime uses its own default').
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1357 · ● 3.7M
| * Ignored when connecting to an existing runtime via {@link RuntimeConnection.forUri}. | ||
| * @default false | ||
| */ | ||
| remote?: boolean; |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency (TypeScript ↔ C#): This property is named remote here, but the equivalent in the C# SDK (added as part of the C# API review in #1343) is EnableRemoteSessions:
// dotnet/src/Types.cs
public bool EnableRemoteSessions { get; set; }Since this PR is bringing TypeScript in line with the C# API review decisions, it might be worth renaming this to enableRemoteSessions to match. The description and semantics are identical – the only difference is the name (remote vs enableRemoteSessions).
This is the one cross-SDK naming inconsistency I found between TypeScript and C# after reviewing all the changes in this PR.
…SDK consistency Matches the C# API review rename in #1343 (EnableRemoteSessions on CopilotClientOptions). The wire-level RPC field stays 'remote' since that is the runtime's contract; only the SDK surface changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency Review (TypeScript vs. C#)As requested in the PR description, this review only assesses consistency with the C# SDK (PR #1343), since that is the only other SDK that has been updated to match the API review decisions so far. ✅ Consistent — No issues foundAll TypeScript API renames and restructurings in this PR align with their C# counterparts. Here is a summary of each change and its corresponding C# equivalent:
No inconsistencies were found between the TypeScript changes in this PR and the already-updated C# SDK. The API surface is semantically aligned, accounting for appropriate language-idiomatic differences (discriminated unions vs. inheritance,
|
7 tasks
SteveSandersonMS
added a commit
that referenced
this pull request
May 21, 2026
* Go SDK API review: Phase A - SessionConfig/ResumeSessionConfig renames
- OnExitPlanMode -> OnExitPlanModeRequest
- OnAutoModeSwitch -> OnAutoModeSwitchRequest
- ExitPlanModeHandler -> ExitPlanModeRequestHandler
- AutoModeSwitchHandler -> AutoModeSwitchRequestHandler
- Session.GetMessages -> Session.GetEvents
- ResumeSessionConfig.DisableResume -> SuppressResumeEvent
- Streaming bool -> *bool on SessionConfig and ResumeSessionConfig
Wire-level RPC method (session.getMessages) and internal request types
are unchanged; only the public Go surface is renamed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phase B+C - ProviderConfig + MCP Tools tweaks
B. ProviderConfig.MaxInputTokens -> MaxPromptTokens (matches the wire
name 'maxPromptTokens' exactly).
C. MCPStdioServerConfig.Tools / MCPHTTPServerConfig.Tools: change JSON
tag from 'tools' to 'tools,omitempty'. Now nil/omitted slice means
'all tools' (CLI default), matching TS/Rust/C# semantics.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phase D - RuntimeConnection refactor
Replaces the flat connection fields on ClientOptions with a single
discriminated Connection RuntimeConnection field constructed from one of:
StdioConnection{Path, Args}
TcpConnection{Port, ConnectionToken, Path, Args}
UriConnection{URL, ConnectionToken}
Removes:
- CLIPath, CLIArgs, CLIUrl, UseStdio, Port, TCPConnectionToken
- AutoStart, AutoRestart
- public ConnectionState type, State* constants, and Client.State() method
Renames:
- CopilotHome -> BaseDirectory
- Remote -> EnableRemoteSessions
- Client.ActualPort() -> Client.RuntimePort()
LogLevel no longer defaults to 'info'. When empty, the SDK does not
pass --log-level to the runtime, matching the TS SDK.
All unit tests, e2e tests, samples, and the Go scenario apps are
migrated. README updated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phase E - lifecycle timestamps as time.Time
SessionMetadata and SessionLifecycleEventMetadata: StartTime and
ModifiedTime change from string to time.Time. The runtime emits these
as ISO 8601 strings, which time.Time's default JSON unmarshal handles
natively.
Matches the equivalent C# (#1343) / TS (#1357) changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phase F - hook timestamps as time.Time
All six hook input types (PreToolUseHookInput, PostToolUseHookInput,
UserPromptSubmittedHookInput, SessionStartHookInput, SessionEndHookInput,
ErrorOccurredHookInput): Timestamp changes from int64 to time.Time. Each
type gains a MarshalJSON/UnmarshalJSON pair that serializes Timestamp as
Unix milliseconds on the wire, matching the runtime protocol.
Mirrors the equivalent C# (#1343) UnixMillisecondsDateTimeOffsetConverter
and TS (#1357) Date-typed hook timestamps.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phases G-K - misc renames and cleanups
G. InputOptions -> UiInputOptions on the Session.UI().Input convenience.
H. Add Session.SendPrompt(ctx, prompt) and Session.SendPromptAndWait(ctx, prompt)
convenience wrappers around the MessageOptions-based methods (mirrors the
string overloads added in C#/TS).
I. SessionFsProvider interface method renames (Go-specific): Mkdir ->
MakeDirectory, Readdir -> ReadDirectory, ReaddirWithTypes ->
ReadDirectoryWithTypes, Rm -> Remove. The adapter still implements the
wire-protocol method names (rpc.SessionFsHandler) unchanged.
J. SessionConfig/ResumeSessionConfig.CreateSessionFsHandler ->
CreateSessionFsProvider (matches the SessionFsProvider type it returns).
K. Remove deprecated Session.Destroy(); callers must use Session.Disconnect().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Go SDK API review: Phase L - docs and scenario migrations
Update Go code samples in docs/ to use the new ClientOptions.Connection
shape (StdioConnection / UriConnection). Also migrate the streaming
scenario to copilot.Bool(true) for the new *bool Streaming field.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix Streaming: true in Go doc snippets after *bool change
CI docs-validate extracts Go code blocks and compiles them. Update the
remaining 4 sites (docs/getting-started.md x3, docs/features/streaming-events.md)
to use copilot.Bool(true) now that SessionConfig.Streaming is *bool.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Apply gofmt
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* MCP Tools: change []string to *[]string to preserve nil vs empty
With `json:"tools,omitempty"` on a bare []string, Go collapses both
nil and []string{} to "omitted", losing the documented distinction
between "all tools" (nil) and "no tools" (empty slice). Switch the
field type to *[]string so a non-nil pointer to an empty slice
serializes as `tools: []` on the wire, matching TS `tools?: string[]`
and C# `IList<string>?` with WhenWritingNull.
Callers use the standard Go idiom for pointer-to-slice literals:
Tools: &[]string{"*"} // explicit all tools
Tools: &[]string{} // no tools
Tools: &[]string{"a","b"} // only those tools
Tools: nil // (default) all tools, field omitted
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: show StdioConnection.Args for Go --log-dir snippet
The Go SDK now supports extra runtime args via StdioConnection.Args /
TcpConnection.Args. Replace the outdated 'Go cannot pass args, run the
CLI manually' snippet with the actual idiomatic call.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Drop TS/C# reference from MCPStdioServerConfig doc comment
Go SDK source shouldn't reference other-language SDKs. Rephrase the
Tools field doc to explain the pointer-to-slice form purely in Go
terms.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 21, 2026
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
- on_exit_plan_mode -> on_exit_plan_mode_request (handler kwarg + TypedDict field) - on_auto_mode_switch -> on_auto_mode_switch_request (handler kwarg + TypedDict field) - CopilotSession.get_messages() -> get_events() (returns SessionEvent[], not messages) - ProviderConfig.max_input_tokens -> max_prompt_tokens (matches wire key maxPromptTokens) Mirrors PR #1357 Phase A (TypeScript SDK API review). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1343 Phase 9 (.NET) and PR #1357 Phase I (TypeScript). Removes the flat `SubprocessConfig` / `ExternalServerConfig` split in favour of a `RuntimeConnection` discriminated hierarchy, with process-management options moved to a new `CopilotClientOptions` dataclass. Public API: * `RuntimeConnection` abstract base with static factories `stdio()`, `tcp()`, `uri()`. * `ChildProcessRuntimeConnection` intermediate base carrying `path` and `args` shared by Stdio + Tcp. * `StdioRuntimeConnection`, `TcpRuntimeConnection`, `UriRuntimeConnection` concrete subclasses; pattern-match / `isinstance` on the class to branch on the transport. * `CopilotClientOptions(connection=..., working_directory=..., log_level=..., env=..., github_token=..., base_directory=..., use_logged_in_user=..., telemetry=..., session_fs=..., session_idle_timeout_seconds=..., enable_remote_sessions=...)`. * `CopilotClient(options=CopilotClientOptions(...) | None, *, auto_start=..., on_list_models=...)`. Renames: * `copilot_home` → `base_directory` * `remote` → `enable_remote_sessions` * `CopilotClient.actual_port` → `CopilotClient.runtime_port` * `CopilotClient.on(...)` → `CopilotClient.on_lifecycle(...)` * `tcp_connection_token` moves off `CopilotClientOptions` and onto the variant: `TcpRuntimeConnection.connection_token` / `UriRuntimeConnection.connection_token` * `use_stdio` bool is gone — the variant class IS the discriminator. Migration: * All hand-written code that previously branched on `isinstance(config, ExternalServerConfig)` now branches on the runtime-connection variant via `isinstance(connection, UriRuntimeConnection)` / `StdioRuntimeConnection` etc., giving proper type narrowing in pyright/mypy without relying on Literal-based tagged-union narrowing. * Samples, scenarios, README, and tests updated in lock-step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 Phase C (TypeScript SDK API review). Changes: * `SessionConfig.streaming` and `ResumeSessionConfig.streaming` now document `Defaults to False` (matches TS PR #1357 4.2). * `ToolBinaryResult.type` tightened from `str` to `Literal["image", "resource"]` with `"image"` default (matches TS PR #1357 2.6). * `scripts/codegen/python.ts` now sets `inferenceFlags: { combineClasses: false }` when invoking quicktype. Previously quicktype's default structural-equality merging produced fuzzy synthesized names (e.g. `PermissionDecisionApproveForIonApproval` for the merge of `PermissionDecisionApproveFor{Session,Location}Approval`), which are unstable: any future divergence between the schema variants would silently change the generated class name. We rely on the schema's named definitions and resolve structural unions explicitly via `postProcessRefBasedDiscriminatedUnionsForPython`, so the merging is also redundant. Removed the `Session→Ion` / `Location→Ion` acronym hack we needed when the merging was on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 Phases D + E (TypeScript SDK API review). Phase D — Session lifecycle event polymorphic hierarchy: * Split the flat `SessionLifecycleEvent` dataclass into a `SessionLifecycleEventBase` + five concrete variant dataclasses (`SessionCreatedEvent`, `SessionDeletedEvent`, `SessionUpdatedEvent`, `SessionForegroundEvent`, `SessionBackgroundEvent`). `SessionLifecycleEvent` is now a `Union` type alias over the five variants; pattern-match on the variant class to branch on the event kind. Each variant exposes `type: ClassVar[Literal["..."]]`, so existing `event.type == "session.created"` consumer code continues to work. * `SessionLifecycleEvent.from_dict` becomes a module-private `_session_lifecycle_event_from_dict` factory that switches on the wire `type` and constructs the matching variant. Phase D / E — Timestamps as `datetime`: * `SessionMetadata.startTime` / `modifiedTime` now `datetime` (previously ISO-8601 `str`). `to_dict` round-trips back to ISO strings. * `SessionLifecycleEventMetadata.startTime` / `modifiedTime` likewise. * All `Pre/PostToolUseHookInput`, `PreMcpToolCallHookInput`, `UserPromptSubmittedHookInput`, `SessionStartHookInput`, `SessionEndHookInput`, `ErrorOccurredHookInput` declare `timestamp: datetime`. `CopilotSession._handle_hooks_invoke` converts the wire epoch-milliseconds integer into a timezone-aware `datetime` at the dispatch boundary (same place the existing `cwd → workingDirectory` remap lives). Tests updated in lock-step: `test_session_e2e.py` asserts the new `datetime` types on `SessionMetadata`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 Phase K + 4.1 (TypeScript SDK API review). Cleanup: * Remove the deprecated `CopilotSession.destroy()` method. Only `disconnect()` remains (matches TS PR #1357 4.1). * Rename `NO_RESULT_PERMISSION_V2_ERROR` → `_NO_RESULT_PERMISSION_V2_ERROR` and `MIN_PROTOCOL_VERSION` → `_MIN_PROTOCOL_VERSION` (module-private constants; Python equivalent of TS `@internal` + `stripInternal`). Public API audit vs C# / TypeScript: * `copilot/__init__.py` now re-exports the full set of types that match the TS public surface from `nodejs/src/index.ts`. Notable additions: - Lifecycle event hierarchy: `SessionCreatedEvent`, `SessionDeletedEvent`, `SessionUpdatedEvent`, `SessionForegroundEvent`, `SessionBackgroundEvent`, `SessionLifecycleEvent`, `SessionLifecycleEventBase`, `SessionLifecycleEventMetadata`, `SessionLifecycleEventType`, `SessionLifecycleHandler` - Session metadata / context: `SessionContext`, `SessionListFilter`, `SessionMetadata` - Generated `PermissionRequest`, `SessionEvent`, `SessionEventType` ( consumers shouldn't have to reach into `copilot.generated.*`) - `PermissionHandler`, `PermissionRequestResult`, `PermissionNoResult`, `UserInputHandler`, `UserInputRequest`, `UserInputResponse` - MCP server configs: `MCPHTTPServerConfig`, `MCPServerConfig`, `MCPStdioServerConfig` - Session config: `SessionConfig`, `ResumeSessionConfig`, `SessionHooks`, `SystemMessageConfig`, `InfiniteSessionConfig` - All hook handler / input / output TypedDicts: `PreToolUseHandler` & friends across the six hook lifecycle points - Model* completion (added `ModelLimits`, `ModelSupports`, `ModelVisionLimits`, `ModelBilling`, `ModelPolicy`, `ModelCapabilities`) - `ConnectionState`, `LogLevel`, `PingResponse`, `GetStatusResponse`, `GetAuthStatusResponse`, `StopError` - `ToolResultType`, `SessionEventHandler` `__all__` is now alphabetically sorted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 phase B + api_review_python.md item B.9 (TypeScript SDK API review). Public Python dataclasses no longer expose camelCase attribute names; the wire JSON keys are unchanged. Renames (Python attribute → wire JSON key): * `PingResponse.protocolVersion` → `protocol_version` (wire: `protocolVersion`) * `SessionContext.gitRoot` → `git_root` (wire: `gitRoot`) * `SessionListFilter.gitRoot` → `git_root` (wire: `gitRoot`) * `SessionMetadata.sessionId` → `session_id` (wire: `sessionId`) * `SessionMetadata.startTime` → `start_time` (wire: `startTime`) * `SessionMetadata.modifiedTime` → `modified_time` (wire: `modifiedTime`) * `SessionMetadata.isRemote` → `is_remote` (wire: `isRemote`) * `SessionLifecycleEventMetadata.startTime` → `start_time` * `SessionLifecycleEventMetadata.modifiedTime` → `modified_time` * `SessionLifecycleEventBase.sessionId` → `session_id` Hook input TypedDicts (`PreToolUseHookInput`, `PreMcpToolCallHookInput`, etc.) intentionally keep camelCase keys — those are wire-format dicts delivered straight to handlers, not Python attributes. Tests + docs updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 Phase G (TypeScript SDK API review) and addresses api_review_python.md item B.5. Hoists the ~30 fields shared between `SessionConfig` and `ResumeSessionConfig` into a new `SessionConfigBase(TypedDict, total=False)`. `SessionConfig` now only adds `session_id` (create-only) and `ResumeSessionConfig` only adds `disable_resume` + `continue_pending_work` (resume-only). Both inherit `SessionConfigBase`. `SessionConfigBase` is re-exported from `copilot.__init__` to match the TS public API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
added a commit
that referenced
this pull request
May 22, 2026
Mirrors PR #1357 Phase K (TypeScript SDK API review): every Python code snippet under \docs/\ and the \python/README.md\ now uses the post-Phase-B/F/G/H public API: * `CopilotClient()` / `CopilotClient(CopilotClientOptions(connection=RuntimeConnection.uri/stdio/tcp(...), ...))` in place of `SubprocessConfig` / `ExternalServerConfig` * `PermissionDecisionApproveOnce()`, `PermissionDecisionReject(feedback=...)`, `PermissionDecisionUserNotAvailable()`, `PermissionNoResult()` in place of `PermissionRequestResult(kind=...)` * `base_directory` (was `copilot_home`) * `enable_remote_sessions` (was `remote`) * `on_exit_plan_mode_request` / `on_auto_mode_switch_request` * `session.get_events()` (was `get_messages`) * `client.on_lifecycle(...)` (was `client.on`) * `max_prompt_tokens` (was `max_input_tokens`) * snake_case dataclass attribute accesses (\session_id\, \start_time\, \modified_time\, \is_remote\, \git_root\, \protocol_version\) TypeScript / Go / C# / Rust / Java snippets in the same files are untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Applies the equivalent of the C# SDK API review fixes (#1343) to the TypeScript SDK, plus the TS-specific items from
api_review_typescript.md.Will be pushed phase-by-phase. Tracking plan:
plan.md(session-local).Phases
fsnames)DatePermissionRequestResult.feedback+ use generated discriminatedPermissionRequestSessionConfigBasedefineTool({ name, ... })single-arg formRuntimeConnectiondiscriminated config +copilotHome->baseDirectorysend/sendAndWaitstring overloadsstripInternal,AsyncDisposable, cleanclient.stop()exit, docs/samples rewritetsc --noEmitfor tests in CIGenerated files (
nodejs/src/generated/**) are untouched. No generator changes required.Notes for cross-language parity reviewer
This is part of a refactoring series. We've already done C#, and this PR handles TypeScript. So, do NOT comment on inconsistencies with Python/Go/Rust at this point. You should ONLY assess consistency with C#, since that's the only other SDK that's been updated to match our API review decisions so far.