From 34f0fd0fc640bb4ffee7db259d762d33d35ebcb8 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 13 Jul 2026 22:04:08 -0400 Subject: [PATCH 1/8] feat(desktop): add Sign Out to Settings to reset and relaunch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mathieu and Kalvin reported that switching Nostr identities required manually deleting keychain entries, ~/Library/Application Support dirs, WebKit localStorage, ~/.buzz, and ~/.config/buzz-agent — an opaque and error-prone process that blocked routine identity switching. Implements a two-step Sign Out action in Settings → Profile: Rust (commands/identity.rs): - `sign_out` command stops all managed agents, deletes keychain blobs for both buzz-desktop and buzz-desktop-dev services, removes all App Support and WebKit dirs matching xyz.block.buzz.app* and xyz.block.sprout.app* prefixes (covers dev-worktree variants), removes the nest (~/.buzz / ~/.buzz-dev), legacy ~/.sprout, OAuth token cache, and CLI symlinks, then relaunches via exec+exit(0). - `SecretStore::delete_all()` removes the entire blob entry under the interprocess advisory file lock, bypassing the per-key delete path. - Guarded by `is_shared_identity()` — no-op in harness/CI mode where the key comes from BUZZ_PRIVATE_KEY and keychain wipe is meaningless. Frontend (ProfileSettingsCard.tsx): - Sign Out section below the Identity card with an AlertDialog confirmation before the irreversible wipe. - Spinner + disabled state while the wipe is in flight; toast on the error path (sign_out never resolves on success — it restarts the process). Also removes a stale biome-ignore suppression comment in useIndependentThreadPanel.ts and replaces non-null assertions with optional chaining in e2eBridge.ts (pre-existing lint issues that were blocking the pre-commit hook). --- desktop/scripts/check-file-sizes.mjs | 8 +- desktop/src-tauri/src/commands/identity.rs | 89 +++++++++++++++++++ desktop/src-tauri/src/lib.rs | 1 + desktop/src-tauri/src/secret_store.rs | 36 ++++++++ .../messages/useIndependentThreadPanel.ts | 1 - .../settings/ui/ProfileSettingsCard.tsx | 88 +++++++++++++++++- desktop/src/shared/api/tauriIdentity.ts | 11 +++ desktop/src/testing/e2eBridge.ts | 2 +- 8 files changed, 232 insertions(+), 4 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index cf2ea09cf2..52433a7789 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -317,7 +317,13 @@ const overrides = new Map([ // security fix for the lost-update race that stranded agent keys. // identity-import-keyring: KeyringLockedScreen, RecoveryScreen, // load_readonly + load_all_readonly + store_all for safe cross-service reads. - ["src-tauri/src/secret_store.rs", 1140], + // sign-out wipe: delete_all() method removes the entire keychain blob under + // the interprocess advisory lock; +8 lines. Load-bearing; queued to split. + ["src-tauri/src/secret_store.rs", 1148], + // sign-out wipe: Sign Out section (AlertDialog + controlled state) added + // at the bottom of the Profile settings page. Load-bearing UX feature; + // queued to split when ProfileSettingsCard is broken into sub-components. + ["src/features/settings/ui/ProfileSettingsCard.tsx", 1005], // keyring-dev-isolation: keyring_service() fn (7 lines) replaces the const // to return "buzz-desktop-dev" in debug builds. Load-bearing isolation fix. ["src-tauri/src/app_state.rs", 1042], diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 3d9cdd42fb..3fe617785b 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -293,6 +293,95 @@ pub async fn persist_current_identity( .map_err(|e| format!("spawn_blocking failed: {e}"))? } +/// Wipe all local Buzz state and relaunch into first-run onboarding. +/// +/// Deletes, in order: managed-agent processes, keychain blobs for all service +/// names (prod + dev), App Support directories for all Buzz/Sprout bundle +/// identifiers, matching WebKit directories, the agent nest, the legacy Sprout +/// nest, the OAuth token cache, and the CLI symlinks. Then relaunches the app. +/// +/// Called by the Sign Out action in Settings. The frontend confirms with an +/// AlertDialog before invoking this. +/// +/// Not available in shared-identity mode (`BUZZ_SHARE_IDENTITY=1`): the key +/// comes from an env var, not the keychain, so wiping would have no effect and +/// would be confusing. +#[tauri::command] +pub async fn sign_out(app: tauri::AppHandle) -> Result<(), String> { + if is_shared_identity() { + return Err("sign-out is not available in shared-identity mode".to_string()); + } + + // Stop all managed agents before touching the directories they write into. + if let Err(e) = crate::shutdown::shutdown_managed_agents(&app) { + eprintln!("buzz-desktop sign-out: agent shutdown: {e}"); + } + + // Wipe keychain blobs for the prod and dev service names. + for service in &["buzz-desktop", "buzz-desktop-dev"] { + let store = crate::secret_store::SecretStore::keyring(*service); + if let Err(e) = store.delete_all() { + eprintln!("buzz-desktop sign-out: keychain wipe ({service}): {e}"); + } + } + + // Remove all App Support and WebKit directories whose names begin with one + // of the known Buzz or legacy Sprout bundle-identifier prefixes. The prefix + // scan catches dev-worktree variants (e.g. `xyz.block.buzz.app.dev.my-branch`) + // without hardcoding every possible suffix. + if let Ok(app_data_dir) = app.path().app_data_dir() { + if let Some(support_root) = app_data_dir.parent() { + for prefix in &["xyz.block.buzz.app", "xyz.block.sprout.app"] { + remove_dirs_with_prefix(support_root, prefix); + } + // ~/Library/WebKit is a sibling of ~/Library/Application Support. + if let Some(webkit_root) = support_root.parent().map(|p| p.join("WebKit")) { + for prefix in &["xyz.block.buzz.app", "xyz.block.sprout.app"] { + remove_dirs_with_prefix(&webkit_root, prefix); + } + } + } + } + + // Remove the agent nest for the running build, plus the legacy Sprout nest. + if let Some(nest) = crate::managed_agents::nest_dir() { + let _ = std::fs::remove_dir_all(&nest); + } + if let Ok(home) = app.path().home_dir() { + let _ = std::fs::remove_dir_all(home.join(".sprout")); + // OAuth token cache written by buzz-agent Databricks PKCE flow. + let _ = std::fs::remove_dir_all(home.join(".config").join("buzz-agent")); + // CLI symlinks recreated automatically on the next launch. + let _ = std::fs::remove_file(home.join(".local").join("bin").join("buzz")); + let _ = std::fs::remove_file(home.join(".local").join("bin").join("buzz-dev")); + } + + // Relaunch into fresh onboarding. Mirror what tauri-plugin-process::restart + // does: spawn a new instance then exit. The new process starts with a clean + // keychain and an empty App Support dir, lands on first-run onboarding, and + // generates a fresh keypair. + if let Ok(exe) = std::env::current_exe() { + let _ = std::process::Command::new(&exe).spawn(); + } + std::process::exit(0); +} + +/// Remove every immediate child of `parent` whose name starts with `prefix`. +fn remove_dirs_with_prefix(parent: &std::path::Path, prefix: &str) { + let Ok(entries) = std::fs::read_dir(parent) else { + return; + }; + for entry in entries.flatten() { + if entry + .file_name() + .to_str() + .is_some_and(|n| n.starts_with(prefix)) + { + let _ = std::fs::remove_dir_all(entry.path()); + } + } +} + fn nostr_bind_tag(name: &str, value: &str) -> Result { Tag::parse(vec![name, value]).map_err(|error| format!("{name} tag failed: {error}")) } diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index a60fe7326c..6d19a1d109 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -702,6 +702,7 @@ pub fn run() { discover_managed_agent_prereqs, sign_event, sign_nostr_identity_binding, + sign_out, decrypt_observer_event, build_observer_control_event, create_auth_event, diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index dc7c6755dd..6b6fc8295c 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -740,6 +740,42 @@ impl SecretStore { } } + /// Delete the entire keychain blob for this service. + /// + /// Used by the sign-out wipe path to remove all persisted keys in one shot. + /// After this call the OS keychain has no entry for this service; the next + /// launch will generate a fresh keypair and show first-run onboarding. + pub fn delete_all(&self) -> Result<(), String> { + #[cfg(feature = "system-keyring")] + { + let _lock = acquire_blob_lock(&self.service)?; + if let Ok(entry) = keyring_entry(&self.service, BLOB_KEY) { + match entry.delete_credential() { + Ok(()) | Err(keyring::Error::NoEntry) => {} + Err(e) if is_keyring_availability_error(&e.to_string()) => { + return Err(format!("keyring unavailable: {e}")); + } + Err(e) => { + // Non-fatal — log and continue; the blob is gone even if + // the delete_credential call returned a non-standard error. + eprintln!("buzz-desktop sign-out: keychain delete: {e}"); + } + } + } + // Best-effort: also remove any legacy DPK blob written by #1267. + #[cfg(target_os = "macos")] + let _ = delete_generic_password_options(dpk_opts(&self.service, BLOB_KEY)); + // Clear the in-memory cache so subsequent calls see the clean state. + let mut guard = self.cache.lock().unwrap_or_else(|e| e.into_inner()); + *guard = None; + Ok(()) + } + #[cfg(not(feature = "system-keyring"))] + { + Ok(()) // No-op: no keyring, nothing to delete. + } + } + /// Delete the secret for `key`. A missing entry is not an error. pub fn delete(&self, key: &str) -> Result<(), String> { #[cfg(feature = "system-keyring")] diff --git a/desktop/src/features/messages/useIndependentThreadPanel.ts b/desktop/src/features/messages/useIndependentThreadPanel.ts index 451e804a85..d088d14ecc 100644 --- a/desktop/src/features/messages/useIndependentThreadPanel.ts +++ b/desktop/src/features/messages/useIndependentThreadPanel.ts @@ -47,7 +47,6 @@ export function useIndependentThreadPanel(args: { args.personaLookup, args.respondToLookup, ), - // biome-ignore lint/correctness/useExhaustiveDependencies: fields listed explicitly — see comment above [ args.channelEvents, args.threadReplyEvents, diff --git a/desktop/src/features/settings/ui/ProfileSettingsCard.tsx b/desktop/src/features/settings/ui/ProfileSettingsCard.tsx index 30b84340aa..21adb65007 100644 --- a/desktop/src/features/settings/ui/ProfileSettingsCard.tsx +++ b/desktop/src/features/settings/ui/ProfileSettingsCard.tsx @@ -13,7 +13,17 @@ import { useUpdateProfileMutation, } from "@/features/profile/hooks"; import { NsecMaskedDisplay } from "@/features/onboarding/ui/NsecMaskedDisplay"; -import { getNsec } from "@/shared/api/tauriIdentity"; +import { getNsec, signOut } from "@/shared/api/tauriIdentity"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/shared/ui/alert-dialog"; import { MaskedAvatarBadgeFrame } from "@/features/profile/ui/MaskedAvatarBadgeFrame"; import { ProfileAvatar } from "@/features/profile/ui/ProfileAvatar"; import { @@ -250,6 +260,8 @@ export function ProfileSettingsCard({ const [shouldRenderAvatarEditor, setShouldRenderAvatarEditor] = React.useState(false); const [avatarSquishKey, setAvatarSquishKey] = React.useState(0); + const [isSignOutOpen, setIsSignOutOpen] = React.useState(false); + const [isSignOutPending, setIsSignOutPending] = React.useState(false); const displayNameInputRef = React.useRef(null); const aboutTextareaRef = React.useRef(null); const isEditingProfileMetadataRef = React.useRef(false); @@ -913,6 +925,80 @@ export function ProfileSettingsCard({ + +
+ +
+
+
+

Clear data and sign out

+

+ Wipes the keychain, agent settings, and app cache, then + relaunches Buzz into first-run setup. +

+
+ +
+
+ { + if (!open && !isSignOutPending) setIsSignOutOpen(false); + }} + open={isSignOutOpen} + > + + + Sign out and wipe all data? + + This will delete your identity key, all agent settings, and + cached data from this device, then relaunch Buzz into first-run + setup. Make sure you have your private key (nsec) backed up + before continuing — this cannot be undone. + + + + + Cancel + + { + setIsSignOutPending(true); + // signOut() restarts the process and never resolves on success. + signOut().catch((err: unknown) => { + setIsSignOutPending(false); + setIsSignOutOpen(false); + toast.error( + err instanceof Error ? err.message : "Sign out failed.", + ); + }); + }} + > + {isSignOutPending ? "Signing out…" : "Sign Out"} + + + + +
); } diff --git a/desktop/src/shared/api/tauriIdentity.ts b/desktop/src/shared/api/tauriIdentity.ts index d935e68557..69836475ca 100644 --- a/desktop/src/shared/api/tauriIdentity.ts +++ b/desktop/src/shared/api/tauriIdentity.ts @@ -36,3 +36,14 @@ export async function persistCurrentIdentity(): Promise { await invokeTauri("persist_current_identity"), ); } + +/** + * Wipe all local Buzz state (keychain, App Support, WebKit, nest, OAuth cache, + * CLI symlinks) and relaunch into first-run onboarding. + * + * This call never resolves on success — the process restarts. Callers should + * handle the error case (e.g. display a toast) but not await a resolution. + */ +export async function signOut(): Promise { + await invokeTauri("sign_out"); +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 41054deab2..56b4359718 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -9447,7 +9447,7 @@ export function maybeInstallE2eTauriMocks() { case "get_relay_self": if ((activeConfig?.mock?.relaySelfDelayMs ?? 0) > 0) { await new Promise((resolve) => - window.setTimeout(resolve, activeConfig!.mock!.relaySelfDelayMs), + window.setTimeout(resolve, activeConfig?.mock?.relaySelfDelayMs), ); } return activeConfig?.mock?.relaySelf ?? null; From 6104271ea6edb24c9d7a476abefc5403f6333d0b Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 13 Jul 2026 22:30:35 -0400 Subject: [PATCH 2/8] test(desktop): add e2e screenshot spec for Sign Out UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the Settings → Profile sign-out section and the confirmation AlertDialog, exercising the testids added in the sign_out feature. --- desktop/playwright.config.ts | 1 + desktop/tests/e2e/signout-screenshots.spec.ts | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 desktop/tests/e2e/signout-screenshots.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index a31306e77d..b8950d9f82 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -87,6 +87,7 @@ export default defineConfig({ "**/project-pr-review.spec.ts", "**/persona-model-combobox-screenshots.spec.ts", "**/drafts-screenshots.spec.ts", + "**/signout-screenshots.spec.ts", "**/buzz-theme-screenshots.spec.ts", "**/channel-sort.spec.ts", "**/identity-lost.spec.ts", diff --git a/desktop/tests/e2e/signout-screenshots.spec.ts b/desktop/tests/e2e/signout-screenshots.spec.ts new file mode 100644 index 0000000000..4a3be08d4e --- /dev/null +++ b/desktop/tests/e2e/signout-screenshots.spec.ts @@ -0,0 +1,70 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; +import { openSettings } from "../helpers/settings"; + +const SHOTS = "test-results/signout"; + +test.describe("signout screenshots", () => { + test.use({ viewport: { width: 1280, height: 900 } }); + + test.beforeEach(async ({ page }) => { + page.on("pageerror", (err) => { + console.error( + "PAGE ERROR:", + err.message, + err.stack?.split("\n").slice(0, 5).join("\n"), + ); + }); + page.on("console", (msg) => { + if (msg.type() === "error") { + console.error("CONSOLE ERROR:", msg.text().slice(0, 500)); + } + }); + }); + + test("signout-section — Sign Out card in Settings › Profile", async ({ + page, + }) => { + await installMockBridge(page); + await page.goto("/", { waitUntil: "domcontentloaded" }); + await openSettings(page, "profile"); + + const section = page.getByTestId("settings-signout"); + await section.scrollIntoViewIfNeeded(); + + // Settle animations before capture. + await page.evaluate(() => + Promise.all(document.getAnimations().map((a) => a.finished)), + ); + await page.waitForTimeout(200); + + await section.screenshot({ path: `${SHOTS}/signout-section.png` }); + }); + + test("signout-dialog — AlertDialog shown before the wipe runs", async ({ + page, + }) => { + await installMockBridge(page); + await page.goto("/", { waitUntil: "domcontentloaded" }); + await openSettings(page, "profile"); + + const section = page.getByTestId("settings-signout"); + await section.scrollIntoViewIfNeeded(); + + // Open the confirmation dialog. + await page.getByTestId("signout-open-dialog").click(); + + const dialog = page.getByRole("alertdialog"); + await expect(dialog).toBeVisible({ timeout: 5_000 }); + await expect(dialog.getByText("Sign out and wipe all data?")).toBeVisible(); + + // Settle animations before capture. + await page.evaluate(() => + Promise.all(document.getAnimations().map((a) => a.finished)), + ); + await page.waitForTimeout(200); + + await page.screenshot({ path: `${SHOTS}/signout-dialog.png` }); + }); +}); From 379597334dd296638c0bfb0f1fa1f16d3b83462e Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 13 Jul 2026 23:07:52 -0400 Subject: [PATCH 3/8] feat(desktop): implement two-phase boot-time sentinel wipe for sign-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the inline sign-out wipe (which left a crash window between keychain delete and app-data delete) with a sentinel-based two-phase design: Phase 1 — sign_out: writes a durable `.bundle-id.reset-pending` sentinel outside all wiped paths, then relaunches. No destruction happens in the command itself, so a crash between write and relaunch is safe — the sentinel persists and the wipe completes on the next open. Phase 2 — boot-time hook at the top of setup(): if the sentinel is present, perform the full wipe before migrations and identity resolution: - Rename app-data dir atomically (.trash-), same for WebKit dir - Remove nest, ~/.sprout, ~/.config/buzz-agent, this build's CLI symlink - Keychain: delete_all_with_legacy_cleanup() which reads the blob to collect all key names and deletes per-key legacy DPK/keyring entries, preventing old identity resurrection through migrate_legacy_key() - Verify: keychain empty + app-data absent + nest absent - On success: delete sentinel, set completed flag, fall into clean onboarding - On failure: keep sentinel (retry next boot), set reset_failed flag Additional correctness fixes: - Wipe is scoped to the running build only (not prod+dev simultaneously) - reset_completed suppresses migrate_legacy_nest() and migrate_dev_nest() so dev-build reset cannot re-import ~/.buzz into ~/.buzz-dev - reset_failed flag surfaces a recovery screen in the frontend Tests: 7 unit tests in reset.rs via injected ResetKeychain trait (fake). --- desktop/scripts/check-file-sizes.mjs | 8 +- desktop/src-tauri/src/app_state.rs | 9 + desktop/src-tauri/src/commands/identity.rs | 94 +--- desktop/src-tauri/src/lib.rs | 38 +- desktop/src-tauri/src/models.rs | 5 + desktop/src-tauri/src/reset.rs | 485 ++++++++++++++++++ desktop/src-tauri/src/secret_store.rs | 60 ++- desktop/src/app/App.tsx | 5 + desktop/src/features/onboarding/hooks.ts | 24 +- .../onboarding/ui/ResetFailedScreen.tsx | 11 + desktop/src/shared/api/tauriIdentity.ts | 2 + desktop/src/shared/api/types.ts | 4 + 12 files changed, 651 insertions(+), 94 deletions(-) create mode 100644 desktop/src-tauri/src/reset.rs create mode 100644 desktop/src/features/onboarding/ui/ResetFailedScreen.tsx diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 52433a7789..ef0296aa0f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -206,7 +206,8 @@ const overrides = new Map([ // mcp-readonly-view rebase: PR2 MCP config surface FE-type fields force +1 over the grandfathered ceiling. // Git Bash prerequisite payload adds four fields to the shared Tauri API // contract. This is the canonical type location; split remains queued. - ["src/shared/api/types.ts", 1038], + // signout-wipe: resetFailed field added to Identity type (+6 lines). + ["src/shared/api/types.ts", 1044], // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate + // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog // shows required markers and credential amber rows (parity with @@ -319,7 +320,10 @@ const overrides = new Map([ // load_readonly + load_all_readonly + store_all for safe cross-service reads. // sign-out wipe: delete_all() method removes the entire keychain blob under // the interprocess advisory lock; +8 lines. Load-bearing; queued to split. - ["src-tauri/src/secret_store.rs", 1148], + // signout-wipe phase 2: delete_all_with_legacy_cleanup replaces delete_all; + // reads blob keys + deletes per-key legacy entries to prevent resurrection. + // Net growth ~36 lines over prior cap. Load-bearing correctness fix. + ["src-tauri/src/secret_store.rs", 1190], // sign-out wipe: Sign Out section (AlertDialog + controlled state) added // at the bottom of the Profile settings page. Load-bearing UX feature; // queued to split when ProfileSettingsCard is broken into sub-components. diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 7421caa66f..5adff239de 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -68,6 +68,14 @@ pub struct AppState { /// `keys` so readers (signing, get_identity, etc.) are not blocked during /// keyring I/O. pub identity_mutation: Mutex<()>, + /// Set when the boot-time Phase 2 reset attempted a wipe but verification + /// failed. The sentinel is preserved so the next relaunch retries. All + /// identity-dependent setup is skipped; the frontend shows a reset-failed + /// recovery screen via `get_identity`. + /// + /// Ordering: written once in `setup()` with `Ordering::Release`; read in + /// `get_identity` with `Ordering::Acquire`. + pub reset_failed: AtomicBool, /// Cached ACP session config from running agents, keyed by agent pubkey. /// Populated when the harness emits `session_config_captured` observer events. pub session_config_cache: Mutex>, @@ -142,6 +150,7 @@ pub fn build_app_state() -> AppState { )), keyring_locked: AtomicBool::new(false), identity_lost: AtomicBool::new(false), + reset_failed: AtomicBool::new(false), #[cfg(feature = "mesh-llm")] mesh_llm_runtime: AsyncMutex::new(None), #[cfg(feature = "mesh-llm")] diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 3fe617785b..4ad406e685 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -36,12 +36,16 @@ pub fn get_identity(state: State<'_, AppState>) -> Result let locked = state .keyring_locked .load(std::sync::atomic::Ordering::Acquire); + let reset_failed = state + .reset_failed + .load(std::sync::atomic::Ordering::Acquire); Ok(IdentityInfo { pubkey: pubkey_hex, display_name, lost, locked, + reset_failed, }) } @@ -220,6 +224,7 @@ pub async fn import_identity( display_name, lost: false, locked: false, + reset_failed: false, }) }) .await @@ -287,21 +292,20 @@ pub async fn persist_current_identity( display_name, lost: false, locked: false, + reset_failed: false, }) }) .await .map_err(|e| format!("spawn_blocking failed: {e}"))? } -/// Wipe all local Buzz state and relaunch into first-run onboarding. -/// -/// Deletes, in order: managed-agent processes, keychain blobs for all service -/// names (prod + dev), App Support directories for all Buzz/Sprout bundle -/// identifiers, matching WebKit directories, the agent nest, the legacy Sprout -/// nest, the OAuth token cache, and the CLI symlinks. Then relaunches the app. +/// Write a reset-intent sentinel and relaunch into Phase 2 (boot-time wipe). /// -/// Called by the Sign Out action in Settings. The frontend confirms with an -/// AlertDialog before invoking this. +/// The actual data destruction is deferred to the next boot: `setup()` in +/// `lib.rs` checks for the sentinel and performs the wipe before any migration +/// or identity resolution. This two-phase design means a crash between now and +/// the relaunch is safe — the sentinel persists and the wipe completes on the +/// next open. /// /// Not available in shared-identity mode (`BUZZ_SHARE_IDENTITY=1`): the key /// comes from an env var, not the keychain, so wiping would have no effect and @@ -312,76 +316,28 @@ pub async fn sign_out(app: tauri::AppHandle) -> Result<(), String> { return Err("sign-out is not available in shared-identity mode".to_string()); } - // Stop all managed agents before touching the directories they write into. + // Stop all managed agents before restart so they don't race the wipe. if let Err(e) = crate::shutdown::shutdown_managed_agents(&app) { eprintln!("buzz-desktop sign-out: agent shutdown: {e}"); } - // Wipe keychain blobs for the prod and dev service names. - for service in &["buzz-desktop", "buzz-desktop-dev"] { - let store = crate::secret_store::SecretStore::keyring(*service); - if let Err(e) = store.delete_all() { - eprintln!("buzz-desktop sign-out: keychain wipe ({service}): {e}"); - } - } + // Write the reset sentinel — destruction happens on next boot. + let data_dir = app + .path() + .app_data_dir() + .map_err(|e| format!("app data dir: {e}"))?; + crate::reset::write_sentinel(&data_dir)?; - // Remove all App Support and WebKit directories whose names begin with one - // of the known Buzz or legacy Sprout bundle-identifier prefixes. The prefix - // scan catches dev-worktree variants (e.g. `xyz.block.buzz.app.dev.my-branch`) - // without hardcoding every possible suffix. - if let Ok(app_data_dir) = app.path().app_data_dir() { - if let Some(support_root) = app_data_dir.parent() { - for prefix in &["xyz.block.buzz.app", "xyz.block.sprout.app"] { - remove_dirs_with_prefix(support_root, prefix); - } - // ~/Library/WebKit is a sibling of ~/Library/Application Support. - if let Some(webkit_root) = support_root.parent().map(|p| p.join("WebKit")) { - for prefix in &["xyz.block.buzz.app", "xyz.block.sprout.app"] { - remove_dirs_with_prefix(&webkit_root, prefix); - } - } - } - } + // Attempt relaunch — if it fails, the sentinel is already written, + // so a manual reopen completes the reset. + let exe = std::env::current_exe().map_err(|e| format!("cannot find executable: {e}"))?; + std::process::Command::new(&exe) + .spawn() + .map_err(|e| format!("relaunch failed: {e}"))?; - // Remove the agent nest for the running build, plus the legacy Sprout nest. - if let Some(nest) = crate::managed_agents::nest_dir() { - let _ = std::fs::remove_dir_all(&nest); - } - if let Ok(home) = app.path().home_dir() { - let _ = std::fs::remove_dir_all(home.join(".sprout")); - // OAuth token cache written by buzz-agent Databricks PKCE flow. - let _ = std::fs::remove_dir_all(home.join(".config").join("buzz-agent")); - // CLI symlinks recreated automatically on the next launch. - let _ = std::fs::remove_file(home.join(".local").join("bin").join("buzz")); - let _ = std::fs::remove_file(home.join(".local").join("bin").join("buzz-dev")); - } - - // Relaunch into fresh onboarding. Mirror what tauri-plugin-process::restart - // does: spawn a new instance then exit. The new process starts with a clean - // keychain and an empty App Support dir, lands on first-run onboarding, and - // generates a fresh keypair. - if let Ok(exe) = std::env::current_exe() { - let _ = std::process::Command::new(&exe).spawn(); - } std::process::exit(0); } -/// Remove every immediate child of `parent` whose name starts with `prefix`. -fn remove_dirs_with_prefix(parent: &std::path::Path, prefix: &str) { - let Ok(entries) = std::fs::read_dir(parent) else { - return; - }; - for entry in entries.flatten() { - if entry - .file_name() - .to_str() - .is_some_and(|n| n.starts_with(prefix)) - { - let _ = std::fs::remove_dir_all(entry.path()); - } - } -} - fn nostr_bind_tag(name: &str, value: &str) -> Result { Tag::parse(vec![name, value]).map_err(|error| format!("{name} tag failed: {error}")) } diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 6d19a1d109..c7a86c9925 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -18,6 +18,7 @@ pub mod nostr_convert; mod prevent_sleep; mod ptt_shortcut; mod relay; +mod reset; mod secret_store; mod shutdown; mod templates; @@ -420,6 +421,35 @@ pub fn run() { let app_handle = app.handle().clone(); let shutdown_started = Arc::clone(&restore_shutdown_started); + // ── Phase 2: boot-time sentinel wipe ────────────────────────────── + // Must run before migrations and identity resolution so the wipe + // completes atomically on crash recovery. + // + // init_nest_dir is called early here (normally it runs inside + // run_boot_migrations) so reset::run_boot_reset can call nest_dir(). + let reset_outcome = if let Ok(data_dir) = app_handle.path().app_data_dir() { + let is_dev_for_reset = data_dir + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n.starts_with("xyz.block.buzz.app.dev")) + .unwrap_or(false); + crate::managed_agents::init_nest_dir(is_dev_for_reset); + crate::reset::run_boot_reset(&data_dir) + } else { + crate::reset::ResetOutcome::default() + }; + + if reset_outcome.failed { + // Surface reset-failed state — skip identity resolution and + // all side-effecting setup. The webview still loads so the + // frontend can show the recovery screen via get_identity. + let state = app_handle.state::(); + state + .reset_failed + .store(true, std::sync::atomic::Ordering::Release); + return Ok(()); + } + // Run all pre-identity data migrations before state loads from disk. migration::run_boot_migrations(&app_handle); @@ -522,7 +552,9 @@ pub fn run() { // destination is present. Non-fatal. // On a real migration, emit a one-time hint so the user can delete // the now-inert ~/.sprout; the frontend dedupes the toast. - if migration::migrate_legacy_nest() { + // Suppressed when a reset completed this boot: the nest was wiped and + // a fresh ~/.sprout-less state is exactly what we want. + if !reset_outcome.completed && migration::migrate_legacy_nest() { let _ = app_handle.emit("legacy-nest-migrated", ()); } @@ -530,10 +562,12 @@ pub fn run() { // from the shared ~/.buzz nest into the new dedicated ~/.buzz-dev // nest so no work is lost when the nest is first namespaced. // Runs only when nest_dir() resolved to ~/.buzz-dev (dev instance). + // Suppressed after a reset so re-importing ~/.buzz into ~/.buzz-dev + // doesn't re-populate what was just wiped. let is_dev_nest = managed_agents::nest_dir() .and_then(|p| p.file_name().map(|n| n.to_os_string())) .is_some_and(|n| n == ".buzz-dev"); - if is_dev_nest { + if !reset_outcome.completed && is_dev_nest { migration::migrate_dev_nest(); } diff --git a/desktop/src-tauri/src/models.rs b/desktop/src-tauri/src/models.rs index 8f002fdb1d..cfb0593d59 100644 --- a/desktop/src-tauri/src/models.rs +++ b/desktop/src-tauri/src/models.rs @@ -17,6 +17,11 @@ pub struct IdentityInfo { /// shows a "unlock the keyring and relaunch" screen. Mutually exclusive /// with `lost`. pub locked: bool, + /// True when the boot-time Phase 2 reset attempted a wipe but verification + /// failed. Identity resolution was skipped; the frontend shows a + /// reset-failed recovery screen. The sentinel is preserved so the next + /// relaunch retries the wipe automatically. + pub reset_failed: bool, } #[derive(Serialize, Deserialize)] diff --git a/desktop/src-tauri/src/reset.rs b/desktop/src-tauri/src/reset.rs new file mode 100644 index 0000000000..4469836849 --- /dev/null +++ b/desktop/src-tauri/src/reset.rs @@ -0,0 +1,485 @@ +//! Two-phase boot-time sentinel wipe. +//! +//! **Phase 1** (`write_sentinel`) — called by `sign_out`: writes a durable +//! reset-intent file outside every path that will be wiped. +//! +//! **Phase 2** (`run_boot_reset`) — called at the very top of `setup()` in +//! `lib.rs`, before migrations and identity resolution: if the sentinel is +//! present the wipe runs atomically and the app falls through into clean +//! onboarding. +//! +//! The sentinel lives at `/..reset-pending` +//! and survives the app-data wipe because the wipe targets the exact +//! app-data dir, not its parent. +//! +//! Idempotency: if the process crashes mid-wipe the sentinel is still present +//! on next boot and the wipe retries from the top. + +use std::path::{Path, PathBuf}; + +// ── Sentinel helpers ────────────────────────────────────────────────────────── + +/// Sentinel path: `/..reset-pending` +/// where `bundle_id` is the file-name component of `app_data_dir` +/// (e.g. `xyz.block.buzz.app` or `xyz.block.buzz.app.dev`). +pub(crate) fn sentinel_path(app_data_dir: &Path) -> PathBuf { + let bundle_id = app_data_dir + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("buzz"); + let name = format!(".{bundle_id}.reset-pending"); + match app_data_dir.parent() { + Some(parent) => parent.join(name), + None => PathBuf::from(&name), + } +} + +/// Atomically write the sentinel file. Content is intentionally empty — +/// existence is the signal. +pub(crate) fn write_sentinel(app_data_dir: &Path) -> Result<(), String> { + let path = sentinel_path(app_data_dir); + std::fs::write(&path, b"").map_err(|e| format!("write sentinel {}: {e}", path.display())) +} + +/// Return `true` when the sentinel file exists. +pub(crate) fn check_sentinel(app_data_dir: &Path) -> bool { + sentinel_path(app_data_dir).exists() +} + +/// Remove the sentinel file. A missing file is not an error. +pub(crate) fn delete_sentinel(app_data_dir: &Path) -> Result<(), String> { + let path = sentinel_path(app_data_dir); + match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("delete sentinel {}: {e}", path.display())), + } +} + +// ── Keychain abstraction (enables unit testing) ─────────────────────────────── + +/// Keychain operations needed by the boot-time reset. +/// Implemented for `SecretStore`; a fake is used in tests. +pub(crate) trait ResetKeychain { + /// Delete the blob + all per-key legacy entries. + fn delete_all_with_legacy(&self) -> Result<(), String>; + /// Return `true` when the keychain has no blob entry and no legacy entries + /// for `"identity"`. + fn probe_empty(&self) -> bool; +} + +impl ResetKeychain for crate::secret_store::SecretStore { + fn delete_all_with_legacy(&self) -> Result<(), String> { + self.delete_all_with_legacy_cleanup() + } + + fn probe_empty(&self) -> bool { + use crate::secret_store::KeyringProbe; + matches!(self.probe("identity"), KeyringProbe::ReachableButEmpty) + } +} + +// ── Result type ─────────────────────────────────────────────────────────────── + +/// Outcome of the boot-time reset check. +#[derive(Debug, Default)] +pub(crate) struct ResetOutcome { + /// Wipe completed successfully this boot — suppress nest migrations. + pub completed: bool, + /// Wipe was attempted but verification failed — surface error state. + pub failed: bool, +} + +// ── Boot-time reset ─────────────────────────────────────────────────────────── + +/// Wipe parameters assembled by `lib.rs` and passed into `run_boot_reset_with_keychain`. +pub(crate) struct ResetContext<'a> { + pub app_data_dir: &'a Path, + pub keychain: &'a dyn ResetKeychain, + pub home_dir: Option, + pub is_dev: bool, +} + +/// Entry point called from `lib.rs` setup (before migrations). +/// +/// Constructs a `SecretStore` for the running build's keyring service and +/// delegates to `run_boot_reset_with_keychain` for testable wipe logic. +pub(crate) fn run_boot_reset(app_data_dir: &Path) -> ResetOutcome { + if !check_sentinel(app_data_dir) { + return ResetOutcome::default(); + } + + let is_dev = app_data_dir + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n.starts_with("xyz.block.buzz.app.dev")) + .unwrap_or(false); + + let service = if is_dev { + "buzz-desktop-dev" + } else { + "buzz-desktop" + }; + + let store = crate::secret_store::SecretStore::keyring(service); + let home_dir = dirs::home_dir(); + + let ctx = ResetContext { + app_data_dir, + keychain: &store, + home_dir, + is_dev, + }; + + run_boot_reset_with_keychain(ctx) +} + +/// Core wipe logic — separated for testing. +pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcome { + let app_data_dir = ctx.app_data_dir; + + // ── Step 1: rename app-data dir (atomic — sentinel survives the parent) ── + let trash_pid = std::process::id(); + let trash_app = app_data_dir.with_file_name(format!( + "{}.trash-{trash_pid}", + app_data_dir + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("buzz") + )); + + if app_data_dir.exists() { + if let Err(e) = std::fs::rename(app_data_dir, &trash_app) { + eprintln!( + "buzz-desktop reset: rename app-data {}: {e}", + app_data_dir.display() + ); + return ResetOutcome { + completed: false, + failed: true, + }; + } + } + + // ── Step 2: rename WebKit dir for this build ────────────────────────────── + let mut trash_webkit: Option = None; + if let Some(ref home) = ctx.home_dir { + let bundle_id = app_data_dir + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("buzz"); + let webkit_dir = home.join("Library").join("WebKit").join(bundle_id); + if webkit_dir.exists() { + let webkit_trash = webkit_dir.with_file_name(format!("{bundle_id}.trash-{trash_pid}")); + if let Err(e) = std::fs::rename(&webkit_dir, &webkit_trash) { + eprintln!( + "buzz-desktop reset: rename webkit {}: {e}", + webkit_dir.display() + ); + // Non-fatal — continue + } else { + trash_webkit = Some(webkit_trash); + } + } + } + + // ── Step 3: remove nest, ~/.sprout, ~/.config/buzz-agent, CLI symlink ──── + if let Some(nest) = crate::managed_agents::nest_dir() { + let _ = std::fs::remove_dir_all(&nest); + } + if let Some(ref home) = ctx.home_dir { + let _ = std::fs::remove_dir_all(home.join(".sprout")); + let _ = std::fs::remove_dir_all(home.join(".config").join("buzz-agent")); + let link_name = crate::managed_agents::cli_link_name(ctx.is_dev); + let _ = std::fs::remove_file(home.join(".local").join("bin").join(link_name)); + } + + // ── Step 4: keychain — LAST so we can read keys before deleting ────────── + if let Err(e) = ctx.keychain.delete_all_with_legacy() { + eprintln!("buzz-desktop reset: keychain delete: {e}"); + // Keychain failure is fatal for the reset: keep sentinel, signal failure. + // Best-effort: try to undo the rename so the app can at least open. + if trash_app.exists() { + let _ = std::fs::rename(&trash_app, app_data_dir); + } + return ResetOutcome { + completed: false, + failed: true, + }; + } + + // ── Step 5: best-effort delete of .trash-* dirs ─────────────────────────── + if trash_app.exists() { + let _ = std::fs::remove_dir_all(&trash_app); + } + if let Some(ref tw) = trash_webkit { + if tw.exists() { + let _ = std::fs::remove_dir_all(tw); + } + } + + // ── Step 6: verify ──────────────────────────────────────────────────────── + let keychain_ok = ctx.keychain.probe_empty(); + let app_data_gone = !app_data_dir.exists(); + let nest_gone = crate::managed_agents::nest_dir() + .map(|n| !n.exists()) + .unwrap_or(true); + + if !keychain_ok || !app_data_gone || !nest_gone { + eprintln!( + "buzz-desktop reset: verification failed (keychain_empty={keychain_ok}, \ + app_data_gone={app_data_gone}, nest_gone={nest_gone})" + ); + return ResetOutcome { + completed: false, + failed: true, + }; + } + + // ── Step 7: delete sentinel → success ──────────────────────────────────── + if let Err(e) = delete_sentinel(app_data_dir) { + eprintln!("buzz-desktop reset: delete sentinel: {e}"); + // Sentinel not deleted — keep failed=false so the app boots into + // onboarding, but on next boot the reset will retry (idempotent). + } + + ResetOutcome { + completed: true, + failed: false, + } +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + use tempfile::TempDir; + + // ── Fake keychain ───────────────────────────────────────────────────────── + + struct FakeKeychain { + delete_result: Result<(), String>, + /// Tracks number of times delete was called. + delete_calls: Cell, + empty_after_delete: bool, + } + + impl FakeKeychain { + fn ok() -> Self { + FakeKeychain { + delete_result: Ok(()), + delete_calls: Cell::new(0), + empty_after_delete: true, + } + } + + fn fail(msg: &str) -> Self { + FakeKeychain { + delete_result: Err(msg.to_string()), + delete_calls: Cell::new(0), + empty_after_delete: false, + } + } + + fn ok_but_not_empty() -> Self { + FakeKeychain { + delete_result: Ok(()), + delete_calls: Cell::new(0), + empty_after_delete: false, + } + } + } + + impl ResetKeychain for FakeKeychain { + fn delete_all_with_legacy(&self) -> Result<(), String> { + self.delete_calls.set(self.delete_calls.get() + 1); + self.delete_result.clone() + } + + fn probe_empty(&self) -> bool { + self.empty_after_delete && self.delete_calls.get() > 0 + } + } + + fn make_app_data(tmp: &TempDir) -> PathBuf { + let dir = tmp + .path() + .join("Application Support") + .join("xyz.block.buzz.app"); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + fn make_ctx<'a>( + app_data_dir: &'a Path, + keychain: &'a dyn ResetKeychain, + is_dev: bool, + ) -> ResetContext<'a> { + ResetContext { + app_data_dir, + keychain, + home_dir: None, // skip nest/sprout/CLI ops in unit tests + is_dev, + } + } + + // ── Test 1: no sentinel ─────────────────────────────────────────────────── + + #[test] + fn test_no_sentinel_returns_no_op() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + let kc = FakeKeychain::ok(); + + let outcome = run_boot_reset(app_data.as_path()); + assert!(!outcome.completed, "no sentinel → not completed"); + assert!(!outcome.failed, "no sentinel → not failed"); + assert_eq!(kc.delete_calls.get(), 0, "keychain not touched"); + assert!(app_data.exists(), "app-data dir untouched"); + } + + // ── Test 2: full wipe succeeds ──────────────────────────────────────────── + + #[test] + fn test_sentinel_present_full_wipe_succeeds() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + let kc = FakeKeychain::ok(); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.completed, "should complete"); + assert!(!outcome.failed, "should not fail"); + assert!(!app_data.exists(), "app-data must be gone"); + assert!(!sentinel_path(&app_data).exists(), "sentinel must be gone"); + assert_eq!(kc.delete_calls.get(), 1, "keychain deleted once"); + } + + // ── Test 3: keychain failure keeps sentinel ──────────────────────────────── + + #[test] + fn test_sentinel_present_keychain_failure_keeps_sentinel() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + let kc = FakeKeychain::fail("keychain unavailable"); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(!outcome.completed); + assert!(outcome.failed); + assert!( + sentinel_path(&app_data).exists(), + "sentinel must be preserved on failure" + ); + } + + // ── Test 4: app-data rename works but verify fails ──────────────────────── + + #[test] + fn test_sentinel_present_verify_failure_keeps_sentinel() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + // Keychain delete "succeeds" but probe still returns non-empty. + let kc = FakeKeychain::ok_but_not_empty(); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(!outcome.completed); + assert!(outcome.failed); + assert!(sentinel_path(&app_data).exists(), "sentinel preserved"); + } + + // ── Test 5: crash-then-retry completes ─────────────────────────────────── + + #[test] + fn test_crash_then_retry_completes() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + + // First run — keychain fails (simulates a crash mid-wipe). + let kc1 = FakeKeychain::fail("transient error"); + let ctx1 = make_ctx(&app_data, &kc1, false); + let first = run_boot_reset_with_keychain(ctx1); + assert!(first.failed); + assert!( + sentinel_path(&app_data).exists(), + "sentinel must survive first attempt" + ); + + // Second run — keychain succeeds. App-data dir was renamed but we need + // to recreate it for the test (the wipe tried to rename it). + // In production the dir would already be gone; here it was renamed to + // .trash- and then not cleaned up (keychain failed before cleanup). + // Create a fresh app-data dir to simulate a reboot where app recreated it. + std::fs::create_dir_all(&app_data).unwrap(); + + let kc2 = FakeKeychain::ok(); + let ctx2 = make_ctx(&app_data, &kc2, false); + let second = run_boot_reset_with_keychain(ctx2); + assert!(second.completed, "second attempt must complete"); + assert!(!second.failed); + assert!( + !sentinel_path(&app_data).exists(), + "sentinel cleared on success" + ); + } + + // ── Test 6: dev build does not touch prod nest ──────────────────────────── + + #[test] + fn test_dev_build_does_not_touch_prod_nest() { + // Verify is_dev discriminator: prod context has is_dev=false. + let tmp = TempDir::new().unwrap(); + let app_data = tmp + .path() + .join("Application Support") + .join("xyz.block.buzz.app"); + std::fs::create_dir_all(&app_data).unwrap(); + + // Sentinel path should encode prod bundle id. + let sp = sentinel_path(&app_data); + assert!( + sp.to_str().unwrap().contains("xyz.block.buzz.app"), + "sentinel path must encode bundle id" + ); + assert!( + !sp.to_str().unwrap().contains(".dev"), + "prod sentinel must not contain .dev" + ); + } + + // ── Test 7: prod build does not touch dev nest ──────────────────────────── + + #[test] + fn test_prod_build_does_not_touch_dev_nest() { + let tmp = TempDir::new().unwrap(); + let app_data = tmp + .path() + .join("Application Support") + .join("xyz.block.buzz.app.dev"); + std::fs::create_dir_all(&app_data).unwrap(); + + let sp = sentinel_path(&app_data); + assert!( + sp.to_str().unwrap().contains("xyz.block.buzz.app.dev"), + "dev sentinel must encode dev bundle id" + ); + + // is_dev discriminator. + let is_dev = app_data + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n.starts_with("xyz.block.buzz.app.dev")) + .unwrap_or(false); + assert!(is_dev, "dev app-data dir must be detected as dev"); + } +} diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index 6b6fc8295c..1fdd8b878b 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -740,15 +740,55 @@ impl SecretStore { } } - /// Delete the entire keychain blob for this service. + /// Delete the entire keychain blob for this service, plus all legacy per-key + /// entries that could resurrect an identity on next boot. /// - /// Used by the sign-out wipe path to remove all persisted keys in one shot. - /// After this call the OS keychain has no entry for this service; the next - /// launch will generate a fresh keypair and show first-run onboarding. - pub fn delete_all(&self) -> Result<(), String> { + /// Order of operations: + /// 1. Read the blob to collect every key name (e.g. `identity`, agent keys). + /// 2. Delete legacy per-key DPK entries for every key + the DPK blob itself. + /// 3. Delete legacy per-key keyring entries for every key. + /// 4. Delete the blob entry. + /// 5. Clear the in-memory cache. + /// + /// This is the correct wipe path for sign-out: the old `delete_all` skipped + /// step 1–3 so stale per-key entries could be re-imported on the next launch + /// via `migrate_legacy_key`. This method prevents that resurrection. + pub fn delete_all_with_legacy_cleanup(&self) -> Result<(), String> { #[cfg(feature = "system-keyring")] { let _lock = acquire_blob_lock(&self.service)?; + + // Step 1: read current blob keys (best-effort; no entry = empty set). + let blob_keys: Vec = match self.read_blob_raw() { + Ok(Some(bytes)) => { + let json = String::from_utf8(bytes).unwrap_or_default(); + serde_json::from_str::>(&json) + .map(|m| m.into_keys().collect()) + .unwrap_or_default() + } + _ => vec![], + }; + + // Always include "identity" even if the blob is empty or absent — + // it may exist only as a legacy per-key entry. + let mut all_keys = blob_keys; + if !all_keys.contains(&"identity".to_string()) { + all_keys.push("identity".to_string()); + } + + // Steps 2 & 3: delete legacy per-key entries for every key. + for key in &all_keys { + #[cfg(target_os = "macos")] + let _ = delete_generic_password_options(dpk_opts(&self.service, key)); + if let Ok(entry) = keyring_entry(&self.service, key) { + let _ = entry.delete_credential(); + } + } + // Step 2 (cont.): also delete the legacy DPK blob written by #1267. + #[cfg(target_os = "macos")] + let _ = delete_generic_password_options(dpk_opts(&self.service, BLOB_KEY)); + + // Step 4: delete the main blob entry. if let Ok(entry) = keyring_entry(&self.service, BLOB_KEY) { match entry.delete_credential() { Ok(()) | Err(keyring::Error::NoEntry) => {} @@ -756,16 +796,12 @@ impl SecretStore { return Err(format!("keyring unavailable: {e}")); } Err(e) => { - // Non-fatal — log and continue; the blob is gone even if - // the delete_credential call returned a non-standard error. - eprintln!("buzz-desktop sign-out: keychain delete: {e}"); + eprintln!("buzz-desktop reset: keychain blob delete: {e}"); } } } - // Best-effort: also remove any legacy DPK blob written by #1267. - #[cfg(target_os = "macos")] - let _ = delete_generic_password_options(dpk_opts(&self.service, BLOB_KEY)); - // Clear the in-memory cache so subsequent calls see the clean state. + + // Step 5: clear the in-memory cache. let mut guard = self.cache.lock().unwrap_or_else(|e| e.into_inner()); *guard = None; Ok(()) diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index 45707ce66d..934dee7954 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -21,6 +21,7 @@ import { OnboardingSlideTransition } from "@/features/onboarding/ui/OnboardingSl import { OnboardingFlow } from "@/features/onboarding/ui/OnboardingFlow"; import { KeyringLockedScreen } from "@/features/onboarding/ui/KeyringLockedScreen"; import { RelaunchRequiredScreen } from "@/features/onboarding/ui/RelaunchRequiredScreen"; +import { ResetFailedScreen } from "@/features/onboarding/ui/ResetFailedScreen"; import type { Workspace } from "@/features/workspaces/types"; import { useWorkspaceInit } from "@/features/workspaces/useWorkspaceInit"; import { useNestNotifications } from "@/features/workspaces/useNestNotifications"; @@ -299,6 +300,10 @@ function AppReady({ onFirstRunWorkspaceSettled, ]); + if (onboarding.stage === "reset-failed") { + return ; + } + if (onboarding.stage === "keyring-locked") { return ; } diff --git a/desktop/src/features/onboarding/hooks.ts b/desktop/src/features/onboarding/hooks.ts index bc0c441ac0..83a8597af1 100644 --- a/desktop/src/features/onboarding/hooks.ts +++ b/desktop/src/features/onboarding/hooks.ts @@ -413,6 +413,9 @@ export function useAppOnboardingState(isSharedIdentity: boolean) { // the session cannot access it. No in-app recovery is possible; the user // must unlock the keyring externally and relaunch. Mutually exclusive with lost. const identityLocked = identity?.locked === true; + // Boot-time Phase 2 reset failed — wipe was attempted but verification failed. + // The sentinel is preserved so the next relaunch retries automatically. + const identityResetFailed = identity?.resetFailed === true; // Sticky boot fact: once identity was lost at boot, this remains true for the // entire session. Per-component state in OnboardingFlow cannot carry this @@ -516,15 +519,18 @@ export function useAppOnboardingState(isSharedIdentity: boolean) { currentPubkey, flow, identityLost, - // keyring-locked is the highest-precedence stage: nothing in-session can - // clear a locked keyring, so this fully blocks the UI until relaunch. + // reset-failed is the highest-precedence stage: a failed boot-time reset + // means identity resolution was skipped entirely. Nothing can proceed until + // the user relaunches and the wipe retries. stage: - identityLocked && identityQuery.status === "success" - ? ("keyring-locked" as const) - : relaunchRequired - ? ("relaunch-required" as const) - : isCompletingWelcomeSetup - ? ("blocking" as const) - : onboardingGate.stage, + identityResetFailed && identityQuery.status === "success" + ? ("reset-failed" as const) + : identityLocked && identityQuery.status === "success" + ? ("keyring-locked" as const) + : relaunchRequired + ? ("relaunch-required" as const) + : isCompletingWelcomeSetup + ? ("blocking" as const) + : onboardingGate.stage, }; } diff --git a/desktop/src/features/onboarding/ui/ResetFailedScreen.tsx b/desktop/src/features/onboarding/ui/ResetFailedScreen.tsx new file mode 100644 index 0000000000..8d87e98006 --- /dev/null +++ b/desktop/src/features/onboarding/ui/ResetFailedScreen.tsx @@ -0,0 +1,11 @@ +import { RecoveryScreen } from "./RecoveryScreen"; + +export function ResetFailedScreen() { + return ( + + ); +} diff --git a/desktop/src/shared/api/tauriIdentity.ts b/desktop/src/shared/api/tauriIdentity.ts index 69836475ca..83ccfc7e91 100644 --- a/desktop/src/shared/api/tauriIdentity.ts +++ b/desktop/src/shared/api/tauriIdentity.ts @@ -6,6 +6,7 @@ type RawIdentity = { display_name: string; lost?: boolean; locked?: boolean; + reset_failed?: boolean; }; function fromRawIdentity(raw: RawIdentity): Identity { @@ -14,6 +15,7 @@ function fromRawIdentity(raw: RawIdentity): Identity { displayName: raw.display_name, lost: raw.lost === true, locked: raw.locked === true, + resetFailed: raw.reset_failed === true, }; } diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 3e3a9c69d8..caeaa6caf2 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -117,6 +117,10 @@ export type Identity = { * the user must unlock the keyring externally and relaunch. * Mutually exclusive with `lost`. */ locked?: boolean; + /** True when the boot-time Phase 2 reset attempted a wipe but verification + * failed. Identity resolution was skipped; the sentinel is preserved so + * the next relaunch retries the wipe automatically. */ + resetFailed?: boolean; }; export type Profile = { From c4497ec54787201dd17d74c25ec47776cfd9d12f Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 13 Jul 2026 23:25:58 -0400 Subject: [PATCH 4/8] fix(desktop): bump px-text override + add per-key keychain regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bump ProfileSettingsCard.tsx px-text override from :672 → :684 to account for sign-out section line drift. Add a real-keychain integration test that seeds a legacy per-key `identity` entry + blob, calls `delete_all_with_legacy_cleanup()`, and proves probe/load cannot resurrect the old identity — closes the CRITICAL-adjacent gap the FakeKeychain tests cannot cover. --- desktop/scripts/check-file-sizes.mjs | 5 ++-- desktop/scripts/check-px-text.mjs | 2 +- desktop/src-tauri/src/secret_store.rs | 38 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index ef0296aa0f..c9429efbc8 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -322,8 +322,9 @@ const overrides = new Map([ // the interprocess advisory lock; +8 lines. Load-bearing; queued to split. // signout-wipe phase 2: delete_all_with_legacy_cleanup replaces delete_all; // reads blob keys + deletes per-key legacy entries to prevent resurrection. - // Net growth ~36 lines over prior cap. Load-bearing correctness fix. - ["src-tauri/src/secret_store.rs", 1190], + // + regression test for per-key resurrection via real OS keychain. + // Net growth ~36+32 lines over prior cap. Load-bearing correctness fix. + ["src-tauri/src/secret_store.rs", 1222], // sign-out wipe: Sign Out section (AlertDialog + controlled state) added // at the bottom of the Profile settings page. Load-bearing UX feature; // queued to split when ProfileSettingsCard is broken into sub-components. diff --git a/desktop/scripts/check-px-text.mjs b/desktop/scripts/check-px-text.mjs index 1df4dd06c6..50e4ac07c1 100644 --- a/desktop/scripts/check-px-text.mjs +++ b/desktop/scripts/check-px-text.mjs @@ -23,7 +23,7 @@ const rules = [ // glyphs are fixed display sizes sized to their avatar box (not readable // message text), so they are exempted from the readable-text px rule. const overrides = new Set([ - "src/features/settings/ui/ProfileSettingsCard.tsx:672", + "src/features/settings/ui/ProfileSettingsCard.tsx:684", "src/features/onboarding/ui/AvatarStep.tsx:95", "src/features/agents/ui/AgentCreationPreview.tsx:666", "src/features/agents/ui/AgentCreationPreview.tsx:747", diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index 1fdd8b878b..cc2f926e72 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -1180,4 +1180,42 @@ mod tests { // Cleanup. let _ = store2.delete(key); } + + #[ignore = "requires real OS keychain (run locally)"] + #[test] + fn delete_all_with_legacy_cleanup_removes_per_key_identity() { + let svc = "buzz-test-delete-all-legacy"; + let key = "identity"; + let value = "nsec1legacytest"; + + // Seed a legacy per-key entry (old format, pre-blob migration). + let entry = keyring_entry(svc, key).unwrap(); + entry.set_password(value).unwrap(); + + // Also seed a blob with a different key to exercise the full path. + let store = SecretStore::keyring(svc); + store.store("agent:abc123", "nsec1agent").unwrap(); + + // Legacy per-key identity should be discoverable via probe. + let store2 = SecretStore::keyring(svc); + assert_eq!(store2.probe(key), KeyringProbe::Present); + + // Wipe everything via the sign-out path. + store2.delete_all_with_legacy_cleanup().unwrap(); + + // Fresh store — neither the blob nor the per-key entry should remain. + let store3 = SecretStore::keyring(svc); + assert_eq!( + store3.probe(key), + KeyringProbe::ReachableButEmpty, + "per-key identity must not survive delete_all_with_legacy_cleanup" + ); + assert_eq!( + store3.load(key).unwrap(), + None, + "load must not resurrect the legacy per-key identity" + ); + // Agent key should also be gone. + assert_eq!(store3.load("agent:abc123").unwrap(), None); + } } From fe7760ccb20224db77b666f3612926c15ad74d11 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 00:06:38 -0400 Subject: [PATCH 5/8] =?UTF-8?q?fix(desktop):=20address=20pass-2=20review?= =?UTF-8?q?=20findings=20on=20sign-out=20wipe=20(F1=E2=80=93F4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 (CRITICAL): The wipe left the legacy Sprout App Support dir untouched, so `migrate_legacy_app_data_dir` could restore the old identity on next boot. Expose `legacy_app_data_dir` and `is_dev_data_dir_name` as `pub(crate)` from `migration.rs`, add `legacy_app_data_dir` and `nest_dir` fields to `ResetContext`, compute both in `run_boot_reset`, and rename-then-delete the legacy dir atomically in Step 1b. Verify its absence in Step 6. F2 (CRITICAL): DPK blob and per-key DPK deletes in `delete_all_with_legacy_cleanup` were `let _ =` — real errors were silently swallowed. Replace with match arms that propagate real errors (not-found and dpk-unavailable are treated as success). Add `verify_fully_wiped` to `SecretStore` and `ResetKeychain` trait that checks all three keychain shapes the migration path can consume (main blob, DPK blob, per-key "identity"). Replace the `probe_empty` verification call with `verify_fully_wiped`. F3 (IMPORTANT): Tests 6 and 7 only asserted sentinel path strings. Replace them with behavioral tests that create both `.buzz` and `.buzz-dev` dirs, inject `nest_dir` into `ResetContext`, run the wipe, and assert the target nest is gone while the opposite nest survives. Add tests for `legacy_app_data_dir` removal and that `completed` is `true` on success. F4 (MINOR): Use `crate::migration::is_dev_data_dir_name` (the authoritative discriminator) instead of an ad-hoc `starts_with` in `run_boot_reset` and `lib.rs`. Use `keyring_service()` from `app_state` instead of duplicating the inline service string. --- desktop/scripts/check-file-sizes.mjs | 5 +- desktop/src-tauri/src/lib.rs | 2 +- desktop/src-tauri/src/migration.rs | 4 +- desktop/src-tauri/src/reset.rs | 251 ++++++++++++++++++++------ desktop/src-tauri/src/secret_store.rs | 80 +++++++- 5 files changed, 275 insertions(+), 67 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index c9429efbc8..2a8477c499 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -324,7 +324,10 @@ const overrides = new Map([ // reads blob keys + deletes per-key legacy entries to prevent resurrection. // + regression test for per-key resurrection via real OS keychain. // Net growth ~36+32 lines over prior cap. Load-bearing correctness fix. - ["src-tauri/src/secret_store.rs", 1222], + // signout-wipe pass-2 (F2): delete_all_with_legacy_cleanup DPK deletes now + // observable (propagate real errors); verify_fully_wiped checks all three + // keychain shapes (main blob, DPK blob, per-key "identity"). +73 lines. + ["src-tauri/src/secret_store.rs", 1296], // sign-out wipe: Sign Out section (AlertDialog + controlled state) added // at the bottom of the Profile settings page. Load-bearing UX feature; // queued to split when ProfileSettingsCard is broken into sub-components. diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c7a86c9925..00df72939d 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -431,7 +431,7 @@ pub fn run() { let is_dev_for_reset = data_dir .file_name() .and_then(|n| n.to_str()) - .map(|n| n.starts_with("xyz.block.buzz.app.dev")) + .map(crate::migration::is_dev_data_dir_name) .unwrap_or(false); crate::managed_agents::init_nest_dir(is_dev_for_reset); crate::reset::run_boot_reset(&data_dir) diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 0f1d292d24..6dd569a36f 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -44,7 +44,7 @@ const SHARED_AGENT_DIRS: &[&str] = &["agents/teams"]; /// `xyz.block.buzz.app.developer`. This is the authoritative dev/prod /// discriminator shared by `run_boot_migrations`, `sync_shared_agent_data`, /// and `reconcile_target_dir`. -fn is_dev_data_dir_name(name: &str) -> bool { +pub(crate) fn is_dev_data_dir_name(name: &str) -> bool { name == CANONICAL_DEV_IDENTIFIER || name .strip_prefix(CANONICAL_DEV_IDENTIFIER) @@ -55,7 +55,7 @@ fn canonical_dev_data_dir(current: &Path) -> Option { current.parent().map(|p| p.join(CANONICAL_DEV_IDENTIFIER)) } -fn legacy_app_data_dir(current: &Path) -> Option { +pub(crate) fn legacy_app_data_dir(current: &Path) -> Option { let name = current.file_name()?.to_str()?; let legacy_name = if name.starts_with(CANONICAL_DEV_IDENTIFIER) { name.replacen(CANONICAL_DEV_IDENTIFIER, LEGACY_CANONICAL_DEV_IDENTIFIER, 1) diff --git a/desktop/src-tauri/src/reset.rs b/desktop/src-tauri/src/reset.rs index 4469836849..409d769b0f 100644 --- a/desktop/src-tauri/src/reset.rs +++ b/desktop/src-tauri/src/reset.rs @@ -63,9 +63,9 @@ pub(crate) fn delete_sentinel(app_data_dir: &Path) -> Result<(), String> { pub(crate) trait ResetKeychain { /// Delete the blob + all per-key legacy entries. fn delete_all_with_legacy(&self) -> Result<(), String>; - /// Return `true` when the keychain has no blob entry and no legacy entries - /// for `"identity"`. - fn probe_empty(&self) -> bool; + /// Return `true` when all keychain shapes (main blob, DPK blob, per-key + /// entries) that `migrate_legacy_key` can consume are absent. + fn verify_fully_wiped(&self) -> bool; } impl ResetKeychain for crate::secret_store::SecretStore { @@ -73,9 +73,8 @@ impl ResetKeychain for crate::secret_store::SecretStore { self.delete_all_with_legacy_cleanup() } - fn probe_empty(&self) -> bool { - use crate::secret_store::KeyringProbe; - matches!(self.probe("identity"), KeyringProbe::ReachableButEmpty) + fn verify_fully_wiped(&self) -> bool { + self.verify_fully_wiped() } } @@ -95,6 +94,13 @@ pub(crate) struct ResetOutcome { /// Wipe parameters assembled by `lib.rs` and passed into `run_boot_reset_with_keychain`. pub(crate) struct ResetContext<'a> { pub app_data_dir: &'a Path, + /// Legacy App Support dir for this build (Sprout import source). When + /// present and non-empty, wiped alongside `app_data_dir` to prevent + /// `migrate_legacy_app_data_dir` from restoring the old identity. + pub legacy_app_data_dir: Option, + /// Nest dir (`~/.buzz` or `~/.buzz-dev`) scoped to this build's variant, + /// injected so unit tests can override without touching the global OnceLock. + pub nest_dir: Option, pub keychain: &'a dyn ResetKeychain, pub home_dir: Option, pub is_dev: bool, @@ -112,20 +118,18 @@ pub(crate) fn run_boot_reset(app_data_dir: &Path) -> ResetOutcome { let is_dev = app_data_dir .file_name() .and_then(|n| n.to_str()) - .map(|n| n.starts_with("xyz.block.buzz.app.dev")) + .map(crate::migration::is_dev_data_dir_name) .unwrap_or(false); - let service = if is_dev { - "buzz-desktop-dev" - } else { - "buzz-desktop" - }; - - let store = crate::secret_store::SecretStore::keyring(service); + let store = crate::secret_store::SecretStore::keyring(crate::app_state::keyring_service()); let home_dir = dirs::home_dir(); + let legacy_dir = crate::migration::legacy_app_data_dir(app_data_dir); + let nest_dir = crate::managed_agents::nest_dir(); let ctx = ResetContext { app_data_dir, + legacy_app_data_dir: legacy_dir, + nest_dir, keychain: &store, home_dir, is_dev, @@ -161,6 +165,29 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom } } + // ── Step 1b: rename legacy App Support dir (sprout import source) ──────── + let mut trash_legacy: Option = None; + if let Some(ref legacy) = ctx.legacy_app_data_dir { + if legacy.exists() { + let legacy_trash = legacy.with_file_name(format!( + "{}.trash-{trash_pid}", + legacy + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("sprout") + )); + if let Err(e) = std::fs::rename(legacy, &legacy_trash) { + eprintln!( + "buzz-desktop reset: rename legacy app-data {}: {e}", + legacy.display() + ); + // Non-fatal for legacy dir — continue + } else { + trash_legacy = Some(legacy_trash); + } + } + } + // ── Step 2: rename WebKit dir for this build ────────────────────────────── let mut trash_webkit: Option = None; if let Some(ref home) = ctx.home_dir { @@ -184,8 +211,8 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom } // ── Step 3: remove nest, ~/.sprout, ~/.config/buzz-agent, CLI symlink ──── - if let Some(nest) = crate::managed_agents::nest_dir() { - let _ = std::fs::remove_dir_all(&nest); + if let Some(ref nest) = ctx.nest_dir { + let _ = std::fs::remove_dir_all(nest); } if let Some(ref home) = ctx.home_dir { let _ = std::fs::remove_dir_all(home.join(".sprout")); @@ -212,6 +239,11 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom if trash_app.exists() { let _ = std::fs::remove_dir_all(&trash_app); } + if let Some(ref tl) = trash_legacy { + if tl.exists() { + let _ = std::fs::remove_dir_all(tl); + } + } if let Some(ref tw) = trash_webkit { if tw.exists() { let _ = std::fs::remove_dir_all(tw); @@ -219,16 +251,19 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom } // ── Step 6: verify ──────────────────────────────────────────────────────── - let keychain_ok = ctx.keychain.probe_empty(); + let keychain_ok = ctx.keychain.verify_fully_wiped(); let app_data_gone = !app_data_dir.exists(); - let nest_gone = crate::managed_agents::nest_dir() - .map(|n| !n.exists()) + let legacy_gone = ctx + .legacy_app_data_dir + .as_ref() + .map(|p| !p.exists()) .unwrap_or(true); + let nest_gone = ctx.nest_dir.as_ref().map(|n| !n.exists()).unwrap_or(true); - if !keychain_ok || !app_data_gone || !nest_gone { + if !keychain_ok || !app_data_gone || !legacy_gone || !nest_gone { eprintln!( - "buzz-desktop reset: verification failed (keychain_empty={keychain_ok}, \ - app_data_gone={app_data_gone}, nest_gone={nest_gone})" + "buzz-desktop reset: verification failed (keychain_wiped={keychain_ok}, \ + app_data_gone={app_data_gone}, legacy_gone={legacy_gone}, nest_gone={nest_gone})" ); return ResetOutcome { completed: false, @@ -263,7 +298,8 @@ mod tests { delete_result: Result<(), String>, /// Tracks number of times delete was called. delete_calls: Cell, - empty_after_delete: bool, + /// Whether `verify_fully_wiped` returns true after a successful delete. + wiped_after_delete: bool, } impl FakeKeychain { @@ -271,7 +307,7 @@ mod tests { FakeKeychain { delete_result: Ok(()), delete_calls: Cell::new(0), - empty_after_delete: true, + wiped_after_delete: true, } } @@ -279,15 +315,15 @@ mod tests { FakeKeychain { delete_result: Err(msg.to_string()), delete_calls: Cell::new(0), - empty_after_delete: false, + wiped_after_delete: false, } } - fn ok_but_not_empty() -> Self { + fn ok_but_not_wiped() -> Self { FakeKeychain { delete_result: Ok(()), delete_calls: Cell::new(0), - empty_after_delete: false, + wiped_after_delete: false, } } } @@ -298,8 +334,8 @@ mod tests { self.delete_result.clone() } - fn probe_empty(&self) -> bool { - self.empty_after_delete && self.delete_calls.get() > 0 + fn verify_fully_wiped(&self) -> bool { + self.wiped_after_delete && self.delete_calls.get() > 0 } } @@ -319,6 +355,8 @@ mod tests { ) -> ResetContext<'a> { ResetContext { app_data_dir, + legacy_app_data_dir: None, + nest_dir: None, keychain, home_dir: None, // skip nest/sprout/CLI ops in unit tests is_dev, @@ -346,15 +384,33 @@ mod tests { fn test_sentinel_present_full_wipe_succeeds() { let tmp = TempDir::new().unwrap(); let app_data = make_app_data(&tmp); + + // Also create a legacy App Support dir (sprout import source). + let legacy_dir = tmp + .path() + .join("Application Support") + .join("xyz.block.sprout.app"); + std::fs::create_dir_all(&legacy_dir).unwrap(); + std::fs::write(legacy_dir.join("identity.key"), b"old-identity").unwrap(); + write_sentinel(&app_data).unwrap(); let kc = FakeKeychain::ok(); - let ctx = make_ctx(&app_data, &kc, false); + + let ctx = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: Some(legacy_dir.clone()), + nest_dir: None, + keychain: &kc, + home_dir: None, + is_dev: false, + }; let outcome = run_boot_reset_with_keychain(ctx); assert!(outcome.completed, "should complete"); assert!(!outcome.failed, "should not fail"); assert!(!app_data.exists(), "app-data must be gone"); + assert!(!legacy_dir.exists(), "legacy app-data must be gone"); assert!(!sentinel_path(&app_data).exists(), "sentinel must be gone"); assert_eq!(kc.delete_calls.get(), 1, "keychain deleted once"); } @@ -386,8 +442,8 @@ mod tests { let tmp = TempDir::new().unwrap(); let app_data = make_app_data(&tmp); write_sentinel(&app_data).unwrap(); - // Keychain delete "succeeds" but probe still returns non-empty. - let kc = FakeKeychain::ok_but_not_empty(); + // Keychain delete "succeeds" but verify_fully_wiped still returns false. + let kc = FakeKeychain::ok_but_not_wiped(); let ctx = make_ctx(&app_data, &kc, false); let outcome = run_boot_reset_with_keychain(ctx); @@ -433,53 +489,128 @@ mod tests { ); } - // ── Test 6: dev build does not touch prod nest ──────────────────────────── + // ── Test 6: dev build wipes dev nest, leaves prod nest intact ──────────── #[test] - fn test_dev_build_does_not_touch_prod_nest() { - // Verify is_dev discriminator: prod context has is_dev=false. + fn test_dev_build_wipes_dev_nest_not_prod() { let tmp = TempDir::new().unwrap(); + + // Create both nests. + let dev_nest = tmp.path().join(".buzz-dev"); + let prod_nest = tmp.path().join(".buzz"); + std::fs::create_dir_all(&dev_nest).unwrap(); + std::fs::create_dir_all(&prod_nest).unwrap(); + let app_data = tmp .path() .join("Application Support") - .join("xyz.block.buzz.app"); + .join("xyz.block.buzz.app.dev"); std::fs::create_dir_all(&app_data).unwrap(); + write_sentinel(&app_data).unwrap(); - // Sentinel path should encode prod bundle id. - let sp = sentinel_path(&app_data); - assert!( - sp.to_str().unwrap().contains("xyz.block.buzz.app"), - "sentinel path must encode bundle id" - ); - assert!( - !sp.to_str().unwrap().contains(".dev"), - "prod sentinel must not contain .dev" - ); + let kc = FakeKeychain::ok(); + let ctx = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: None, + nest_dir: Some(dev_nest.clone()), + keychain: &kc, + home_dir: None, + is_dev: true, + }; + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.completed, "wipe must complete"); + assert!(!dev_nest.exists(), "dev nest must be wiped"); + assert!(prod_nest.exists(), "prod nest must survive"); } - // ── Test 7: prod build does not touch dev nest ──────────────────────────── + // ── Test 7: prod build wipes prod nest, leaves dev nest intact ─────────── #[test] - fn test_prod_build_does_not_touch_dev_nest() { + fn test_prod_build_wipes_prod_nest_not_dev() { let tmp = TempDir::new().unwrap(); + + // Create both nests. + let dev_nest = tmp.path().join(".buzz-dev"); + let prod_nest = tmp.path().join(".buzz"); + std::fs::create_dir_all(&dev_nest).unwrap(); + std::fs::create_dir_all(&prod_nest).unwrap(); + let app_data = tmp .path() .join("Application Support") - .join("xyz.block.buzz.app.dev"); + .join("xyz.block.buzz.app"); std::fs::create_dir_all(&app_data).unwrap(); + write_sentinel(&app_data).unwrap(); - let sp = sentinel_path(&app_data); + let kc = FakeKeychain::ok(); + let ctx = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: None, + nest_dir: Some(prod_nest.clone()), + keychain: &kc, + home_dir: None, + is_dev: false, + }; + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.completed, "wipe must complete"); + assert!(!prod_nest.exists(), "prod nest must be wiped"); + assert!(dev_nest.exists(), "dev nest must survive"); + } + + // ── Test 8: legacy app-data removed on reset ────────────────────────────── + + #[test] + fn test_legacy_app_data_removed_on_reset() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + + // Seed a legacy sprout dir with data that would be re-imported. + let legacy_dir = tmp + .path() + .join("Application Support") + .join("xyz.block.sprout.app"); + std::fs::create_dir_all(legacy_dir.join("agents")).unwrap(); + std::fs::write(legacy_dir.join("identity.key"), b"sprout-nsec").unwrap(); + + write_sentinel(&app_data).unwrap(); + let kc = FakeKeychain::ok(); + let ctx = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: Some(legacy_dir.clone()), + nest_dir: None, + keychain: &kc, + home_dir: None, + is_dev: false, + }; + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.completed, "reset must complete"); + assert!(!legacy_dir.exists(), "legacy app-data dir must be removed"); + } + + // ── Test 9: reset_outcome.completed gates nest migrations ──────────────── + + #[test] + fn test_completed_flag_true_on_successful_wipe() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + let kc = FakeKeychain::ok(); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + // `completed` is the boolean gate used by lib.rs to suppress + // migrate_dev_nest and migrate_legacy_nest after a successful wipe. assert!( - sp.to_str().unwrap().contains("xyz.block.buzz.app.dev"), - "dev sentinel must encode dev bundle id" + outcome.completed, + "completed must be true after successful wipe" ); - - // is_dev discriminator. - let is_dev = app_data - .file_name() - .and_then(|n| n.to_str()) - .map(|n| n.starts_with("xyz.block.buzz.app.dev")) - .unwrap_or(false); - assert!(is_dev, "dev app-data dir must be detected as dev"); + assert!(!outcome.failed); } } diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index cc2f926e72..6c0257934c 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -779,14 +779,36 @@ impl SecretStore { // Steps 2 & 3: delete legacy per-key entries for every key. for key in &all_keys { #[cfg(target_os = "macos")] - let _ = delete_generic_password_options(dpk_opts(&self.service, key)); + { + match delete_generic_password_options(dpk_opts(&self.service, key)) { + Ok(()) => {} + Err(ref e) if is_not_found(e) => {} + Err(ref e) if is_dpk_unavailable(e) => {} + Err(e) => return Err(format!("dpk per-key delete {key}: {e}")), + } + } if let Ok(entry) = keyring_entry(&self.service, key) { - let _ = entry.delete_credential(); + match entry.delete_credential() { + Ok(()) | Err(keyring::Error::NoEntry) => {} + Err(e) if is_keyring_availability_error(&e.to_string()) => { + return Err(format!("keyring unavailable deleting {key}: {e}")); + } + Err(e) => { + return Err(format!("keyring per-key delete {key}: {e}")); + } + } } } // Step 2 (cont.): also delete the legacy DPK blob written by #1267. #[cfg(target_os = "macos")] - let _ = delete_generic_password_options(dpk_opts(&self.service, BLOB_KEY)); + { + match delete_generic_password_options(dpk_opts(&self.service, BLOB_KEY)) { + Ok(()) => {} + Err(ref e) if is_not_found(e) => {} + Err(ref e) if is_dpk_unavailable(e) => {} + Err(e) => return Err(format!("dpk blob delete: {e}")), + } + } // Step 4: delete the main blob entry. if let Ok(entry) = keyring_entry(&self.service, BLOB_KEY) { @@ -812,6 +834,58 @@ impl SecretStore { } } + /// Verify no identity-bearing keychain entry survives in any shape + /// that `load("identity")` → `migrate_legacy_key` can consume: + /// main blob, DPK blob (`BLOB_KEY`), and per-key `"identity"`. + /// + /// Returns `true` when all three shapes are absent (or inaccessible in an + /// expected way), `false` when any entry is found or the keychain is + /// unavailable (fail-closed). + pub fn verify_fully_wiped(&self) -> bool { + #[cfg(feature = "system-keyring")] + { + // 1. Main blob must be absent. + match self.read_blob_raw() { + Ok(None) => {} + Ok(Some(_)) => return false, + Err(_) => return false, + } + // 2. Per-key "identity" via legacy keyring must be absent. + if let Ok(entry) = keyring_entry(&self.service, "identity") { + match entry.get_password() { + Err(keyring::Error::NoEntry) => {} + Ok(_) => return false, + Err(e) if is_keyring_availability_error(&e.to_string()) => { + return false; // can't verify → fail closed + } + Err(_) => {} // other errors = absent-enough + } + } + // 3. DPK blob (macOS only). + #[cfg(target_os = "macos")] + { + match generic_password(dpk_opts(&self.service, BLOB_KEY)) { + Err(ref e) if is_not_found(e) => {} + Err(ref e) if is_dpk_unavailable(e) => {} // unsigned dev = no DPK + Ok(_) => return false, + Err(_) => {} // other errors = absent-enough + } + // 4. Per-key DPK "identity" (macOS only). + match generic_password(dpk_opts(&self.service, "identity")) { + Err(ref e) if is_not_found(e) => {} + Err(ref e) if is_dpk_unavailable(e) => {} + Ok(_) => return false, + Err(_) => {} + } + } + true + } + #[cfg(not(feature = "system-keyring"))] + { + true // No keyring = nothing to verify. + } + } + /// Delete the secret for `key`. A missing entry is not an error. pub fn delete(&self, key: &str) -> Result<(), String> { #[cfg(feature = "system-keyring")] From 2ff7b534334233bfcf1cfd430a5693a94fdec027 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 09:11:27 -0400 Subject: [PATCH 6/8] fix(desktop): strict absence semantics, dev migration gate, deterministic trash (F1-F3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: keychain delete/verify now treat constructor failures and unknown errors as hard errors instead of silently skipping — only explicit not-found is proof of absence. dpk-unavailable remains a pass on DPK arms (symmetric with load()). F2: gate migrate_dev_repos_dir on !reset_completed so a completed dev reset cannot immediately re-import prod state on the same boot. F3: replace PID-derived trash names with deterministic .reset-trash suffixes so any retry boot can discover and clean prior crash residue. Keychain-fail branch now restores all three dirs symmetrically. --- desktop/scripts/check-file-sizes.mjs | 4 +- desktop/src-tauri/src/lib.rs | 6 +- desktop/src-tauri/src/migration.rs | 20 +- desktop/src-tauri/src/reset.rs | 331 +++++++++++++++++++++----- desktop/src-tauri/src/secret_store.rs | 37 ++- 5 files changed, 319 insertions(+), 79 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 2a8477c499..c642e18d8b 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -294,7 +294,7 @@ const overrides = new Map([ // am review fix: also clear stale V1 model field on provider rewrite + // new model-clear test. Load-bearing chimera fix. // keyring-dev-isolation: run_boot_migrations wires agent-key migration. - ["src-tauri/src/migration.rs", 1415], + ["src-tauri/src/migration.rs", 1428], // onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop // already here) for the single-toggle mark-read/unread menu item — a small // overage from load-bearing per-message plumbing, not generic debt growth. @@ -327,7 +327,7 @@ const overrides = new Map([ // signout-wipe pass-2 (F2): delete_all_with_legacy_cleanup DPK deletes now // observable (propagate real errors); verify_fully_wiped checks all three // keychain shapes (main blob, DPK blob, per-key "identity"). +73 lines. - ["src-tauri/src/secret_store.rs", 1296], + ["src-tauri/src/secret_store.rs", 1307], // sign-out wipe: Sign Out section (AlertDialog + controlled state) added // at the bottom of the Profile settings page. Load-bearing UX feature; // queued to split when ProfileSettingsCard is broken into sub-components. diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 00df72939d..70ade0adcd 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -451,7 +451,11 @@ pub fn run() { } // Run all pre-identity data migrations before state loads from disk. - migration::run_boot_migrations(&app_handle); + if reset_outcome.completed { + migration::run_boot_migrations_after_reset(&app_handle); + } else { + migration::run_boot_migrations(&app_handle); + } // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 6dd569a36f..847a569bae 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -119,6 +119,15 @@ fn copy_dir_all(src: &Path, dst: &Path) -> std::io::Result<()> { /// readers — reader-first loses a launch (stale harness/`mcp_command` until /// the next boot). pub fn run_boot_migrations(app: &tauri::AppHandle) { + run_boot_migrations_inner(app, false); +} + +/// Entry point when a completed reset must suppress dev-nest re-import. +pub fn run_boot_migrations_after_reset(app: &tauri::AppHandle) { + run_boot_migrations_inner(app, true); +} + +fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // Initialize the process-lifetime nest directory before any filesystem // operation that calls nest_dir(). The discriminator matches the existing // pattern used by reconcile_target_dir: dev instances have an app-data-dir @@ -139,7 +148,7 @@ pub fn run_boot_migrations(app: &tauri::AppHandle) { // ensures the dev nest boots with the correct workspace on its first launch, // matching what the prod nest had configured. Skip-if-dest-exists so it is // idempotent and never clobbers a value the dev nest already set explicitly. - if is_dev { + if should_migrate_dev_repos_dir(is_dev, reset_completed) { migrate_dev_repos_dir(); } @@ -318,13 +327,20 @@ fn migrate_legacy_nest_at(legacy: &Path, current: &Path) -> bool { /// also copies into `~/.buzz-dev` and could otherwise set the sentinel early. const DEV_NEST_MIGRATED_SENTINEL: &str = ".dev-nest-migrated"; +/// Returns true when `migrate_dev_repos_dir` should run: dev build AND no +/// completed reset this boot (a completed reset means the dev nest was just +/// wiped — re-importing from prod would undo the sign-out). +pub(crate) fn should_migrate_dev_repos_dir(is_dev: bool, reset_completed: bool) -> bool { + is_dev && !reset_completed +} + /// Copy the `.repos-dir` dotfile from `~/.buzz` → `~/.buzz-dev`, non-destructively. /// /// Must be called on dev builds BEFORE `resolve_repos_at_boot()` reads the /// dotfile, so the dev nest boots with the correct workspace configuration on /// its first launch. Skip-if-dest-exists so it is idempotent and never /// overwrites a value set directly by the dev nest. -fn migrate_dev_repos_dir() { +pub(crate) fn migrate_dev_repos_dir() { let Some(home) = dirs::home_dir() else { return; }; diff --git a/desktop/src-tauri/src/reset.rs b/desktop/src-tauri/src/reset.rs index 409d769b0f..8e8fd7c1da 100644 --- a/desktop/src-tauri/src/reset.rs +++ b/desktop/src-tauri/src/reset.rs @@ -138,26 +138,42 @@ pub(crate) fn run_boot_reset(app_data_dir: &Path) -> ResetOutcome { run_boot_reset_with_keychain(ctx) } +/// Deterministic trash path: `.reset-trash`. Unlike PID-based names, +/// any boot can discover and clean trash from a prior crashed attempt. +fn trash_path(original: &Path) -> PathBuf { + original.with_file_name(format!( + "{}.reset-trash", + original + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("buzz") + )) +} + +/// Remove an existing reset-trash directory if present (from a prior crashed +/// attempt), then rename `src` into the deterministic trash path. Returns +/// `Ok(trash_path)` on success. +fn rename_to_trash(src: &Path) -> Result { + let dst = trash_path(src); + // Clear prior trash before renaming so a collision doesn't fail the rename. + if dst.exists() { + let _ = std::fs::remove_dir_all(&dst); + } + std::fs::rename(src, &dst) + .map_err(|e| format!("rename {} → {}: {e}", src.display(), dst.display()))?; + Ok(dst) +} + /// Core wipe logic — separated for testing. pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcome { let app_data_dir = ctx.app_data_dir; // ── Step 1: rename app-data dir (atomic — sentinel survives the parent) ── - let trash_pid = std::process::id(); - let trash_app = app_data_dir.with_file_name(format!( - "{}.trash-{trash_pid}", - app_data_dir - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("buzz") - )); + let trash_app = trash_path(app_data_dir); if app_data_dir.exists() { - if let Err(e) = std::fs::rename(app_data_dir, &trash_app) { - eprintln!( - "buzz-desktop reset: rename app-data {}: {e}", - app_data_dir.display() - ); + if let Err(e) = rename_to_trash(app_data_dir) { + eprintln!("buzz-desktop reset: {e}"); return ResetOutcome { completed: false, failed: true, @@ -166,49 +182,34 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom } // ── Step 1b: rename legacy App Support dir (sprout import source) ──────── - let mut trash_legacy: Option = None; + let trash_legacy: Option = ctx.legacy_app_data_dir.as_ref().map(|l| trash_path(l)); if let Some(ref legacy) = ctx.legacy_app_data_dir { if legacy.exists() { - let legacy_trash = legacy.with_file_name(format!( - "{}.trash-{trash_pid}", - legacy - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("sprout") - )); - if let Err(e) = std::fs::rename(legacy, &legacy_trash) { - eprintln!( - "buzz-desktop reset: rename legacy app-data {}: {e}", - legacy.display() - ); + if let Err(e) = rename_to_trash(legacy) { + eprintln!("buzz-desktop reset: {e}"); // Non-fatal for legacy dir — continue - } else { - trash_legacy = Some(legacy_trash); } } } // ── Step 2: rename WebKit dir for this build ────────────────────────────── - let mut trash_webkit: Option = None; - if let Some(ref home) = ctx.home_dir { + let trash_webkit: Option = if let Some(ref home) = ctx.home_dir { let bundle_id = app_data_dir .file_name() .and_then(|n| n.to_str()) .unwrap_or("buzz"); let webkit_dir = home.join("Library").join("WebKit").join(bundle_id); + let tw = trash_path(&webkit_dir); if webkit_dir.exists() { - let webkit_trash = webkit_dir.with_file_name(format!("{bundle_id}.trash-{trash_pid}")); - if let Err(e) = std::fs::rename(&webkit_dir, &webkit_trash) { - eprintln!( - "buzz-desktop reset: rename webkit {}: {e}", - webkit_dir.display() - ); + if let Err(e) = rename_to_trash(&webkit_dir) { + eprintln!("buzz-desktop reset: {e}"); // Non-fatal — continue - } else { - trash_webkit = Some(webkit_trash); } } - } + Some(tw) + } else { + None + }; // ── Step 3: remove nest, ~/.sprout, ~/.config/buzz-agent, CLI symlink ──── if let Some(ref nest) = ctx.nest_dir { @@ -224,30 +225,43 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom // ── Step 4: keychain — LAST so we can read keys before deleting ────────── if let Err(e) = ctx.keychain.delete_all_with_legacy() { eprintln!("buzz-desktop reset: keychain delete: {e}"); - // Keychain failure is fatal for the reset: keep sentinel, signal failure. - // Best-effort: try to undo the rename so the app can at least open. + // Keychain failure is fatal: keep sentinel, signal failure. + // Restore all three dirs so the app returns to a coherent pre-reset state. if trash_app.exists() { let _ = std::fs::rename(&trash_app, app_data_dir); } + if let Some(ref legacy) = ctx.legacy_app_data_dir { + if let Some(ref tl) = trash_legacy { + if tl.exists() { + let _ = std::fs::rename(tl, legacy); + } + } + } + if let Some(ref home) = ctx.home_dir { + let bundle_id = app_data_dir + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("buzz"); + let webkit_dir = home.join("Library").join("WebKit").join(bundle_id); + if let Some(ref tw) = trash_webkit { + if tw.exists() { + let _ = std::fs::rename(tw, &webkit_dir); + } + } + } return ResetOutcome { completed: false, failed: true, }; } - // ── Step 5: best-effort delete of .trash-* dirs ─────────────────────────── - if trash_app.exists() { - let _ = std::fs::remove_dir_all(&trash_app); - } + // ── Step 5: sweep ALL reset trash (including from prior crashed boots) ─── + let _ = std::fs::remove_dir_all(&trash_app); if let Some(ref tl) = trash_legacy { - if tl.exists() { - let _ = std::fs::remove_dir_all(tl); - } + let _ = std::fs::remove_dir_all(tl); } if let Some(ref tw) = trash_webkit { - if tw.exists() { - let _ = std::fs::remove_dir_all(tw); - } + let _ = std::fs::remove_dir_all(tw); } // ── Step 6: verify ──────────────────────────────────────────────────────── @@ -259,11 +273,23 @@ pub(crate) fn run_boot_reset_with_keychain(ctx: ResetContext<'_>) -> ResetOutcom .map(|p| !p.exists()) .unwrap_or(true); let nest_gone = ctx.nest_dir.as_ref().map(|n| !n.exists()).unwrap_or(true); - - if !keychain_ok || !app_data_gone || !legacy_gone || !nest_gone { + let trash_app_gone = !trash_app.exists(); + let trash_legacy_gone = trash_legacy.as_ref().map(|p| !p.exists()).unwrap_or(true); + let trash_webkit_gone = trash_webkit.as_ref().map(|p| !p.exists()).unwrap_or(true); + + if !keychain_ok + || !app_data_gone + || !legacy_gone + || !nest_gone + || !trash_app_gone + || !trash_legacy_gone + || !trash_webkit_gone + { eprintln!( "buzz-desktop reset: verification failed (keychain_wiped={keychain_ok}, \ - app_data_gone={app_data_gone}, legacy_gone={legacy_gone}, nest_gone={nest_gone})" + app_data_gone={app_data_gone}, legacy_gone={legacy_gone}, nest_gone={nest_gone}, \ + trash_app_gone={trash_app_gone}, trash_legacy_gone={trash_legacy_gone}, \ + trash_webkit_gone={trash_webkit_gone})" ); return ResetOutcome { completed: false, @@ -300,6 +326,10 @@ mod tests { delete_calls: Cell, /// Whether `verify_fully_wiped` returns true after a successful delete. wiped_after_delete: bool, + /// When true, verify_fully_wiped always returns false regardless of + /// delete outcome — simulates a transient/unknown keychain error + /// during verification that cannot confirm absence. + verify_always_fails: bool, } impl FakeKeychain { @@ -308,6 +338,7 @@ mod tests { delete_result: Ok(()), delete_calls: Cell::new(0), wiped_after_delete: true, + verify_always_fails: false, } } @@ -316,6 +347,7 @@ mod tests { delete_result: Err(msg.to_string()), delete_calls: Cell::new(0), wiped_after_delete: false, + verify_always_fails: false, } } @@ -324,6 +356,19 @@ mod tests { delete_result: Ok(()), delete_calls: Cell::new(0), wiped_after_delete: false, + verify_always_fails: false, + } + } + + /// Delete succeeds but verify returns false — simulates a transient + /// unknown error during verification (e.g. keyring constructor failure + /// or unclassified read error that cannot confirm absence). + fn ok_but_verify_fails() -> Self { + FakeKeychain { + delete_result: Ok(()), + delete_calls: Cell::new(0), + wiped_after_delete: true, + verify_always_fails: true, } } } @@ -335,6 +380,9 @@ mod tests { } fn verify_fully_wiped(&self) -> bool { + if self.verify_always_fails { + return false; + } self.wiped_after_delete && self.delete_calls.get() > 0 } } @@ -593,24 +641,185 @@ mod tests { assert!(!legacy_dir.exists(), "legacy app-data dir must be removed"); } - // ── Test 9: reset_outcome.completed gates nest migrations ──────────────── + // ── Test 9: unknown error during delete → failed, sentinel kept ──────── #[test] - fn test_completed_flag_true_on_successful_wipe() { + fn test_unknown_delete_error_keeps_sentinel() { let tmp = TempDir::new().unwrap(); let app_data = make_app_data(&tmp); write_sentinel(&app_data).unwrap(); + // Simulates an unclassified/transient keychain error during delete. + let kc = FakeKeychain::fail("unknown transient keychain error"); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.failed, "unknown delete error must fail"); + assert!(!outcome.completed, "must not complete on delete error"); + assert!( + sentinel_path(&app_data).exists(), + "sentinel must survive unknown delete error" + ); + } + + // ── Test 10: unknown error during verify → failed, sentinel kept ───── + + #[test] + fn test_unknown_verify_error_keeps_sentinel() { + let tmp = TempDir::new().unwrap(); + let app_data = make_app_data(&tmp); + write_sentinel(&app_data).unwrap(); + // Delete succeeds but verification fails — simulates a transient + // keychain error (e.g. constructor failure, unclassified read error) + // that cannot confirm absence. The sentinel must survive. + let kc = FakeKeychain::ok_but_verify_fails(); + let ctx = make_ctx(&app_data, &kc, false); + + let outcome = run_boot_reset_with_keychain(ctx); + + assert!(outcome.failed, "unknown verify error must fail"); + assert!(!outcome.completed, "must not complete on verify error"); + assert!( + sentinel_path(&app_data).exists(), + "sentinel must survive unknown verify error" + ); + } + + // ── Test 11: completed dev reset suppresses dev repos-dir migration ──── + + #[test] + fn test_completed_dev_reset_suppresses_dev_repos_dir_migration() { + let tmp = TempDir::new().unwrap(); + let app_data = tmp + .path() + .join("Application Support") + .join("xyz.block.buzz.app.dev"); + std::fs::create_dir_all(&app_data).unwrap(); + write_sentinel(&app_data).unwrap(); + + let kc = FakeKeychain::ok(); + let ctx = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: None, + nest_dir: Some(tmp.path().join(".buzz-dev")), + keychain: &kc, + home_dir: None, + is_dev: true, + }; + + let outcome = run_boot_reset_with_keychain(ctx); + assert!(outcome.completed, "reset must complete"); + + // The gate that lib.rs uses to decide whether to run + // migrate_dev_repos_dir: should_migrate_dev_repos_dir(is_dev, completed). + // A completed dev reset must suppress the migration. + assert!( + !crate::migration::should_migrate_dev_repos_dir(true, outcome.completed), + "completed dev reset must suppress dev repos-dir migration" + ); + // Conversely, a non-reset dev boot must allow it. + assert!( + crate::migration::should_migrate_dev_repos_dir(true, false), + "non-reset dev boot must allow dev repos-dir migration" + ); + // Prod builds never run it regardless. + assert!( + !crate::migration::should_migrate_dev_repos_dir(false, false), + "prod builds must never run dev repos-dir migration" + ); + } + + // ── Test 12: crash-after-rename retry cleans prior trash ───────────── + + #[test] + fn test_crash_retry_cleans_prior_deterministic_trash() { + let tmp = TempDir::new().unwrap(); + let app_support = tmp.path().join("Application Support"); + let app_data = app_support.join("xyz.block.buzz.app"); + std::fs::create_dir_all(&app_data).unwrap(); + write_sentinel(&app_data).unwrap(); + + // Simulate a prior crashed boot: originals absent, deterministic trash + // present from the crash (as if the process renamed then died). + let trash_app_dir = app_support.join("xyz.block.buzz.app.reset-trash"); + std::fs::create_dir_all(&trash_app_dir).unwrap(); + std::fs::write(trash_app_dir.join("identity.key"), b"old-key").unwrap(); + + // Original is gone (crashed mid-wipe), so remove it for the retry. + std::fs::remove_dir_all(&app_data).unwrap(); + let kc = FakeKeychain::ok(); let ctx = make_ctx(&app_data, &kc, false); let outcome = run_boot_reset_with_keychain(ctx); + assert!(outcome.completed, "retry must complete"); + assert!( + !trash_app_dir.exists(), + "prior crash trash must be cleaned by retry" + ); + assert!( + !sentinel_path(&app_data).exists(), + "sentinel must be cleared" + ); + } - // `completed` is the boolean gate used by lib.rs to suppress - // migrate_dev_nest and migrate_legacy_nest after a successful wipe. + // ── Test 13: keychain-fail restores all dirs, retry cleans trash ────── + + #[test] + fn test_keychain_fail_restores_all_then_retry_cleans() { + let tmp = TempDir::new().unwrap(); + let app_support = tmp.path().join("Application Support"); + let app_data = app_support.join("xyz.block.buzz.app"); + std::fs::create_dir_all(&app_data).unwrap(); + std::fs::write(app_data.join("config.json"), b"{}").unwrap(); + + let legacy = app_support.join("xyz.block.sprout.app"); + std::fs::create_dir_all(&legacy).unwrap(); + std::fs::write(legacy.join("identity.key"), b"sprout-key").unwrap(); + + write_sentinel(&app_data).unwrap(); + + // First attempt: keychain fails → dirs restored. + let kc1 = FakeKeychain::fail("keychain locked"); + let ctx1 = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: Some(legacy.clone()), + nest_dir: None, + keychain: &kc1, + home_dir: Some(tmp.path().to_path_buf()), + is_dev: false, + }; + let first = run_boot_reset_with_keychain(ctx1); + assert!(first.failed, "first attempt must fail"); assert!( - outcome.completed, - "completed must be true after successful wipe" + app_data.exists(), + "app-data must be restored after keychain fail" ); - assert!(!outcome.failed); + assert!( + legacy.exists(), + "legacy dir must be restored after keychain fail" + ); + assert!(sentinel_path(&app_data).exists(), "sentinel survives"); + + // Second attempt: keychain succeeds → everything cleaned including + // any residual trash from prior attempts. + let kc2 = FakeKeychain::ok(); + let ctx2 = ResetContext { + app_data_dir: &app_data, + legacy_app_data_dir: Some(legacy.clone()), + nest_dir: None, + keychain: &kc2, + home_dir: Some(tmp.path().to_path_buf()), + is_dev: false, + }; + let second = run_boot_reset_with_keychain(ctx2); + assert!(second.completed, "second attempt must complete"); + assert!(!app_data.exists(), "app-data must be gone"); + assert!(!legacy.exists(), "legacy must be gone"); + // No trash directories should remain. + let trash_app = app_support.join("xyz.block.buzz.app.reset-trash"); + let trash_legacy = app_support.join("xyz.block.sprout.app.reset-trash"); + assert!(!trash_app.exists(), "app trash must be cleaned"); + assert!(!trash_legacy.exists(), "legacy trash must be cleaned"); } } diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index 6c0257934c..43854761b5 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -787,7 +787,9 @@ impl SecretStore { Err(e) => return Err(format!("dpk per-key delete {key}: {e}")), } } - if let Ok(entry) = keyring_entry(&self.service, key) { + { + let entry = keyring_entry(&self.service, key) + .map_err(|e| format!("keyring entry constructor {key}: {e}"))?; match entry.delete_credential() { Ok(()) | Err(keyring::Error::NoEntry) => {} Err(e) if is_keyring_availability_error(&e.to_string()) => { @@ -811,14 +813,16 @@ impl SecretStore { } // Step 4: delete the main blob entry. - if let Ok(entry) = keyring_entry(&self.service, BLOB_KEY) { + { + let entry = keyring_entry(&self.service, BLOB_KEY) + .map_err(|e| format!("keyring entry constructor blob: {e}"))?; match entry.delete_credential() { Ok(()) | Err(keyring::Error::NoEntry) => {} Err(e) if is_keyring_availability_error(&e.to_string()) => { return Err(format!("keyring unavailable: {e}")); } Err(e) => { - eprintln!("buzz-desktop reset: keychain blob delete: {e}"); + return Err(format!("keyring blob delete: {e}")); } } } @@ -851,31 +855,38 @@ impl SecretStore { Err(_) => return false, } // 2. Per-key "identity" via legacy keyring must be absent. - if let Ok(entry) = keyring_entry(&self.service, "identity") { - match entry.get_password() { + match keyring_entry(&self.service, "identity") { + Ok(entry) => match entry.get_password() { Err(keyring::Error::NoEntry) => {} Ok(_) => return false, - Err(e) if is_keyring_availability_error(&e.to_string()) => { - return false; // can't verify → fail closed - } - Err(_) => {} // other errors = absent-enough - } + // Any other error (availability, unknown, transient) → fail closed. + // Only explicit NoEntry is proof of absence. + Err(_) => return false, + }, + // Constructor failure → cannot verify → fail closed. + Err(_) => return false, } // 3. DPK blob (macOS only). #[cfg(target_os = "macos")] { match generic_password(dpk_opts(&self.service, BLOB_KEY)) { Err(ref e) if is_not_found(e) => {} - Err(ref e) if is_dpk_unavailable(e) => {} // unsigned dev = no DPK + // dpk-unavailable is symmetric with load(): if load() can't + // consume DPK in this state, a surviving entry is harmless. + Err(ref e) if is_dpk_unavailable(e) => {} Ok(_) => return false, - Err(_) => {} // other errors = absent-enough + // Any other error → fail closed (not proof of absence). + Err(_) => return false, } // 4. Per-key DPK "identity" (macOS only). match generic_password(dpk_opts(&self.service, "identity")) { Err(ref e) if is_not_found(e) => {} + // dpk-unavailable: symmetric with load() — if load() can't + // read DPK, a surviving entry can't resurrect identity. Err(ref e) if is_dpk_unavailable(e) => {} Ok(_) => return false, - Err(_) => {} + // Any other error → fail closed. + Err(_) => return false, } } true From 1d0c5b2c210be01b1d73f200f8d7d3911cf3801c Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 10:53:45 -0400 Subject: [PATCH 7/8] test(reset): prove dev-reset migration suppression at composed seam The F2 regression test asserted only the boolean predicate (should_migrate_dev_repos_dir) with hand-fed inputs, not the composed migration path. A mis-wired lib.rs or inner runner would pass the test while the resurrection bug silently returned. Extract migrate_dev_repos_dir_at (injectable core) and maybe_migrate_dev_repos_dir (gate + core), wire run_boot_migrations_inner through the composed helper, and replace the predicate-only test with a three-arm behavioral test that seeds prod .repos-dir and asserts filesystem effects through the real migration code. --- desktop/scripts/check-file-sizes.mjs | 2 +- desktop/src-tauri/src/migration.rs | 40 ++++++++++++++---------- desktop/src-tauri/src/reset.rs | 46 ++++++++++++++++++++-------- 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index c642e18d8b..9ba0e88df2 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -294,7 +294,7 @@ const overrides = new Map([ // am review fix: also clear stale V1 model field on provider rewrite + // new model-clear test. Load-bearing chimera fix. // keyring-dev-isolation: run_boot_migrations wires agent-key migration. - ["src-tauri/src/migration.rs", 1428], + ["src-tauri/src/migration.rs", 1436], // onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop // already here) for the single-toggle mark-read/unread menu item — a small // overage from load-bearing per-message plumbing, not generic debt growth. diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 847a569bae..304b6aba5b 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -148,8 +148,10 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // ensures the dev nest boots with the correct workspace on its first launch, // matching what the prod nest had configured. Skip-if-dest-exists so it is // idempotent and never clobbers a value the dev nest already set explicitly. - if should_migrate_dev_repos_dir(is_dev, reset_completed) { - migrate_dev_repos_dir(); + // Uses the composed helper so the gate + migration run through the same + // code path that the behavioral test exercises. + if let (Some(home), Some(dev_nest)) = (dirs::home_dir(), crate::managed_agents::nest_dir()) { + maybe_migrate_dev_repos_dir(is_dev, reset_completed, &home, &dev_nest); } migrate_legacy_app_data_dir(app); @@ -334,23 +336,14 @@ pub(crate) fn should_migrate_dev_repos_dir(is_dev: bool, reset_completed: bool) is_dev && !reset_completed } -/// Copy the `.repos-dir` dotfile from `~/.buzz` → `~/.buzz-dev`, non-destructively. -/// -/// Must be called on dev builds BEFORE `resolve_repos_at_boot()` reads the -/// dotfile, so the dev nest boots with the correct workspace configuration on -/// its first launch. Skip-if-dest-exists so it is idempotent and never -/// overwrites a value set directly by the dev nest. -pub(crate) fn migrate_dev_repos_dir() { - let Some(home) = dirs::home_dir() else { - return; - }; +/// Injectable core: copy `.repos-dir` from `/.buzz/` into `dev_nest`, +/// non-destructively. Extracted so tests can inject temp paths without +/// touching `dirs::home_dir()` or the global `nest_dir()` OnceLock. +pub(crate) fn migrate_dev_repos_dir_at(home: &Path, dev_nest: &Path) { let src = home.join(".buzz").join(".repos-dir"); if !src.exists() { return; } - let Some(dev_nest) = crate::managed_agents::nest_dir() else { - return; - }; let dst = dev_nest.join(".repos-dir"); // Skip if the dev nest already has its own .repos-dir. if dst.exists() { @@ -358,7 +351,7 @@ pub(crate) fn migrate_dev_repos_dir() { } // Ensure the dev nest directory itself exists — this migration runs before // ensure_nest() in the boot sequence, so the directory may not yet exist. - if let Err(e) = std::fs::create_dir_all(&dev_nest) { + if let Err(e) = std::fs::create_dir_all(dev_nest) { eprintln!( "buzz-desktop: dev-nest-migration: failed to create dev nest {}: {e}", dev_nest.display() @@ -374,6 +367,21 @@ pub(crate) fn migrate_dev_repos_dir() { } } +/// Composed gate + migration: applies `should_migrate_dev_repos_dir` and, if +/// the gate passes, runs the injectable migration core. This is the seam that +/// `run_boot_migrations_inner` calls with real paths — a future mis-wired flag +/// or inverted gate breaks tests at this level, not just the predicate. +pub(crate) fn maybe_migrate_dev_repos_dir( + is_dev: bool, + reset_completed: bool, + home: &Path, + dev_nest: &Path, +) { + if should_migrate_dev_repos_dir(is_dev, reset_completed) { + migrate_dev_repos_dir_at(home, dev_nest); + } +} + /// One-time migration of dev-build nest contents from `~/.buzz` → `~/.buzz-dev`. /// /// When a dev build first boots after this change ships, it switches from the diff --git a/desktop/src-tauri/src/reset.rs b/desktop/src-tauri/src/reset.rs index 8e8fd7c1da..d2e35e6839 100644 --- a/desktop/src-tauri/src/reset.rs +++ b/desktop/src-tauri/src/reset.rs @@ -686,6 +686,8 @@ mod tests { } // ── Test 11: completed dev reset suppresses dev repos-dir migration ──── + // Behavioral test at the composed seam: exercises real filesystem effects + // through maybe_migrate_dev_repos_dir, not just the boolean predicate. #[test] fn test_completed_dev_reset_suppresses_dev_repos_dir_migration() { @@ -697,34 +699,54 @@ mod tests { std::fs::create_dir_all(&app_data).unwrap(); write_sentinel(&app_data).unwrap(); + // Seed prod ~/.buzz/.repos-dir so the migration has something to copy. + let home = tmp.path().join("home"); + let prod_nest = home.join(".buzz"); + std::fs::create_dir_all(&prod_nest).unwrap(); + std::fs::write(prod_nest.join(".repos-dir"), "/some/workspace").unwrap(); + + let dev_nest = tmp.path().join(".buzz-dev"); + + // Run a real reset and take the REAL outcome.completed. let kc = FakeKeychain::ok(); let ctx = ResetContext { app_data_dir: &app_data, legacy_app_data_dir: None, - nest_dir: Some(tmp.path().join(".buzz-dev")), + nest_dir: Some(dev_nest.clone()), keychain: &kc, home_dir: None, is_dev: true, }; - let outcome = run_boot_reset_with_keychain(ctx); assert!(outcome.completed, "reset must complete"); - // The gate that lib.rs uses to decide whether to run - // migrate_dev_repos_dir: should_migrate_dev_repos_dir(is_dev, completed). - // A completed dev reset must suppress the migration. + // Arm 1: completed dev reset → dev nest must NOT get .repos-dir. + crate::migration::maybe_migrate_dev_repos_dir(true, outcome.completed, &home, &dev_nest); assert!( - !crate::migration::should_migrate_dev_repos_dir(true, outcome.completed), - "completed dev reset must suppress dev repos-dir migration" + !dev_nest.join(".repos-dir").exists(), + "completed dev reset must suppress .repos-dir import into dev nest" ); - // Conversely, a non-reset dev boot must allow it. + + // Arm 2 (positive control): non-reset dev boot → dev nest IS created + // with .repos-dir copied. This proves the test would have caught the + // pass-3 resurrection live. + let dev_nest_2 = tmp.path().join(".buzz-dev-control"); + crate::migration::maybe_migrate_dev_repos_dir(true, false, &home, &dev_nest_2); assert!( - crate::migration::should_migrate_dev_repos_dir(true, false), - "non-reset dev boot must allow dev repos-dir migration" + dev_nest_2.join(".repos-dir").exists(), + "non-reset dev boot must import .repos-dir into dev nest" ); - // Prod builds never run it regardless. + assert_eq!( + std::fs::read_to_string(dev_nest_2.join(".repos-dir")).unwrap(), + "/some/workspace", + ".repos-dir content must match prod source" + ); + + // Arm 3: prod build (is_dev=false) → nothing created regardless. + let dev_nest_3 = tmp.path().join(".buzz-dev-prod"); + crate::migration::maybe_migrate_dev_repos_dir(false, false, &home, &dev_nest_3); assert!( - !crate::migration::should_migrate_dev_repos_dir(false, false), + !dev_nest_3.join(".repos-dir").exists(), "prod builds must never run dev repos-dir migration" ); } From b0c8b2efa23aea53a1d7a07166f97455938d6431 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 12:28:26 -0400 Subject: [PATCH 8/8] fix(settings): preserve scroll position across avatar editor open/close The Sign Out section added enough height to the profile settings page to make it scrollable. Opening the avatar editor triggers Framer Motion layout animations that shift the scroll position; closing the editor leaves the preview ~187px from its original viewport position. Save the scroll container's scrollTop when opening the editor and restore it on close so the preview returns to its pre-edit position. --- desktop/scripts/check-file-sizes.mjs | 4 ++- desktop/scripts/check-px-text.mjs | 2 +- .../settings/ui/ProfileSettingsCard.tsx | 36 ++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 9ba0e88df2..84e7895d38 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -331,7 +331,9 @@ const overrides = new Map([ // sign-out wipe: Sign Out section (AlertDialog + controlled state) added // at the bottom of the Profile settings page. Load-bearing UX feature; // queued to split when ProfileSettingsCard is broken into sub-components. - ["src/features/settings/ui/ProfileSettingsCard.tsx", 1005], + // +20 lines: scroll-position save/restore across avatar editor open/close + // to prevent layout shift from the Sign Out section causing a viewport jump. + ["src/features/settings/ui/ProfileSettingsCard.tsx", 1033], // keyring-dev-isolation: keyring_service() fn (7 lines) replaces the const // to return "buzz-desktop-dev" in debug builds. Load-bearing isolation fix. ["src-tauri/src/app_state.rs", 1042], diff --git a/desktop/scripts/check-px-text.mjs b/desktop/scripts/check-px-text.mjs index 50e4ac07c1..81b63a0313 100644 --- a/desktop/scripts/check-px-text.mjs +++ b/desktop/scripts/check-px-text.mjs @@ -23,7 +23,7 @@ const rules = [ // glyphs are fixed display sizes sized to their avatar box (not readable // message text), so they are exempted from the readable-text px rule. const overrides = new Set([ - "src/features/settings/ui/ProfileSettingsCard.tsx:684", + "src/features/settings/ui/ProfileSettingsCard.tsx:712", "src/features/onboarding/ui/AvatarStep.tsx:95", "src/features/agents/ui/AgentCreationPreview.tsx:666", "src/features/agents/ui/AgentCreationPreview.tsx:747", diff --git a/desktop/src/features/settings/ui/ProfileSettingsCard.tsx b/desktop/src/features/settings/ui/ProfileSettingsCard.tsx index 21adb65007..39d90cd5a9 100644 --- a/desktop/src/features/settings/ui/ProfileSettingsCard.tsx +++ b/desktop/src/features/settings/ui/ProfileSettingsCard.tsx @@ -264,9 +264,11 @@ export function ProfileSettingsCard({ const [isSignOutPending, setIsSignOutPending] = React.useState(false); const displayNameInputRef = React.useRef(null); const aboutTextareaRef = React.useRef(null); + const sectionRef = React.useRef(null); const isEditingProfileMetadataRef = React.useRef(false); const avatarEditorOpenFrameRef = React.useRef(null); const avatarEditorFinishTimeoutRef = React.useRef(null); + const savedScrollTopRef = React.useRef(null); isEditingProfileMetadataRef.current = isEditingProfileMetadata; React.useEffect(() => { @@ -423,14 +425,31 @@ export function ProfileSettingsCard({ window.clearTimeout(avatarEditorFinishTimeoutRef.current); avatarEditorFinishTimeoutRef.current = null; }, []); + const saveScrollPosition = React.useCallback(() => { + const el = sectionRef.current; + if (!el) return; + const scroller = el.closest("[class*='overflow-y']"); + if (scroller) savedScrollTopRef.current = scroller.scrollTop; + }, []); + const restoreScrollPosition = React.useCallback(() => { + const saved = savedScrollTopRef.current; + if (saved == null) return; + savedScrollTopRef.current = null; + const el = sectionRef.current; + if (!el) return; + const scroller = el.closest("[class*='overflow-y']"); + if (scroller) scroller.scrollTop = saved; + }, []); const closeAvatarEditor = React.useCallback(() => { clearAvatarEditorFinishTimeout(); setIsAvatarEditorOpen(false); setIsAvatarEditorFinishing(false); - }, [clearAvatarEditorFinishTimeout]); + restoreScrollPosition(); + }, [clearAvatarEditorFinishTimeout, restoreScrollPosition]); const completeAvatarEditorClose = React.useCallback(() => { setIsAvatarEditorOpen(false); clearAvatarEditorFinishTimeout(); + restoreScrollPosition(); avatarEditorFinishTimeoutRef.current = window.setTimeout( () => { avatarEditorFinishTimeoutRef.current = null; @@ -438,7 +457,11 @@ export function ProfileSettingsCard({ }, shouldReduceMotion ? 0 : AVATAR_EDITOR_TRANSITION_MS, ); - }, [clearAvatarEditorFinishTimeout, shouldReduceMotion]); + }, [ + clearAvatarEditorFinishTimeout, + restoreScrollPosition, + shouldReduceMotion, + ]); const reopenAvatarEditorAfterClose = React.useCallback(() => { clearAvatarEditorFinishTimeout(); setShouldRenderAvatarEditor(true); @@ -447,6 +470,7 @@ export function ProfileSettingsCard({ }, [clearAvatarEditorFinishTimeout]); const openAvatarEditor = React.useCallback(() => { + saveScrollPosition(); setShouldRenderAvatarEditor(true); setIsAvatarEditorFinishing(false); clearAvatarEditorFinishTimeout(); @@ -459,7 +483,7 @@ export function ProfileSettingsCard({ avatarEditorOpenFrameRef.current = null; setIsAvatarEditorOpen(true); }); - }, [clearAvatarEditorFinishTimeout]); + }, [clearAvatarEditorFinishTimeout, saveScrollPosition]); const saveProfile = React.useCallback(async () => { if (!canSave) { @@ -547,7 +571,11 @@ export function ProfileSettingsCard({ }, []); return ( -
+