Open
Conversation
Adds the Copilot Rust SDK (`copilot-sdk` crate) under `rust/`, alongside Rust codegen plumbed into `scripts/codegen/` and CI under `.github/workflows/rust-sdk-tests.yml`. The crate ships a JSON-RPC client, session lifecycle management, system message transforms, permission policy helpers, the `define_tool` adapter, and per-event `SessionHandler`/`SessionHooks` traits. Includes: - 14 ported E2E scenarios under `rust/tests/` driving the replay-proxy harness, plus a hand-curated set of unit tests. - A rust-coding-skill (`.github/skills/rust-coding-skill/`) capturing conventions for error handling, async/concurrency, tracing, and the intentional trait exceptions in the SDK's public API. - Release tooling: `rust-publish-release.yml`, `RELEASING.md`, and protocol-version generation wired into the existing automation. - `PermissionResult` extended with `Deferred` and `Custom` variants for richer permission decisions. Public API is held at 0.1.0-pre. Marked protocol-evolving public enums `#[non_exhaustive]` so additive variants stay non-breaking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Christopher Schleiden <cschleiden@github.com> Co-authored-by: David Dossett <25163139+daviddossett@users.noreply.github.com> Co-authored-by: Devraj Mehta <devm33@github.com> Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> Co-authored-by: Evan Boyle <EvanBoyle@users.noreply.github.com> Co-authored-by: Jeremy Moseley <jemoseley@microsoft.com> Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
- **Broadcast subscriptions for lifecycle and session events.** `Client::subscribe_lifecycle()` and `Session::subscribe()` return `tokio::sync::broadcast::Receiver`; dropping the receiver unsubscribes. Replaces the prior callback-based `Client::on`, `Client::on_event_type`, `Session::on`, and `Unsubscribe` API. Spawned consumer tasks isolate panics naturally. - **`PermissionResult` gains `Deferred` and `Custom` variants.** `Deferred` lets handlers resolve a request asynchronously via `session.permissions.handlePendingPermissionRequest` (notification path only — falls back to `Approved` on the direct RPC path). `Custom(Value)` lets handlers send arbitrary response payloads beyond the standard `approve-once` / `reject` shapes. - **`#[non_exhaustive]` on protocol-evolving public enums** (`PermissionResult`, `SessionLifecycleEventType`, `GitHubReferenceType`, others) so additive variants stay non-breaking. - **`ToolHandlerRouter` overrides per-event `SessionHandler` methods** so consumers can call `router.on_external_tool(...)` directly without unwrapping `HandlerResponse`. - **`define_tool` accepts bare `async fn` items** in addition to closures, matching `tower::service_fn` / `hyper::service::service_fn` conventions. Documented in rustdoc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ypes Generated code emitted `pub session_id: String` for every schema field named `sessionId` and likewise for `requestId`, leaving consumers with mixed types: `Session::id()` returned `SessionId` but `session.events_subscribe()` events exposed `session_id: String`. Same papercut for request IDs in permission and elicitation event payloads. The newtypes are `#[serde(transparent)]` so the wire format is unchanged. This adds a property-name override map to `scripts/codegen/rust.ts` that maps `sessionId`, `remoteSessionId`, and `requestId` to the hand-authored types in `crate::types`, and emits the matching `use` statement in both generated modules. `mc_session_id` (MCP protocol metadata, not a Copilot session) stays as `String`. After regeneration: 27 fields converted to `SessionId` (including the handoff event's `remoteSessionId`) and 25 to `RequestId`. The existing `PartialEq<str>` / `PartialEq<String>` impls on both newtypes mean test code like Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
define_tool's Fn(P) -> Fut bound gave closures only the deserialized
arguments, leaving session_id, tool_call_id, and tool_name unreachable.
That blocked the helper for any tool that needs to scope DB lookups to
a session, emit per-tool-call telemetry, or stream UI updates back to
the originating session — patterns that hit dozens of sites across
realistic tool suites.
Change the closure bound to Fn(ToolInvocation, P) -> Fut. The arguments
are moved out via mem::take before deserialization, so there is no
clone cost on the hot path. Closures that don't need the metadata
write |_inv, params|.
Also add ToolInvocation::params<P>() so long-form impl ToolHandler
blocks can deserialize without naming serde_json directly:
async fn call(&self, inv: ToolInvocation) -> Result<ToolResult, Error> {
let params: MyParams = inv.params()?;
// …use inv.session_id alongside params…
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Node, Python, and .NET all expose ping with an optional message. Go requires it only because Go has no Option type — Rust has one, so the API should match the languages with the same expressive power rather than the one without. Change ping(&self, message: &str) to ping(&self, message: Option<&str>). When None, the message field is omitted from the request payload rather than sent as an empty string. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Rust SDK crate (copilot-sdk, library name copilot) to the monorepo, mirroring the existing SDKs’ JSON-RPC client/session model, and wires it into repo workflows and scenario coverage.
Changes:
- Introduces the Rust SDK crate with protocol types, JSON-RPC transport, session/handler abstractions, examples, and tests.
- Extends scenario samples and verification scripts to build/run Rust implementations alongside TS/Python/Go/C#.
- Updates repo automation: codegen,
justtasks, scenario-build CI, Rust SDK CI, and release/publish workflows.
Show a summary per file
| File | Description |
|---|---|
| test/scenarios/verify.sh | Shows Rust status in the aggregated scenario runner UI. |
| test/scenarios/transport/tcp/verify.sh | Builds/runs the Rust TCP transport scenario. |
| test/scenarios/transport/tcp/rust/Cargo.toml | Rust TCP scenario crate manifest. |
| test/scenarios/transport/tcp/rust/src/main.rs | Rust TCP sample using external host:port CLI server. |
| test/scenarios/transport/stdio/verify.sh | Builds/runs the Rust stdio transport scenario. |
| test/scenarios/transport/stdio/rust/Cargo.toml | Rust stdio scenario crate manifest. |
| test/scenarios/transport/stdio/rust/src/main.rs | Rust stdio sample spawning the CLI child process. |
| test/scenarios/transport/stdio/README.md | Documents Rust sample location/package name. |
| test/scenarios/tools/tool-overrides/verify.sh | Builds/runs Rust tool override scenario. |
| test/scenarios/tools/tool-overrides/rust/Cargo.toml | Rust tool-overrides scenario crate manifest (+derive). |
| test/scenarios/tools/tool-overrides/rust/src/main.rs | Rust sample overriding built-in tool behavior. |
| test/scenarios/tools/tool-filtering/verify.sh | Builds/runs Rust tool filtering scenario. |
| test/scenarios/tools/tool-filtering/rust/Cargo.toml | Rust tool-filtering scenario crate manifest. |
| test/scenarios/tools/tool-filtering/rust/src/main.rs | Rust sample limiting available tools. |
| test/scenarios/tools/skills/verify.sh | Builds/runs Rust skills scenario. |
| test/scenarios/tools/skills/rust/Cargo.toml | Rust skills scenario crate manifest. |
| test/scenarios/tools/skills/rust/src/main.rs | Rust sample configuring skill directories + hooks. |
| test/scenarios/tools/no-tools/verify.sh | Builds/runs Rust no-tools scenario. |
| test/scenarios/tools/no-tools/rust/Cargo.toml | Rust no-tools scenario crate manifest. |
| test/scenarios/tools/no-tools/rust/src/main.rs | Rust sample disabling tools + replacing system prompt. |
| test/scenarios/tools/mcp-servers/verify.sh | Builds/runs Rust MCP servers scenario. |
| test/scenarios/tools/mcp-servers/rust/Cargo.toml | Rust mcp-servers scenario crate manifest. |
| test/scenarios/tools/mcp-servers/rust/src/main.rs | Rust sample passing MCP servers config to CLI. |
| test/scenarios/tools/custom-agents/verify.sh | Builds/runs Rust custom agents scenario. |
| test/scenarios/tools/custom-agents/rust/Cargo.toml | Rust custom-agents scenario crate manifest (+derive). |
| test/scenarios/tools/custom-agents/rust/src/main.rs | Rust sample defining custom agents + custom tool. |
| test/scenarios/sessions/streaming/verify.sh | Builds/runs Rust streaming scenario. |
| test/scenarios/sessions/streaming/rust/Cargo.toml | Rust streaming scenario crate manifest. |
| test/scenarios/sessions/streaming/rust/src/main.rs | Rust sample counting streaming delta events. |
| test/scenarios/sessions/session-resume/verify.sh | Builds/runs Rust session resume scenario. |
| test/scenarios/sessions/session-resume/rust/Cargo.toml | Rust session-resume scenario crate manifest. |
| test/scenarios/sessions/session-resume/rust/src/main.rs | Rust sample creating + resuming session by ID. |
| test/scenarios/sessions/infinite-sessions/verify.sh | Builds/runs Rust infinite sessions scenario. |
| test/scenarios/sessions/infinite-sessions/rust/Cargo.toml | Rust infinite-sessions scenario crate manifest. |
| test/scenarios/sessions/infinite-sessions/rust/src/main.rs | Rust sample exercising infinite session thresholds. |
| test/scenarios/sessions/concurrent-sessions/verify.sh | Builds/runs Rust concurrent sessions scenario. |
| test/scenarios/sessions/concurrent-sessions/rust/Cargo.toml | Rust concurrent-sessions scenario crate manifest. |
| test/scenarios/sessions/concurrent-sessions/rust/src/main.rs | Rust sample running two sessions concurrently. |
| test/scenarios/prompts/system-message/verify.sh | Builds/runs Rust system-message scenario. |
| test/scenarios/prompts/system-message/rust/Cargo.toml | Rust system-message scenario crate manifest. |
| test/scenarios/prompts/system-message/rust/src/main.rs | Rust sample replacing system message. |
| test/scenarios/prompts/reasoning-effort/verify.sh | Builds/runs Rust reasoning-effort scenario. |
| test/scenarios/prompts/reasoning-effort/rust/Cargo.toml | Rust reasoning-effort scenario crate manifest. |
| test/scenarios/prompts/reasoning-effort/rust/src/main.rs | Rust sample setting reasoning_effort. |
| test/scenarios/modes/default/verify.sh | Builds/runs Rust default mode scenario. |
| test/scenarios/modes/default/rust/Cargo.toml | Rust default-mode scenario crate manifest. |
| test/scenarios/modes/default/rust/src/main.rs | Rust sample using default tool-enabled mode. |
| test/scenarios/callbacks/user-input/verify.sh | Builds/runs Rust user-input callback scenario. |
| test/scenarios/callbacks/user-input/rust/Cargo.toml | Rust user-input scenario crate manifest. |
| test/scenarios/callbacks/user-input/rust/src/main.rs | Rust sample handling ask_user prompts. |
| test/scenarios/callbacks/permissions/verify.sh | Builds/runs Rust permission callback scenario. |
| test/scenarios/callbacks/permissions/rust/Cargo.toml | Rust permissions scenario crate manifest. |
| test/scenarios/callbacks/permissions/rust/src/main.rs | Rust sample logging/approving permissions. |
| test/scenarios/callbacks/hooks/verify.sh | Builds/runs Rust hooks callback scenario. |
| test/scenarios/callbacks/hooks/rust/Cargo.toml | Rust hooks scenario crate manifest. |
| test/scenarios/callbacks/hooks/rust/src/main.rs | Rust sample implementing SessionHooks logging. |
| test/scenarios/RUST_COVERAGE.md | Documents Rust scenario parity coverage/gaps. |
| scripts/codegen/package.json | Adds Rust generation to codegen scripts. |
| nodejs/scripts/update-protocol-version.ts | Generates Rust SDK protocol version constant. |
| justfile | Adds Rust format/lint/test/codegen tasks. |
| .gitignore | Ignores Rust scenario build artifacts/lockfiles. |
| .github/workflows/scenario-builds.yml | Adds CI job building all Rust scenario crates. |
| .github/workflows/rust-sdk-tests.yml | Adds Rust SDK CI (fmt/clippy/doc/test/semver-checks). |
| .github/workflows/rust-release-pr.yml | Adds release-plz workflow to open Rust release PRs. |
| .github/workflows/rust-publish-release.yml | Adds release-plz workflow to publish Rust crate. |
| .github/workflows/codegen-check.yml | Ensures Rust codegen + protocol version regen in CI. |
| .github/skills/rust-coding-skill/SKILL.md | Adds repo-specific Rust engineering guidance. |
| .github/skills/rust-coding-skill/examples.md | Adds Rust SDK examples/patterns for contributors. |
| .github/copilot-instructions.md | Updates repo guidance to include Rust SDK + Rust skill. |
| rust/Cargo.toml | Defines the new copilot-sdk crate (features, deps, MSRV). |
| rust/Cargo.lock | Locks Rust dependencies for deterministic builds. |
| rust/README.md | Documents Rust SDK usage, architecture, and API surface. |
| rust/CHANGELOG.md | Establishes initial changelog and release-plz plan. |
| rust/RELEASING.md | Documents release/publish operations for maintainers. |
| rust/LICENSE | Rust crate license file. |
| rust/rust-toolchain.toml | Pins Rust toolchain version/components for the crate. |
| rust/release-plz.toml | Configures release-plz behavior for the Rust crate. |
| rust/clippy.toml | Configures Rust clippy rules (e.g., disallowed macros). |
| rust/.rustfmt.toml | Stable rustfmt config (edition 2024). |
| rust/.rustfmt.nightly.toml | Nightly rustfmt config enabling unstable formatting opts. |
| rust/.gitignore | Ignores Rust target dir and backup lock files. |
| rust/build.rs | Build-time CLI bundling/extraction codegen support. |
| rust/src/sdk_protocol_version.rs | Generated SDK protocol version constant for Rust. |
| rust/src/generated/mod.rs | Rust generated-type module root + re-exports. |
| rust/src/jsonrpc.rs | Content-Length framed JSON-RPC transport implementation. |
| rust/src/router.rs | Per-session routing of notifications/requests. |
| rust/src/handler.rs | Session handler traits/events + default handlers. |
| rust/src/permission.rs | Permission policy wrappers over SessionHandler. |
| rust/src/transforms.rs | System message transform extension point + dispatcher. |
| rust/src/session.rs | Session lifecycle/event loop plumbing (core runtime). |
| rust/tests/jsonrpc_test.rs | Tests for JSON-RPC framing/routing (feature-gated). |
| rust/tests/protocol_version_test.rs | Tests protocol version negotiation behavior. |
| rust/tests/integration_test.rs | Ignored integration tests against real CLI. |
| rust/examples/chat.rs | Interactive streaming chat example. |
| rust/examples/hooks.rs | Hooks example for logging/auditing. |
| rust/examples/tool_server.rs | Tool server example (feature-gated on derive). |
| rust/examples/lifecycle_observer.rs | Observer example for lifecycle/session event streams. |
Copilot's findings
- Files reviewed: 102/107 changed files
- Comments generated: 5
cargo doc was running with --features test-support, which left the derive feature off and made intra-doc links to define_tool and schema_for resolve to nothing — failing under the crate's deny(rustdoc::broken_intra_doc_links). docs.rs already uses all-features (see Cargo.toml's [package.metadata.docs.rs]); align CI with that so the docs job matches what users will see on docs.rs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
emitted from the loop correlate to a session in traces. Matches
the pattern documented in the rust-coding-skill.
- README.md / embeddedcli.rs: correct the embedded-CLI documentation
to match what build.rs and embeddedcli.rs actually do — archives
come from the github/copilot-cli GitHub Releases, integrity is
SHA-256 against SHA256SUMS.txt, and the runtime cache path is
~/.cache/copilot-sdk-{version}/copilot.
- test/scenarios/sessions/streaming/verify.sh: drop a duplicate
'# Go: build' comment.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Picks up the new model.call_failure session event (with its ModelCallFailureData payload and ModelCallFailureSource enum) and the new optional 'tip' field on session_info. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes path triggers and the regenerate step for other languages' protocol-version files. Those drift checks are a pre-existing gap on main and out of scope for the Rust SDK port. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 23-line setup checklist duplicated content already in rust/RELEASING.md. One-line pointer is enough. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two scenarios still used the old `Fn(P) -> Fut` shape and broke when the SDK switched to `Fn(ToolInvocation, P) -> Fut`. They don't use the invocation field, so just bind it as `_inv`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 #1164 · ● 2.5M
Cross-SDK consistency: every other SDK (Node, Python, Go, .NET) uses `send`/`Send`/`SendAsync` plus `MessageOptions` as the public parameter type. Rust was the outlier with `send_message` and `SendOptions`, and the asymmetry with the existing `send_and_wait` method made it read awkwardly. - Rename `Session::send_message` -> `Session::send` (and the private helper `send_message_inner` -> `send_inner`). - Rename the public `SendOptions` type -> `MessageOptions`. - Delete the previous wire-level `MessageOptions` struct: it had no internal callers (the wire payload is hand-rolled in send_inner) and freeing the name was the cleanest path to parity. Pre-1.0 type rename, no protocol or behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
stephentoub
reviewed
Apr 29, 2026
Collaborator
|
Thanks for getting this up. It'd be helpful to run an agent over the other SDKs and this new one to look for any inconsistencies / gaps / divergences, so that we can then evaluate each and decide whether it's acceptable or should be addressed. The more consistent we can be across the SDKs, the easier it'll be to maintain them moving forward, the more information / docs about one will translate to consumption of the others, the better we'll be able to evolve with reduced concerns for how something we want to add may not fit well in a particular SDK, etc. |
This comment has been minimized.
This comment has been minimized.
Previously Session::subscribe and Client::subscribe_lifecycle returned
raw tokio::sync::broadcast::Receiver<T> values. A survey of mature Rust
crates (tonic, lapin, rdkafka, redis-rs, tokio-tungstenite, iroh-gossip,
tokio-stream's BroadcastStream itself) found that none of them expose a
raw broadcast::Receiver in their public API; the dominant pattern is a
named newtype implementing futures::Stream, with overflow surfaced
explicitly in the item type.
Introduce a copilot::subscription module with:
- EventSubscription / LifecycleSubscription newtypes
- Inherent recv() returning Result<T, RecvError> for existing
while-let loop ergonomics
- Stream impl yielding Result<T, Lagged> so callers can use
tokio_stream::StreamExt or futures::StreamExt combinators
- Lagged / RecvError types owned by the SDK so consumers no longer
import tokio's broadcast error types
Net effect: the channel choice is now an internal implementation detail.
We can swap broadcast for async-broadcast / flume / a custom backpressure
policy, or convert lag into an Event::Lagged variant, without a breaking
change to the public surface.
Existing while-let loops in tests and examples continue to compile and
behave identically: close and lag both exit the loop, matching
tokio::sync::broadcast::Receiver.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Local cargo +nightly fmt --check passed without `--config-path .rustfmt.nightly.toml`, but CI runs with the explicit config and flagged two diffs: import group flattening and test-mod import order. Applied with the same flags CI uses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1164 · ● 3.1M
Two more wire-shape bugs in the same class as `c58e2f2` (elicitation `requestedSchema`) and `64541af` (list_sessions filter wrapping): hand-authored RPC wrappers calling JSON-RPC methods that don't exist on the CLI runtime, masked by mock-server tests asserting the same wrong names the implementation sent. The CLI runtime (verified via `@github/copilot@1.0.39` app.js) registers two top-level RPCs: - `status.get` (returns version + protocol_version) - `auth.getStatus` (returns is_authenticated + auth_type) Every other public SDK calls those exact names: - Node `nodejs/src/client.ts:980, 992` - Go `go/client.go:1239, 1257` - Python `python/copilot/client.py:1806, 1827` - .NET `dotnet/src/Client.cs:718, 732` The Rust hand-authored layer was sending `getStatus` and `getAuthStatus` — names that don't exist on the wire. Result: both methods would have returned a method-not-found error (or a misleading no-such-method log on the CLI side) instead of the expected payload. The schema doesn't currently typed-define these top-level RPCs (only `session.auth.getStatus` is in the schema, a different namespaced endpoint), so the codegen didn't catch the hand-authored layer's drift. The mock-server test `client_rpc_methods_send_correct_method_names` asserted on the same wrong names the implementation used, so the bugs round-tripped through both ends. Same root cause as the elicitation and list_sessions tests. Fix: - `lib.rs:1301`: `"getStatus"` → `"status.get"` - `lib.rs:1308`: `"getAuthStatus"` → `"auth.getStatus"` - `tests/session_test.rs:488-509`: assertions migrated to the canonical names AND new explicit-absence assertions (`assert_ne!(request["method"], "getStatus")` / `"getAuthStatus"`) so the same regression class can't be reintroduced silently. Doc-comment on the test names the source- of-truth references (CLI runtime registration, four-SDK call sites). - `CHANGELOG.md`: new `### Fixed` entry documenting the wire-name fix and the test gap that masked it. Validation: - 209 tests pass (no count change — same test, stricter assertions). - `cargo doc -D warnings` clean. - `cargo +nightly-2026-04-14 fmt --check` clean. - `cargo clippy --all-features --all-targets -- -D warnings` clean. Caught by the github-actions bot on PR #1164 (comments 3164826435 and 3164826441). Verify-before-drafting tally now 14 / 15 for these two saves. This brings the wire-shape audit save count to 4 (elicitation, list_sessions, get_status, get_auth_status) — all four were hand-authored RPC wrappers asserting on the implementation's wrong shape rather than the schema/runtime's expected shape. The structural fix (regression-prevention asserts in tests) is now in place for all four; the same bug class is no longer silently reproducible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Closes RFD-400 review findings #1 (writer mutex desync hazard) and #4 (send_request pending-entry leak on cancellation). See cancel-safety review session db4b1ac8-... for the full report. The previous design held a `tokio::sync::Mutex<Box<dyn AsyncWrite>>` across `write_all` + `flush` for Content-Length-framed JSON-RPC messages. Caller cancellation mid-frame (e.g. wrapping `Session::send` or `Client::call` in `tokio::time::timeout` or `select!`) could leave a partial frame on the wire, permanently desyncing the transport for the rest of the connection. The reader on the CLI side would block forever waiting for the missing bytes, and the next outbound message would be interpreted as the tail of the previous body. Fix: writer-actor task fed by an `mpsc::UnboundedSender<WriteCommand>`. Public `write` and `send_request` pre-serialize the body, enqueue the frame + an ack `oneshot`, and await the ack. Caller cancellation drops the ack receiver but the actor still completes the in-flight write — the frame either lands atomically or surfaces an io::Error to the (possibly already-dropped) ack receiver. A partial frame can never appear on the wire. Design notes: - Pre-serialize before enqueue: actor receives `Vec<u8>` (header + body, already concatenated). Serde errors fail synchronously at enqueue time before touching the actor — cleanest cancel-safety story. - Unbounded sender: RFD 400 explicitly permits this for cancel-safety, and the producer-consumer ratio is naturally bounded by request/ response semantics on the wire. - Writer actor lives until the JsonRpcClient drops. Channel closes on Drop -> actor sees recv None -> flushes and exits. `pending_requests` cleanup (#4): added a small `PendingGuard` RAII helper in `send_request` that arms after the map insert and disarms before the success return. Caller cancellation drops the guard, which removes the pending-entry synchronously via parking_lot::RwLock (the map mutex is converted in this commit too — it was never held across .await anyway, so the conversion is a pure cleanup). Mutex conversions in this commit: - `JsonRpcClient::pending_requests`: `tokio::sync::RwLock` -> `parking_lot::RwLock`. Lock is never held across .await; conversion is also what enables the synchronous PendingGuard drop. - The previous `Arc<Mutex<Box<dyn AsyncWrite>>>` is gone entirely. The actor task owns the writer directly with no shared synchronization. Tests: two new regression tests in tests/jsonrpc_test.rs - `write_actor_completes_on_caller_cancel`: tiny duplex (8 bytes) + gated reader so the actor's write_all blocks waiting for room. Race the caller's future against a sleep; sleep wins, future is dropped while suspended on ack_rx.await. Then enqueue a second write and release the reader. Both frames must land on the wire intact, in order, with no partial frame from the cancelled write. - `send_request_cancellation_does_not_leak_pending`: cancel an in-flight request, then send a (late) response for the cancelled id to verify the read loop logs and discards it without colliding, then send a fresh request and verify it works normally. Existing 209 tests continue to pass without modification — the wire semantics are unchanged. Total now 211. Validation: cargo test, cargo doc -D warnings, cargo +nightly fmt --check, cargo clippy --all-features --all-targets -- -D warnings all clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes RFD-400 review finding #2 (idle_waiter slot leak on external cancellation). See cancel-safety review session db4b1ac8-... for the full report. `Session::send_and_wait` installs an `IdleWaiter` slot under `self.idle_waiter`, then awaits the response. Internal failure paths (send_inner error, oneshot closure, internal timeout) clean up the slot. External cancellation — caller wraps `send_and_wait` in `tokio::time::timeout`, races it in `select!`, or aborts the JoinHandle — does not. The cleanup at the event loop's `else` branch only fires when the channel itself closes (i.e., the entire session is going away), not when an individual call is cancelled. Effect: any external cancellation between "install waiter" and "drain response" leaves `idle_waiter = Some(...)` forever. All subsequent `send` and `send_and_wait` calls return `SendWhileWaiting`, permanently bricking the session. Fix: `WaiterGuard` RAII helper that takes the slot on Drop. Construct right after install, let RAII handle every exit path. The explicit `self.idle_waiter.lock().take()` calls inside the function disappear, and the slot is cleared on every cancellation path automatically. Mutex conversion (folded in from finding #5): - `Session::idle_waiter`: `tokio::sync::Mutex<Option<IdleWaiter>>` -> `parking_lot::Mutex<Option<IdleWaiter>>`. Lock is never held across `.await` in any code path; the conversion is what makes WaiterGuard's synchronous Drop work without needing an async-spawn fallback. `event_loop` mutex stays `tokio::sync::Mutex` for now — commit C converts it as part of cooperative shutdown (so the conversion lands alongside the matching change to Drop for Session). Tests: two new regression tests in tests/session_test.rs - `send_and_wait_outer_cancellation_clears_waiter`: outer `tokio::time::timeout(50ms, ...)` around `send_and_wait` with a 60-second inner timeout. Outer fires, dropping the future. Verify the next `send` succeeds (no SendWhileWaiting). - `send_and_wait_drop_clears_waiter`: explicit `JoinHandle::abort` of an in-flight send_and_wait. Verify the next `send` succeeds. Existing 211 tests continue to pass; total now 213. Validation: cargo test --all-features, cargo doc -D warnings, cargo +nightly fmt --check, cargo clippy -- -D warnings all clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes RFD-400 review finding #3 (JoinHandle::abort lands at any await point in the event loop body) and the remaining bit of #5 (Session::event_loop tokio::sync::Mutex -> parking_lot::Mutex). See cancel-safety review session db4b1ac8-... for the full report. The previous design called `JoinHandle::abort()` in two places to stop the event-loop task: explicitly in `Session::stop_event_loop` and best-effort in `Drop for Session`. RFD 400's strongest single recommendation about task aborts: > With the abort(), the future returned by do_long_running_operation() > can be cancelled at *any await point*. It is unlikely that arbitrary > async Rust code is resilient to cancellations anywhere during > control flow. ... task aborts should be avoided entirely. The event loop body has many await points: - The outer `select!` itself - `handle_notification`'s nested awaits (acquire idle_waiter lock, send on broadcast, dispatch RPC callbacks for permission/tool/ elicitation requests) - `handle_request`'s inline awaits (deserialize, dispatch handler, build response, write back to client) If `abort()` landed mid-handler — say partway through writing the permission response — the CLI would keep that requestId in flight forever. Same hazard for the idle_waiter oneshot: an abort between "observed session.idle" and "send on waiter.tx" leaves the caller's `send_and_wait` await blocked until the entire transport tears down. Fix: cooperative cancellation via `Arc<tokio::sync::Notify>`. The event-loop body adds a fourth select arm: tokio::select! { _ = shutdown.notified() => break, Some(notification) = notifications.recv() => { ... } Some(request) = requests.recv() => { ... } else => break, } `Notify::notified()` is cancel-safe per RFD 400's framework: the unselected branch losing a poll is fine — it's reconstructed on the next iteration. `notify_one()` (NOT `notify_waiters`) is used because it buffers a single signal: if the loop is currently inside `handle_request(...).await` (no `.notified()` future registered), the signal is held until the handler returns and the loop re-enters select on the next iteration. `notify_waiters()` would silently lose the signal in this case. `stop_event_loop` calls `shutdown.notify_one()` then awaits the JoinHandle. The loop drains its current iteration's handler before observing the buffered signal and exiting cleanly — no mid-protocol state, no orphaned RPC ids on the CLI side. `Drop for Session` calls `notify_one()` and unregisters from the router. The handle is left in `event_loop` for tokio to reap when the loop's spawned task exits naturally; we intentionally don't await it because Drop is sync. Spawned child tasks inside `handle_notification` (permission/tool/elicitation callbacks) intentionally outlive the parent loop — RFD 400's "spawn background tasks to perform cancel-unsafe operations" pattern, documented in the loop's comment. Mutex conversion (folded in from finding #5): - `Session::event_loop`: `tokio::sync::Mutex<Option<JoinHandle<()>>>` -> `parking_lot::Mutex<Option<JoinHandle<()>>>`. Lock is never held across `.await` and the conversion eliminates the `try_lock()` Drop footgun (silent no-op on contention with stop_event_loop). This is the last tokio::sync::Mutex on Session — the import is no longer needed. Tests: two new regression tests in tests/session_test.rs - `stop_event_loop_completes_in_flight_handler`: install a `SlowHandler` whose userInput.request callback sleeps 150ms. Send the request from the server side, then 20ms in (handler is mid- flight) call `stop_event_loop`. Verify the response with `answer: "completed"` lands on the wire BEFORE stop_event_loop returns — proves the handler ran to completion, not aborted mid-await. - `drop_session_does_not_abort_handler`: same shape but via `drop(session)` instead of `stop_event_loop`. Tracks handler completion via an AtomicBool that the handler sets only after its sleep completes; verifies both that the response lands on the wire AND that the handler actually completed despite the Session being dropped. Existing 213 tests continue to pass; total now 215. Validation: cargo test --all-features, cargo doc -D warnings, cargo +nightly fmt --check, cargo clippy -- -D warnings all clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes RFD-400 review finding #6 (per-method cancel-safety documentation). Doc-only follow-up to commits A/B/C; no behavioral change. RFD 400 § "Marking APIs as cancel-unsafe" calls for explicit documentation; with the underlying behavior now correct after A/B/C, documenting the contract makes it inspectable from rustdoc rather than requiring callers to read the implementation. Added `# Cancel safety` rustdoc sections to: - `Session::send` — cancel-safe (writer-actor); message lands on the wire even if the await is cancelled. - `Session::send_and_wait` — cancel-safe (WaiterGuard RAII); outer-cancellation distinct from internal-timeout, both clear the slot. - `Session::abort` — cancel-safe (single RPC via writer-actor). - `Client::call` — cancel-safe at the wire level; documents the caveat that the CLI may still process the request even if the caller doesn't see the response (idempotent vs non-idempotent methods). - `Client::stop` — cancel-unsafe-but-recoverable; the body sequentially destroys sessions (each individually cancel-safe) before killing the child. The existing `tokio::time::timeout` example with `force_stop` fallback is the documented recovery path. - `Client::force_stop` — synchronous, cannot be cancelled. Designed as the recovery path for `stop`'s timeout case. - `Subscription::recv` — cancel-safe by virtue of `tokio::sync::broadcast` + `BroadcastStream`. Validation: cargo doc -D warnings clean, cargo test (215 pass), cargo +nightly fmt --check clean, cargo clippy clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Closes "Mode 1 hands-off loop" gap A from the SDK release plan-doc: update-copilot-dependency.yml is workflow_dispatch only today, so when a new @github/copilot publishes nobody automatically opens the regen PR. Today's bot-driven bump (commit ecdc5ec, "Update @github/copilot to 1.0.39 (#1167)") was triggered manually after the plan-doc session noticed the gap. Goal: the SDK auto-detects new CLI versions, opens a draft PR with all per-language regen + formatting + lockfile updates, and a human just reviews + merges. github-app's bundled-binary tracker picks the new SDK up via crates.io dependabot in the symmetric direction. Changes: 1. **`schedule` trigger.** Daily cron at 10:00 UTC, deliberately symmetric with github-app's check-bundled-binary-versions.yml so the SDK and consumer halves of the loop tick in lockstep on release days. 2. **`version` input is now optional.** Manual override still works for emergency back-patches or cherry-picking specific prereleases; an empty input on workflow_dispatch flows through the same auto-detect path the schedule uses. 3. **Two-job split: `detect` -> `update`.** The detect job is cheap (one `npm view @github/copilot version` + a jq read of nodejs/package.json + a strip of the `^`/`~` prefix). It sets two outputs: - `version`: the resolved target (input override, or latest from npm) - `should_continue`: 'true' if the resolved version differs from the current pin, 'false' otherwise The update job has `needs: detect` and `if: needs.detect.outputs.should_continue == 'true'`, so it skips entirely when there's nothing to do. This keeps the no-op daily cost effectively free. 4. **Validation moves into the resolve step.** Both manual and auto-detected versions go through the same semver regex check. The previous standalone "Validate version input" step is retired; the detect job's resolve step now owns format validation, version comparison, and skip logic. 5. **All `${{ inputs.version }}` references become `${{ needs.detect.outputs.version }}`.** Same value, just routed through the detect job's outputs so manual override and auto-detect both feed the update job uniformly. Doesn't touch SDK source; cargo test / cargo publish --dry-run are no-op confirmation post-merge. Companion gap on the github-app side (auto-open sync PR after each new SDK release) lives on Sync session PR #4140; this commit unblocks the SDK half. Plan-doc capture: tracked under "Steady-state CLI bump automation (Mode 1 hands-off loop)" — gap A closed; gap B (github-app sync-PR auto-open) remains. 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 #1164 · ● 1.7M
…t-only Bot review caught an inaccuracy in the rustdoc and README: the prior wording claimed the underlying account.getQuota JSON-RPC endpoint was exposed only by the Rust SDK. In fact only the top-level Client::get_quota convenience wrapper is Rust-unique; the endpoint itself is generated and exposed cross-SDK via each SDK's typed rpc() namespace. Verified cross-SDK exposure: - Node: nodejs/src/generated/rpc.ts:2203 (account.getQuota in rpc() namespace) - Python: python/copilot/generated/rpc.py:5647 (rpc().account.get_quota) - Go: go/rpc/generated_rpc.go:2252 (RPC.Account.GetQuota) - .NET: dotnet/src/Generated/Rpc.cs:3140 (Rpc().Account().GetQuotaAsync) - Rust: rust/src/generated/rpc.rs:90 (rpc().account().get_quota) Updated rustdoc on Client::get_quota in rust/src/lib.rs and the "Rust-only API" bullet in rust/README.md to correctly state that the wrapper is Rust-only while the underlying endpoint is available across all SDKs via their typed rpc() namespaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirrors the existing MessageOptions::new() / with_* builder pattern on the two other public #[non_exhaustive] config types that consumers construct most often. Closes the cross-crate ergonomics gap that #[non_exhaustive] introduces: external callers can't use struct- literal syntax (with or without ..Default::default()) on these types, so every field assignment becomes its own mut-let statement. The builder shape collapses that into a single chained expression. Caught by github-app PR #4140 review on src-tauri/src/session/cli.rs: the mut-let-and-assign pattern read as awkward to reviewers even after trimming to the four fields that diverge from default. Same ergonomic friction that prompted the make_tool() helper consumers were writing for Tool. Builder ships zero-cost convenience without giving up #[non_exhaustive]'s SemVer protection on growable config struct. ClientOptions builder (rust/src/lib.rs): - new() -> Self (documented entry point) - with_program(impl Into<CliProgram>) - with_prefix_args<I: IntoIterator<...>>(args) - with_cwd(impl Into<PathBuf>) - with_env<I: IntoIterator<(K, V)>> - with_env_remove<I: IntoIterator<S>> - with_extra_args<I: IntoIterator<S>> - with_transport(Transport) - with_github_token(impl Into<String>) - with_use_logged_in_user(bool) - with_log_level(LogLevel) - with_session_idle_timeout_seconds(u64) - with_list_models_handler<H: ListModelsHandler + 'static> (Arc-wrapped internally) - with_session_fs(SessionFsConfig) - with_trace_context_provider<P: TraceContextProvider + 'static> (Arc-wrapped internally) - with_telemetry(TelemetryConfig) Tool builder (rust/src/types.rs): - new(impl Into<String>) -> Self (name + defaults) - with_namespaced_name(impl Into<String>) - with_description(impl Into<String>) - with_instructions(impl Into<String>) - with_parameters(serde_json::Value) - with_overrides_built_in_tool(bool) - with_skip_permission(bool) Pure additive: no existing ClientOptions::default() / mut-let patterns break, no public field visibility changes. Both builders compose by wrapping ::default() and mutating-and-returning self, matching MessageOptions's idiom. Tests: client_options_builder_composes verifies all 10 chained builder methods produce the expected internal state. tool_builder_composes does the same for Tool's seven methods, plus tool_with_parameters_handles_non_object_value covers the HashMap-from-Value extraction edge case (json!(null) -> empty map rather than panic). CHANGELOG: new "Builder ergonomics" subsection under Documentation, explaining the rationale and listing both builders. Validation: 220 tests pass (was 215 + 5 new builder tests across unit + doctest), cargo doc -D warnings clean, cargo fmt --check clean, cargo clippy --all-features --all-targets -- -D warnings clean. Companion ask from github-app PR #4140 review (Sync session) follows up: github-app's `cli.rs` and ~25 Tool::new() call sites will switch to the builder form once this lands and the next sync round-trip pulls it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Mirrors the just-shipped ClientOptions/Tool builder pattern (commit 2d725a5) on the two structs that consumers reach for most when wiring up a session. Both types are #[non_exhaustive], so external callers have been writing the same mut-let-and-assign block at every construction site: let mut config = ResumeSessionConfig::new(session_id); config.client_name = Some("...".into()); config.streaming = Some(true); config.tools = Some(tools); // ...10+ more lines The new chainable form collapses that to: ResumeSessionConfig::new(session_id) .with_client_name("...") .with_streaming(true) .with_tools(tools) // ... Both types have ~30 public fields each. Coverage: SessionConfig: with_session_id, with_model, with_client_name, with_reasoning_effort, with_streaming, with_tools, with_available_tools, with_excluded_tools, with_mcp_servers, with_disabled_mcp_servers, with_env_value_mode, with_enable_config_discovery, with_request_user_input, with_request_permission, with_request_exit_plan_mode, with_request_auto_mode_switch, with_request_elicitation, with_skill_directories, with_disabled_skills, with_agent, with_custom_agents, with_config_dir, with_working_directory, with_github_token, with_include_sub_agent_streaming_events, with_system_message, with_skip_compaction_history. ResumeSessionConfig: same shape minus model/reasoning_effort/ available_tools/disabled_skills/disabled_mcp_servers (which the resume RPC doesn't accept), plus with_disable_resume. Existing closure-installing chains (with_handler, with_hooks, with_transform, with_commands, with_session_fs_provider, approve_all_permissions, deny_all_permissions, approve_permissions_if) and the manual mut-let pattern continue to work unchanged. The hooks bool is intentionally not exposed via a new with_* setter; with_hooks(handler) auto-enables the flag, and power users wanting to override can still set the .hooks field directly. Adds two integration-style tests covering composition across all field categories (scalars, bool flags, Vec setters, struct setters, HashMap setters) for both types. CHANGELOG entry extends the existing "Builder ergonomics" subsection. This is the third commit in the pre-1.0 builder-pattern series (MessageOptions in the original branch, ClientOptions/Tool in 2d725a5, SessionConfig/ResumeSessionConfig here). Pure additive, zero migration cost. Driven by github-app PR #4140 reviewer feedback on src-tauri/src/session/cli.rs and core.rs construction sites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The copilot-sdk repo is public; github-app is private. Public-repo content must not name or depend on github-app, even in comments or test fixtures. - .github/workflows/update-copilot-dependency.yml: drop the "symmetric with github-app's scheduled" framing on the daily cron; rewrite the comment to describe what the schedule does in SDK-local terms only. - rust/CHANGELOG.md: rephrase the Tool builder rationale to talk about "real-world consumer code" rather than github-app's tool catalog specifically. - rust/src/lib.rs: change the `source_name` test fixture from "github-app" to "my-app" — it was just an example string for OTEL source-name plumbing, but the explicit consumer naming is unnecessary and confusing in a public repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
After landing per-field builders on the four highest-traffic consumer-facing config structs (MessageOptions, ClientOptions, Tool, SessionConfig/ResumeSessionConfig), this commit closes the long tail. Rust ecosystem research (tokio, reqwest, tonic, axum, AWS SDK) confirms two design points: 1. The dominant pattern for #[non_exhaustive] config builders is the single `with_<field>(impl Into<T>) -> Self` setter that always Some-wraps. AWS SDK's dual `field` + `set_field(Option<T>)` shape is justified by codegen, not by ergonomics — and the `impl Into<Option<T>>` magic that some older crates use has fallen out of fashion (it conflicts with `impl Into<T>` chaining and produces poor type-inference errors). 2. Direct field assignment on a `pub` field is idiomatic Rust for forwarding `Option<T>` values from upstream code. The http::request::Parts / hyper::Body::Builder pattern. We already support it; we just hadn't documented it. This commit therefore: - Adds full builders to the four bare consumer-facing structs: CustomAgentConfig (name + prompt required, six with_* setters), InfiniteSessionConfig (three with_* setters), ProviderConfig (base_url required, six with_* setters), SystemMessageConfig (three with_* setters). - Expands TraceContext with a symmetric new() + with_traceparent pair alongside the existing from_traceparent shorthand. The shorthand is now expressed in terms of the chain (`new().with_traceparent(x)`) for consistency. - Documents the direct-field-assignment escape hatch on SessionConfig and ResumeSessionConfig, with a doc example showing how to mix the fluent chain (for compile-time-known values) with direct mutation (for Option<T> pass-throughs). Per-field `with_<x>_opt(Option<T>)` setters were considered and deliberately rejected: doubling the method count for ~30-field configs would hurt rustdoc discoverability and conflict with the single-setter precedent we just shipped. The mut-let escape hatch covers the same use case without polluting the API surface. Adds 5 unit tests (one per new builder + a TraceContext suite) and extends the existing CHANGELOG "Builder ergonomics" subsection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit a88c3eb. The scheduled trigger was added to close a hands-off-loop gap that was framed in terms of a downstream consumer's needs. On reflection, the SDK should not be the only language in this repo with scheduled auto-detect; that's a cross-language platform decision, not a single- SDK one. Reverting brings update-copilot-dependency.yml back to manual-only (workflow_dispatch with a required version input) — the same shape as every other language's CLI update workflow in this repo. When the SDK collectively decides to add scheduled CLI tracking, that should land as a unified change across all languages, not as a Rust-only special case driven by a downstream consumer's automation desires. Manual dispatch remains the supported path until then. Conflict resolution: the e93ce8e ("Scrub github-app references") commit landed after a88c3eb and rewrote the cron block's comment. The revert drops the entire schedule: block (including the rewritten comment), restoring the workflow to its pre-a88c3eb manual-only shape exactly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Final missing piece in the consumer-facing #[non_exhaustive] config
struct audit. `TelemetryConfig` had public optional fields (otlp_endpoint,
file_path, exporter_type, source_name, capture_content) and an
is_empty() helper, but no `new()` or `with_*` setters — leaving callers
to write `TelemetryConfig { otlp_endpoint: Some(...), ..Default::default() }`
which doesn't compile from outside the crate due to #[non_exhaustive].
Adds `TelemetryConfig::new()` (delegates to Default) and `with_*`
setters for all five fields, matching the existing pattern.
The is_empty() helper stays unchanged.
After this commit, every consumer-facing #[non_exhaustive] struct
in the SDK has a fluent builder: ClientOptions, TelemetryConfig,
Tool, CommandDefinition, CustomAgentConfig, InfiniteSessionConfig,
ProviderConfig, SessionConfig, ResumeSessionConfig,
SystemMessageConfig, MessageOptions, SessionFsConfig, TraceContext.
One unit test (telemetry_config_builder_composes) covering all five
setters + is_empty behavior on a fresh new(). Extends the existing
CHANGELOG "Builder ergonomics" round-out bullet.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…e-prep # Conflicts: # .gitignore
This comment has been minimized.
This comment has been minimized.
The bundled-CLI build path (gated by the embedded-cli feature and the
COPILOT_CLI_VERSION env var) previously shelled out to `curl` to fetch
SHA256SUMS.txt and the platform tarball from the copilot-cli releases.
Two problems with that:
1. Implicit system dependency. `expect("curl is required ...")` panics
when curl isn't on PATH — true on minimal Windows MSVC environments,
some Docker images, and the CI matrices crates.io publishers tend
to validate against. crates.io build environments themselves don't
guarantee curl.
2. No retry on transient errors. Network blips during CI bundling
surfaced as build failures rather than self-healing waits.
Switch to ureq 2 with default-features=false + ["tls"] (rustls +
webpki-roots, pure-Rust TLS, no OpenSSL or native-tls). Adds about
1.7MB and ~12 transitive deps to the build graph but only when the
embedded-cli feature is on; the default cargo build is unaffected.
Retry policy:
- Up to 3 retries (4 total attempts) with exponential backoff:
1s, 2s, 4s.
- 5xx responses, connect timeouts, read timeouts, and other
transport-layer errors are treated as transient and retried.
- 4xx responses fail fast (the URL is wrong; retrying won't help).
Connect/read timeouts (30s/120s respectively) prevent indefinite
hangs on stalled connections. The cache-hit path still bypasses
the network entirely; only fresh downloads pay the HTTP cost.
End-to-end smoke test against
https://github.com/github/copilot-cli/releases/download/v1.0.39 :
SHA256SUMS download succeeded, tarball download succeeded
(134MB extracted, 44MB compressed), SHA-256 verified, embedded
build succeeded.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The default `cargo test` job in rust-sdk-tests.yml runs on all three supported platforms but does not set `COPILOT_CLI_VERSION` — which means `build.rs` short-circuits immediately and the embedded-cli bundle path (download + SHA-256 verify + extract + zstd compress + embed) is never exercised in CI. After the recent switch from shelling out to `curl` over to `ureq` with retry logic in `build.rs`, that gap matters: the new HTTP client + cross-platform TLS code path needs cross-platform CI coverage before it ships. Adds a parallel `bundle` job to the same workflow: - 3-OS matrix: ubuntu-latest, macos-latest, windows-latest. - Reads the pinned CLI version from `nodejs/package.json` so the bundle test always tracks the same version the rest of the SDK ships against — no separate version source to drift. - Caches the downloaded archive in `./rust/.bundled-cli-cache` keyed on OS + CLI version, so steady-state CI doesn't refetch ~130 MB on every run. Cache miss (e.g. first run after a CLI bump) exercises the full download path end-to-end, which is the regression surface this job is meant to catch. - Runs `cargo build --features embedded-cli` to drive `build.rs` through the full pipeline. Doesn't run `cargo test` — the test job already covers runtime behavior; this job is specifically validating the bundle build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency ReviewThis PR adds a comprehensive new Rust SDK — overall well structured and strongly aligned with the existing Node.js, Python, Go, and .NET SDKs. The core client/session lifecycle, transport options, permission handling, tool registration, hooks, session-fs, streaming, and codegen pipeline all follow the same patterns as the other SDKs. Scenario coverage is thorough. ✅ Consistent across all SDKs
|
| Method | Rust | Node | Python | Go | .NET |
|---|---|---|---|---|---|
Session::get_model() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::set_mode() / get_mode() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::get_name() / set_name() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::set_approve_all_permissions() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::list_workspace_files() / read_workspace_file() / create_workspace_file() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::read_plan() / update_plan() / delete_plan() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::start_fleet() |
✅ | ❌ | ❌ | ❌ | ❌ |
Session::send_telemetry() |
✅ | ❌ | ❌ | ❌ | ❌ |
Client::get_quota() |
✅ | ❌ | ❌ | ❌ | ❌ |
Client::send_telemetry() |
✅ | ❌ | ❌ | ❌ | ❌ |
These are not blockers for this PR — given the "technical preview" framing this is expected — but they represent opportunities for follow-up parity work in the other SDK implementations.
Minor design note
resume_session in Rust silently defaults to DenyAllHandler when no handler is provided, while Node.js, Go, and Python return an error if onPermissionRequest is omitted. Both are safe defaults but it's worth noting the behavioral difference in case callers expect parity here.
Overall: the Rust SDK is a solid addition with strong cross-SDK alignment on the core API surface. The gaps noted above are additive (Rust does more, not less) and are reasonable deferred work for the other languages.
Generated by SDK Consistency Review Agent for issue #1164 · ● 2.7M · ◷
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1164 · ● 2.7M
| } | ||
|
|
||
| /// Get the current model. | ||
| pub async fn get_model(&self) -> Result<Option<String>, Error> { |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency note: get_model() is a new high-level method in Rust that doesn't exist in any other SDK. Every SDK already exposes setModel/set_model/SetModel, but none expose the read side as a convenience wrapper — callers must go through the raw RPC layer today. This is a useful addition that would be worth backfilling in Node.js (getModel(): Promise<string | undefined>), Python (get_model()), Go (GetModel(ctx) (string, error)), and .NET (GetModelAsync()).
| } | ||
|
|
||
| /// List files in the session workspace. | ||
| pub async fn list_workspace_files(&self) -> Result<Vec<String>, Error> { |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency note: list_workspace_files(), read_workspace_file(), and create_workspace_file() (lines 481–507), plus read_plan() / update_plan() / delete_plan() (lines 509–530) and start_fleet() (line 591) are all new high-level session helpers not present in any other SDK. The other SDKs have the matching generated RPC stubs (workspaces.listFiles, workspaces.readFile, workspaces.createFile, session.plan.*, session.fleet.start) but no first-class Session methods wrapping them. These convenience wrappers seem broadly useful across all SDKs.
| /// .NET `client.Rpc().Account().GetQuotaAsync()`), and in Rust at | ||
| /// `client.rpc().account().get_quota()`. This wrapper is a thin shortcut | ||
| /// for that same call. | ||
| pub async fn get_quota(&self) -> Result<generated::api_types::AccountGetQuotaResult, Error> { |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency note: Client::get_quota() and Client::send_telemetry() / Session::send_telemetry() (lines 1529, 553 in session.rs) are new convenience wrappers not present in the Node.js, Python, Go, or .NET client APIs. get_quota in particular is noted in the doc comment as "Go client.Rpc().Account().GetQuota()" which confirms the other SDKs require going through the raw RPC layer today. Fine to have these in Rust now, but good candidates for cross-SDK parity follow-ups.
| } | ||
|
|
||
| /// Get the current session mode. | ||
| pub async fn get_mode(&self) -> Result<String, Error> { |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency note: set_mode() and get_mode() are also new high-level wrappers introduced here (alongside get_name() / set_name() below at lines 429–443). These have no counterpart in the Node.js, Python, Go, or .NET SDKs. All four are protocol-level capabilities (session.mode.set, session.mode.get, session.name.get, session.name.set) that the other SDKs only expose through their generated RPC layer. Worth considering for parity.
| } | ||
|
|
||
| /// Enable or disable session-wide auto-approval for tool permission requests. | ||
| pub async fn set_approve_all_permissions(&self, enabled: bool) -> Result<(), Error> { |
Contributor
There was a problem hiding this comment.
Cross-SDK consistency note (design difference): set_approve_all_permissions() provides a runtime escape hatch to flip the setApproveAll flag on a live session. The other SDKs take a callback-centric approach — they accept a onPermissionRequest handler at createSession/resumeSession time instead. Both approaches are valid, but the runtime mutability here is a meaningful ergonomic difference. If this proves useful, the other SDKs could adopt a similar session-level method alongside their callback patterns.
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.
Adds a Rust SDK alongside the existing Node, Python, Go, and .NET SDKs in this repo. Same JSON-RPC client model, same protocol, same session lifecycle — just in Rust.
Important
Technical preview. This is published as
github-copilot-sdk = "0.1"(pre-1.0) and the public API is subject to breaking changes as we iterate. Pin to an exact version, expect churn, and please file issues for friction or missing parity.See
rust/README.mdfor the full overview, examples, and the build/test commands. Generated types follow the same schema-driven flow used by the other SDKs (scripts/codegen/rust.ts).CI for the new crate runs in
.github/workflows/rust-sdk-tests.yml.