From 45756bd0b5e3eb29a12e1efd6209c122df6f3e65 Mon Sep 17 00:00:00 2001 From: Richard Date: Sun, 20 Sep 2026 17:42:16 +0800 Subject: [PATCH 1/2] fix(web-ui): delete sessions through the backend when the workspace path is missing FlowChatStore.deleteSession kept a stale workspace-path guard, so the ID-first backend delete was never invoked and the session reappeared after reload. Refs #3150 --- .../local/LocalSessionDriver.ts | 20 +-- .../src/flow_chat/store/FlowChatStore.test.ts | 129 ++++++++++++++++++ .../src/flow_chat/store/FlowChatStore.ts | 42 ++++-- 3 files changed, 173 insertions(+), 18 deletions(-) diff --git a/src/web-ui/src/flow_chat/session-drivers/local/LocalSessionDriver.ts b/src/web-ui/src/flow_chat/session-drivers/local/LocalSessionDriver.ts index 64fd224a6d..a22cceba46 100644 --- a/src/web-ui/src/flow_chat/session-drivers/local/LocalSessionDriver.ts +++ b/src/web-ui/src/flow_chat/session-drivers/local/LocalSessionDriver.ts @@ -145,15 +145,17 @@ export const localSessionDriver: SessionDriver = { sessionId, hasWorkspacePath: Boolean(session && sessionProjectWorkspacePath(session)), }); - await context.flowChatStore.deleteSession( - sessionId, - removal.removedActiveSession ? { nextActiveSessionId: null } : undefined, - ); - - removal.removedSessionIds.forEach(id => { - context.processingManager.clearSessionStatus(id); - cleanupSaveState(context, id); - }); + try { + await context.flowChatStore.deleteSession( + sessionId, + removal.removedActiveSession ? { nextActiveSessionId: null } : undefined, + ); + } finally { + removal.removedSessionIds.forEach(id => { + context.processingManager.clearSessionStatus(id); + cleanupSaveState(context, id); + }); + } }, async archiveSession( diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts index c0c8f7f81f..e15d79912b 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts @@ -750,6 +750,135 @@ describe('FlowChatStore session removal active selection', () => { }); }); +describe('FlowChatStore session deletion persistence', () => { + beforeEach(() => { + apiMocks.deleteSession.mockClear(); + }); + + afterEach(() => { + resetStore(); + }); + + it('deletes a session without any workspace path by workspace ID', async () => { + // Regression: invalid session projects (lost workspace directory) keep + // their workspace ID but have no path fields; the ID-first backend API + // must still be called instead of failing on the missing path. + const session = createSession({ + sessionId: 'orphan-1', + title: 'Orphaned session', + workspacePath: undefined, + config: { agentType: 'Standard' }, + }); + expect(session.workspaceId).toBeTruthy(); + expect(session.projectWorkspacePath).toBeUndefined(); + expect(session.config.workspacePath).toBeUndefined(); + + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + await flowChatStore.deleteSession('orphan-1', { nextActiveSessionId: null }); + + expect(apiMocks.deleteSession).toHaveBeenCalledTimes(1); + expect(apiMocks.deleteSession).toHaveBeenCalledWith('orphan-1', session.workspaceId); + expect(flowChatStore.getState().sessions.has('orphan-1')).toBe(false); + expect(flowChatStore.getState().activeSessionId).toBeNull(); + }); + + it('rejects with a session-scoped error when no workspace ID exists', async () => { + const session = createSession({ + sessionId: 'pre-id-1', + title: 'Pre-ID session', + workspaceId: undefined, + workspacePath: undefined, + config: { agentType: 'Standard' }, + }); + + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + await expect(flowChatStore.deleteSession('pre-id-1')).rejects.toThrow( + 'Failed to delete session pre-id-1 on backend: ' + + 'Session workspace ID is unavailable for session pre-id-1', + ); + expect(apiMocks.deleteSession).not.toHaveBeenCalled(); + expect(flowChatStore.getState().sessions.has('pre-id-1')).toBe(false); + }); + + it('keeps local cleanup and surfaces an error when the backend delete rejects', async () => { + apiMocks.deleteSession.mockRejectedValueOnce(new Error('backend unavailable')); + const session = createSession({ + sessionId: 'reject-1', + title: 'Rejected session', + workspacePath: undefined, + config: { agentType: 'Standard' }, + }); + + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + await expect(flowChatStore.deleteSession('reject-1', { nextActiveSessionId: null })) + .rejects.toThrow('Failed to delete session reject-1 on backend: backend unavailable'); + + expect(apiMocks.deleteSession).toHaveBeenCalledWith('reject-1', session.workspaceId); + expect(flowChatStore.getState().sessions.has('reject-1')).toBe(false); + expect(flowChatStore.getState().activeSessionId).toBeNull(); + }); + + it('resolves without error when all backend deletes succeed', async () => { + const session = createSession({ sessionId: 'success-1', title: 'Success session' }); + + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + await expect(flowChatStore.deleteSession('success-1', { nextActiveSessionId: null })) + .resolves.toBeUndefined(); + + expect(apiMocks.deleteSession).toHaveBeenCalledTimes(1); + expect(apiMocks.deleteSession).toHaveBeenCalledWith('success-1', session.workspaceId); + expect(flowChatStore.getState().sessions.has('success-1')).toBe(false); + expect(flowChatStore.getState().activeSessionId).toBeNull(); + }); + + it('deletes every cascade member by workspace ID even when children lack paths', async () => { + const parent = createSession({ + sessionId: 'cascade-parent', + title: 'Cascade parent', + workspacePath: undefined, + config: { agentType: 'Standard' }, + }); + const child = createSession({ + sessionId: 'cascade-child', + title: 'Cascade child', + parentSessionId: 'cascade-parent', + workspacePath: undefined, + config: { agentType: 'Standard' }, + }); + + flowChatStore.setState(() => ({ + sessions: new Map([ + [parent.sessionId, parent], + [child.sessionId, child], + ]), + activeSessionId: parent.sessionId, + })); + + await flowChatStore.deleteSession('cascade-parent', { nextActiveSessionId: null }); + + expect(apiMocks.deleteSession).toHaveBeenCalledTimes(2); + expect(apiMocks.deleteSession).toHaveBeenCalledWith('cascade-parent', parent.workspaceId); + expect(apiMocks.deleteSession).toHaveBeenCalledWith('cascade-child', child.workspaceId); + expect(Array.from(flowChatStore.getState().sessions.keys())).toEqual([]); + }); +}); + describe('FlowChatStore token usage', () => { afterEach(() => { resetStore(); diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.ts index e90215e373..573f1227e5 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.ts @@ -1,4 +1,4 @@ -import { requireSessionWorkspaceId } from '../utils/sessionWorkspace'; +import { requireSessionWorkspaceId, sessionWorkspaceId } from '../utils/sessionWorkspace'; import { workspaceManager } from '@/infrastructure/services/business/workspaceManager'; import { resolveLegacySessionWorkspace } from '@/infrastructure/api/service-api/legacyWorkspaceCompatibility'; import { projectUserQuestionTiming } from '../utils/userQuestionTiming'; @@ -5182,25 +5182,37 @@ export class FlowChatStore { this.pendingRemoveSessionOptions.set(sessionId, options); } + // Snapshot workspace IDs synchronously from the same state that produced + // the cascade list: backend deletion is workspace-ID-first and must not + // depend on the session still being present once awaited work resolves. + // Sessions whose workspace directory disappeared (invalid session + // projects) keep their workspace ID and stay deletable; only pre-ID + // sessions without any workspace ID fail explicitly instead of silently + // skipping the request. + const workspaceIdBySessionId = new Map(); + sessionIdsToDelete.forEach(id => { + const workspaceId = sessionWorkspaceId(this.state.sessions.get(id)); + if (workspaceId) { + workspaceIdBySessionId.set(id, workspaceId); + } + }); + const { stateMachineManager } = await import('../state-machine'); sessionIdsToDelete.forEach(id => { stateMachineManager.delete(id); }); + const deleteFailures: { sessionId: string; reason: unknown }[] = []; try { const { agentAPI } = await import('@/infrastructure/api/service-api/AgentAPI'); const deleteResults = await Promise.allSettled( sessionIdsToDelete.map(async id => { - const sess = this.state.sessions.get(id); - const workspacePath = sess ? sessionProjectWorkspacePath(sess) : undefined; - if (!workspacePath) { - throw new Error(`Workspace path not found for session ${id}`); + const workspaceId = workspaceIdBySessionId.get(id); + if (!workspaceId) { + throw new Error(`Session workspace ID is unavailable for session ${id}`); } - await agentAPI.deleteSession( - id, - requireSessionWorkspaceId(sess!) - ); + await agentAPI.deleteSession(id, workspaceId); }) ); @@ -5210,16 +5222,28 @@ export class FlowChatStore { sessionId: sessionIdsToDelete[index], error: result.reason, }); + deleteFailures.push({ sessionId: sessionIdsToDelete[index], reason: result.reason }); } }); } catch (error) { log.error('Failed to delete session on backend', { sessionId, error }); + deleteFailures.push({ sessionId, reason: error }); } const removedSessionIds = this.removeSession(sessionId, options); sessionComposerStore.getState().removeDrafts(removedSessionIds); askUserQuestionDraftStore.getState().removeSessionDrafts(removedSessionIds); this.pendingRemoveSessionOptions.delete(sessionId); + + if (deleteFailures.length > 0) { + const firstFailure = deleteFailures[0]; + const reasonText = firstFailure.reason instanceof Error + ? firstFailure.reason.message + : String(firstFailure.reason); + throw new Error( + `Failed to delete session ${firstFailure.sessionId} on backend: ${reasonText}` + ); + } } public removeSession(sessionId: string, options?: RemoveSessionOptions): string[] { From fe1a15dc6b1fa3b3c73866911f1f2f31982d962f Mon Sep 17 00:00:00 2001 From: Richard Date: Sun, 20 Sep 2026 17:42:37 +0800 Subject: [PATCH 2/2] fix(desktop): allow deleting legacy-rooted or orphaned assistant workspaces Assistant workspace records left at the legacy BitFun root (~/.bitfun/personal_assistant/) by the brand migration, and orphaned records whose directory is already gone, can now be deleted or reset. Existing directories outside the managed layouts are still refused. The legacy_hidden_data_directory() constant now comes from core-types, replacing the private copy in legacy-migration. Fixes #3150 --- src/apps/desktop/src/api/commands.rs | 114 +++++++-- .../core/src/service/workspace/service.rs | 219 +++++++++++++++--- .../core-types/src/product_identity.rs | 13 +- .../services/legacy-migration/src/paths.rs | 7 +- 4 files changed, 308 insertions(+), 45 deletions(-) diff --git a/src/apps/desktop/src/api/commands.rs b/src/apps/desktop/src/api/commands.rs index 85496af0d7..705a211d8e 100644 --- a/src/apps/desktop/src/api/commands.rs +++ b/src/apps/desktop/src/api/commands.rs @@ -1563,6 +1563,38 @@ pub async fn set_primary_assistant_workspace( Ok(WorkspaceInfoDto::from_workspace_info(&workspace)) } +/// How deleting an assistant workspace record may touch its directory. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AssistantWorkspaceDirectoryPlan { + /// Managed layout (current or legacy): delete the directory when present. + DeleteDirectoryWhenPresent, + /// The directory does not exist: remove the registry entry only. + RegistryEntryOnly, +} + +/// Guards `delete_assistant_workspace` against deleting arbitrary directories. +/// +/// Existing directories outside the managed assistant roots (the current +/// OpenBitFun layout and the legacy BitFun layout left behind by the +/// BitFun-to-OpenBitFun brand migration) are refused; a record whose +/// directory is already gone resolves to a registry-only cleanup. +fn assistant_workspace_directory_plan( + path: &Path, + is_managed_path: bool, +) -> Result { + if is_managed_path { + return Ok(AssistantWorkspaceDirectoryPlan::DeleteDirectoryWhenPresent); + } + if path.exists() { + Err(format!( + "Workspace path is not a managed assistant workspace: {}", + path.display() + )) + } else { + Ok(AssistantWorkspaceDirectoryPlan::RegistryEntryOnly) + } +} + #[tauri::command] pub async fn delete_assistant_workspace( state: State<'_, AppState>, @@ -1590,15 +1622,11 @@ pub async fn delete_assistant_workspace( return Err("Primary assistant workspace cannot be deleted".to_string()); } - if !state + let is_managed_path = state .workspace_service - .is_assistant_workspace_path(&workspace_info.root_path) - { - return Err(format!( - "Workspace path is not a managed assistant workspace: {}", - workspace_info.root_path.display() - )); - } + .is_manageable_assistant_workspace_path(&workspace_info.root_path); + let directory_plan = + assistant_workspace_directory_plan(&workspace_info.root_path, is_managed_path)?; let is_active_workspace = state .workspace_service @@ -1617,11 +1645,26 @@ pub async fn delete_assistant_workspace( let workspace_path = workspace_info.root_path.to_string_lossy().to_string(); - state - .filesystem_service - .delete_directory(&workspace_path, true) - .await - .map_err(|e| format!("Failed to delete assistant workspace files: {}", e))?; + // Re-check presence so a directory that disappears between the guard and + // the deletion still resolves as a registry-only cleanup. + let delete_files = matches!( + directory_plan, + AssistantWorkspaceDirectoryPlan::DeleteDirectoryWhenPresent + ) && workspace_info.root_path.exists(); + + if delete_files { + state + .filesystem_service + .delete_directory(&workspace_path, true) + .await + .map_err(|e| format!("Failed to delete assistant workspace files: {}", e))?; + } else { + info!( + "Assistant workspace directory is missing; removed registry entry only: workspace_id={}, path={}", + request.workspace_id, + workspace_info.root_path.display() + ); + } state .workspace_service @@ -1711,6 +1754,47 @@ async fn clear_directory_contents(directory: &Path) -> Result<(), String> { Ok(()) } +#[cfg(test)] +mod assistant_workspace_delete_tests { + use super::{assistant_workspace_directory_plan, AssistantWorkspaceDirectoryPlan}; + + #[test] + fn managed_paths_may_delete_even_when_the_directory_is_missing() { + let temp = tempfile::tempdir().expect("temp root"); + let missing = temp.path().join("workspace-44e0e781"); + + assert_eq!( + assistant_workspace_directory_plan(&missing, true), + Ok(AssistantWorkspaceDirectoryPlan::DeleteDirectoryWhenPresent) + ); + } + + #[test] + fn unmanaged_existing_directories_are_refused() { + let temp = tempfile::tempdir().expect("temp root"); + let unmanaged = temp.path().join("Documents").join("foo"); + std::fs::create_dir_all(&unmanaged).expect("create unmanaged directory"); + + let error = assistant_workspace_directory_plan(&unmanaged, false) + .expect_err("an existing unmanaged directory must be refused"); + + assert!(error.starts_with("Workspace path is not a managed assistant workspace")); + assert!(error.contains(&unmanaged.to_string_lossy().to_string())); + assert!(unmanaged.exists(), "the guard must not delete anything"); + } + + #[test] + fn unmanaged_missing_records_resolve_to_registry_only_cleanup() { + let temp = tempfile::tempdir().expect("temp root"); + let orphan = temp.path().join("missing-root").join("workspace-44e0e781"); + + assert_eq!( + assistant_workspace_directory_plan(&orphan, false), + Ok(AssistantWorkspaceDirectoryPlan::RegistryEntryOnly) + ); + } +} + #[tauri::command] pub async fn reset_assistant_workspace( state: State<'_, AppState>, @@ -1732,7 +1816,7 @@ pub async fn reset_assistant_workspace( if !state .workspace_service - .is_assistant_workspace_path(&workspace_info.root_path) + .is_manageable_assistant_workspace_path(&workspace_info.root_path) { return Err(format!( "Workspace path is not a managed assistant workspace: {}", @@ -2965,7 +3049,7 @@ pub async fn reset_workspace_persona_files( }; if workspace.workspace_kind != WorkspaceKind::Assistant - || !service.is_assistant_workspace_path(&workspace.root_path) + || !service.is_manageable_assistant_workspace_path(&workspace.root_path) { return Err(format!( "Workspace {} is not a managed assistant workspace", diff --git a/src/crates/assembly/core/src/service/workspace/service.rs b/src/crates/assembly/core/src/service/workspace/service.rs index 1ca978af20..5bdcf92d2e 100644 --- a/src/crates/assembly/core/src/service/workspace/service.rs +++ b/src/crates/assembly/core/src/service/workspace/service.rs @@ -29,7 +29,7 @@ use crate::service::workspace_runtime::{ }; use crate::util::errors::*; use log::{info, warn}; -use openbitfun_core_types::product_identity::product_id; +use openbitfun_core_types::product_identity::{legacy_hidden_data_directory, product_id}; use openbitfun_services_core::workspace_identity::{ canonicalize_local_workspace_root, local_workspace_roots_equal, normalize_remote_workspace_path, remote_workspace_stable_id, @@ -123,6 +123,59 @@ struct AssistantWorkspaceDescriptor { display_name: String, } +/// Assistant workspace layout check against one personal-assistant root. +/// +/// A path belongs to the layout when it is the default `/workspace` +/// directory or a named `/workspace-` sibling. Comparison is +/// literal, matching how those directories are created and how their paths +/// are persisted in workspace records; the directory does not need to exist. +/// Both the current OpenBitFun root and the legacy BitFun root are checked +/// with the same layout rules, so records written before the +/// BitFun-to-OpenBitFun brand migration stay recognizable. +fn assistant_descriptor_in_root( + path: &Path, + assistant_root: &Path, +) -> Option { + if path == assistant_root.join("workspace") { + return Some(AssistantWorkspaceDescriptor { + path: path.to_path_buf(), + assistant_id: None, + display_name: WorkspaceService::assistant_display_name(None), + }); + } + + if path.parent()? != assistant_root { + return None; + } + + let file_name = path.file_name()?.to_string_lossy(); + let assistant_id = file_name.strip_prefix("workspace-")?; + if assistant_id.trim().is_empty() { + return None; + } + + Some(AssistantWorkspaceDescriptor { + path: path.to_path_buf(), + assistant_id: Some(assistant_id.to_string()), + display_name: WorkspaceService::assistant_display_name(Some(assistant_id)), + }) +} + +/// Legacy BitFun-brand personal-assistant root: `~/.bitfun/personal_assistant/`. +/// +/// `BITFUN_HOME` / `BITFUN_E2E_HOME` replace the legacy `.bitfun` data root +/// itself, with the same resolution as the BitFun-to-OpenBitFun data +/// migrator, so both sides resolve one legacy root. Returns `None` when no +/// user home directory is available. +fn legacy_assistant_workspace_root() -> Option { + let legacy_home_root = std::env::var_os("BITFUN_HOME") + .or_else(|| std::env::var_os("BITFUN_E2E_HOME")) + .map(PathBuf::from) + .filter(|path| !path.as_os_str().is_empty()) + .or_else(|| dirs::home_dir().map(|home| home.join(legacy_hidden_data_directory())))?; + Some(legacy_home_root.join("personal_assistant")) +} + impl WorkspaceService { fn normalize_workspace_name(name: String) -> OpenBitFunResult { let name = name.trim(); @@ -1981,31 +2034,7 @@ impl WorkspaceService { } fn assistant_descriptor_from_path(&self, path: &Path) -> Option { - let default_workspace = self.path_manager.default_assistant_workspace_dir(None); - if path == default_workspace { - return Some(AssistantWorkspaceDescriptor { - path: path.to_path_buf(), - assistant_id: None, - display_name: Self::assistant_display_name(None), - }); - } - - let assistant_root = self.path_manager.assistant_workspace_base_dir(None); - if path.parent()? != assistant_root { - return None; - } - - let file_name = path.file_name()?.to_string_lossy(); - let assistant_id = file_name.strip_prefix("workspace-")?; - if assistant_id.trim().is_empty() { - return None; - } - - Some(AssistantWorkspaceDescriptor { - path: path.to_path_buf(), - assistant_id: Some(assistant_id.to_string()), - display_name: Self::assistant_display_name(Some(assistant_id)), - }) + assistant_descriptor_in_root(path, &self.path_manager.assistant_workspace_base_dir(None)) } fn normalize_workspace_options_for_path( @@ -2225,6 +2254,22 @@ impl WorkspaceService { self.assistant_descriptor_from_path(path).is_some() } + /// Returns whether a path is an assistant workspace record the product can + /// still manage: either the current OpenBitFun layout or the legacy + /// BitFun layout (`~/.bitfun/personal_assistant/`) left behind by the + /// BitFun-to-OpenBitFun brand migration. Deletion and reset guards must + /// accept both so a pre-migration record stays an explicit user action + /// instead of being rejected as an unmanaged path. + pub fn is_manageable_assistant_workspace_path(&self, path: &Path) -> bool { + self.is_assistant_workspace_path(path) || self.is_legacy_assistant_workspace_path(path) + } + + fn is_legacy_assistant_workspace_path(&self, path: &Path) -> bool { + legacy_assistant_workspace_root().is_some_and(|assistant_root| { + assistant_descriptor_in_root(path, &assistant_root).is_some() + }) + } + /// Clears all persisted data. pub async fn clear_persistent_data(&self) -> OpenBitFunResult<()> { self.persistence @@ -3120,6 +3165,128 @@ mod tests { Some("Legacy TypeScript implementation".to_string()) ); } + + #[test] + fn assistant_layout_check_accepts_only_managed_layouts_under_its_root() { + let temp = tempfile::tempdir().expect("temp root"); + let assistant_root = temp.path().join(".bitfun").join("personal_assistant"); + + let named = assistant_root.join("workspace-abc"); + assert_eq!( + assistant_descriptor_in_root(&named, &assistant_root) + .expect("named legacy layout should be recognized") + .assistant_id + .as_deref(), + Some("abc") + ); + assert!( + assistant_descriptor_in_root(&assistant_root.join("workspace"), &assistant_root) + .is_some(), + "default legacy layout should be recognized" + ); + assert!( + assistant_descriptor_in_root(&assistant_root.join("workspace-"), &assistant_root) + .is_none(), + "a blank assistant id must not be recognized" + ); + assert!( + assistant_descriptor_in_root( + &temp + .path() + .join(".bitfun") + .join("other") + .join("workspace-x"), + &assistant_root + ) + .is_none(), + "directories outside the personal_assistant root must be rejected" + ); + assert!( + assistant_descriptor_in_root( + &temp.path().join("Documents").join("foo"), + &assistant_root + ) + .is_none(), + "unrelated directories must be rejected" + ); + } + + #[tokio::test] + async fn manageable_check_still_accepts_current_assistant_layout() { + let env = TestEnvironment::new(); + let service = build_test_workspace_service(env.path_manager.clone()).await; + let assistant_root = env.path_manager.assistant_workspace_base_dir(None); + let named = assistant_root.join("workspace-current-id"); + + assert!(service.is_manageable_assistant_workspace_path(&named)); + assert!(service.is_manageable_assistant_workspace_path( + &env.path_manager.default_assistant_workspace_dir(None) + )); + assert!( + !service.is_manageable_assistant_workspace_path(&named.join("child")), + "paths below an assistant workspace are not assistant workspaces" + ); + assert!(!service + .is_manageable_assistant_workspace_path(&env.root.join("Documents").join("foo"))); + assert_eq!( + service.is_manageable_assistant_workspace_path(&named), + service.is_assistant_workspace_path(&named), + "current-layout behavior must not regress" + ); + } + + #[tokio::test] + async fn manageable_check_accepts_legacy_bitfun_assistant_roots() { + let env = TestEnvironment::new(); + let service = build_test_workspace_service(env.path_manager.clone()).await; + let temp = tempfile::tempdir().expect("temp root"); + + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _guard = ENV_LOCK.lock().expect("env lock poisoned"); + let previous_home = std::env::var_os("BITFUN_HOME"); + let previous_e2e_home = std::env::var_os("BITFUN_E2E_HOME"); + std::env::remove_var("BITFUN_HOME"); + // Mirror the data migrator: the override points at the legacy + // `.bitfun` data root itself, not at the user home above it. + std::env::set_var("BITFUN_E2E_HOME", temp.path().join(".bitfun")); + + let legacy_assistant_root = temp.path().join(".bitfun").join("personal_assistant"); + assert!( + service.is_manageable_assistant_workspace_path( + &legacy_assistant_root.join("workspace-abc") + ), + "legacy BitFun assistant workspaces must stay manageable" + ); + assert!( + service + .is_manageable_assistant_workspace_path(&legacy_assistant_root.join("workspace")), + "the legacy default assistant workspace must stay manageable" + ); + assert!( + !service.is_manageable_assistant_workspace_path( + &temp + .path() + .join(".bitfun") + .join("other") + .join("workspace-x") + ), + "directories outside the legacy personal_assistant root must be rejected" + ); + assert!( + !service + .is_manageable_assistant_workspace_path(&temp.path().join("Documents").join("foo")), + "unrelated directories must be rejected" + ); + + match previous_home { + Some(value) => std::env::set_var("BITFUN_HOME", value), + None => std::env::remove_var("BITFUN_HOME"), + } + match previous_e2e_home { + Some(value) => std::env::set_var("BITFUN_E2E_HOME", value), + None => std::env::remove_var("BITFUN_E2E_HOME"), + } + } } fn workspace_catalog_revision() -> &'static tokio::sync::watch::Sender { diff --git a/src/crates/contracts/core-types/src/product_identity.rs b/src/crates/contracts/core-types/src/product_identity.rs index 3c11980d17..f8d2774284 100644 --- a/src/crates/contracts/core-types/src/product_identity.rs +++ b/src/crates/contracts/core-types/src/product_identity.rs @@ -24,7 +24,7 @@ pub const fn data_namespace() -> &'static str { } } -/// Hidden directory name derived from [`data_namespace`] by the build adapter. +/// Hidden directory name derived from [`hidden_data_directory`] by the build adapter. /// /// The same name is used for the product home and project-local product data. pub const fn hidden_data_directory() -> &'static str { @@ -34,6 +34,16 @@ pub const fn hidden_data_directory() -> &'static str { } } +/// Hidden directory name used by the legacy BitFun-branded artifacts. +/// +/// Records written before the BitFun-to-OpenBitFun brand migration may still +/// point at the legacy data root, so migration-aware code needs this name to +/// keep reading (and to keep explicit deletion or reset of) those records +/// working instead of rejecting them as unmanaged paths. +pub const fn legacy_hidden_data_directory() -> &'static str { + ".bitfun" +} + #[cfg(test)] mod tests { use super::*; @@ -43,5 +53,6 @@ mod tests { assert_eq!(product_id(), "openbitfun"); assert_eq!(data_namespace(), "openbitfun"); assert_eq!(hidden_data_directory(), ".openbitfun"); + assert_eq!(legacy_hidden_data_directory(), ".bitfun"); } } diff --git a/src/crates/services/legacy-migration/src/paths.rs b/src/crates/services/legacy-migration/src/paths.rs index e4dbaac88f..28a8e0feb5 100644 --- a/src/crates/services/legacy-migration/src/paths.rs +++ b/src/crates/services/legacy-migration/src/paths.rs @@ -1,10 +1,11 @@ use crate::{LegacyMigrationError, LegacyMigrationResult}; -use openbitfun_services_core::product_identity::{data_namespace, hidden_data_directory}; +use openbitfun_services_core::product_identity::{ + data_namespace, hidden_data_directory, legacy_hidden_data_directory, +}; use std::env; use std::path::{Path, PathBuf}; pub const LEGACY_PRODUCT_ID: &str = "bitfun"; -const LEGACY_HIDDEN_DATA_DIRECTORY: &str = ".bitfun"; #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] @@ -46,7 +47,7 @@ impl MigrationRoots { .unwrap_or_else(|| config_root.join(LEGACY_PRODUCT_ID)); let legacy_home_root = env_path("BITFUN_HOME") .or_else(|| env_path("BITFUN_E2E_HOME")) - .unwrap_or_else(|| home.join(LEGACY_HIDDEN_DATA_DIRECTORY)); + .unwrap_or_else(|| home.join(legacy_hidden_data_directory())); let target_user_root = env_path("OPENBITFUN_USER_ROOT") .or_else(|| env_path("OPENBITFUN_E2E_USER_ROOT")) .unwrap_or_else(|| config_root.join(data_namespace()));