From f44fbb9c85d0350ce9aa4a7dce6b0ade561e8ba4 Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Fri, 23 Oct 2026 15:00:59 -0700 Subject: [PATCH] test(desktop): verify session launch reviews, cleanup, and account isolation Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-desktop/src/controller.rs | 163 +++++++++ crates/switchyard-desktop/src/sessions.rs | 354 ++++++++++++++++++++ 2 files changed, 517 insertions(+) diff --git a/crates/switchyard-desktop/src/controller.rs b/crates/switchyard-desktop/src/controller.rs index a199a03e5..2233ffcec 100644 --- a/crates/switchyard-desktop/src/controller.rs +++ b/crates/switchyard-desktop/src/controller.rs @@ -1553,6 +1553,169 @@ fn update(settings: &std::path::Path) -> Result { #[cfg(test)] mod tests { use super::*; + // The subprocess isolates harness discovery and every launch destination. + #[cfg(unix)] + #[test] + fn session_launch_requires_an_unchanged_review_before_creating_files() { + use std::os::unix::fs::PermissionsExt; + use std::{fs, process::Command}; + if let Some(home) = std::env::var_os("SWITCHYARD_LAUNCH_REVIEW_FIXTURE") { + let home = PathBuf::from(home); + let project = home.join("project"); + let git = |args: &[&str]| { + let result = Command::new("/usr/bin/git") + .arg("-C") + .arg(&project) + .args(args) + .output() + .expect("git"); + assert!( + result.status.success(), + "{}", + String::from_utf8_lossy(&result.stderr) + ); + }; + fs::create_dir(&project).expect("project"); + git(&["init"]); + git(&[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.test", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "initial", + ]); + let settings = home.join("desktop.toml"); + let server = home.join("server.toml"); + let text = "[llm_clients.c]\n\ + base_url='http://localhost:1234'\n\ + [targets.m]\n\ + id='actual'\n\ + llm_client='c'\n\ + [routes.r]\n\ + id='public'\n\ + type='passthrough'\n\ + target='m'\n"; + fs::write(&server, text).expect("server"); + fs::write( + &settings, + format!( + "config_file={server:?}\nrouting_log={:?}\n", + home.join("routing.jsonl") + ), + ) + .expect("settings"); + let mut controller = + Controller::new(Config::load(&settings).expect("settings"), settings.clone()); + let preview = |controller: &mut Controller| { + controller + .dispatch(Action::PreviewLaunch { + tool: Harness::CodexCli, + account: None, + route: "r".into(), + id: "public".into(), + project: project.display().to_string(), + }) + .expect("preview") + }; + let launch = |token: Option, path: String| Action::Launch { + tool: Harness::CodexCli, + account: None, + route: "r".into(), + id: "public".into(), + project: path, + preview_token: token, + }; + let reviewed = preview(&mut controller); + let token = reviewed.data["preview_token"] + .as_str() + .expect("token") + .to_string(); + for rejected in [ + launch(None, project.display().to_string()), + launch(Some("wrong".into()), project.display().to_string()), + launch(Some(token.clone()), home.display().to_string()), + ] { + assert!(controller.dispatch(rejected).is_err()); + } + fs::write(&server, format!("{text}# external edit\n")).expect("edit"); + assert!( + controller + .dispatch(launch(Some(token), project.display().to_string())) + .is_err() + ); + assert_eq!( + fs::read_to_string(&server).expect("preserved"), + format!("{text}# external edit\n") + ); + let reviewed = preview(&mut controller); + let token = reviewed.data["preview_token"] + .as_str() + .expect("token") + .to_string(); + let original = fs::read_to_string(&settings).expect("settings"); + fs::write(&settings, format!("{original}# external settings\n")).expect("edit"); + assert!( + controller + .dispatch(launch(Some(token), project.display().to_string())) + .is_err() + ); + let reviewed = preview(&mut controller); + let token = reviewed.data["preview_token"] + .as_str() + .expect("token") + .to_string(); + git(&[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.test", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "changed", + ]); + assert!( + controller + .dispatch(launch(Some(token), project.display().to_string())) + .err() + .expect("changed HEAD") + .contains("project changed") + ); + assert!(!home.join(".switchyard/worktrees").exists()); + assert!(!home.join("terminal-called").exists()); + assert_eq!(fs::read_dir(&home).expect("files").count(), 4); + return; + } + let home = tempfile::tempdir().expect("fixture"); + let bin = home.path().join("bin"); + fs::create_dir(&bin).expect("bin"); + for name in ["codex", "open", "osascript"] { + let path = bin.join(name); + fs::write( + &path, + "#!/bin/sh\ntouch \"$HOME/terminal-called\"\nexit 0\n", + ) + .expect("stub"); + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).expect("executable"); + } + let output=Command::new(std::env::current_exe().expect("test binary")) + .args(["--exact", "controller::tests::session_launch_requires_an_unchanged_review_before_creating_files"]) + .env("SWITCHYARD_LAUNCH_REVIEW_FIXTURE",home.path()).env("HOME",home.path()) + .env("PATH",format!("{}:/usr/bin:/bin",bin.display())).output().expect("child"); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } #[test] // An algorithm switch must not give stale choices a revision for externally edited bytes. fn algorithm_switches_keep_the_first_revision_and_external_edits_reject_it() { diff --git a/crates/switchyard-desktop/src/sessions.rs b/crates/switchyard-desktop/src/sessions.rs index 9f9455fa6..076d0c32b 100644 --- a/crates/switchyard-desktop/src/sessions.rs +++ b/crates/switchyard-desktop/src/sessions.rs @@ -475,3 +475,357 @@ pub fn launch( worktree.display() )) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn failed_session_setup_removes_its_worktree_branch_and_private_settings() { + let dir = tempfile::tempdir().expect("directory"); + let project = dir.path().join("project"); + std::fs::create_dir(&project).expect("project"); + let git = |args: &[&str]| { + let output = std::process::Command::new("git") + .arg("-C") + .arg(&project) + .args(args) + .output() + .expect("git"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + output + }; + git(&["init"]); + git(&[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "fixture", + ]); + let worktree = dir.path().join("worktree"); + let private = dir.path().join("settings"); + git(&[ + "worktree", + "add", + "-b", + "session-fixture", + worktree.to_str().expect("path"), + ]); + std::fs::create_dir(&private).expect("private"); + std::fs::write(private.join("settings.json"), "{}").expect("settings"); + let revision = String::from_utf8_lossy(&git(&["rev-parse", "HEAD"]).stdout) + .trim() + .to_string(); + // An unrelated HEAD in the original checkout makes `branch -d` refuse the session branch. + git(&["checkout", "--orphan", "unrelated"]); + git(&[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "unrelated", + ]); + drop(PendingSession { + project: project.clone(), + worktree: worktree.clone(), + private: private.clone(), + branch: "session-fixture".into(), + revision, + opened: false, + }); + assert!(!worktree.exists()); + assert!(!private.exists()); + assert!( + !String::from_utf8_lossy(&git(&["branch", "--list", "session-fixture"]).stdout) + .contains("session-fixture") + ); + } + // The subprocess isolates HOME and PATH. + // The Terminal stub injects a conflicting directory when the script must select one. + // Claude's default case requires an unset CLAUDE_CONFIG_DIR, not an explicit ~/.claude. + #[cfg(unix)] + #[test] + fn generated_sessions_use_explicit_accounts_without_changing_user_defaults() { + use std::os::unix::fs::PermissionsExt; + if let Ok(fixture) = std::env::var("SWITCHYARD_SESSION_FIXTURE") { + let fixture = std::path::PathBuf::from(fixture); + let tool = if std::env::var("SWITCHYARD_SESSION_TOOL").expect("tool") == "codex" { + Harness::CodexCli + } else { + Harness::Claude + }; + let account = + std::env::var_os("SWITCHYARD_SESSION_ACCOUNT").map(std::path::PathBuf::from); + let revision = preview(&fixture.join("project")).expect("preview")["revision"] + .as_str() + .expect("revision") + .to_string(); + // Advancing the original checkout makes a launch from HEAD differ from the review. + if tool == Harness::CodexCli && account.is_none() { + let committed = std::process::Command::new("git") + .arg("-C") + .arg(fixture.join("project")) + .args([ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "advanced", + ]) + .output() + .expect("advance HEAD"); + assert!(committed.status.success()); + } + launch( + tool, + &fixture.join("project"), + "route'$(literal)", + "http://localhost:4123", + tool == Harness::Claude, + account.as_deref(), + &revision, + ) + .expect("launch"); + if tool == Harness::CodexCli && account.is_none() { + let records = inventory().expect("inventory"); + assert_eq!(records.len(), 1); + assert_eq!(records[0]["revision"], revision); + assert_ne!( + preview(&fixture.join("project")).expect("advanced project")["revision"], + revision + ); + let id = records[0]["id"].as_str().expect("session id"); + let receipt = session_root() + .expect("session root") + .join(format!("{id}-settings/session.json")); + let original = std::fs::read(&receipt).expect("receipt"); + // Valid JSON roots must still be objects before inventory adds process_status. + for malformed in ["null", "[]", "false", "42"] { + std::fs::write(&receipt, malformed).expect("malformed receipt"); + assert!(inventory().is_err()); + assert!(cleanup_preview(id).is_err()); + } + std::fs::write(&receipt, original).expect("restore receipt"); + let clean = cleanup_preview(id).expect("preview cleanup"); + let worktree = Path::new(records[0]["worktree"].as_str().expect("worktree")); + let unsaved = worktree.join("unsaved-user-work.txt"); + std::fs::write(&unsaved, "keep this work").expect("user work"); + assert!(cleanup(id, &clean).is_err()); + assert!(unsaved.exists()); + let dirty = cleanup_preview(id).expect("dirty preview"); + assert!(cleanup(id, &dirty).is_err()); + assert!(worktree.exists()); + std::fs::remove_file(&unsaved).expect("save fixture work"); + let clean = cleanup_preview(id).expect("clean preview"); + // Cleanup deletes ignored files, so a new ignored file requires a new review. + let info = fixture.join("project/.git/info"); + std::fs::create_dir_all(&info).expect("git info"); + std::fs::write(info.join("exclude"), "ignored-notes.txt\n").expect("exclude"); + std::fs::write(worktree.join("ignored-notes.txt"), "notes").expect("notes"); + assert!(cleanup(id, &clean).is_err()); + assert!(worktree.join("ignored-notes.txt").exists()); + let reviewed = cleanup_preview(id).expect("ignored preview"); + assert_eq!( + reviewed["ignored"], + serde_json::json!(["ignored-notes.txt"]) + ); + // An unrelated HEAD in the original checkout makes `branch -d` refuse the branch. + let project_git = |args: &[&str]| { + let output = std::process::Command::new("git") + .arg("-C") + .arg(fixture.join("project")) + .args(args) + .output() + .expect("git"); + assert!(output.status.success()); + output + }; + project_git(&["checkout", "--orphan", "unrelated"]); + project_git(&[ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "unrelated", + ]); + cleanup(id, &reviewed).expect("safe cleanup"); + assert!(!worktree.exists()); + assert!(inventory().expect("inventory after cleanup").is_empty()); + assert!(project_git(&["branch", "--list", id]).stdout.is_empty()); + } + return; + } + for (name, named, inherited_dir) in [ + ("codex", false, true), + ("codex", true, true), + ("claude", false, true), + ("claude", true, true), + ("claude", false, false), + ("claude", true, false), + ] { + let fixture = tempfile::tempdir().expect("fixture"); + let home = fixture.path().join("home"); + let bin = fixture.path().join("bin"); + let project = fixture.path().join("project"); + for path in [&home, &bin, &project] { + std::fs::create_dir(path).expect("directory"); + } + for args in [ + vec!["init"], + vec![ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-m", + "fixture", + ], + ] { + assert!( + std::process::Command::new("git") + .arg("-C") + .arg(&project) + .args(args) + .output() + .expect("git") + .status + .success() + ); + } + let default = home.join("default-login"); + std::fs::create_dir(&default).expect("default"); + std::fs::write(default.join("settings.json"), "{\"user\":true}").expect("settings"); + let account = home.join(".switchyard/accounts").join(name).join("named"); + if named { + std::fs::create_dir_all(&account).expect("account"); + } + let output = fixture.path().join("arguments"); + let captured = fixture.path().join("session.command"); + // macOS runs the script through `open`; Linux runs it through the program in + // TERMINAL, which receives `-e` before the script path. + let open = bin.join(if cfg!(target_os = "linux") { + "terminal" + } else { + "open" + }); + let terminal_claude = if inherited_dir || named { + "export CLAUDE_CONFIG_DIR=wrong-terminal-login" + } else { + "unset CLAUDE_CONFIG_DIR" + }; + std::fs::write( + &open, + format!( + "#!/bin/bash\n\ + [ \"$1\" = -e ] && shift\n\ + cp -- \"$1\" {}\n\ + export CODEX_HOME=wrong-terminal-login ANTHROPIC_AUTH_TOKEN=wrong-token\n\ + {terminal_claude}\n\ + bash \"$1\"\n", + quote(&captured.display().to_string()) + ), + ) + .expect("open"); + let tool = bin.join(name); + std::fs::write( + &tool, + format!( + "#!/bin/bash\n\ + printf '%s\\n' \"${{CODEX_HOME:-}}\" \"${{CLAUDE_CONFIG_DIR:-}}\" \ + \"${{ANTHROPIC_AUTH_TOKEN:-}}\" \"$@\" > {0}.partial && mv {0}.partial {0}\n", + quote(&output.display().to_string()) + ), + ) + .expect("tool"); + for path in [&open, &tool] { + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)) + .expect("executable"); + } + let mut child = std::process::Command::new(std::env::current_exe().expect("runner")); + child + .args([ + "--exact", + "sessions::tests::\ + generated_sessions_use_explicit_accounts_without_changing_user_defaults", + ]) + .env("SWITCHYARD_SESSION_FIXTURE", fixture.path()) + .env("SWITCHYARD_SESSION_TOOL", name) + .env("HOME", &home) + .env("CODEX_HOME", &default) + .env("CLAUDE_CONFIG_DIR", &default) + .env("PATH", format!("{}:/usr/bin:/bin", bin.display())) + .env("TERMINAL", &open); + if named { + child.env("SWITCHYARD_SESSION_ACCOUNT", &account); + } + if !inherited_dir { + child.env_remove("CLAUDE_CONFIG_DIR"); + } + let result = child.output().expect("child"); + assert!( + result.status.success(), + "{}\n{}", + String::from_utf8_lossy(&result.stdout), + String::from_utf8_lossy(&result.stderr) + ); + // A Linux terminal starts apart from the launch, so the arguments can arrive + // after the child exits. The tool stub renames the file into place when done. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while !output.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(20)); + } + let arguments = std::fs::read_to_string(output).expect("arguments"); + let lines: Vec<_> = arguments.lines().collect(); + let expected = if named { + account.to_str().expect("path") + } else if inherited_dir { + default.to_str().expect("path") + } else { + "" + }; + assert_eq!(lines[usize::from(name == "claude")], expected); + if name == "claude" { + assert_eq!(lines[2], ""); + } + assert!(arguments.contains("route'$(literal)")); + let script = std::fs::read_to_string(captured).expect("script"); + assert!(script.contains("x-switchyard-session-id")); + if name == "claude" && !named && !inherited_dir { + assert!(!script.contains("export CLAUDE_CONFIG_DIR=")); + } + assert_eq!( + std::fs::read_to_string(default.join("settings.json")).expect("settings"), + "{\"user\":true}" + ); + } + } +}