Commit 8272a2a
TypeScript SDK API review fixes (#1357)
* Phase A: property/method renames on SessionConfig/ResumeSessionConfig
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>
* Phase C: CopilotClientOptions / MCP / streaming shape changes
- 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>
* Phase D: lifecycle event polymorphic union + Date timestamps
- 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>
* Phase E: hook input timestamps as Date
- 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>
* Phase F: PermissionRequestResult.feedback + use generated PermissionRequest 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>
* Phase G: extract SessionConfigBase
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>
* Phase H: defineTool({ name, ... }) single-arg form
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>
* Revert "Phase H: defineTool({ name, ... }) single-arg form"
This reverts commit 49da911.
* Phase I: RuntimeConnection discriminated config
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>
* Phase J: send / sendAndWait string overloads
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>
* Phase K: stripInternal, AsyncDisposable, clean stop(), drop destroy()
- 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>
* Phase L: fix githubToken typos in scenario fixtures
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>
* Phase L follow-up: run prettier --write over all modified files
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>
* Fix Phase I/E/L test failures surfaced by CI
- 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>
* Add E2E equivalents of removed createSession/resumeSession-without-permission 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>
* Add E2E tests for createSession/resumeSession without onPermissionRequest
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>
* Harness: preserve caller-supplied RuntimeConnection while still injecting 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>
* session_fs.e2e: fix unconverted tcpConnectionToken flat key on inner 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>
* Move hook input deserialization next to the cast that types it
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>
* Phase K refinement: use unref() instead of destroy() in client.stop()
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>
* client.stop(): await child exit and socket close
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>
* Revert incorrect feedback widening of PermissionRequestResult
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>
* Rename client.on -> client.onLifecycle for cross-SDK consistency
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>
* Reformat src/types.ts after PermissionRequestResult revert
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review comments from #1357
- 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>
* logLevel: don't impose any default, match C#/Go
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>
* Rename CopilotClientOptions.remote -> enableRemoteSessions for cross-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>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>1 parent 3c4ebf6 commit 8272a2a
70 files changed
Lines changed: 969 additions & 752 deletions
File tree
- docs/troubleshooting
- nodejs
- examples
- src
- test
- e2e
- harness
- test
- scenarios
- auth
- byok-anthropic/typescript/src
- byok-azure/typescript/src
- byok-ollama/typescript/src
- byok-openai/typescript/src
- gh-app/typescript/src
- bundling/fully-bundled/typescript/src
- callbacks
- hooks/typescript/src
- permissions/typescript/src
- user-input/typescript/src
- modes
- default/typescript/src
- minimal/typescript/src
- prompts
- attachments/typescript/src
- reasoning-effort/typescript/src
- system-message/typescript/src
- sessions
- concurrent-sessions/typescript/src
- infinite-sessions/typescript/src
- session-resume/typescript/src
- streaming/typescript/src
- tools
- custom-agents/typescript/src
- mcp-servers/typescript/src
- no-tools/typescript/src
- skills/typescript/src
- tool-filtering/typescript/src
- tool-overrides/typescript/src
- virtual-filesystem/typescript/src
- transport
- reconnect/typescript/src
- stdio/typescript/src
- tcp/typescript/src
- snapshots
- hooks_extended
- multi_client
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
178 | 178 | | |
179 | 179 | | |
180 | 180 | | |
181 | | - | |
| 181 | + | |
182 | 182 | | |
183 | 183 | | |
184 | 184 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
89 | 89 | | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
94 | 93 | | |
95 | 94 | | |
96 | 95 | | |
| |||
173 | 172 | | |
174 | 173 | | |
175 | 174 | | |
176 | | - | |
| 175 | + | |
177 | 176 | | |
178 | 177 | | |
179 | 178 | | |
| |||
183 | 182 | | |
184 | 183 | | |
185 | 184 | | |
186 | | - | |
| 185 | + | |
187 | 186 | | |
188 | 187 | | |
189 | 188 | | |
| |||
277 | 276 | | |
278 | 277 | | |
279 | 278 | | |
280 | | - | |
| 279 | + | |
281 | 280 | | |
282 | 281 | | |
283 | 282 | | |
| |||
415 | 414 | | |
416 | 415 | | |
417 | 416 | | |
418 | | - | |
| 417 | + | |
419 | 418 | | |
420 | 419 | | |
421 | 420 | | |
| |||
856 | 855 | | |
857 | 856 | | |
858 | 857 | | |
859 | | - | |
860 | | - | |
861 | | - | |
862 | | - | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
863 | 862 | | |
864 | 863 | | |
865 | | - | |
866 | | - | |
867 | | - | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
868 | 867 | | |
869 | 868 | | |
870 | 869 | | |
| |||
1026 | 1025 | | |
1027 | 1026 | | |
1028 | 1027 | | |
1029 | | - | |
| 1028 | + | |
1030 | 1029 | | |
1031 | 1030 | | |
1032 | 1031 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
24 | | - | |
25 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
28 | | - | |
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
32 | 33 | | |
33 | | - | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
39 | 38 | | |
40 | | - | |
| 39 | + | |
41 | 40 | | |
42 | 41 | | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | 42 | | |
0 commit comments