Address Rust E2E review feedback

Move struct-only E2E placeholders into unit tests, exercise invalid external-auth client options, avoid logging session IDs from test assertions, and harden failing Rust E2Es on CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: stephentoub

rust/src/lib.rs

@@ -927,6 +927,24 @@ impl Client {
         if let Some(cfg) = &options.session_fs {
             validate_session_fs_config(cfg)?;
         }
+        // Auth options only make sense when the SDK spawns the CLI; with an
+        // external server, the server manages its own auth.
+        if matches!(options.transport, Transport::External { .. }) {
+            if options.github_token.is_some() {
+                return Err(Error::InvalidConfig(
+                    "github_token cannot be used with Transport::External \
+                     (external server manages its own auth)"
+                        .to_string(),
+                ));
+            }
+            if options.use_logged_in_user.is_some() {
+                return Err(Error::InvalidConfig(
+                    "use_logged_in_user cannot be used with Transport::External \
+                     (external server manages its own auth)"
+                        .to_string(),
+                ));
+            }
+        }
         // Validate token + transport combination. Stdio cannot use a
         // connection token; auto-generate a UUID when the SDK spawns
         // its own CLI in TCP mode and no explicit token was set.

rust/tests/e2e/rpc_extensions_loaded.rs rust/tests/api_types_test.rsrust/tests/e2e/rpc_extensions_loaded.rs renamed to rust/tests/api_types_test.rs

@@ -1,70 +1,89 @@
+// Unit tests for generated API types -- struct construction and field
+// access. These do not require a client, session, or replay proxy.
+
+#![allow(clippy::unwrap_used)]
+
 use github_copilot_sdk::generated::api_types::{
     Extension, ExtensionList, ExtensionSource, ExtensionStatus, ExtensionsDisableRequest,
-    ExtensionsEnableRequest,
+    ExtensionsEnableRequest, FleetStartRequest, FleetStartResult, TasksStartAgentRequest,
 };
 
-#[tokio::test]
-async fn discovers_loads_and_reports_running_extension() {
+#[test]
+fn extension_running_has_expected_status_and_source() {
     let extension = running_extension("project:demo", "demo");
-
     assert_eq!(extension.status, ExtensionStatus::Running);
     assert_eq!(extension.source, ExtensionSource::Project);
 }
 
-#[tokio::test]
-async fn disable_then_enable_cycles_extension_status() {
+#[test]
+fn disable_and_enable_requests_share_the_same_id() {
     let disable = ExtensionsDisableRequest {
         id: "project:demo".to_string(),
     };
     let enable = ExtensionsEnableRequest {
         id: disable.id.clone(),
     };
-
     assert_eq!(disable.id, enable.id);
 }
 
-#[tokio::test]
-async fn reload_picks_up_extension_added_after_session_create() {
+#[test]
+fn extension_list_contains_newly_added_extension_by_name() {
     let list = ExtensionList {
         extensions: vec![running_extension("project:late", "late")],
     };
-
-    assert!(
-        list.extensions
-            .iter()
-            .any(|extension| extension.name == "late")
-    );
+    assert!(list.extensions.iter().any(|e| e.name == "late"));
 }
 
-#[tokio::test]
-async fn failed_extension_reports_failed_status() {
+#[test]
+fn failed_extension_reports_failed_status() {
     let mut extension = running_extension("project:broken", "broken");
     extension.status = ExtensionStatus::Failed;
-
     assert_eq!(extension.status, ExtensionStatus::Failed);
 }
 
-#[tokio::test]
-async fn multiple_extensions_are_discovered_independently() {
+#[test]
+fn multiple_extensions_have_distinct_ids() {
     let list = ExtensionList {
         extensions: vec![
             running_extension("project:first", "first"),
             running_extension("user:second", "second"),
         ],
     };
-
     assert_eq!(list.extensions.len(), 2);
     assert_ne!(list.extensions[0].id, list.extensions[1].id);
 }
 
-#[tokio::test]
-async fn reload_preserves_disabled_state_across_calls() {
+#[test]
+fn disabled_extension_preserves_disabled_status() {
     let mut extension = running_extension("project:disabled", "disabled");
     extension.status = ExtensionStatus::Disabled;
-
     assert_eq!(extension.status, ExtensionStatus::Disabled);
 }
 
+#[test]
+fn fleet_start_request_and_result_fields_are_accessible() {
+    let request = FleetStartRequest {
+        prompt: Some("Use the custom tool".to_string()),
+    };
+    let result = FleetStartResult { started: true };
+    assert_eq!(request.prompt.as_deref(), Some("Use the custom tool"));
+    assert!(result.started);
+}
+
+#[test]
+fn tasks_start_agent_request_fields_are_accessible() {
+    let request = TasksStartAgentRequest {
+        agent_type: "general-purpose".to_string(),
+        prompt: "Say hi".to_string(),
+        name: "sdk-test-task".to_string(),
+        description: Some("SDK task agent".to_string()),
+        model: None,
+    };
+    assert_eq!(request.agent_type, "general-purpose");
+    assert_eq!(request.name, "sdk-test-task");
+    assert_eq!(request.description.as_deref(), Some("SDK task agent"));
+}
+
 fn running_extension(id: &str, name: &str) -> Extension {
     Extension {
         id: id.to_string(),

rust/tests/e2e.rs

@@ -51,8 +51,6 @@ mod rpc_additional_edge_cases;
 mod rpc_agent;
 #[path = "e2e/rpc_event_side_effects.rs"]
 mod rpc_event_side_effects;
-#[path = "e2e/rpc_extensions_loaded.rs"]
-mod rpc_extensions_loaded;
 #[path = "e2e/rpc_mcp_and_skills.rs"]
 mod rpc_mcp_and_skills;
 #[path = "e2e/rpc_mcp_config.rs"]

rust/tests/e2e/abort.rs

@@ -93,6 +93,7 @@ async fn should_abort_during_active_tool_execution() {
                     )
                     .await
                     .expect("create session");
+                let events = session.subscribe();
 
                 session
                     .send("Use slow_analysis with value 'test_abort'. Wait for the result.")
@@ -106,6 +107,10 @@ async fn should_abort_during_active_tool_execution() {
                 release_tx
                     .send("RELEASED_AFTER_ABORT".to_string())
                     .expect("release slow tool");
+                wait_for_event(events, "session.idle after abort", |event| {
+                    event.parsed_type() == SessionEventType::SessionIdle
+                })
+                .await;
 
                 let recovery = session
                     .send_and_wait("Say 'tool_abort_recovery_ok'.")

rust/tests/e2e/client_options.rs

@@ -1,7 +1,7 @@
 use std::net::{Ipv4Addr, SocketAddrV4, TcpListener};
 
 use github_copilot_sdk::{
-    Client, ClientOptions, LogLevel, MessageOptions, OtelExporterType, SessionConfig,
+    Client, ClientOptions, Error, LogLevel, MessageOptions, OtelExporterType, SessionConfig,
     TelemetryConfig, Transport,
 };
 use serde_json::json;
@@ -242,8 +242,18 @@ async fn should_throw_when_githubtoken_used_with_cliurl() {
         })
         .with_github_token("token");
 
-    assert!(matches!(options.transport, Transport::External { .. }));
-    assert_eq!(options.github_token.as_deref(), Some("token"));
+    let err = Client::start(options).await.unwrap_err();
+    assert!(
+        matches!(err, Error::InvalidConfig(_)),
+        "expected InvalidConfig, got {err:?}"
+    );
+    let Error::InvalidConfig(msg) = err else {
+        unreachable!()
+    };
+    assert!(
+        msg.contains("github_token"),
+        "error message should mention github_token, got: {msg}"
+    );
 }
 
 #[tokio::test]
@@ -255,8 +265,18 @@ async fn should_throw_when_useloggedinuser_used_with_cliurl() {
         })
         .with_use_logged_in_user(true);
 
-    assert!(matches!(options.transport, Transport::External { .. }));
-    assert_eq!(options.use_logged_in_user, Some(true));
+    let err = Client::start(options).await.unwrap_err();
+    assert!(
+        matches!(err, Error::InvalidConfig(_)),
+        "expected InvalidConfig, got {err:?}"
+    );
+    let Error::InvalidConfig(msg) = err else {
+        unreachable!()
+    };
+    assert!(
+        msg.contains("use_logged_in_user"),
+        "error message should mention use_logged_in_user, got: {msg}"
+    );
 }
 
 fn get_available_tcp_port() -> u16 {

rust/tests/e2e/rpc_shell_and_fleet.rs

@@ -1,6 +1,4 @@
-use github_copilot_sdk::generated::api_types::{
-    FleetStartRequest, FleetStartResult, ShellExecRequest, ShellKillRequest,
-};
+use github_copilot_sdk::generated::api_types::{ShellExecRequest, ShellKillRequest};
 
 use super::support::{wait_for_condition, with_e2e_context};
 
@@ -82,17 +80,6 @@ async fn should_kill_shell_process() {
     .await;
 }
 
-#[tokio::test]
-async fn should_start_fleet_and_complete_custom_tool_task() {
-    let request = FleetStartRequest {
-        prompt: Some("Use the custom tool".to_string()),
-    };
-    let result = FleetStartResult { started: true };
-
-    assert_eq!(request.prompt.as_deref(), Some("Use the custom tool"));
-    assert!(result.started);
-}
-
 async fn wait_for_file_text(path: &std::path::Path, expected: &'static str) {
     wait_for_condition("shell command output file", || async {
         match std::fs::read_to_string(path) {

rust/tests/e2e/rpc_tasks_and_handlers.rs

@@ -159,21 +159,6 @@ async fn should_report_implemented_error_for_invalid_task_agent_model() {
     .await;
 }
 
-#[tokio::test]
-async fn should_start_background_agent_and_report_task_details() {
-    let request = TasksStartAgentRequest {
-        agent_type: "general-purpose".to_string(),
-        prompt: "Say hi".to_string(),
-        name: "sdk-test-task".to_string(),
-        description: Some("SDK task agent".to_string()),
-        model: None,
-    };
-
-    assert_eq!(request.agent_type, "general-purpose");
-    assert_eq!(request.name, "sdk-test-task");
-    assert_eq!(request.description.as_deref(), Some("SDK task agent"));
-}
-
 #[tokio::test]
 async fn should_return_expected_results_for_missing_pending_handler_requestids() {
     with_e2e_context(

rust/tests/e2e/session_fs.rs

@@ -38,7 +38,7 @@ async fn should_route_file_operations_through_the_session_fs_provider() {
                 assert!(assistant_message_content(&answer).contains("300"));
                 let events_path = provider_root
                     .join(session.id().as_ref())
-                    .join("session-state")
+                    .join(provider_relative_path(&session_state_path()))
                     .join("events.jsonl");
                 wait_for_file_containing(&events_path, "300").await;
                 let content = std::fs::read_to_string(events_path).expect("read events");
@@ -214,7 +214,7 @@ async fn should_reject_setprovider_when_sessions_already_exist() {
     let config = session_fs_config();
 
     assert_eq!(config.initial_cwd, "/");
-    assert_eq!(config.session_state_path, "/session-state");
+    assert_eq!(config.session_state_path, session_state_path());
 }
 
 #[tokio::test]
@@ -277,7 +277,7 @@ async fn should_persist_plan_md_via_sessionfs() {
                     .expect("update plan");
                 let plan_path = provider_root
                     .join(session.id().as_ref())
-                    .join("session-state")
+                    .join(provider_relative_path(&session_state_path()))
                     .join("plan.md");
                 wait_for_file_containing(&plan_path, "This is a test.").await;
                 assert!(
@@ -383,7 +383,7 @@ async fn should_write_workspace_metadata_via_sessionfs() {
                 assert!(assistant_message_content(&answer).contains("56"));
                 let workspace_path = provider_root
                     .join(session.id().as_ref())
-                    .join("session-state")
+                    .join(provider_relative_path(&session_state_path()))
                     .join("workspace.yaml");
                 wait_for_file_containing(&workspace_path, session.id().as_ref()).await;
 
@@ -413,7 +413,23 @@ fn session_config(
 }
 
 fn session_fs_config() -> SessionFsConfig {
-    SessionFsConfig::new("/", "/session-state", SessionFsConventions::Posix)
+    SessionFsConfig::new("/", session_state_path(), SessionFsConventions::Posix)
+}
+
+fn session_state_path() -> String {
+    if cfg!(windows) {
+        "/session-state".to_string()
+    } else {
+        std::env::temp_dir()
+            .join("copilot-rust-sessionfs-state")
+            .join("session-state")
+            .to_string_lossy()
+            .replace('\\', "/")
+    }
+}
+
+fn provider_relative_path(path: &str) -> PathBuf {
+    PathBuf::from(path.trim_start_matches(['/', '\\']))
 }
 
 async fn wait_for_file_containing(path: &Path, needle: &str) {

rust/tests/e2e/support.rs

@@ -424,7 +424,7 @@ pub fn assert_uuid_like(session_id: &SessionId) {
     assert_eq!(text.len(), 36, "session id should be UUID-shaped");
     assert!(
         text.chars().all(|ch| ch.is_ascii_hexdigit() || ch == '-'),
-        "session id should be UUID-shaped: {text}"
+        "session id should be UUID-shaped"
     );
 }
 

rust/tests/e2e/suspend.rs

@@ -86,19 +86,3 @@ async fn should_allow_resume_and_continue_conversation_after_suspend() {
     )
     .await;
 }
-
-#[tokio::test]
-async fn should_cancel_pending_permission_request_when_suspending() {
-    let config =
-        ResumeSessionConfig::new("suspend-permission".into()).with_continue_pending_work(false);
-
-    assert_eq!(config.continue_pending_work, Some(false));
-}
-
-#[tokio::test]
-async fn should_reject_pending_external_tool_when_suspending() {
-    let config =
-        ResumeSessionConfig::new("suspend-external-tool".into()).with_continue_pending_work(false);
-
-    assert_eq!(config.continue_pending_work, Some(false));
-}