Make in-container OAuth logins actually complete #57

Merged
jknapp merged 9 commits from fix/auth-callback-and-opener into main 2026-09-18 04:32:07 +00:00
5 changed files with 311 additions and 44 deletions
Showing only changes of commit 90b7e4ccb2 - Show all commits
+46 -7
View File
@@ -15,6 +15,12 @@ use crate::AppState;
/// non-`Running` status carrying an explanation rather than an error, so the /// 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 /// pane always has something specific to say. This is host-side only — no
/// container recreation is involved either way. /// 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] #[tauri::command]
pub async fn set_browser_view_enabled( pub async fn set_browser_view_enabled(
project_id: String, project_id: String,
@@ -23,9 +29,15 @@ pub async fn set_browser_view_enabled(
state: State<'_, AppState>, state: State<'_, AppState>,
) -> Result<BrowserViewStatus, String> { ) -> Result<BrowserViewStatus, String> {
if !enabled { 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. // Awaits the supervisor, so the host port is released before we return.
manager().stop(&project_id).await; 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?; 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 .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] #[tauri::command]
pub async fn get_browser_view_status(project_id: String) -> Result<BrowserViewStatus, String> { pub async fn get_browser_view_status(
Ok(manager().status(&project_id).await) project_id: String,
state: State<'_, AppState>,
) -> Result<BrowserViewStatus, String> {
Ok(manager().status(&project_id, enabled_for(&state, &project_id)).await)
} }
/// Probe the container for Playwright without starting anything. /// Probe the container for Playwright without starting anything.
@@ -110,7 +129,9 @@ pub async fn open_browser_view_popout(
app_handle: AppHandle, app_handle: AppHandle,
state: State<'_, AppState>, state: State<'_, AppState>,
) -> Result<(), String> { ) -> 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 { let (BrowserViewState::Running, Some(url)) = (status.state, status.url.as_deref()) else {
return Err( return Err(
"The browser view isn't running. Start it before opening it in its own window." "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 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. // 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. // 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 { if status.state != BrowserViewState::Running {
crate::commands::project_commands::emit_progress( crate::commands::project_commands::emit_progress(
&app_handle, &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 // 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. // window of its own or it lands somewhere the user isn't looking.
if show_window { 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() { if let Some(url) = status.url.as_deref() {
let name = state let name = state
.projects_store .projects_store
@@ -311,6 +336,20 @@ pub async fn get_browser_view_match_window(project_id: String) -> Result<bool, S
Ok(popout::match_window(&project_id)) Ok(popout::match_window(&project_id))
} }
/// The project's stored browser-view opt-in.
///
/// The manager holds no copy of this — see
/// [`BrowserViewManager`](crate::browser_view::BrowserViewManager) — so every
/// status call reads it here, the way `get_auth_bridge_status` does. A project
/// that has gone away reads as off, which is the only answer that can be given
/// about a record that no longer exists.
fn enabled_for(state: &State<'_, AppState>, 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. /// 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 /// Every command here needs a *running* container, and every one of them used
+81 -32
View File
@@ -34,14 +34,22 @@
//! //!
//! ## Lifecycle //! ## 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 //! 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 //! is the only thing that tears them down, so every way a session can end funnels
//! through one code path: //! through one code path:
//! //!
//! | Trigger | 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 | //! | Container stopped, by the UI or otherwise | supervisor's `is_container_running` check |
//! | Project deleted | supervisor's `store.get()` check | //! | Project deleted | supervisor's `store.get()` check |
//! | Container rebuilt | old container stops → supervisor exits; the new one is not auto-started | //! | 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 //! 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 //! 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 //! 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 commands;
pub mod detect; pub mod detect;
@@ -134,7 +145,10 @@ pub enum BrowserViewState {
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub struct BrowserViewStatus { 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 enabled: bool,
pub state: BrowserViewState, pub state: BrowserViewState,
/// Fully-formed, token-bearing URL for the pane's iframe. Loopback only. /// Fully-formed, token-bearing URL for the pane's iframe. Loopback only.
@@ -201,17 +215,20 @@ struct Session {
type SessionMap = Arc<Mutex<HashMap<String, Session>>>; type SessionMap = Arc<Mutex<HashMap<String, Session>>>;
/// 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)] #[derive(Default)]
pub struct BrowserViewManager { pub struct BrowserViewManager {
sessions: SessionMap, 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<std::collections::HashSet<String>>,
next_epoch: AtomicU64, next_epoch: AtomicU64,
} }
@@ -226,22 +243,15 @@ pub fn manager() -> &'static Arc<BrowserViewManager> {
} }
impl BrowserViewManager { 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. /// 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) { match self.sessions.lock().await.get(project_id) {
Some(session) => BrowserViewStatus { Some(session) => BrowserViewStatus {
enabled, enabled,
@@ -261,6 +271,14 @@ impl BrowserViewManager {
/// ///
/// Idempotent: a call while a live session exists returns that session's /// Idempotent: a call while a live session exists returns that session's
/// status untouched, so re-opening the tab does not restart the dashboard. /// 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( pub async fn start(
&self, &self,
project_id: String, project_id: String,
@@ -268,7 +286,7 @@ impl BrowserViewManager {
app: AppHandle, app: AppHandle,
store: Arc<ProjectsStore>, store: Arc<ProjectsStore>,
) -> Result<BrowserViewStatus, String> { ) -> Result<BrowserViewStatus, String> {
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, // Bind the answer before acting on it: `status()` takes the same lock,
// and this mutex is not reentrant. // and this mutex is not reentrant.
@@ -279,7 +297,7 @@ impl BrowserViewManager {
.get(&project_id) .get(&project_id)
.is_some_and(|s| !s.supervisor.is_finished()); .is_some_and(|s| !s.supervisor.is_finished());
if already_live { 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?; 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); emit(&app, &project_id, &status);
Ok(status) Ok(status)
} }
/// Stop one project's view and wait until its host port has been released. /// 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) { 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 // Remove under the lock, then release it before awaiting: the
// supervisor takes the same lock to deregister itself on exit. // supervisor takes the same lock to deregister itself on exit.
let session = self.sessions.lock().await.remove(project_id); 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. // longer exists. The session owns it, and this is where the session ends.
let _ = popout::close(&app, &project_id); 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)); emit(&app, &project_id, &BrowserViewStatus::off(enabled));
} }
@@ -915,6 +945,25 @@ mod tests {
assert!(s.url.is_none()); 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] #[test]
fn an_unavailable_status_keeps_the_detail_the_user_needs() { fn an_unavailable_status_keeps_the_detail_the_user_needs() {
let mut d = PlaywrightDetection::default(); let mut d = PlaywrightDetection::default();
@@ -1100,6 +1100,15 @@ pub async fn update_project(
project.container_id = stored.container_id; project.container_id = stored.container_id;
project.status = stored.status; 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.created_at = stored.created_at;
project.updated_at = chrono::Utc::now().to_rfc3339(); project.updated_at = chrono::Utc::now().to_rfc3339();
+103 -5
View File
@@ -132,6 +132,26 @@ fn default_use_shared_auth_token() -> bool {
true 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:<that port>`.
/// 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. /// How much autonomy Claude Code is granted inside the container.
/// ///
/// Maps onto Claude Code CLI flags — see [`PermissionMode::cli_args`], which is /// Maps onto Claude Code CLI flags — see [`PermissionMode::cli_args`], which is
@@ -336,17 +356,30 @@ pub struct Project {
pub sandbox_mode_enabled: bool, pub sandbox_mode_enabled: bool,
#[serde(default)] #[serde(default)]
pub mission_control_enabled: bool, pub mission_control_enabled: bool,
/// Opt in to the auth bridge: while the container runs, its loopback /// The auth bridge: while the container runs, its loopback listeners are
/// listeners are mirrored onto the host's loopback so browser OAuth /// mirrored onto the host's loopback so browser OAuth callbacks
/// callbacks (`claude login`, `fly login`, `aws sso login`) can reach them. /// (`claude login`, `fly login`, `aws sso login`) can reach them.
/// Purely host-side — it deliberately has no container-recreation label, /// Purely host-side — it deliberately has no container-recreation label,
/// because toggling it changes nothing about the container itself. /// 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, pub auth_bridge_enabled: bool,
/// Opt in to the browser-view pane, which watches and takes over the /// Opt in to the browser-view pane, which watches and takes over the
/// browser Claude drives with Playwright inside the container. Purely /// browser Claude drives with Playwright inside the container. Purely
/// host-side like `auth_bridge_enabled`, so it likewise has no /// host-side like `auth_bridge_enabled`, so it likewise has no
/// container-recreation label. /// 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)] #[serde(default)]
pub browser_view_enabled: bool, pub browser_view_enabled: bool,
/// Grant the container what a VPN client needs to build a tunnel: /// Grant the container what a VPN client needs to build a tunnel:
@@ -639,7 +672,7 @@ impl Project {
allow_docker_access: false, allow_docker_access: false,
sandbox_mode_enabled: false, sandbox_mode_enabled: false,
mission_control_enabled: false, mission_control_enabled: false,
auth_bridge_enabled: false, auth_bridge_enabled: default_auth_bridge_enabled(),
browser_view_enabled: false, browser_view_enabled: false,
vpn_support_enabled: false, vpn_support_enabled: false,
use_shared_auth_token: default_use_shared_auth_token(), 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(); let round_tripped: ClaudeCodeSettings = serde_json::from_str(&json).unwrap();
assert_eq!(round_tripped, partial); 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"));
}
} }
@@ -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<String>) -> Result<(), String> { pub fn set_container_id(&self, project_id: &str, container_id: Option<String>) -> Result<(), String> {
let mut projects = self.lock(); let mut projects = self.lock();
if let Some(p) = projects.iter_mut().find(|p| p.id == project_id) { 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(); 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<Project>) -> 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<Project> =
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();
}
} }