Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src-tauri/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dunce = "1"
doctor = { git = "https://github.com/block/builderbot", rev = "73ff9a0521dcc784c9514911a655187e5dd3b6ca" }
etcetera = "0.11.0"
flate2 = "1"
tempfile = "3"
hex = "0.4"
ignore = "0.4.25"
infer = "0.19.0"
Expand Down Expand Up @@ -142,7 +143,6 @@ block-voice-dictation = []
admin-runtime-config = []

[dev-dependencies]
tempfile = "3"
# Mirrors goose's bare-name command resolver (crates/goose Cargo.toml): the
# native Windows gate resolves the bridge launcher through the exact
# `which_in_global` path goosed uses, so PATHEXT resolution is under test.
Expand Down
21 changes: 21 additions & 0 deletions src-tauri/src/commands/installation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use tauri::State;

use crate::services::installation_cohort::{
InstallationCohort, InstallationCohortReadiness, InstallationCohortState,
};

#[tauri::command]
pub async fn get_installation_cohort(
state: State<'_, InstallationCohortState>,
) -> Result<InstallationCohort, String> {
let mut receiver = state.0.clone();
loop {
let readiness = *receiver.borrow_and_update();
if let InstallationCohortReadiness::Ready(cohort) = readiness {
return Ok(cohort);
}
if receiver.changed().await.is_err() {
return Ok(InstallationCohort::Unknown);
}
}
}
1 change: 1 addition & 0 deletions src-tauri/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub mod git;
pub mod git_changes;
pub mod global_shortcut;
pub mod home_widget_media;
pub mod installation;
pub mod layout;
pub mod message_queues;
pub mod migration;
Expand Down
26 changes: 26 additions & 0 deletions src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,31 @@ pub fn run() {
app.manage(commands::pocket_voice::PocketVoiceState::default());
app.manage(commands::native_voice::NativeVoiceState::default());
app.manage(commands::voice_capture::VoiceCaptureState::default());
let (installation_cohort_sender, installation_cohort_state) =
services::installation_cohort::installation_cohort_channel();
app.manage(installation_cohort_state);
let current_layout_exists =
services::installation_cohort::layout_database_exists(&app_data_dir);
let legacy_layout_exists = if app.try_state::<services::e2e_mode::E2eMode>().is_some() {
Ok(false)
} else {
services::app_data_migration::legacy_layout_database_exists(app.handle())
};
let installation_cohort =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 P2 · Cohort classification races migration (blocking)

Startup inspects current and legacy databases and permanently publishes the cohort marker before running legacy migration. Atomic no-clobber publication coordinates marker writers, but no inter-process lock covers the database observations and migration. A concurrently running Berd or legacy Goose Internal process can publish established data after inspection but before migration, allowing this process to persist fresh-with-landing-v1 and then copy/open established data.

User effect: An installation with established data can be permanently treated as new and unexpectedly routed through onboarding because of launch timing between processes.

Recommended fix: Serialize classification and legacy migration with an inter-process startup/migration lock. Under the lock, inspect pre-migration evidence, perform migration, and publish the marker only after the evidence and resulting layout are stable.

Test: Add a lock-boundary or multi-process test that pauses after an absent observation, creates a valid legacy layout from another writer, resumes startup, and asserts the final marker is established rather than fresh.

services::installation_cohort::initialize_installation_cohort(
&app_data_dir,
current_layout_exists,
legacy_layout_exists,
)
.unwrap_or_else(|error| {
log::warn!("Failed to initialize installation cohort: {error}");
services::installation_cohort::InstallationCohort::Unknown
});
installation_cohort_sender.send_replace(
services::installation_cohort::InstallationCohortReadiness::Ready(
installation_cohort,
),
);
let release_channel_state = commands::updates::ReleaseChannelState::load(app.handle())?;
app.manage(release_channel_state);

Expand Down Expand Up @@ -510,6 +535,7 @@ pub fn run() {
commands::git::git_create_worktree,
commands::git::git_remove_worktree,
commands::home_widget_media::import_home_widget_photo,
commands::installation::get_installation_cohort,
commands::layout::get_layout,
commands::layout::save_layout_items,
commands::layout::save_layout_camera,
Expand Down
10 changes: 10 additions & 0 deletions src-tauri/src/services/app_data_migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ struct AppDataMigrationSummary {
/// Copy old app-owned local data before Berd services open files in the new
/// location. Failures are logged and non-fatal so a single locked/cache file
/// does not prevent app startup.
pub(crate) fn legacy_layout_database_exists<R: Runtime>(
app: &AppHandle<R>,
) -> Result<bool, String> {
let pair = legacy_directory_pairs(app)?
.into_iter()
.find(|pair| pair.kind == AppDirectoryKind::Data)
.ok_or_else(|| "Legacy app data directory is unavailable".to_string())?;
crate::services::installation_cohort::file_exists(&pair.old.join(OLD_LAYOUT_DATABASE))
}

pub(crate) fn migrate_legacy_app_data<R: Runtime>(app: &AppHandle<R>) {
if app
.try_state::<crate::services::e2e_mode::E2eMode>()
Expand Down
235 changes: 235 additions & 0 deletions src-tauri/src/services/installation_cohort.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
use serde::{Deserialize, Serialize};
use std::fs;
use std::io::Write;
use std::path::Path;
use tokio::sync::watch;

const MARKER_FILE_NAME: &str = "installation-cohort-v1.json";
const MARKER_VERSION: u32 = 1;
const CURRENT_LAYOUT_DATABASE: &str = "berd.sqlite";

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum InstallationCohort {
FreshWithLandingV1,
EstablishedBeforeLandingV1,
Unknown,
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum InstallationCohortReadiness {
Initializing,
Ready(InstallationCohort),
}

#[derive(Clone, Debug)]
pub struct InstallationCohortState(pub watch::Receiver<InstallationCohortReadiness>);

#[derive(Deserialize, Serialize)]
struct InstallationCohortRecord {
version: u32,
cohort: InstallationCohort,
}

pub fn layout_database_exists(app_data_dir: &Path) -> Result<bool, String> {
file_exists(&app_data_dir.join(CURRENT_LAYOUT_DATABASE))
}

pub(crate) fn file_exists(path: &Path) -> Result<bool, String> {
match fs::metadata(path) {
Ok(metadata) => Ok(metadata.is_file()),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 P2 · Ambiguous database paths become fresh (blocking)

file_exists returns false for every existing path whose metadata is not a regular file. A directory, FIFO, or other unexpected entry at either database path is therefore treated exactly like NotFound, and startup may permanently publish the fresh cohort even though layout initialization or migration cannot safely use that path. This bypasses the otherwise fail-closed metadata-error behavior.

User effect: After a filesystem conflict or malformed installation path, Berd can save the wrong permanent cohort and later force an established user through onboarding once the path problem is repaired.

Recommended fix: Return an error for an existing non-file path and explicitly define symlink handling, so ambiguous evidence resolves to unknown without publishing a marker.

Test: Create a directory at the current and legacy database paths and assert detection errors and no cohort marker is written; cover the intended symlink policy separately.

Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false),
Err(error) => Err(format!("Failed to inspect {}: {error}", path.display())),
}
}

pub fn installation_cohort_channel() -> (
watch::Sender<InstallationCohortReadiness>,
InstallationCohortState,
) {
let (sender, receiver) = watch::channel(InstallationCohortReadiness::Initializing);
(sender, InstallationCohortState(receiver))
}

pub fn initialize_installation_cohort(
app_data_dir: &Path,
current_layout_exists: Result<bool, String>,
legacy_layout_exists: Result<bool, String>,
) -> Result<InstallationCohort, String> {
fs::create_dir_all(app_data_dir)
.map_err(|error| format!("Failed to create app data directory: {error}"))?;
let marker_path = app_data_dir.join(MARKER_FILE_NAME);

if marker_path.exists() {
let bytes = fs::read(&marker_path)
.map_err(|error| format!("Failed to read installation cohort marker: {error}"))?;
let Ok(record) = serde_json::from_slice::<InstallationCohortRecord>(&bytes) else {
return Ok(InstallationCohort::Unknown);
};
if record.version != MARKER_VERSION || record.cohort == InstallationCohort::Unknown {
return Ok(InstallationCohort::Unknown);
}
return Ok(record.cohort);
}

let current_layout_exists = current_layout_exists?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 P2 · Definitive established evidence is discarded (blocking)

The current and legacy database probe Results are unwrapped sequentially. If either probe is Ok(true) but the other probe errors, initialization returns Unknown instead of using the already definitive evidence that an established database exists. For example, a valid current berd.sqlite is ignored when permissions prevent inspecting the obsolete legacy directory.

User effect: An established user can be sent into first-run onboarding because an irrelevant secondary filesystem probe failed, even though Berd already found their existing database.

Recommended fix: Aggregate the probes as three-state evidence: classify established if either is Ok(true), classify fresh only when both are Ok(false), and return unknown/error only when no positive evidence exists and at least one probe is uncertain.

Test: Add cases for Ok(true) plus Err and Err plus Ok(true), asserting both classify and persist established; retain both-false and uncertainty-without-positive-evidence cases.

let legacy_layout_exists = legacy_layout_exists?;
let cohort = if current_layout_exists || legacy_layout_exists {
InstallationCohort::EstablishedBeforeLandingV1
} else {
InstallationCohort::FreshWithLandingV1
};
persist_marker(&marker_path, cohort)
}

fn persist_marker(path: &Path, cohort: InstallationCohort) -> Result<InstallationCohort, String> {
let record = InstallationCohortRecord {
version: MARKER_VERSION,
cohort,
};
let parent = path
.parent()
.ok_or_else(|| "Installation cohort marker has no parent directory".to_string())?;
let bytes = serde_json::to_vec(&record)
.map_err(|error| format!("Failed to serialize installation cohort marker: {error}"))?;
let mut temporary = tempfile::NamedTempFile::new_in(parent)
.map_err(|error| format!("Failed to create installation cohort marker: {error}"))?;
temporary
.write_all(&bytes)
.and_then(|_| temporary.as_file().sync_all())
.map_err(|error| format!("Failed to write installation cohort marker: {error}"))?;

match temporary.persist_noclobber(path) {
Ok(_) => {
if let Err(error) = sync_parent_directory(path) {
log::warn!(
"Installation cohort marker was published but directory sync failed: {error}"
);
}
Ok(cohort)
}
Err(error) if error.error.kind() == std::io::ErrorKind::AlreadyExists => {
read_published_cohort(path)
}
Err(error) => Err(format!(
"Failed to publish installation cohort marker: {}",
error.error
)),
}
}

fn read_published_cohort(path: &Path) -> Result<InstallationCohort, String> {
let bytes = fs::read(path)
.map_err(|error| format!("Failed to read published installation cohort marker: {error}"))?;
let record: InstallationCohortRecord = serde_json::from_slice(&bytes)
.map_err(|error| format!("Invalid published installation cohort marker: {error}"))?;
if record.version != MARKER_VERSION || record.cohort == InstallationCohort::Unknown {
return Err("Published installation cohort marker is unsupported".to_string());
}
Ok(record.cohort)
}

#[cfg(unix)]
fn sync_parent_directory(path: &Path) -> Result<(), String> {
let parent = path
.parent()
.ok_or_else(|| "Installation cohort marker has no parent directory".to_string())?;
fs::File::open(parent)
.and_then(|directory| directory.sync_all())
.map_err(|error| format!("Failed to sync installation cohort directory: {error}"))
}

#[cfg(not(unix))]
fn sync_parent_directory(_path: &Path) -> Result<(), String> {
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
use tempfile::tempdir;

#[test]
fn classifies_and_persists_a_fresh_installation() {
let root = tempdir().unwrap();
let first = initialize_installation_cohort(root.path(), Ok(false), Ok(false)).unwrap();
assert_eq!(first, InstallationCohort::FreshWithLandingV1);

fs::write(root.path().join(CURRENT_LAYOUT_DATABASE), b"later").unwrap();
let second = initialize_installation_cohort(root.path(), Ok(false), Ok(false)).unwrap();
assert_eq!(second, InstallationCohort::FreshWithLandingV1);
}

#[test]
fn classifies_current_or_legacy_layouts_as_established() {
let current = tempdir().unwrap();
fs::write(current.path().join(CURRENT_LAYOUT_DATABASE), b"existing").unwrap();
assert_eq!(
initialize_installation_cohort(current.path(), Ok(true), Ok(false)).unwrap(),
InstallationCohort::EstablishedBeforeLandingV1
);

let legacy = tempdir().unwrap();
assert_eq!(
initialize_installation_cohort(legacy.path(), Ok(false), Ok(true)).unwrap(),
InstallationCohort::EstablishedBeforeLandingV1
);
}

#[test]
fn concurrent_initializers_use_the_published_winner() {
use std::sync::{Arc, Barrier};

let root = tempdir().unwrap();
let path = Arc::new(root.path().to_path_buf());
let barrier = Arc::new(Barrier::new(3));
let handles = [false, false].map(|legacy_exists| {
let path = Arc::clone(&path);
let barrier = Arc::clone(&barrier);
std::thread::spawn(move || {
barrier.wait();
initialize_installation_cohort(&path, Ok(false), Ok(legacy_exists)).unwrap()
})
});
barrier.wait();

let cohorts = handles.map(|handle| handle.join().unwrap());
assert_eq!(cohorts[0], InstallationCohort::FreshWithLandingV1);
assert_eq!(cohorts[1], cohorts[0]);
assert_eq!(
initialize_installation_cohort(&path, Ok(false), Ok(false)).unwrap(),
cohorts[0]
);
}

#[test]
fn detection_failure_does_not_publish_a_fresh_marker() {
let root = tempdir().unwrap();
assert!(initialize_installation_cohort(
root.path(),
Err("metadata unavailable".into()),
Ok(false),
)
.is_err());
assert!(!root.path().join(MARKER_FILE_NAME).exists());
}

#[test]
fn treats_an_unsupported_marker_as_unknown_and_preserves_it() {
let root = tempdir().unwrap();
let marker = root.path().join(MARKER_FILE_NAME);
fs::write(
&marker,
br#"{"version":2,"cohort":"fresh-with-landing-v1"}"#,
)
.unwrap();

assert_eq!(
initialize_installation_cohort(root.path(), Ok(false), Ok(false)).unwrap(),
InstallationCohort::Unknown
);
assert!(String::from_utf8(fs::read(marker).unwrap())
.unwrap()
.contains("\"version\":2"));
}
}
1 change: 1 addition & 0 deletions src-tauri/src/services/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod distro_bundle;
pub(crate) mod e2e_mode;
pub(crate) mod env_key;
pub(crate) mod goose_config;
pub(crate) mod installation_cohort;
#[cfg(target_os = "macos")]
pub(crate) mod installer_media;
#[cfg_attr(
Expand Down
2 changes: 2 additions & 0 deletions src/app/AppShell.berdctl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { gooseServeSelectionFromExecutionTarget } from "@/features/chat/lib/goos
import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore";
import type { ChatSession } from "@/features/chat/stores/chatSessionStore";
import { useProjectStore } from "@/features/projects/stores/projectStore";
import { dispatchOnboarding } from "@/features/onboarding/model";
import { useShortcutsDialogStore } from "@/features/shortcuts/stores/shortcutsDialogStore";
import { useRuntimeConfigStore } from "@/shared/runtime-config/runtimeConfigStore";
import { useDefaultProviderReadinessStore } from "@/features/providers/stores/defaultProviderReadinessStore";
Expand Down Expand Up @@ -253,6 +254,7 @@ describe("AppShell berdctl integration", () => {
vi.stubEnv("VITE_AUTOMATIONS", "1");
window.history.replaceState(null, "", "/");
window.localStorage.clear();
dispatchOnboarding({ type: "complete" });
useShortcutsDialogStore.setState({ open: false });
mockAcpCreateSession.mockReset();
mockAcpCreateSession.mockResolvedValue({ sessionId: "created-session" });
Expand Down
2 changes: 2 additions & 0 deletions src/app/AppShell.navigation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { OPEN_SETTINGS_EVENT } from "@/features/settings/lib/settingsEvents";
import { SHORTCUT_PREFERENCES_STORAGE_KEY } from "@/features/shortcuts/lib/shortcutRegistry";
import { useShortcutsDialogStore } from "@/features/shortcuts/stores/shortcutsDialogStore";
import { useProjectStore } from "@/features/projects/stores/projectStore";
import { dispatchOnboarding } from "@/features/onboarding/model";
import {
resetHomeWidgetStoreForTests,
useHomeWidgetStore,
Expand Down Expand Up @@ -893,6 +894,7 @@ describe("AppShell global navigation", () => {
afterEach(cleanup);

beforeEach(() => {
dispatchOnboarding({ type: "complete" });
resetHomeWidgetStoreForTests();
resetStarterWidgetPickerRequestForTests();
mockRepairManagedGooseModelSelection.mockReset();
Expand Down
2 changes: 2 additions & 0 deletions src/app/AppShell.startupDiagnostics.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useAgentStore } from "@/features/agents/stores/agentStore";
import { useChatStore } from "@/features/chat/stores/chatStore";
import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore";
import { useProjectStore } from "@/features/projects/stores/projectStore";
import { dispatchOnboarding } from "@/features/onboarding/model";
import { AppShell } from "./AppShell";

const mocks = vi.hoisted(() => ({
Expand Down Expand Up @@ -117,6 +118,7 @@ describe("AppShell startup diagnostics", () => {
vi.clearAllMocks();
window.history.replaceState(null, "", "/");
window.localStorage.clear();
dispatchOnboarding({ type: "complete" });
mocks.startupState.ready = true;
mocks.startupState.error = null;
mocks.migrationState.status = "ready";
Expand Down
Loading