Refactor Rust SDK errors to use structs with a kind() method

.github/copilot-instructions.md

@@ -29,6 +29,7 @@
   - **.NET testing note:** Never add `InternalsVisibleTo` to any project file when writing tests. Tests must only access public APIs.
   - Java: `cd java && mvn clean verify` (full build + tests), `mvn spotless:apply` (format code before commit)
   - **Java testing note:** Always use `mvn verify` without `-q` and without piping through `grep`. Never add `InternalsVisibleTo` equivalent — tests must only access public APIs.
+- Use configured LSPs for supported operations like finding references instead of pattern matching, renaming symbols, etc.
 
 ## Testing & E2E tips ⚙️
 

.github/lsp.json

@@ -21,6 +21,24 @@
         ".go": "go"
       },
       "rootUri": "go"
+    },
+    "rust-analyzer": {
+      "command": "rust-analyzer",
+      "fileExtensions": {
+        ".rs": "rust"
+      },
+      "initializationOptions": {
+        "cargo": {
+          "buildScripts": {
+            "enable": true
+          },
+          "allFeatures": true
+        },
+        "checkOnSave": true,
+        "check": {
+          "command": "clippy"
+        }
+      }
     }
   }
 }

.github/skills/rust-coding-skill/SKILL.md

@@ -13,11 +13,10 @@ Opinionated Rust rules for the Copilot Rust SDK (`rust/`). Priority order:
 
 ## Error handling
 
-The SDK's public error type is `crate::Error` (`rust/src/error.rs`). Add new
-variants there rather than introducing parallel error enums per module — every
-public failure mode is part of the API contract and should be expressible in one
-type. Internal modules can use `thiserror` enums when a richer local taxonomy
-helps; convert at the boundary.
+The SDK's public error type is `crate::Error` (`rust/src/errors.rs`). Add new
+variants to `crate::ErrorKind` rather than introducing parallel error enums
+per module — every public failure mode is part of the API contract and should
+be expressible in one type.
 
 `anyhow` is reserved for binaries and example code. Library code never returns
 `anyhow::Result` — callers can't pattern-match on `anyhow::Error`, so it would
@@ -42,7 +41,7 @@ it on shutdown. Fire-and-forget spawns silently swallow panics and outlive the
 session; don't.
 
 Blocking calls (filesystem, subprocess wait) belong in
-`tokio::task::spawn_blocking`, *not* on the async runtime. The blocking pool is
+`tokio::task::spawn_blocking`, _not_ on the async runtime. The blocking pool is
 bounded, so for genuinely long-lived workers (think: file watchers that run for
 the lifetime of a session) prefer `std::thread::spawn` with a channel back into
 async land.
