From afe9d5cdb2a827eb9917d9ff8c1407c53d5f97a7 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 17 Sep 2026 10:01:29 -0700 Subject: [PATCH 1/9] docs: correct Linux packaging in BUILDING.md BUILDING.md listed AppImage, .deb and .rpm as build artifacts, but Linux ships as AppImage only -- CI passes `--bundles appimage`, and the .deb and .rpm were dropped because neither could self-update. A bare `npx tauri build` still emits all three, since tauri.conf.json keeps "targets": "all" to leave macOS and Windows untouched, so the table now marks which are actually released rather than pretending the others do not exist. Co-Authored-By: Claude Opus 5 (1M context) --- BUILDING.md | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 2d3580a..d43c786 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -71,13 +71,29 @@ npm ci npx tauri build ``` +Linux ships as **AppImage only**. To match what CI produces, pass the bundle +explicitly: + +```bash +npx tauri build --bundles appimage +``` + +The `.deb` and `.rpm` bundles were dropped — two more artifacts to build and +publish for an audience the AppImage already serves, and neither could +self-update. A bare `npx tauri build` still emits them, because +`tauri.conf.json` keeps `"targets": "all"` so that macOS and Windows are +untouched; they are not released and not tested. + Build artifacts are located in `app/src-tauri/target/release/bundle/`: -| Format | Path | -|------------|-------------------------------| -| AppImage | `appimage/*.AppImage` | -| Debian pkg | `deb/*.deb` | -| RPM pkg | `rpm/*.rpm` | +| Format | Path | Released | +|------------|-------------------------------|----------| +| AppImage | `appimage/*.AppImage` | yes | +| Debian pkg | `deb/*.deb` | no | +| RPM pkg | `rpm/*.rpm` | no | + +`scripts/finalize-appimage.sh` post-processes the AppImage; see the Packaging +section of `CLAUDE.md` for why both of its steps are load-bearing. ## macOS From 90b7e4ccb2911329e6d980aaa5b83adc0d8ad29c Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 17 Sep 2026 10:06:09 -0700 Subject: [PATCH 2/9] fix: default the auth bridge on, and make the browser-view flag durable A CLI running `claude login` inside the container binds a random ephemeral loopback port and hands the provider a redirect pointing at it. The browser is on the host, so the callback lands on a closed host port and the login hangs with no diagnostic. The auth bridge is the thing that fixes this -- it mirrors container loopback listeners onto the same host port -- so having it default to off made a hang the out-of-the-box experience. `auth_bridge_enabled` now defaults to true through a `default_auth_bridge_enabled()` serde helper, matching the shape already used by `use_shared_auth_token`. Because the default is applied at deserialisation, projects stored before the bridge existed pick it up too; `migrate_from_value` writes neither flag, so nothing defeats it, and a regression test pins that. Separately, `BrowserViewManager.enabled` was in-memory only and the durable `browser_view_enabled` field on the project record was never implemented. Rather than sync the two, the cache is removed and the record becomes the single home for the flag, mirroring how `AuthBridgeManager` already works. `stop()` deliberately does not clear it, since container teardown and migration reach that path and neither is the user changing their mind. Durable does not mean auto-started: a restarted app reports enabled with the viewer off. Co-Authored-By: Claude Opus 5 (1M context) --- app/src-tauri/src/browser_view/commands.rs | 53 ++++++-- app/src-tauri/src/browser_view/mod.rs | 113 +++++++++++++----- .../src/commands/project_commands.rs | 9 ++ app/src-tauri/src/models/project.rs | 108 ++++++++++++++++- app/src-tauri/src/storage/projects_store.rs | 72 +++++++++++ 5 files changed, 311 insertions(+), 44 deletions(-) diff --git a/app/src-tauri/src/browser_view/commands.rs b/app/src-tauri/src/browser_view/commands.rs index 0b3a58e..15a0639 100644 --- a/app/src-tauri/src/browser_view/commands.rs +++ b/app/src-tauri/src/browser_view/commands.rs @@ -15,6 +15,12 @@ use crate::AppState; /// non-`Running` status carrying an explanation rather than an error, so the /// pane always has something specific to say. This is host-side only — no /// container recreation is involved either way. +/// +/// Either way the choice is persisted, so it survives an app restart. This is +/// the only caller allowed to write `false`: every other path to +/// [`BrowserViewManager::stop`](crate::browser_view::BrowserViewManager::stop) +/// is a teardown rather than the user changing their mind. Enabling persists +/// inside `start`, which is the single funnel for it. #[tauri::command] pub async fn set_browser_view_enabled( project_id: String, @@ -23,9 +29,15 @@ pub async fn set_browser_view_enabled( state: State<'_, AppState>, ) -> Result { if !enabled { + // Persist first, then tear down: the supervisor's own teardown emit + // reads this flag back out of the store, and reading it mid-stop would + // announce a view that is going away as still enabled. + state + .projects_store + .set_browser_view_enabled(&project_id, false)?; // Awaits the supervisor, so the host port is released before we return. manager().stop(&project_id).await; - return Ok(manager().status(&project_id).await); + return Ok(manager().status(&project_id, false).await); } let container_id = running_container(&state, &project_id, "opening the browser view").await?; @@ -40,10 +52,17 @@ pub async fn set_browser_view_enabled( .await } -/// Current status. Cheap: reads in-process state only, never the container. +/// Current status. Cheap: the session map in this process plus the stored flag, +/// never the container. +/// +/// The two are independent on purpose — this is what the pane reads on mount, +/// and after an app restart the honest answer is "enabled, nothing running". #[tauri::command] -pub async fn get_browser_view_status(project_id: String) -> Result { - Ok(manager().status(&project_id).await) +pub async fn get_browser_view_status( + project_id: String, + state: State<'_, AppState>, +) -> Result { + Ok(manager().status(&project_id, enabled_for(&state, &project_id)).await) } /// Probe the container for Playwright without starting anything. @@ -110,7 +129,9 @@ pub async fn open_browser_view_popout( app_handle: AppHandle, state: State<'_, AppState>, ) -> Result<(), String> { - let status = manager().status(&project_id).await; + let status = manager() + .status(&project_id, enabled_for(&state, &project_id)) + .await; let (BrowserViewState::Running, Some(url)) = (status.state, status.url.as_deref()) else { return Err( "The browser view isn't running. Start it before opening it in its own window." @@ -209,7 +230,9 @@ pub async fn open_page_in_container_browser( // the user to go and press Start in the Browser tab themselves — and from // the terminal's URL prompt, with no indication that was even needed. // Asking for a page *is* asking to watch it, so the viewer comes up too. - let status = manager().status(&project_id).await; + let status = manager() + .status(&project_id, enabled_for(&state, &project_id)) + .await; if status.state != BrowserViewState::Running { crate::commands::project_commands::emit_progress( &app_handle, @@ -229,7 +252,9 @@ pub async fn open_page_in_container_browser( // From the terminal there is no pane on screen to fill, so the page needs a // window of its own or it lands somewhere the user isn't looking. if show_window { - let status = manager().status(&project_id).await; + let status = manager() + .status(&project_id, enabled_for(&state, &project_id)) + .await; if let Some(url) = status.url.as_deref() { let name = state .projects_store @@ -311,6 +336,20 @@ pub async fn get_browser_view_match_window(project_id: String) -> Result, project_id: &str) -> bool { + state + .projects_store + .get(project_id) + .is_some_and(|p| p.browser_view_enabled) +} + /// The project's container, or a sentence saying why there isn't one. /// /// Every command here needs a *running* container, and every one of them used diff --git a/app/src-tauri/src/browser_view/mod.rs b/app/src-tauri/src/browser_view/mod.rs index b816749..dd35dcb 100644 --- a/app/src-tauri/src/browser_view/mod.rs +++ b/app/src-tauri/src/browser_view/mod.rs @@ -34,14 +34,22 @@ //! //! ## Lifecycle //! -//! Off by default and per-project opt-in, exactly like `auth_bridge_enabled`. +//! Off by default and per-project opt-in. The opt-in itself is +//! [`Project::browser_view_enabled`](crate::models::Project), persisted like +//! `auth_bridge_enabled` and read from the store on demand rather than cached +//! here — so the pane comes back the way it was left. What does *not* persist +//! is the session: nothing starts a viewer on app start, so a project left +//! enabled reports `enabled: true` with a state of `Off` until the pane asks +//! for one. That is deliberate, and the reason the flag and the session are +//! separate ideas — see [`BrowserViewManager::status`]. +//! //! One supervisor task per session owns the proxy and the viewer process, and it //! is the only thing that tears them down, so every way a session can end funnels //! through one code path: //! //! | Trigger | Path | //! |---|---| -//! | Turned off in the UI | `set_browser_view_enabled(false)` → [`BrowserViewManager::stop`] | +//! | Turned off in the UI | `set_browser_view_enabled(false)` → persist `false`, then [`BrowserViewManager::stop`] | //! | Container stopped, by the UI or otherwise | supervisor's `is_container_running` check | //! | Project deleted | supervisor's `store.get()` check | //! | Container rebuilt | old container stops → supervisor exits; the new one is not auto-started | @@ -59,7 +67,10 @@ //! orphan is reachable on container loopback only: the host-side port dies with //! the app, and [`crate::auth_bridge::RESERVED_CONTAINER_PORTS`] is a constant //! precisely so the bridge will not mirror an orphan the next time the app -//! starts. The next [`BrowserViewManager::start`] reclaims it. +//! starts. The next [`BrowserViewManager::start`] reclaims it — and since the +//! opt-in is now durable, the restarted app says `enabled` with nothing running, +//! which is exactly the state that invites the user to press the button that +//! reclaims it. Nothing reclaims it on its own, because nothing auto-starts. pub mod commands; pub mod detect; @@ -134,7 +145,10 @@ pub enum BrowserViewState { #[derive(Debug, Clone, Serialize)] pub struct BrowserViewStatus { - /// The per-project opt-in. Off by default. + /// The per-project opt-in, read from the persisted project record. Off by + /// default, and true without a `Running` state whenever the view is turned + /// on but has nothing up — a stopped container, or an app that has just + /// restarted and does not auto-start viewers. pub enabled: bool, pub state: BrowserViewState, /// Fully-formed, token-bearing URL for the pane's iframe. Loopback only. @@ -201,17 +215,20 @@ struct Session { type SessionMap = Arc>>; +/// Live sessions, and nothing else. +/// +/// The per-project opt-in deliberately is **not** a field here. It lives on +/// the project record as +/// [`browser_view_enabled`](crate::models::Project::browser_view_enabled) and +/// is read from [`ProjectsStore`] at each use, exactly as +/// [`crate::auth_bridge::AuthBridgeManager`] treats `auth_bridge_enabled`: +/// one copy, durable across a restart, and impossible to get out of step with +/// what the Config tab shows. A cached copy here was the previous design and +/// its only observable behaviour was forgetting the user's choice on every +/// app start. #[derive(Default)] pub struct BrowserViewManager { sessions: SessionMap, - /// The per-project opt-in. - /// - /// NOTE: in memory only, so it does not survive an app restart. The durable - /// home for this is a `browser_view_enabled: bool` field on - /// `models::Project` (see the report) — `models/project.rs` is out of scope - /// for this change, so the flag lives here and the wiring is otherwise - /// identical to `auth_bridge_enabled`. - enabled: Mutex>, next_epoch: AtomicU64, } @@ -226,22 +243,15 @@ pub fn manager() -> &'static Arc { } impl BrowserViewManager { - pub async fn is_enabled(&self, project_id: &str) -> bool { - self.enabled.lock().await.contains(project_id) - } - - async fn set_enabled(&self, project_id: &str, enabled: bool) { - let mut set = self.enabled.lock().await; - if enabled { - set.insert(project_id.to_string()); - } else { - set.remove(project_id); - } - } - /// Current status without touching the container. - pub async fn status(&self, project_id: &str) -> BrowserViewStatus { - let enabled = self.is_enabled(project_id).await; + /// + /// `enabled` is passed in rather than looked up, the way + /// [`crate::auth_bridge::AuthBridgeManager::status`] takes it: the flag is + /// the caller's to read from the store, and keeping it out of here is what + /// stops a second copy of it appearing. A project whose view is enabled but + /// whose container is stopped — or whose app has just restarted — reports + /// `enabled: true` with a state of `Off`, which is the honest answer. + pub async fn status(&self, project_id: &str, enabled: bool) -> BrowserViewStatus { match self.sessions.lock().await.get(project_id) { Some(session) => BrowserViewStatus { enabled, @@ -261,6 +271,14 @@ impl BrowserViewManager { /// /// Idempotent: a call while a live session exists returns that session's /// status untouched, so re-opening the tab does not restart the dashboard. + /// + /// This is the single funnel for turning the view **on**, so it is also + /// where the durable flag is written — both call sites (the toggle and + /// `open_page_in_container_browser`, which opens a page and then shows it) + /// mean "on", and neither can forget. The **off** direction is not + /// symmetric and must not be: [`Self::stop`] is reached by teardown paths + /// that are not the user changing their mind, so the command owns that + /// write. See [`Self::stop`]. pub async fn start( &self, project_id: String, @@ -268,7 +286,7 @@ impl BrowserViewManager { app: AppHandle, store: Arc, ) -> Result { - self.set_enabled(&project_id, true).await; + store.set_browser_view_enabled(&project_id, true)?; // Bind the answer before acting on it: `status()` takes the same lock, // and this mutex is not reentrant. @@ -279,7 +297,7 @@ impl BrowserViewManager { .get(&project_id) .is_some_and(|s| !s.supervisor.is_finished()); if already_live { - return Ok(self.status(&project_id).await); + return Ok(self.status(&project_id, true).await); } let detection = detect::detect(&container_id).await?; @@ -364,14 +382,21 @@ impl BrowserViewManager { }, ); - let status = self.status(&project_id).await; + let status = self.status(&project_id, true).await; emit(&app, &project_id, &status); Ok(status) } /// Stop one project's view and wait until its host port has been released. + /// + /// Tears the *session* down and deliberately leaves the durable flag alone. + /// Most callers are not the user turning the feature off — a migration + /// removes the container out from under a running view + /// (`migration_commands`), and the container can stop for any other reason + /// — and persisting `false` for those would quietly opt the project out of + /// a feature it never asked to lose. `set_browser_view_enabled(false)` is + /// the one caller that means it, and it writes the flag itself first. pub async fn stop(&self, project_id: &str) { - self.set_enabled(project_id, false).await; // Remove under the lock, then release it before awaiting: the // supervisor takes the same lock to deregister itself on exit. let session = self.sessions.lock().await.remove(project_id); @@ -483,7 +508,12 @@ async fn supervise( // longer exists. The session owns it, and this is where the session ends. let _ = popout::close(&app, &project_id); - let enabled = manager().is_enabled(&project_id).await; + // Straight from the store, like the auth bridge's own teardown emit: the + // session is over, but the project may well still be opted in — a stopped + // container is not a changed mind, and the pane has to show the difference. + let enabled = store + .get(&project_id) + .is_some_and(|p| p.browser_view_enabled); emit(&app, &project_id, &BrowserViewStatus::off(enabled)); } @@ -915,6 +945,25 @@ mod tests { assert!(s.url.is_none()); } + #[tokio::test] + async fn the_opt_in_and_the_live_session_are_separate_answers() { + let manager = BrowserViewManager::default(); + + // Exactly what the pane reads on mount after an app restart of a + // project that was left enabled: the durable flag says on, and nothing + // auto-starts, so the state is honestly `Off`. The old in-memory flag + // could not express this — it came back `false` and the pane silently + // showed the feature as never having been turned on. + let status = manager.status("p1", true).await; + assert!(status.enabled); + assert_eq!(status.state, BrowserViewState::Off); + assert!(status.url.is_none()); + + // The flag belongs to the caller, read from the store. The manager + // keeps no copy, so it has nothing to contradict it with. + assert!(!manager.status("p1", false).await.enabled); + } + #[test] fn an_unavailable_status_keeps_the_detail_the_user_needs() { let mut d = PlaywrightDetection::default(); diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs index ac93bfe..25912c4 100644 --- a/app/src-tauri/src/commands/project_commands.rs +++ b/app/src-tauri/src/commands/project_commands.rs @@ -1100,6 +1100,15 @@ pub async fn update_project( project.container_id = stored.container_id; project.status = stored.status; + // `browser_view_enabled` is owned by `set_browser_view_enabled` and is + // restored here rather than taken from the payload, exactly like + // `container_id` and `status` above. The Config tab has no control for it + // — the Browser tab's toggle is the only way it ever changes — so the + // project object the frontend round-trips carries whatever it was told at + // load time and would silently undo a toggle made since. `auth_bridge_enabled` + // is different and does arrive through this save: the Config tab edits it, + // which is why the reconcile below follows whatever was just persisted. + project.browser_view_enabled = stored.browser_view_enabled; project.created_at = stored.created_at; project.updated_at = chrono::Utc::now().to_rfc3339(); diff --git a/app/src-tauri/src/models/project.rs b/app/src-tauri/src/models/project.rs index 5c22e15..70905fd 100644 --- a/app/src-tauri/src/models/project.rs +++ b/app/src-tauri/src/models/project.rs @@ -132,6 +132,26 @@ fn default_use_shared_auth_token() -> bool { true } +/// `auth_bridge_enabled` defaults to **on**, and the default is what makes +/// `claude login` work at all. +/// +/// The login flow binds a *random* ephemeral loopback port inside the +/// container and then sends the host's browser to `127.0.0.1:`. +/// On the host nothing is listening there, so the callback lands on a closed +/// port and the CLI waits for a redirect that can never arrive. The bridge +/// mirrors the container's loopback listeners onto the same host port, which +/// is the only thing that closes that loop — so off-by-default made a hang the +/// out-of-the-box experience. +/// +/// Returning `true` from a `#[serde(default)]` helper (rather than flipping the +/// constructor alone) is deliberate: existing `projects.json` records were +/// written before this field existed, or while it was off, and an absent key is +/// what the default is read for. A project that wants the old behaviour turns +/// the toggle off, which persists an explicit `false`. +fn default_auth_bridge_enabled() -> bool { + true +} + /// How much autonomy Claude Code is granted inside the container. /// /// Maps onto Claude Code CLI flags — see [`PermissionMode::cli_args`], which is @@ -336,17 +356,30 @@ pub struct Project { pub sandbox_mode_enabled: bool, #[serde(default)] pub mission_control_enabled: bool, - /// Opt in to the auth bridge: while the container runs, its loopback - /// listeners are mirrored onto the host's loopback so browser OAuth - /// callbacks (`claude login`, `fly login`, `aws sso login`) can reach them. + /// The auth bridge: while the container runs, its loopback listeners are + /// mirrored onto the host's loopback so browser OAuth callbacks + /// (`claude login`, `fly login`, `aws sso login`) can reach them. /// Purely host-side — it deliberately has no container-recreation label, /// because toggling it changes nothing about the container itself. - #[serde(default)] + /// + /// **On by default**, and opt-*out* rather than opt-in — see + /// [`default_auth_bridge_enabled`] for why the default is the feature. + #[serde(default = "default_auth_bridge_enabled")] pub auth_bridge_enabled: bool, /// Opt in to the browser-view pane, which watches and takes over the /// browser Claude drives with Playwright inside the container. Purely /// host-side like `auth_bridge_enabled`, so it likewise has no /// container-recreation label. + /// + /// This is the *durable* home of the flag: `BrowserViewManager` reads it + /// rather than keeping its own copy, so the pane comes back the way it was + /// left. Off by default, and unlike the auth bridge it stays that way — a + /// view costs a container exec, a Node daemon and a host port, and a + /// container without Playwright cannot serve one at all. + /// + /// Durable does **not** mean auto-started: nothing brings a viewer up on + /// app start, so a project left enabled reports `enabled` with a state of + /// `Off` until the pane (or `open_page_in_container_browser`) asks for one. #[serde(default)] pub browser_view_enabled: bool, /// Grant the container what a VPN client needs to build a tunnel: @@ -639,7 +672,7 @@ impl Project { allow_docker_access: false, sandbox_mode_enabled: false, mission_control_enabled: false, - auth_bridge_enabled: false, + auth_bridge_enabled: default_auth_bridge_enabled(), browser_view_enabled: false, vpn_support_enabled: false, use_shared_auth_token: default_use_shared_auth_token(), @@ -885,4 +918,69 @@ mod tests { let round_tripped: ClaudeCodeSettings = serde_json::from_str(&json).unwrap(); assert_eq!(round_tripped, partial); } + + // ── The host-side per-project toggles ───────────────────────────────── + + #[test] + fn a_project_stored_before_the_auth_bridge_existed_gets_it_turned_on() { + // The whole point of the serde default: `MAIN_SHAPE_PROJECT` is a real + // record written by a shipped binary and has no `auth_bridge_enabled` + // key at all. Without this, every existing project keeps hanging on + // `claude login` until its owner finds the toggle. + assert!(!MAIN_SHAPE_PROJECT.contains("auth_bridge_enabled")); + let project: Project = serde_json::from_str(MAIN_SHAPE_PROJECT).unwrap(); + assert!(project.auth_bridge_enabled); + + // The browser view is the other way round and must stay so: it costs a + // Node daemon, a container exec loop and a host port, and most + // containers have no Playwright to serve it with. + assert!(!project.browser_view_enabled); + } + + #[test] + fn turning_the_auth_bridge_off_survives_the_default() { + // Opt-out has to be expressible, or the toggle does nothing across a + // restart. An explicit `false` in the file beats the default. + let json = r#"{ "auth_bridge_enabled": false }"#; + #[derive(Deserialize)] + struct JustTheFlag { + #[serde(default = "default_auth_bridge_enabled")] + auth_bridge_enabled: bool, + } + let parsed: JustTheFlag = serde_json::from_str(json).unwrap(); + assert!(!parsed.auth_bridge_enabled); + + // And a saved project always writes the key, so the choice is pinned + // rather than re-defaulted on the next load. + let mut p = Project::new("demo".to_string(), Vec::new()); + p.auth_bridge_enabled = false; + let round_tripped: Project = + serde_json::from_str(&serde_json::to_string(&p).unwrap()).unwrap(); + assert!(!round_tripped.auth_bridge_enabled); + } + + #[test] + fn a_new_project_starts_with_the_bridge_on_and_the_view_off() { + let p = Project::new("demo".to_string(), Vec::new()); + assert!(p.auth_bridge_enabled); + assert!(!p.browser_view_enabled); + } + + #[test] + fn the_path_migration_never_writes_the_flags_and_so_cannot_defeat_the_default() { + // `ProjectsStore::new` runs every record through this before + // deserialising. If it inserted either key — even as `false` — the + // serde default above would never be consulted for an existing project + // and this change would be a no-op on exactly the projects it is for. + let legacy = serde_json::json!({ + "id": "p1", + "name": "demo", + "path": "/home/u/demo", + }); + let migrated = Project::migrate_from_value(legacy); + let obj = migrated.as_object().unwrap(); + assert!(obj.contains_key("paths"), "the migration should still do its own job"); + assert!(!obj.contains_key("auth_bridge_enabled")); + assert!(!obj.contains_key("browser_view_enabled")); + } } diff --git a/app/src-tauri/src/storage/projects_store.rs b/app/src-tauri/src/storage/projects_store.rs index e3ed311..338da50 100644 --- a/app/src-tauri/src/storage/projects_store.rs +++ b/app/src-tauri/src/storage/projects_store.rs @@ -241,6 +241,21 @@ impl ProjectsStore { } } + /// Granular setter for the browser view's opt-in, for the same reason + /// [`Self::set_auth_bridge_enabled`] has one: the pane toggles this while + /// the Config tab may be holding an older copy of the whole record. + pub fn set_browser_view_enabled(&self, project_id: &str, enabled: bool) -> Result<(), String> { + let mut projects = self.lock(); + if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) { + p.browser_view_enabled = enabled; + p.updated_at = chrono::Utc::now().to_rfc3339(); + self.save(&projects)?; + Ok(()) + } else { + Err(format!("Project {} not found", project_id)) + } + } + pub fn set_container_id(&self, project_id: &str, container_id: Option) -> Result<(), String> { let mut projects = self.lock(); if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) { @@ -338,4 +353,61 @@ mod tests { fs::remove_dir_all(&dir).ok(); } + + /// A store over a temp file. `new()` insists on `dirs::data_dir()`, which + /// is the real user's; the fields are right here, so the granular setters + /// can be exercised against a directory the test owns. + fn store_over(dir: &Path, projects: Vec) -> ProjectsStore { + ProjectsStore { + projects: Mutex::new(projects), + file_path: dir.join("projects.json"), + } + } + + #[test] + fn the_browser_view_flag_is_written_to_disk_and_read_back() { + // The point of the whole exercise: before this the flag lived in a + // `HashSet` in `BrowserViewManager` and an app restart forgot it. + let dir = temp_dir("browser-view"); + let project = Project::new("demo".to_string(), Vec::new()); + let id = project.id.clone(); + let store = store_over(&dir, vec![project]); + + assert!(!store.get(&id).unwrap().browser_view_enabled); + store.set_browser_view_enabled(&id, true).unwrap(); + assert!(store.get(&id).unwrap().browser_view_enabled); + + // Durable, not merely in memory — this is what a restart reads. + let on_disk: Vec = + serde_json::from_str(&fs::read_to_string(dir.join("projects.json")).unwrap()).unwrap(); + assert!(on_disk[0].browser_view_enabled); + + store.set_browser_view_enabled(&id, false).unwrap(); + assert!(!store.get(&id).unwrap().browser_view_enabled); + + assert!(store.set_browser_view_enabled("no-such-project", true).is_err()); + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_granular_toggle_leaves_every_other_field_alone() { + // Why these setters exist at all: the Config tab can be holding an + // older copy of the whole record while the pane flips one flag. + let dir = temp_dir("granular"); + let mut project = Project::new("demo".to_string(), Vec::new()); + project.claude_instructions = Some("keep me".to_string()); + let id = project.id.clone(); + let store = store_over(&dir, vec![project]); + + store.set_browser_view_enabled(&id, true).unwrap(); + store.set_auth_bridge_enabled(&id, false).unwrap(); + + let saved = store.get(&id).unwrap(); + assert_eq!(saved.claude_instructions.as_deref(), Some("keep me")); + assert!(saved.browser_view_enabled); + assert!(!saved.auth_bridge_enabled); + + fs::remove_dir_all(&dir).ok(); + } } From bf8094dbc47f972b8d1cd3cf2ac3088489ff0161 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 17 Sep 2026 10:07:50 -0700 Subject: [PATCH 3/9] fix: route sign-in links by what can actually catch the callback `isAnthropicSignInUrl` made the container the default action for every Anthropic sign-in link, justified by "the host has nothing to catch it with". That was wrong in both directions. The host does have something -- the auth bridge -- and the container side is not a general browser at all but Playwright's dashboard, whose packages and chromium are deliberately not baked into the image. So the default pointed at the one path that is uninstalled on a fresh project, on every platform, while the path that works sat behind a switch. The decision now lives in `useSignInOpenTarget`: a live auth bridge picks the host, otherwise a container that can actually launch a browser picks the container, otherwise the host. It resolves at mount rather than when a URL arrives, so the buttons do not swap under a moving mouse, and it re-decides on `auth-bridge-changed` so flipping the switch during a hanging login takes effect. A bridge with port conflicts reads as not live; an empty `active_ports` does not, since there is nothing to bridge until the CLI binds its listener and that races the URL. Both buttons still render either way -- this changes which one leads. `sanitizeRelayUrl` is byte-for-byte unchanged, so the embedded copy in web_terminal/terminal.html needs no matching edit. The host "Open" path also failed silently: `dismissUrlPrompt()` ran before `openUrl`, so the toast vanished and a rejected promise reached only the devtools console. Dismissal now happens on success only, leaving "In container" one click away after a failure, and the error surfaces through the same toast the container path already used. On Linux this catch will not fire for the common case -- `xdg-open` routinely exits 0 having done nothing -- so it complements the AppImage environment fix rather than replacing it. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/projects/home/BrowserTab.tsx | 10 +- .../components/terminal/TerminalView.test.tsx | 236 ++++++++++++++++++ app/src/components/terminal/TerminalView.tsx | 57 ++++- app/src/components/terminal/UrlToast.test.tsx | 56 ++++- app/src/components/terminal/UrlToast.tsx | 56 +++-- app/src/hooks/useSignInOpenTarget.ts | 176 +++++++++++++ app/src/lib/browserViewSupport.ts | 53 ++++ app/src/lib/urlRelay.test.ts | 51 ++++ app/src/lib/urlRelay.ts | 21 +- 9 files changed, 677 insertions(+), 39 deletions(-) create mode 100644 app/src/hooks/useSignInOpenTarget.ts create mode 100644 app/src/lib/browserViewSupport.ts diff --git a/app/src/components/projects/home/BrowserTab.tsx b/app/src/components/projects/home/BrowserTab.tsx index 8e5aac3..1fe5a80 100644 --- a/app/src/components/projects/home/BrowserTab.tsx +++ b/app/src/components/projects/home/BrowserTab.tsx @@ -23,6 +23,7 @@ import { setBrowserViewMatchWindow, setBrowserViewPopoutAlwaysOnTop, } from "../../../lib/tauri-commands"; +import { isBrowserViewUsable } from "../../../lib/browserViewSupport"; import { useAppState } from "../../../store/appState"; import OpenPageDialog from "./OpenPageDialog"; import AccordionSection from "../../ui/AccordionSection"; @@ -338,7 +339,7 @@ export default function BrowserTab({ project, active }: Props) { // Prefer the probe: it is the fresher of the two, and it is the one that // reflects an install that just finished. const probed = detection ?? status.detection; - const ready = isUsable(probed); + const ready = isBrowserViewUsable(probed); // Mirrors Rust `PlaywrightDetection::needs_browser`: the Chrome channel is an // apt package, so it never shows up in `browsers`, and a container that has // it is not missing a browser. @@ -539,11 +540,6 @@ export default function BrowserTab({ project, active }: Props) { ); } -/** Mirrors Rust `PlaywrightDetection::is_usable`. */ -function isUsable(d: PlaywrightDetection | null): boolean { - return d !== null && d.playwright_version !== null && d.has_bind && d.cli_entry !== null; -} - /** * Mirrors Rust `PlaywrightDetection::revision_skew`. * @@ -627,7 +623,7 @@ function Setup({ onInstall: (which: Exclude) => void; }) { const busy = job !== null; - const havePackages = isUsable(detection); + const havePackages = isBrowserViewUsable(detection); const missing = missingParts(detection); const browsers = detection?.browsers ?? []; const chrome = detection?.chrome_channel ?? null; diff --git a/app/src/components/terminal/TerminalView.test.tsx b/app/src/components/terminal/TerminalView.test.tsx index 5f71ad0..093d471 100644 --- a/app/src/components/terminal/TerminalView.test.tsx +++ b/app/src/components/terminal/TerminalView.test.tsx @@ -3,6 +3,12 @@ import { render, fireEvent, cleanup, act } from "@testing-library/react"; import TerminalView, { supersedes } from "./TerminalView"; import { useAppState } from "../../store/appState"; import { uploadHostFileToTerminal } from "../../lib/tauri-commands"; +import { openUrl } from "@tauri-apps/plugin-opener"; +import { + chooseSignInTarget, + resetBrowserSupportCache, +} from "../../hooks/useSignInOpenTarget"; +import type { AuthBridgeStatus, PlaywrightDetection } from "../../lib/types"; import { URL_TOAST_SELECTOR } from "./UrlToast"; /** @@ -16,6 +22,18 @@ const dragDrop = vi.hoisted(() => ({ handler: null as null | ((event: unknown) => unknown), })); +/** + * What the project's container answers about itself. + * + * `TerminalView` asks two questions on mount — is the auth bridge live, and is + * there a browser inside to open a page in — because together they decide which + * of the URL toast's two buttons leads for a sign-in link. + */ +const containerEnv = vi.hoisted(() => ({ + bridge: { enabled: false, active_ports: [], conflicts: [] } as unknown, + detection: null as unknown, +})); + /** The `terminal-output-{id}` listeners, so a test can be the PTY. */ const ptyOutput = vi.hoisted(() => ({ listeners: new Map void>(), @@ -45,6 +63,8 @@ vi.mock("../../lib/tauri-commands", () => ({ awsSsoRefresh: vi.fn(async () => {}), openPageInContainerBrowser: vi.fn(async () => ({ error: null })), uploadHostFileToTerminal: vi.fn(async () => ""), + getAuthBridgeStatus: vi.fn(async () => containerEnv.bridge), + checkBrowserViewSupport: vi.fn(async () => containerEnv.detection), })); vi.mock("@tauri-apps/api/event", () => ({ @@ -128,6 +148,14 @@ beforeEach(() => { vi.mocked(uploadHostFileToTerminal).mockResolvedValue("/workspace/api/dropped.txt"); dragDrop.handler = null; ptyOutput.listeners.clear(); + vi.mocked(openUrl).mockReset(); + vi.mocked(openUrl).mockResolvedValue(undefined); + containerEnv.bridge = { enabled: false, active_ports: [], conflicts: [] }; + containerEnv.detection = null; + // The Playwright probe is memoized across mounts (it is a container exec), so + // a case that changes the answer has to drop what an earlier one cached. + resetBrowserSupportCache(); + useAppState.setState({ toasts: [] }); document.body.innerHTML = ""; useAppState.setState({ sessions: [] }); }); @@ -559,6 +587,214 @@ describe("TerminalView — reaching the URL prompt without a mouse", () => { }); }); +/** + * A container with Playwright *and* a browser in the cache — i.e. one where + * "In container" would actually open something. + */ +function usableDetection( + over: Partial = {}, +): PlaywrightDetection { + return { + node_version: "v22.11.0", + playwright_version: "1.56.0", + playwright_path: "/workspace/node_modules/playwright", + playwright_cli: "/workspace/node_modules/playwright/cli.js", + has_bind: true, + cli_version: "1.56.0", + cli_entry: "/workspace/node_modules/@playwright/cli/index.js", + browsers: ["chromium-1200"], + chrome_channel: null, + chromium_executable: "/home/claude/.cache/ms-playwright/chromium-1200/chrome", + chromium_executable_exists: true, + script_playwright_version: "1.56.0", + script_chromium_executable: null, + script_chromium_executable_exists: false, + searched: [], + ...over, + }; +} + +const LIVE_BRIDGE: AuthBridgeStatus = { + enabled: true, + active_ports: [], + conflicts: [], +}; + +describe("chooseSignInTarget — which action leads for a sign-in link", () => { + // The rule this replaced was "container, always", justified by the callback + // listener living inside the container. Both halves of that justification + // stopped being true: the auth bridge mirrors that listener onto the host, + // and the container-side target is Playwright's pane, whose browsers are not + // in the image. + it("prefers the host browser whenever the bridge is live", () => { + expect(chooseSignInTarget(LIVE_BRIDGE, usableDetection())).toBe("host"); + }); + + it("does not call a bridge live while it is holding a port conflict", () => { + // Enabled and unable to catch the callback anyway — the one state where + // "on" must not read as "will work". + const conflicted: AuthBridgeStatus = { + enabled: true, + active_ports: [], + conflicts: [{ port: 54545, reason: "already in use on the host" }], + }; + expect(chooseSignInTarget(conflicted, usableDetection())).toBe("container"); + }); + + it("does not wait for a bridged port before trusting an enabled bridge", () => { + // There is nothing to bridge until the CLI binds its listener, and that + // races the URL reaching the transcript. Requiring a port would make the + // default flip between two identical sign-ins. + expect(chooseSignInTarget(LIVE_BRIDGE, null)).toBe("host"); + }); + + it("falls to the container only when it has a browser to open", () => { + const off: AuthBridgeStatus = { enabled: false, active_ports: [], conflicts: [] }; + expect(chooseSignInTarget(off, usableDetection())).toBe("container"); + expect(chooseSignInTarget(off, null)).toBe("host"); + // Packages installed, cache empty — the fresh-project state, and the one + // that used to be the silent default. + expect( + chooseSignInTarget( + off, + usableDetection({ browsers: [], chromium_executable_exists: false }), + ), + ).toBe("host"); + // Playwright too old to bind: the pane cannot show it either. + expect(chooseSignInTarget(off, usableDetection({ has_bind: false }))).toBe("host"); + }); + + it("answers host when nothing is known at all", () => { + expect(chooseSignInTarget(null, null)).toBe("host"); + }); +}); + +describe("TerminalView — the sign-in default follows the project", () => { + const SIGN_IN = + "https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code"; + + function relaySequence(url: string): number[] { + return Array.from( + new TextEncoder().encode(`\x1b]7777;open;${btoa(url)}\x07`), + ); + } + + async function mountWithPrompt() { + const view = mountSession("claude"); + await act(async () => {}); + const emit = ptyOutput.listeners.get("terminal-output-s1"); + if (!emit) throw new Error("no terminal-output listener registered"); + await act(async () => { + emit({ payload: relaySequence(SIGN_IN) }); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + }); + return view; + } + + function primaryLabel(): string | null { + return document.querySelector( + '[data-url-toast-primary="true"]', + )?.textContent ?? null; + } + + function actionOrder(): (string | null)[] { + return Array.from(document.querySelectorAll("button")) + .map((b) => b.textContent) + .filter((t) => t === "Open" || t === "In container"); + } + + it("leads with the host browser when the auth bridge is on", async () => { + containerEnv.bridge = LIVE_BRIDGE; + containerEnv.detection = usableDetection(); + await mountWithPrompt(); + expect(primaryLabel()).toBe("Open"); + // Both are still offered — this changes which leads, never which exist. + expect(actionOrder()).toEqual(["Open", "In container"]); + }); + + it("leads with the container when the bridge is off and a browser is there", async () => { + containerEnv.detection = usableDetection(); + await mountWithPrompt(); + expect(primaryLabel()).toBe("In container"); + expect(actionOrder()).toEqual(["In container", "Open"]); + }); + + it("leads with the host on a fresh project, where neither is set up", async () => { + // Playwright is deliberately not baked into the image, so this is what a + // project looks like until someone presses install — and pointing the + // default at it failed on every platform, silently. + await mountWithPrompt(); + expect(primaryLabel()).toBe("Open"); + }); +}); + +describe("TerminalView — a host open that fails says so", () => { + const URL = "https://github.com/login/device?code=ABCD-EFGH"; + + function relaySequence(url: string): number[] { + return Array.from( + new TextEncoder().encode(`\x1b]7777;open;${btoa(url)}\x07`), + ); + } + + async function mountWithPrompt() { + const view = mountSession("claude"); + await act(async () => {}); + const emit = ptyOutput.listeners.get("terminal-output-s1"); + if (!emit) throw new Error("no terminal-output listener registered"); + await act(async () => { + emit({ payload: relaySequence(URL) }); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + }); + return view; + } + + function openButton(): HTMLElement { + const el = Array.from(document.querySelectorAll("button")).find( + (b) => b.textContent === "Open", + ); + if (!el) throw new Error("Open button not found"); + return el as HTMLElement; + } + + it("pushes a toast instead of a console line nobody reads", async () => { + vi.mocked(openUrl).mockRejectedValueOnce(new Error("no opener")); + await mountWithPrompt(); + await act(async () => { + fireEvent.click(openButton()); + await Promise.resolve(); + }); + const toasts = useAppState.getState().toasts; + expect(toasts).toHaveLength(1); + expect(toasts[0].kind).toBe("error"); + expect(toasts[0].detail).toContain("no opener"); + }); + + it("keeps the prompt on screen, so the other route is still one click away", async () => { + // Dismissing first is what this replaced: the toast vanished, nothing + // opened, and the URL only existed in the container's transcript. + vi.mocked(openUrl).mockRejectedValueOnce(new Error("no opener")); + await mountWithPrompt(); + await act(async () => { + fireEvent.click(openButton()); + await Promise.resolve(); + }); + expect(document.querySelector(URL_TOAST_SELECTOR)).not.toBeNull(); + }); + + it("dismisses the prompt once the handoff actually succeeded", async () => { + await mountWithPrompt(); + await act(async () => { + fireEvent.click(openButton()); + await Promise.resolve(); + }); + expect(openUrl).toHaveBeenCalledWith(URL); + expect(document.querySelector(URL_TOAST_SELECTOR)).toBeNull(); + }); +}); + describe("TerminalView — focus on request", () => { /** Mount, then deliberately give focus away, so what the assertions below * observe is the *request* taking effect and never the focus `active` diff --git a/app/src/components/terminal/TerminalView.tsx b/app/src/components/terminal/TerminalView.tsx index ff17972..7f5cba2 100644 --- a/app/src/components/terminal/TerminalView.tsx +++ b/app/src/components/terminal/TerminalView.tsx @@ -23,6 +23,7 @@ import { sanitizeRelayUrl, } from "../../lib/urlRelay"; import { classifyDrop, DROP_BLOCKED_TOAST } from "../../lib/dropTarget"; +import { useSignInOpenTarget } from "../../hooks/useSignInOpenTarget"; import UrlToast, { URL_TOAST_PRIMARY_SELECTOR, URL_TOAST_SELECTOR, @@ -409,7 +410,18 @@ export default function TerminalView({ sessionId, active }: Props) { console.warn("Refusing to open a link that failed validation"); return; } - openUrl(safe).catch((e) => console.error("Failed to open URL:", e)); + // Same failure reporting as the toast's Open button — see the long note + // on `handleOpenUrl`, including what this catch does *not* catch on + // Linux. A click that appears to do nothing is the complaint either way. + openUrl(safe).catch((e) => + useAppState.getState().pushToast({ + kind: "error", + message: "Could not open that link in your browser", + detail: String(e), + // A dead opener fails for every link in the buffer. One card. + dedupeKey: "host-open-failed", + }), + ); }, { urlRegex }); term.loadAddon(webLinksAddon); @@ -786,20 +798,58 @@ export default function TerminalView({ sessionId, active }: Props) { return () => clearTimeout(timer); }, [imagePasteMsg]); + /** + * Hand the prompted URL to the host's browser. + * + * Two things here are ordering, not decoration: + * + * - **The toast is dismissed on success only.** It used to go first, so a + * failed open left the user with an empty screen and no way back to a URL + * that only exists in the container's transcript. Now a failure keeps the + * prompt exactly where it was, which also leaves "In container" one click + * away — the fallback this failure is the argument for. + * - **The failure is a toast, not a `console.error`.** Same `pushToast` the + * container-browser branch below uses, because from the user's side the + * two actions fail identically: nothing happens. + * + * What this does *not* cover, and must not be described as covering: on Linux + * `xdg-open` routinely exits 0 having done nothing useful, so the most common + * Linux failure resolves this promise and reports success. Stripping the + * leaked AppImage environment before the browser is spawned is what addresses + * that; this is the complement that catches everything which does report. + */ const handleOpenUrl = useCallback(() => { if (!urlPrompt) return; // Validated again at the sink. `promptUrl` is the only writer and already // sanitizes, so this can only fail if that invariant is broken — which is // precisely when it matters that the last thing before `openUrl` checks. const safe = sanitizeRelayUrl(urlPrompt.url); - dismissUrlPrompt(); if (!safe) { console.warn("Refusing to open a URL that failed validation"); + dismissUrlPrompt(); return; } - openUrl(safe).catch((e) => console.error("Failed to open URL:", e)); + openUrl(safe) + .then(() => dismissUrlPrompt()) + .catch((e) => + useAppState.getState().pushToast({ + kind: "error", + message: "Could not open it in your browser", + detail: String(e), + dedupeKey: "host-open-failed", + }), + ); }, [urlPrompt, dismissUrlPrompt]); + /** + * Which action leads when the prompt is holding an Anthropic sign-in link. + * + * Resolved per project, not per URL — see `useSignInOpenTarget`. The toast + * offers both regardless; this is only which one is filled in and reachable + * with {@link URL_TOAST_SHORTCUT}. + */ + const signInDefault = useSignInOpenTarget(projectId); + /** * Open the prompted URL in the container's own browser instead of the host's. * @@ -896,6 +946,7 @@ export default function TerminalView({ sessionId, active }: Props) { label={urlPrompt.label} onOpen={handleOpenUrl} onOpenInContainer={handleOpenUrlInContainer} + signInDefault={signInDefault} onDismiss={dismissUrlPrompt} /> )} diff --git a/app/src/components/terminal/UrlToast.test.tsx b/app/src/components/terminal/UrlToast.test.tsx index ad705d0..60e68d1 100644 --- a/app/src/components/terminal/UrlToast.test.tsx +++ b/app/src/components/terminal/UrlToast.test.tsx @@ -105,6 +105,7 @@ describe("UrlToast", () => { url={SIGN_IN} onOpen={noop} onOpenInContainer={noop} + signInDefault="container" onDismiss={noop} />, ); @@ -150,6 +151,7 @@ describe("UrlToast", () => { url={SIGN_IN} onOpen={noop} onOpenInContainer={noop} + signInDefault="container" onDismiss={noop} />, ); @@ -166,10 +168,11 @@ describe("UrlToast", () => { describe("Anthropic sign-in links", () => { // The callback listener a `claude login` is waiting on is *inside* the - // container. Sending the user to their host browser completes the sign-in - // and then posts the result where nothing is listening, and the terminal - // hangs to its timeout — so for these, and only these, the container-side - // browser leads. + // container, so a sign-in is the one case where the host browser may be the + // wrong lead. Whether it actually is depends on the project — a live auth + // bridge carries the callback back, and the container-side alternative is + // not installed on a fresh project — so the owner decides and passes + // `signInDefault`. This component only renders the decision. const SIGN_IN = "https://claude.ai/oauth/authorize?code=true&client_id=abc&response_type=code"; @@ -180,12 +183,13 @@ describe("UrlToast", () => { .filter((t) => t === "Open" || t === "In container"); } - it("puts the container browser first", () => { + it("puts the container browser first when the caller asks for it", () => { render( , ); @@ -195,6 +199,42 @@ describe("UrlToast", () => { ); }); + it("leads with the host when the caller says so, without hiding the other", () => { + // A live auth bridge, or a container with no browser installed. The pair + // is unchanged; only the order and which one is filled. + render( + , + ); + expect(actions()).toEqual(["Open", "In container"]); + expect( + document.querySelector(URL_TOAST_PRIMARY_SELECTOR), + ).toHaveTextContent("Open"); + // Still recognised as a sign-in, so the explanation stays. + expect(screen.getByTestId("url-toast-signin-hint")).toHaveTextContent( + /auth bridge/i, + ); + }); + + it("defaults to the host when the caller passes nothing", () => { + // The safe fallback: the answer more likely to work, and the one that + // reports its own failure. + render( + , + ); + expect(actions()).toEqual(["Open", "In container"]); + }); + it("keeps the host browser available as a fallback", () => { const onOpen = vi.fn(); render( @@ -202,6 +242,7 @@ describe("UrlToast", () => { url={SIGN_IN} onOpen={onOpen} onOpenInContainer={noop} + signInDefault="container" onDismiss={noop} />, ); @@ -211,12 +252,14 @@ describe("UrlToast", () => { it("leaves an ordinary URL alone", () => { // A `gh auth login` device code, a docs page, a preview build — the host - // browser is the right answer for all of them and stays the default. + // browser is the right answer for all of them and stays the default, + // whatever the project's sign-in preference happens to be. render( , ); @@ -232,6 +275,7 @@ describe("UrlToast", () => { url="https://claude.ai.evil.tld/oauth/authorize?x=1" onOpen={noop} onOpenInContainer={noop} + signInDefault="container" onDismiss={noop} />, ); diff --git a/app/src/components/terminal/UrlToast.tsx b/app/src/components/terminal/UrlToast.tsx index ddad469..148c723 100644 --- a/app/src/components/terminal/UrlToast.tsx +++ b/app/src/components/terminal/UrlToast.tsx @@ -37,6 +37,18 @@ interface Props { /** Open it in the container's own browser instead of the host's. Omitted when * the project has no browser to open it in. */ onOpenInContainer?: () => void; + /** + * Which action leads for a *sign-in* link (see the note below). Nothing else + * in the toast moves: both buttons are offered either way, in either order. + * + * This component does not work it out, because the answer depends on the + * project's auth bridge and on what is installed inside its container — + * neither of which a presentational component should be reaching for. + * `hooks/useSignInOpenTarget.ts` owns the rule. `"host"` is the default here + * for the same reason it is the fallback there: it is the answer that is more + * likely to work, and the one that reports its own failure. + */ + signInDefault?: "host" | "container"; onDismiss: () => void; } @@ -57,17 +69,20 @@ interface Props { * text swaps with no animation, and a user reading URL A can click Open on URL * B that arrived a second later. * - * ## Anthropic sign-in links default to the container's browser + * ## Anthropic sign-in links get their default from the caller * * For an ordinary URL the host browser is the right answer and stays the - * default. For a sign-in it is the *wrong* one: the callback listener the CLI - * is waiting on is inside the container, so a host browser completes the sign-in - * and then posts the result somewhere nothing is listening, and the terminal - * hangs until it times out. Making the host button primary there was quietly - * steering every user into that. The container-side browser closes the loop - * with no host round trip and no auth bridge, so it leads — and the host button - * stays, because a user who has the auth bridge on, or who wants their existing - * browser session, still needs it. + * default, unconditionally. A sign-in is the one case where it might not be: + * the callback listener the CLI is waiting on is inside the container, so a + * host browser can complete the sign-in and then post the result where nothing + * is listening, leaving the terminal to hang to its timeout. + * + * *Can*, not *does* — which is why this is no longer decided from the URL. The + * auth bridge mirrors that container listener onto the same host port, and the + * container-side alternative is Playwright's dashboard pane, which a fresh + * project has not installed. Both of those are project facts, so the owner + * passes {@link Props.signInDefault} and this only renders it: the leading + * button is filled and comes first, the other keeps its place beside it. * * ## Reachable without a mouse, and it does not take focus to manage it * @@ -96,6 +111,7 @@ export default function UrlToast({ label = "Long URL detected", onOpen, onOpenInContainer, + signInDefault = "host", onDismiss, }: Props) { const origin = urlOrigin(url); @@ -103,18 +119,22 @@ export default function UrlToast({ // Only when there is somewhere to send it: without `onOpenInContainer` the // host button is the only action there is, so it stays primary. const signIn = !!onOpenInContainer && isAnthropicSignInUrl(url); + // A sign-in link the caller has decided is better completed inside the + // container. Everything below keys off this rather than off `signIn`, so the + // two orderings differ only in which of the pair leads. + const containerLeads = signIn && signInDefault === "container"; // `Button` already owns the filled/outlined variants — including the rule // that filled uses `--accent-emphasis` and never `--accent`, which is the // foreground/link accent and fails WCAG AA behind white text. const hostButton = (