Cross-SDK cleanup follow-ups from PR #1376#1378
Merged
Merged
Conversation
Every scenario explicitly threaded GITHUB_TOKEN from the parent env into
the SDK client options:
- Python: github_token=os.environ.get("GITHUB_TOKEN")
- TypeScript: gitHubToken: process.env.GITHUB_TOKEN
- .NET: GitHubToken = Environment.GetEnvironmentVariable("GITHUB_TOKEN")
- Go: GitHubToken: os.Getenv("GITHUB_TOKEN")
- Rust: opts.github_token = std::env::var("GITHUB_TOKEN").ok()
This is pure boilerplate noise: the runtime CLI already reads GITHUB_TOKEN
from its inherited environment when no token is provided to the SDK, so
the explicit pass-through is redundant and nothing a real consumer would
write. Drop the line in every scenario across all 5 languages.
Exception: `auth/gh-app` keeps explicit github_token plumbing because the
whole point of that scenario is to demonstrate passing an OAuth-derived
token obtained at runtime.
Also clean up the resulting unused imports:
- Python: `import os` removed where it was only used for GITHUB_TOKEN
(ruff autofixed).
- Go: `"os"` import removed where it was only used for GITHUB_TOKEN
(gofmt happy).
- Rust: `let mut opts = ClientOptions::default(); ...; Client::start(opts)`
collapsed to `Client::start(ClientOptions::default())` where opts is
no longer mutated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
.NET removed `CopilotClient.State` and `ConnectionState` in commit b00fd8c (Phase 4d/4e). Python matched this in PR #1376. TypeScript was the last SDK still exposing both — drop them for cross-SDK consistency. - `CopilotClient.getState(): ConnectionState` removed. - Public `ConnectionState` type alias removed from `types.ts` and the index re-exports. - The private `state` field on `CopilotClient` stays (still used by start/stop to gate behavior); its type is now an inline union declaration instead of the exported alias. - Unit test for unexpected child-process death now reads the private `state` field via `(client as any).state` since the behaviour being tested (state transition on async disconnect) is genuinely internal. - E2E `client.getState() === "..."` assertions are dropped — they were pure tautologies once `start()` returned without throwing. - `nodejs/README.md` no longer documents `getState()`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Most scenarios explicitly set up a stdio connection from COPILOT_CLI_PATH:
- .NET: Connection = RuntimeConnection.ForStdio(path: Environment.GetEnvironmentVariable("COPILOT_CLI_PATH"))
- TS: connection: RuntimeConnection.forStdio({ path: process.env.COPILOT_CLI_PATH })
This is redundant boilerplate:
- The default `RuntimeConnection` is already stdio across every SDK.
- When no `path` is given, the SDKs already fall back to the
`COPILOT_CLI_PATH` env var (and then the bundled binary), so the
explicit pass-through adds nothing.
- No real consumer of the SDK would write this — the docs reasonably
expect `new CopilotClient()` (or the language-equivalent) to Just Work.
Drop the connection setup entirely in every scenario *except* those whose
whole purpose is to demonstrate transport / bundling configuration:
- `transport/stdio` — explicit stdio path is the point.
- `transport/tcp` / `transport/reconnect` — TCP transport is the point.
- `bundling/*` — bundled-binary lookup is the point.
Also clean up the resulting cruft:
- Empty `new CopilotClientOptions {}` / `new CopilotClient({})` literals
collapsed to parameter-less constructors.
- Unused `RuntimeConnection` imports stripped from TS scenarios.
- Go scenarios that previously passed `&copilot.ClientOptions{}` now
pass `nil` (idiomatic when no fields are set).
- The `cliPath` local var in `tools/custom-agents` (.NET) deleted.
Rust scenarios keep `ClientOptions::default()` because the API requires
the argument — that's idiomatic Rust, not redundant boilerplate.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. docs/getting-started.md: Python snippet still used the removed `CopilotClientOptions` wrapper. Switch to the flat-kwargs form (`CopilotClient(connection=RuntimeConnection.for_uri(...))`). 2. python/README.md custom-permission-handler example imported per-kind variants (`PermissionRequestShell`, `PermissionDecisionApproveOnce`, `PermissionDecisionReject`) from `copilot` — but only `PermissionRequest` and `PermissionRequestResult` are re-exported at the top level. Fix the example to import the variant classes from `copilot.generated.session_events`. 3. python/copilot/client.py `RuntimeConnection` docstring referenced the old factory names `stdio`/`tcp`/`uri`. Update to `for_stdio`/`for_tcp`/`for_uri` to match the renamed methods. 4. python/copilot/session.py comment above `PermissionRequestResult` told users to construct variants with `kind=...` arguments — but Phase L baked the discriminator into a `ClassVar` default, so the generated variants reject `kind=` at the call site. Update the comment to reflect the new ergonomics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…(.NET, Go) Python (#1376) drove out its own hand-written ``PermissionRequestResult`` wrapper in favour of the generated discriminated ``PermissionDecision`` union plus a small ``PermissionNoResult`` sentinel. This commit lands the same refactor for the .NET and Go SDKs. **.NET** The old ``PermissionRequestResult`` struct (just ``Kind`` + optional ``Feedback``) collapsed every decision variant into a flat string-tagged DTO, losing the rich per-variant payloads — ``Feedback`` on rejection, per-kind ``Approval`` lists on ``ApproveForSession`` / ``ApproveForLocation``, ``Domain`` on ``ApprovePermanently``, etc. The generated ``PermissionDecision`` (``Rpc.cs:4760``) is already a proper polymorphic hierarchy with ``[JsonDerivedType]`` wired up for every variant. Switch ``OnPermissionRequest`` to return ``Task<PermissionDecision>`` and route the variant straight onto the wire — StreamJsonRpc handles the discriminator via the existing attributes. Additions: - ``PermissionDecisionNoResult`` — hand-written subclass of ``PermissionDecision`` used when the handler declines to respond so another connected client can answer. The SDK suppresses the wire response when it sees this variant. - Static factories on ``PermissionDecision`` for discoverability: ``ApproveOnce()``, ``Reject(feedback)``, ``UserNotAvailable()``, ``NoResult()``. For richer decisions that need an ``Approval`` payload, instantiate the variant class directly. Deletions: - ``PermissionRequestResult`` class (``Types.cs``) - ``PermissionRequestResultKind`` struct + obsolete enum-like wrappers - ``PermissionRequestResultKindTests.cs`` - ``PermissionRequestResult`` JSON serialization metadata **Go** Same shape: drop ``PermissionRequestResult`` + ``PermissionRequestResultKind`` in favour of ``rpc.PermissionDecision`` (already a sealed interface implemented by every variant). Added ``rpc.PermissionDecisionNoResult`` as a hand-written variant that satisfies the unexported ``permissionDecision()`` method — kept inside the ``rpc`` package since the sealing method is unexported. Handler signature changes from ``(PermissionRequestResult, error)`` to ``(rpc.PermissionDecision, error)``. ``PermissionHandler.ApproveAll`` now returns ``&rpc.PermissionDecisionApproveOnce{}``. Removed the ``rpcPermissionDecisionFromKind`` helper used to convert the flat kind back to a variant — no longer needed when the handler already returns the variant directly. **Tests / scenarios** All E2E tests and scenarios across .NET and Go updated to construct ``rpc.PermissionDecision*`` variants directly. The Go interface return type means explicit ``Task.FromResult<PermissionDecision>(...)`` casts are needed in C# where lambdas previously inferred the wrapper type. **Doc style** Per repo convention, public docs do not reference protocol v1/v2 or internal transport details. The README copy for each SDK describes the behavioural semantics ("decline to respond so another client can answer") rather than the wire mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Same shape as the .NET and Go refactors in the previous commit. The hand-written ``PermissionResult`` enum lost almost everything the generated ``PermissionDecision`` discriminated union exposes — feedback strings on rejection, per-kind approval lists on ``ApproveForSession``/``ApproveForLocation``, domains on ``ApprovePermanently``, the rich denial reasons (``DeniedByRules``, ``DeniedByContentExclusionPolicy``, …). It also carried two SDK-only escape hatches (``Deferred``, ``Custom(serde_json::Value)``) that other SDKs don't expose and that consumers never actually needed. This commit: - Replaces ``PermissionResult`` with a thin two-variant wrapper: ``PermissionResult::Decision(PermissionDecision)`` and ``PermissionResult::NoResult``. The ``Decision`` variant carries any wire-level decision; ``NoResult`` tells the SDK to suppress the response so another connected client can answer. - Adds discoverable factory methods on ``PermissionResult``: ``approve_once()``, ``reject(feedback)``, ``user_not_available()``, ``no_result()``. For richer decisions (per-session, per-location, permanent) consumers construct the ``PermissionDecision`` variant directly and wrap it in ``PermissionResult::Decision`` (the ``From<PermissionDecision>`` impl also covers this). - Drops ``PermissionResult::Approved`` / ``Denied`` / ``UserNotAvailable`` / ``Deferred`` / ``Custom`` and the ``permission_request_response`` / ``pending_permission_result_kind`` helpers — the new wrapper serializes its inner ``PermissionDecision`` directly via serde. Updates ``ApproveAllHandler`` / ``DenyAllHandler`` / ``PolicyHandler``, all session dispatch logic, internal unit tests, the E2E test suite, scenarios, and the README to construct decisions via the factories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI flagged formatting issues introduced by the previous .NET/Go/Rust PermissionDecision refactor commits: - Three Go test files had `rpc` imported above `testharness` — gofmt prefers alphabetical-within-group order. - nodejs/test/e2e/client_options.e2e.test.ts had a prettier wrap miss from the earlier scenarios cleanup commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Bumps the minimum supported wire protocol version from 2 to 3 across
TypeScript, Python, .NET, Go, and Rust (Java intentionally left at 2 —
out of scope for this PR).
Before this change every SDK registered v2-only RPC handlers for
``tool.call`` and ``permission.request`` alongside the v3
broadcast-event model. The handlers were dead code on a v3 server (the
server only ever sends broadcast events) but were kept "just in case"
the SDK ever connected to a v2 server.
Now that ``MIN_PROTOCOL_VERSION = 3``, a v2 server is rejected at the
``connect`` handshake before any session-level RPC can fire, so the v2
adapters can be deleted outright.
Deletions per SDK:
- **.NET** (``Client.cs``):
- ``OnToolCallV2`` and ``OnPermissionRequestV2`` RPC handler methods
- ``ToolCallResponseV2`` and ``PermissionRequestResponseV2`` records
- Their ``[JsonSerializable]`` entries
- ``NoResultPermissionDirectRpcErrorMessage`` constant (no longer
reachable)
- **TypeScript** (``client.ts``, ``session.ts``):
- ``handleToolCallRequestV2`` / ``handlePermissionRequestV2`` private
methods on ``CopilotClient``
- ``normalizeToolResultV2`` / ``isToolResultObject`` helpers
- ``CopilotSession._handlePermissionRequestV2``
- ``NO_RESULT_PERMISSION_V2_ERROR`` constant
- Unused ``ToolCallRequestPayload`` / ``ToolCallResponsePayload`` /
``ToolResultObject`` imports in ``client.ts``
- **Python** (``client.py``):
- ``_handle_tool_call_request_v2`` and ``_handle_permission_request_v2``
methods (both create_session and resume_session paths)
- The whole "Protocol v2 backward-compatibility adapters" section
- ``_NO_RESULT_PERMISSION_V2_ERROR`` constant
- ``test_v2_permission_adapter_rejects_no_result`` unit test
- **Go** (``client.go``):
- ``handleToolCallRequestV2`` and ``handlePermissionRequestV2`` methods
- ``toolCallRequestV2`` / ``toolCallResponseV2`` /
``permissionRequestV2`` / ``permissionResponseV2`` payload types
- ``noResultPermissionDirectRpcError`` constant
- **Rust** (``session.rs``, ``lib.rs``):
- ``permission.request`` direct-RPC match arm in the session router
- ``direct_permission_payload`` helper + its unit tests
What's preserved:
- ``MIN_PROTOCOL_VERSION`` constant in each SDK (now ``= 3``)
- The handshake check that rejects servers reporting older versions
- ``negotiated_protocol_version`` field on the client (no longer
branched on anywhere, but harmless and may grow back if we add a v4
later)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- `nodejs/src/session.ts` had an unused `PermissionRequestResult` import after the v2 drop. eslint flagged it (`no-unused-vars`). - `nodejs/test/client.test.ts` had a leftover test for the deleted `handlePermissionRequestV2` v2 adapter. Delete the test. - `rust/tests/session_test.rs` had two tests that drove the deleted `permission.request` direct-RPC arm (`permission_request_dispatches_to_handler`, `approve_all_handler_approves_permission`). Delete the first one and rewrite the second to exercise the broadcast-event path (`permission.requested` event + `handlePendingPermissionRequest` RPC response), which is the only remaining permission flow. - Drop unused `PermissionHandler` / `PermissionResult` imports that the deleted tests left behind. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI's clippy treats unused-imports as a denied warning. The mock handler for the v2 `permission_request_dispatches_to_handler` test was the only consumer of `PermissionRequestData`; remove the import now that the test is gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Pull request overview
Cross-SDK follow-up cleanup aligning TypeScript, .NET, Go, Rust, and Python with the post-#1376 permission API (return PermissionDecision directly), dropping protocol v2 support (min protocol now 3), and simplifying scenario fixtures (removing redundant GITHUB_TOKEN / stdio connection boilerplate). Also trims the Node public API by removing getState() / ConnectionState.
Changes:
- Removed protocol v2 compatibility layers across SDKs and bumped minimum protocol version to 3.
- Replaced
PermissionRequestResultwrappers withPermissionDecision(plus SDK-only “no-result” variants where needed) across Go/.NET/Rust and updated docs/tests/scenarios. - Simplified test scenarios and docs-validation plumbing (e.g., removed
GITHUB_TOKENboilerplate; updated C# snippet validation to avoid ambiguous usings).
Show a summary per file
| File | Description |
|---|---|
| test/scenarios/transport/tcp/typescript/src/index.ts | Scenario formatting + explicit forUri args formatting. |
| test/scenarios/transport/tcp/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/transport/tcp/go/main.go | Scenario: gofmt cleanup for response printing. |
| test/scenarios/transport/stdio/typescript/src/index.ts | Scenario: remove token boilerplate; format stdio connection args. |
| test/scenarios/transport/stdio/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/transport/stdio/python/main.py | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/transport/stdio/go/main.go | Scenario: remove GITHUB_TOKEN plumbing; gofmt cleanup. |
| test/scenarios/transport/stdio/csharp/Program.cs | Scenario: remove GitHubToken boilerplate. |
| test/scenarios/transport/reconnect/typescript/src/index.ts | Scenario: formatting tweaks for URI + console output. |
| test/scenarios/transport/reconnect/go/main.go | Scenario: gofmt cleanup for response printing. |
| test/scenarios/tools/virtual-filesystem/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/tools/virtual-filesystem/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/virtual-filesystem/go/main.go | Scenario: switch permission callback to rpc.PermissionDecision; remove token boilerplate. |
| test/scenarios/tools/virtual-filesystem/csharp/Program.cs | Scenario: switch permission callback to PermissionDecision; remove client options boilerplate. |
| test/scenarios/tools/tool-overrides/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/tools/tool-overrides/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/tool-overrides/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/tool-overrides/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/tools/tool-overrides/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/tools/tool-filtering/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/tools/tool-filtering/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/tool-filtering/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/tool-filtering/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/tools/tool-filtering/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/tools/skills/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/tools/skills/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/skills/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/skills/go/main.go | Scenario: switch permission callback to rpc.PermissionDecision; remove token boilerplate. |
| test/scenarios/tools/skills/csharp/Program.cs | Scenario: switch permission callback to PermissionDecision; remove client options boilerplate. |
| test/scenarios/tools/no-tools/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/tools/no-tools/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/no-tools/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/no-tools/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/tools/no-tools/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/tools/mcp-servers/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting for MCP args/logging. |
| test/scenarios/tools/mcp-servers/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/mcp-servers/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/mcp-servers/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/tools/mcp-servers/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/tools/custom-agents/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; reformat tool + agent configs. |
| test/scenarios/tools/custom-agents/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/tools/custom-agents/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/tools/custom-agents/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/tools/custom-agents/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/sessions/streaming/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/sessions/streaming/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/sessions/streaming/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/sessions/streaming/go/main.go | Scenario: remove token boilerplate. |
| test/scenarios/sessions/streaming/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/sessions/session-resume/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/sessions/session-resume/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/sessions/session-resume/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/sessions/session-resume/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/sessions/session-resume/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/sessions/multi-user-short-lived/typescript/src/index.ts | Scenario: formatting for skip message. |
| test/scenarios/sessions/multi-user-long-lived/typescript/src/index.ts | Scenario: formatting for skip message. |
| test/scenarios/sessions/infinite-sessions/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; normalize numeric literal; formatting. |
| test/scenarios/sessions/infinite-sessions/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/sessions/infinite-sessions/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/sessions/infinite-sessions/go/main.go | Scenario: remove token boilerplate; minor formatting. |
| test/scenarios/sessions/infinite-sessions/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/sessions/concurrent-sessions/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/sessions/concurrent-sessions/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/sessions/concurrent-sessions/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/sessions/concurrent-sessions/go/main.go | Scenario: remove token boilerplate. |
| test/scenarios/sessions/concurrent-sessions/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/prompts/system-message/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/prompts/system-message/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/prompts/system-message/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/prompts/system-message/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/prompts/system-message/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/prompts/reasoning-effort/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/prompts/reasoning-effort/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/prompts/reasoning-effort/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/prompts/reasoning-effort/go/main.go | Scenario: remove token boilerplate. |
| test/scenarios/prompts/reasoning-effort/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/prompts/attachments/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/prompts/attachments/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/prompts/attachments/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/prompts/attachments/go/main.go | Scenario: remove token boilerplate. |
| test/scenarios/prompts/attachments/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/modes/minimal/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/modes/minimal/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/modes/minimal/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/modes/minimal/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/modes/default/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; prompt formatting. |
| test/scenarios/modes/default/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/modes/default/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/modes/default/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/modes/default/csharp/Program.cs | Scenario: remove client options boilerplate. |
| test/scenarios/Directory.Build.props | Scenarios: suppress GHCP001 warning for snippet builds. |
| test/scenarios/callbacks/user-input/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/callbacks/user-input/rust/src/main.rs | Scenario: update permission result helper + remove GITHUB_TOKEN. |
| test/scenarios/callbacks/user-input/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/callbacks/user-input/go/main.go | Scenario: switch permission callback to rpc.PermissionDecision; remove token boilerplate. |
| test/scenarios/callbacks/user-input/csharp/Program.cs | Scenario: switch permission callback to PermissionDecision; remove client options boilerplate. |
| test/scenarios/callbacks/permissions/typescript/src/index.ts | Scenario: remove stdio/token boilerplate. |
| test/scenarios/callbacks/permissions/rust/src/main.rs | Scenario: update permission result helper + remove GITHUB_TOKEN. |
| test/scenarios/callbacks/permissions/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/callbacks/permissions/go/main.go | Scenario: switch permission callback to rpc.PermissionDecision; remove token boilerplate. |
| test/scenarios/callbacks/permissions/csharp/Program.cs | Scenario: switch permission callback to PermissionDecision; remove client options boilerplate. |
| test/scenarios/callbacks/hooks/typescript/src/index.ts | Scenario: remove stdio/token boilerplate; formatting. |
| test/scenarios/callbacks/hooks/rust/src/main.rs | Scenario: remove GITHUB_TOKEN plumbing. |
| test/scenarios/callbacks/hooks/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/callbacks/hooks/go/main.go | Scenario: switch permission callback to rpc.PermissionDecision; remove token boilerplate. |
| test/scenarios/callbacks/hooks/csharp/Program.cs | Scenario: switch permission callback to PermissionDecision; remove client options boilerplate. |
| test/scenarios/bundling/fully-bundled/typescript/src/index.ts | Scenario: remove token boilerplate; format stdio args. |
| test/scenarios/bundling/fully-bundled/python/main.py | Scenario: remove GITHUB_TOKEN boilerplate. |
| test/scenarios/bundling/fully-bundled/go/main.go | Scenario: remove token boilerplate; gofmt cleanup. |
| test/scenarios/bundling/fully-bundled/csharp/Program.cs | Scenario: remove GitHubToken boilerplate. |
| test/scenarios/bundling/container-proxy/go/main.go | Scenario: gofmt cleanup for response printing. |
| test/scenarios/bundling/app-direct-server/go/main.go | Scenario: gofmt cleanup for response printing. |
| test/scenarios/bundling/app-backend-to-server/typescript/src/index.ts | Scenario: formatting for CLI URL selection. |
| test/scenarios/auth/gh-app/typescript/src/index.ts | Scenario: formatting; remove explicit stdio connection when not essential. |
| test/scenarios/auth/gh-app/go/main.go | Scenario: gofmt cleanup for response printing. |
| test/scenarios/auth/gh-app/csharp/Program.cs | Scenario: remove explicit stdio connection when not essential. |
| test/scenarios/auth/byok-openai/typescript/src/index.ts | Scenario: remove explicit stdio connection; format constants. |
| test/scenarios/auth/byok-openai/go/main.go | Scenario: use default client opts; gofmt cleanup. |
| test/scenarios/auth/byok-openai/csharp/Program.cs | Scenario: remove explicit stdio connection. |
| test/scenarios/auth/byok-ollama/typescript/src/index.ts | Scenario: remove explicit stdio connection; format constants. |
| test/scenarios/auth/byok-ollama/go/main.go | Scenario: use default client opts; gofmt cleanup. |
| test/scenarios/auth/byok-ollama/csharp/Program.cs | Scenario: remove explicit stdio connection. |
| test/scenarios/auth/byok-azure/typescript/src/index.ts | Scenario: remove explicit stdio connection. |
| test/scenarios/auth/byok-azure/go/main.go | Scenario: use default client opts; gofmt cleanup. |
| test/scenarios/auth/byok-azure/csharp/Program.cs | Scenario: remove explicit stdio connection. |
| test/scenarios/auth/byok-anthropic/typescript/src/index.ts | Scenario: remove explicit stdio connection. |
| test/scenarios/auth/byok-anthropic/go/main.go | Scenario: use default client opts; gofmt cleanup. |
| test/scenarios/auth/byok-anthropic/csharp/Program.cs | Scenario: remove explicit stdio connection. |
| scripts/docs-validation/validate.ts | C# snippet validation: suppress GHCP001 warning. |
| scripts/docs-validation/extract.ts | C# snippet extraction: smarter Copilot/Rpc using injection to avoid ambiguities. |
| rust/tests/session_test.rs | Rust tests: update permission handling tests for v3 broadcast model. |
| rust/tests/e2e/tools.rs | Rust E2E: update permission result construction helpers. |
| rust/tests/e2e/permissions.rs | Rust E2E: update permission result construction + assertions for wrapper type. |
| rust/tests/e2e/multi_client.rs | Rust E2E: update permission result construction for multi-client tests. |
| rust/tests/e2e/multi_client_commands_elicitation.rs | Rust E2E: update permission result construction helper usage. |
| rust/tests/e2e/elicitation.rs | Rust E2E: update permission result construction helper usage. |
| rust/tests/e2e/ask_user.rs | Rust E2E: update permission result construction helper usage. |
| rust/src/types.rs | Rust public types: re-export generated PermissionDecision variants; update permission policy tests. |
| rust/src/session.rs | Rust session: remove v2 direct-RPC permission path; serialize PermissionDecision directly for pending permission handling. |
| rust/src/permission.rs | Rust permission policy handler: return PermissionResult wrapper helpers. |
| rust/src/lib.rs | Rust SDK: bump MIN_PROTOCOL_VERSION to 3. |
| rust/src/handler.rs | Rust permission API: redefine PermissionResult as PermissionDecision wrapper + add factories (approve_once, reject, etc.). |
| rust/README.md | Rust docs: update permission handler examples to new factories. |
| python/test_client.py | Python tests: remove protocol v2 adapter test; update imports. |
| python/README.md | Python docs: update permission handler section and imports (needs a small fix). |
| python/copilot/session.py | Python session types: clarify decision construction (no explicit kind argument). |
| python/copilot/client.py | Python client: bump min protocol to 3; remove v2 request handler adapters. |
| nodejs/test/e2e/client.e2e.test.ts | Node E2E: remove assertions relying on getState(). |
| nodejs/test/e2e/client_options.e2e.test.ts | Node E2E: remove getState() assertions. |
| nodejs/test/client.test.ts | Node unit tests: remove v2 no-result test; adjust state assertions to internal field. |
| nodejs/src/types.ts | Node public types: remove exported ConnectionState. |
| nodejs/src/session.ts | Node session: remove v2 permission request adapter + related error constant. |
| nodejs/src/index.ts | Node public surface: stop exporting ConnectionState. |
| nodejs/src/client.ts | Node client: bump min protocol to 3; remove v2 adapters; remove getState(); inline state type. |
| nodejs/README.md | Node docs: remove getState() documentation. |
| go/types.go | Go public API: remove PermissionRequestResult wrapper; permission handler now returns rpc.PermissionDecision. |
| go/types_test.go | Go unit tests: remove tests for deleted PermissionRequestResultKind/wrapper. |
| go/session.go | Go session: forward rpc.PermissionDecision directly; suppress response on NoResult. |
| go/session_test.go | Go unit tests: remove tests tied to deleted kind→decision mapping helper. |
| go/rpc/permission_decision_no_result.go | Go RPC: add SDK-only PermissionDecisionNoResult variant. |
| go/README.md | Go docs: update permission handler docs to new rpc.PermissionDecision variants. |
| go/permissions.go | Go helpers: update built-in ApproveAll to return PermissionDecisionApproveOnce. |
| go/internal/e2e/tools_e2e_test.go | Go E2E: update permission handlers to return rpc.PermissionDecision variants. |
| go/internal/e2e/suspend_e2e_test.go | Go E2E: update permission handler channel types to rpc.PermissionDecision. |
| go/internal/e2e/permissions_e2e_test.go | Go E2E: update permission handlers to new decision API. |
| go/internal/e2e/pending_work_resume_e2e_test.go | Go E2E: update pending-work resume permission plumbing to new decision API. |
| go/internal/e2e/multi_client_e2e_test.go | Go E2E: update multi-client permission tests to new decision API. |
| go/client.go | Go client: bump min protocol to 3; remove v2 tool/permission RPC adapters. |
| dotnet/test/Unit/SerializationTests.cs | .NET unit tests: serialize PermissionDecision via factories. |
| dotnet/test/Unit/PermissionRequestResultKindTests.cs | .NET unit tests: remove coverage for deleted PermissionRequestResultKind. |
| dotnet/test/E2E/ToolsE2ETests.cs | .NET E2E: update permission handlers to return PermissionDecision. |
| dotnet/test/E2E/SuspendE2ETests.cs | .NET E2E: update pending permission release types to PermissionDecision. |
| dotnet/test/E2E/PermissionE2ETests.cs | .NET E2E: migrate permission handler return types to PermissionDecision. |
| dotnet/test/E2E/PendingWorkResumeE2ETests.cs | .NET E2E: migrate pending permission plumbing to PermissionDecision. |
| dotnet/test/E2E/MultiClientE2ETests.cs | .NET E2E: migrate multi-client permission tests to PermissionDecision. |
| dotnet/src/Types.cs | .NET public API: remove PermissionRequestResult + kind types; update SessionConfig signature. |
| dotnet/src/Session.cs | .NET session: execute permission responses using PermissionDecision directly + support NoResult. |
| dotnet/src/PermissionHandlers.cs | .NET helpers: ApproveAll now returns PermissionDecision. |
| dotnet/src/PermissionDecision.cs | .NET: add SDK-only PermissionDecisionNoResult + static factories on PermissionDecision. |
| dotnet/src/Client.cs | .NET client: bump min protocol to 3; remove v2 RPC adapters. |
| dotnet/README.md | .NET docs: update permission handler docs to PermissionDecision factories and switch-based patterns. |
| docs/getting-started.md | Docs: update Python snippet to new client construction shape. |
| docs/features/streaming-events.md | Docs: update Go permission handler snippets to rpc.PermissionDecision. |
| docs/features/steering-and-queueing.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
| docs/features/skills.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
| docs/features/session-persistence.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
| docs/features/remote-sessions.md | Docs: update Go/.NET/Rust snippets for new permission decision APIs. |
| docs/features/image-input.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
| docs/features/hooks.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
| docs/features/custom-agents.md | Docs: update Go/.NET snippets for PermissionDecision usage + add Rpc using. |
Copilot's findings
- Files reviewed: 185/190 changed files
- Comments generated: 3
- go/session.go: guard against nil PermissionDecision from handler - go/README.md: fix non-compiling permission handler example - python/README.md: fix PermissionDecision* import path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency Review ✅The changes in this PR maintain cross-SDK consistency across the five targeted SDKs (TypeScript, Python, Go, .NET, Rust). The API transformations are parallel and well-aligned:
Known Gap: JavaAs explicitly noted in the PR description, Java is intentionally left out of this PR and retains the old API ( No additional consistency issues were found beyond the acknowledged Java gap.
|
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.
Cross-SDK API cleanup follow-ups to PR #1376 (Python SDK API review). Applies the equivalent changes across TypeScript, .NET, Go, Rust, and Python. Java is intentionally out of scope for this PR and will be aligned in a follow-up.
Highlights
PermissionRequestResultis gone in five SDKs. Handlers now return the generatedPermissionDecisionunion directly (with a small SDK-onlyNoResultvariant where needed). Discoverable static factories let you writePermissionDecision.ApproveOnce()/rpc.PermissionDecisionApproveOnce{}/PermissionResult::approve_once()etc. without thinking aboutkindstrings.MIN_PROTOCOL_VERSIONis now3and all v2-only RPC handlers, response envelopes, and error-string fallbacks are deleted. This removed ~1,100 lines of dead code.GITHUB_TOKENplumbing boilerplate and the redundantRuntimeConnection.ForStdio(COPILOT_CLI_PATH)plumbing from scenarios where it wasn't the point of the scenario.client.getState()and theConnectionStatetype are gone (matching what .NET and Python had already done).Breaking changes
These break user code. Migration is mechanical.
All affected SDKs:
PermissionRequestResult→PermissionDecision{ kind: "approve-once" }{ kind: "approve-once" }(no change — alias removed)new PermissionRequestResult { Kind = PermissionRequestResultKind.ApproveOnce }PermissionDecision.ApproveOnce()&rpc.PermissionRequestResult{Kind: rpc.PermissionRequestResultKindApproveOnce}&rpc.PermissionDecisionApproveOnce{}PermissionResult::ApproveOncePermissionResult::approve_once()(wrapsPermissionDecision)TypeScript
CopilotClient.getState()removed.ConnectionStatetype removed from the public surface.Protocol version
MIN_PROTOCOL_VERSIONbumped from2to3in all five SDKs. Clients connecting to a CLI that only speaks v2 will fail to handshake. The current public Copilot CLI speaks v3.Out of scope (Java)
Java is intentionally untouched in this PR. It still has the old
PermissionRequestResult/PermissionRequestResultKindAPI andMIN_PROTOCOL_VERSION = 2. A follow-up will align Java with the other five SDKs.Commits
f3321b23Dropgithub_tokenboilerplate from scenarios2a77f124TS: removegetState()/ConnectionStatef6a669d9Drop redundantRuntimeConnection/COPILOT_CLI_PATHfrom scenarios85f17805Address 4 post-merge review comments on Python SDK API review fixes #1376a95b69ae.NET + Go:PermissionRequestResult→PermissionDecisioneb591c9dRust:PermissionResultis now a wrapper aroundPermissionDecision8b51c986Format fixes (gofmt + prettier)13f71167Drop protocol v2 support (−667 net LOC)712eaeffLint cleanup + delete v2-direct-RPC testse7886edeDrop unused Rust importac10d3f1,e3a2ef58,0af04780Doc + scenario validation fixes for the new API