@@ -81,12 +80,12 @@ Trivial field re-shaping is best inlined. Closures should stay short (under ~10
 
 **Channels, not callback closures, for event flow.** Closures fight `Send + Sync + 'static` and don't compose with `select!`. Channel choice by semantics:
 
-| Use case | Primitive |
-|---|---|
-| One producer → one consumer with backpressure | `tokio::sync::mpsc` (cap 1) or `tokio::sync::oneshot` for single value |
-| Many producers → one consumer | `tokio::sync::mpsc` |
-| One producer → many consumers, every event delivered (pub/sub) | `tokio::sync::broadcast` |
-| One producer → many consumers, only the latest value matters | `tokio::sync::watch` |
+| Use case                                                       | Primitive                                                              |
+| -------------------------------------------------------------- | ---------------------------------------------------------------------- |
+| One producer → one consumer with backpressure                  | `tokio::sync::mpsc` (cap 1) or `tokio::sync::oneshot` for single value |
+| Many producers → one consumer                                  | `tokio::sync::mpsc`                                                    |
+| One producer → many consumers, every event delivered (pub/sub) | `tokio::sync::broadcast`                                               |
+| One producer → many consumers, only the latest value matters   | `tokio::sync::watch`                                                   |
 
 For the **public** API, prefer returning `impl Stream<Item = Event>` (wrap a `broadcast::Receiver` in `tokio_stream::wrappers::BroadcastStream`). `Stream` composes with `select!`, `take`, `map`, `filter`, `timeout`. See `EventSubscription` and `LifecycleSubscription`.
 
@@ -115,7 +114,7 @@ JSON: `#[serde(rename_all = "camelCase")]` at the type level, per-field `#[serde
 Banned via `clippy.toml`. Use manual spans with `error_span!`:
 
 - **Almost always use `error_span!`**, not `info_span!`. Span level controls
-  the *minimum* filter at which the span appears. An `info_span` disappears when
+  the _minimum_ filter at which the span appears. An `info_span` disappears when
   the filter is `warn` or `error` — taking all child events with it, even
   errors. `error_span!` ensures the span is always present.
 - **Spawned tasks lose parent context.** Attach a span with `.instrument()` or
@@ -239,9 +238,9 @@ Match those exact commands locally before pushing.
 
 JSON-RPC and session-event types are generated from the Copilot CLI schema:
 
-| Source | Output |
-|---|---|
-| `nodejs/node_modules/@github/copilot/schemas/api.schema.json` | `rust/src/generated/api_types.rs` |
+| Source                                                                   | Output                                 |
+| ------------------------------------------------------------------------ | -------------------------------------- |
+| `nodejs/node_modules/@github/copilot/schemas/api.schema.json`            | `rust/src/generated/api_types.rs`      |
 | `nodejs/node_modules/@github/copilot/schemas/session-events.schema.json` | `rust/src/generated/session_events.rs` |
 
 Regenerate with:

.vscode/settings.json

@@ -14,6 +14,11 @@
   "python.testing.pytestEnabled": true,
   "python.testing.unittestEnabled": false,
   "python.testing.pytestArgs": ["python"],
+  "rust-analyzer.cargo.features": "all",
+  "rust-analyzer.check.command": "clippy",
+  "[rust]": {
+    "editor.defaultFormatter": "rust-lang.rust-analyzer"
+  },
   "[python]": {
     "editor.defaultFormatter": "charliermarsh.ruff"
   },

rust/Cargo.toml

@@ -43,7 +43,6 @@ async-trait = "0.1"
 schemars = { version = "1", optional = true }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-thiserror = "2"
 tokio = { version = "1", features = ["io-util", "sync", "rt", "process", "net", "time", "macros"] }
 tokio-stream = { version = "0.1", features = ["sync"] }
 tokio-util = { version = "0.7", default-features = false }
@@ -68,6 +67,18 @@ serial_test = "3"
 tempfile = "3"
 tokio = { version = "1", features = ["rt-multi-thread"] }
 
+# Integration tests that call test-support-only Client methods (e.g.
+# `from_streams_with_connection_token`, `from_streams_with_trace_provider`)
+# require the `test-support` feature because `cfg(test)` is not set on the
+# library when Cargo compiles it for integration tests.
+[[test]]
+name = "session_test"
+required-features = ["test-support"]
+
+[[test]]
+name = "protocol_version_test"
+required-features = ["test-support"]
+
 [build-dependencies]
 flate2 = "1"
 sha2 = "0.10"

rust/examples/manual_tool_resume.rs

@@ -10,7 +10,7 @@ use github_copilot_sdk::generated::session_events::{
     AssistantMessageData, ExternalToolRequestedData, PermissionRequestedData, SessionEventType,
 };
 use github_copilot_sdk::{
-    Client, ClientOptions, EventSubscription, RecvError, ResumeSessionConfig, SessionConfig,
+    Client, ClientOptions, EventSubscription, subscription::RecvError, ResumeSessionConfig, SessionConfig,
 };
 use serde_json::json;
 

rust/examples/session_fs.rs

@@ -15,7 +15,7 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use github_copilot_sdk::handler::ApproveAllHandler;
 use github_copilot_sdk::session_fs::{
-    DirEntry, DirEntryKind, FileInfo, FsError, SessionFsConfig, SessionFsConventions,
+    DirEntry, DirEntryKind, FileInfo, FsError, FsErrorKind, SessionFsConfig, SessionFsConventions,
     SessionFsProvider,
 };
 use github_copilot_sdk::types::{MessageOptions, SessionConfig};
@@ -46,7 +46,7 @@ impl SessionFsProvider for InMemoryProvider {
             .lock()
             .get(path)
             .cloned()
-            .ok_or_else(|| FsError::NotFound(path.to_string()))
+            .ok_or_else(|| FsError::from(FsErrorKind::NotFound(path.to_string())))
     }
 
     async fn write_file(
@@ -69,7 +69,7 @@ impl SessionFsProvider for InMemoryProvider {
         let files = self.files.lock();
         let content = files
             .get(path)
-            .ok_or_else(|| FsError::NotFound(path.to_string()))?;
+            .ok_or_else(|| FsError::from(FsErrorKind::NotFound(path.to_string())))?;
         Ok(FileInfo::new(
             true,
             false,
@@ -101,7 +101,7 @@ impl SessionFsProvider for InMemoryProvider {
 
     async fn rm(&self, path: &str, _recursive: bool, force: bool) -> Result<(), FsError> {
         if self.files.lock().remove(path).is_none() && !force {
-            return Err(FsError::NotFound(path.to_string()));
+            return Err(FsError::from(FsErrorKind::NotFound(path.to_string())));
         }
         Ok(())
     }

rust/src/embeddedcli.rs

@@ -15,7 +15,7 @@ use std::fs;
 #[cfg(all(has_bundled_cli, not(windows)))]
 use std::io::Read;
 #[cfg(has_bundled_cli)]
-use std::io::{self, Write};
+use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 
@@ -109,7 +109,7 @@ fn default_install_dir(version: &str) -> PathBuf {
 fn install(install_dir: &Path, archive: &[u8]) -> Result<PathBuf, EmbeddedCliError> {
     let verbose = std::env::var("COPILOT_CLI_INSTALL_VERBOSE").ok().as_deref() == Some("1");
 
-    fs::create_dir_all(install_dir).map_err(EmbeddedCliError::CreateDir)?;
+    fs::create_dir_all(install_dir).map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::CreateDir, e))?;
 
     let final_path = install_dir.join(build_time::CLI_BINARY_NAME);
 
@@ -141,35 +141,35 @@ fn install(install_dir: &Path, archive: &[u8]) -> Result<PathBuf, EmbeddedCliErr
 fn extract_binary(archive: &[u8], binary_name: &str) -> Result<Vec<u8>, EmbeddedCliError> {
     let gz = flate2::read::GzDecoder::new(archive);
     let mut tar = tar::Archive::new(gz);
-    for entry in tar.entries().map_err(EmbeddedCliError::Archive)? {
-        let mut entry = entry.map_err(EmbeddedCliError::Archive)?;
-        let path = entry.path().map_err(EmbeddedCliError::Archive)?;
+    for entry in tar.entries().map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Archive, e))? {
+        let mut entry = entry.map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Archive, e))?;
+        let path = entry.path().map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Archive, e))?;
         let name = path.to_string_lossy();
         if name == binary_name || name.ends_with(&format!("/{binary_name}")) {
             let mut bytes = Vec::with_capacity(entry.size() as usize);
             entry
                 .read_to_end(&mut bytes)
-                .map_err(EmbeddedCliError::Archive)?;
+                .map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Archive, e))?;
             return Ok(bytes);
         }
     }
-    Err(EmbeddedCliError::BinaryNotFoundInArchive)
+    Err(EmbeddedCliErrorKind::BinaryNotFoundInArchive.into())
 }
 
 #[cfg(all(has_bundled_cli, windows))]
 fn extract_binary(archive: &[u8], binary_name: &str) -> Result<Vec<u8>, EmbeddedCliError> {
     let cursor = std::io::Cursor::new(archive);
-    let mut zip = zip::ZipArchive::new(cursor).map_err(EmbeddedCliError::Zip)?;
+    let mut zip = zip::ZipArchive::new(cursor).map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Zip, e))?;
     for i in 0..zip.len() {
-        let mut entry = zip.by_index(i).map_err(EmbeddedCliError::Zip)?;
+        let mut entry = zip.by_index(i).map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Zip, e))?;
         let name = entry.name().to_string();
         if name == binary_name || name.ends_with(&format!("/{binary_name}")) {
             let mut bytes = Vec::with_capacity(entry.size() as usize);
-            std::io::copy(&mut entry, &mut bytes).map_err(EmbeddedCliError::Io)?;
+            std::io::copy(&mut entry, &mut bytes).map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Io, e))?;
             return Ok(bytes);
         }
     }
-    Err(EmbeddedCliError::BinaryNotFoundInArchive)
+    Err(EmbeddedCliErrorKind::BinaryNotFoundInArchive.into())
 }
 
 #[cfg(has_bundled_cli)]
@@ -190,38 +190,107 @@ fn write_binary(path: &Path, data: &[u8]) -> Result<(), EmbeddedCliError> {
         .create(true)
         .truncate(true)
         .open(path)
-        .map_err(EmbeddedCliError::Io)?;
+        .map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Io, e))?;
 
-    file.write_all(data).map_err(EmbeddedCliError::Io)?;
+    file.write_all(data).map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Io, e))?;
 
     #[cfg(unix)]
     {
         use std::os::unix::fs::PermissionsExt;
         fs::set_permissions(path, fs::Permissions::from_mode(0o755))
-            .map_err(EmbeddedCliError::Io)?;
+            .map_err(|e| EmbeddedCliError::new(EmbeddedCliErrorKind::Io, e))?;
     }
 
     Ok(())
 }
 
 #[cfg(has_bundled_cli)]
-#[derive(Debug, thiserror::Error)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 #[allow(dead_code)]
-enum EmbeddedCliError {
-    #[error("failed to create install directory: {0}")]
-    CreateDir(io::Error),
-
+enum EmbeddedCliErrorKind {
+    CreateDir,
     #[cfg(not(windows))]
-    #[error("failed to read archive entry: {0}")]
-    Archive(io::Error),
-
+    Archive,
     #[cfg(windows)]
-    #[error("failed to read zip archive: {0}")]
-    Zip(zip::result::ZipError),
-
-    #[error("CLI binary not found in embedded archive")]
+    Zip,
     BinaryNotFoundInArchive,
+    Io,
+}
+
+#[cfg(has_bundled_cli)]
+impl std::fmt::Display for EmbeddedCliErrorKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            EmbeddedCliErrorKind::CreateDir => f.write_str("failed to create install directory"),
+            #[cfg(not(windows))]
+            EmbeddedCliErrorKind::Archive => f.write_str("failed to read archive entry"),
+            #[cfg(windows)]
+            EmbeddedCliErrorKind::Zip => f.write_str("failed to read zip archive"),
+            EmbeddedCliErrorKind::BinaryNotFoundInArchive => {
+                f.write_str("CLI binary not found in embedded archive")
+            }
+            EmbeddedCliErrorKind::Io => f.write_str("I/O error"),
+        }
+    }
+}
 
-    #[error("I/O error: {0}")]
-    Io(io::Error),
+#[cfg(has_bundled_cli)]
+#[allow(dead_code)]
+struct EmbeddedCliError {
+    repr: crate::errors::Repr<EmbeddedCliErrorKind>,
+}
+
+#[cfg(has_bundled_cli)]
+impl EmbeddedCliError {
+    fn new<E>(kind: EmbeddedCliErrorKind, error: E) -> Self
+    where
+        E: Into<Box<dyn std::error::Error + Send + Sync>>,
+    {
+        Self {
+            repr: crate::errors::Repr::Custom(crate::errors::Custom {
+                kind,
+                error: error.into(),
+            }),
+        }
+    }
+
+}
+
+#[cfg(has_bundled_cli)]
+impl From<EmbeddedCliErrorKind> for EmbeddedCliError {
+    fn from(kind: EmbeddedCliErrorKind) -> Self {
+        Self {
+            repr: crate::errors::Repr::Simple(kind),
+        }
+    }
+}
+
+#[cfg(has_bundled_cli)]
+impl std::fmt::Display for EmbeddedCliError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match &self.repr {
+            crate::errors::Repr::Simple(kind) => write!(f, "{kind}"),
+            crate::errors::Repr::SimpleMessage(_, msg) => write!(f, "{msg}"),
+            crate::errors::Repr::Custom(crate::errors::Custom { kind, error }) => {
+                write!(f, "{kind}: {error}")
+            }
+        }
+    }
+}
+
+#[cfg(has_bundled_cli)]
+impl std::fmt::Debug for EmbeddedCliError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "EmbeddedCliError({self})")
+    }
+}
+
+#[cfg(has_bundled_cli)]
+impl std::error::Error for EmbeddedCliError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match &self.repr {
+            crate::errors::Repr::Custom(crate::errors::Custom { error, .. }) => Some(&**error),
+            _ => None,
+        }
+    }
 }