From b5d11d0a1920ab0861aacc6cc12cbe2d614365c6 Mon Sep 17 00:00:00 2001 From: Boii Date: Tue, 11 Aug 2026 19:21:42 +0800 Subject: [PATCH 1/6] feat(sidebar): add user_order ordering base and sidebar grouping API (#820) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Backend half of PR-A: the ordering base + sidebar grouping/paging API that moves the left conversation list off "pull everything and group in the browser" onto a server-driven, paged model with a real source of truth for pins. ### Migration 038 - New `user_order` table — the pin ordering source of truth (v1 scene `'pinned'`; `order_key` ascending = most-recently pinned first). - `conversations.archived_at` column + partial indexes (`idx_user_order_scene`, archived indexes) landed here so PR-B (archive) needs no further migration. - No backfill: historical `extra.pinned` is intentionally NOT migrated into `user_order` (consistent with team-pin localStorage not migrating — preference data is never migrated). ### aionui-db - `IUserOrderStore` + `SqliteUserOrderStore`: pin (`order_key = scene global min − 1000`, empty scene → `1000`), unpin, keyset paging on `(order_key, item_type, item_id)`, `BEGIN IMMEDIATE` to serialize read-min-then-insert against concurrent pins. - Sidebar read store: thin-row query + batched hydration (no N+1), anti-join against `user_order` for the unpinned side (never reads the deprecated `pinned` column). ### aionui-sidebar (new crate) - `GET /api/sidebar` — first screen: pinned → projects (real project groups + dir pseudo-groups) → chats. - `GET /api/sidebar/items` — per-group `+10` keyset paging; five-case classification + path merge live in the display layer only (no `resolve_existing`, no fs touch, no writes). - `PUT`/`DELETE /api/order/pinned/{item_type}/{item_id}` — pin/unpin, idempotent, scene enum validated (unknown → 400). - Group order = render order; in-group item order = render order. ### Cascade (§4.3, best-effort) - Conversation delete, team delete, and a conversation becoming a live team member each drop the matching `user_order` rows. Orphans self-heal on read. ### Wiring - Routes / state / service wired into `aionui-app` mirroring the project module. ## Tests - `EXPLAIN QUERY PLAN` asserts the hot pinned reads (base + expanded keyset predicate) ride `idx_user_order_scene` and never full-scan `user_order`. - Keyset continuity (no repeat / no gap), concurrent-pin serialization into distinct rows, per-user scoping. - Sidebar classification matrix (five cases × conversation/team), path-merge does not write the DB, dangling `project_id`, join-team read-side member exclusion. - Cascade coverage across the three paths. - Full workspace gate green: `just push` = 8526 passed, 47 skipped. ## Notes - Pairs with AionUi `boii/feat/sidebar` (PR #3969). Frontend requires this branch running. - Not yet live-verified end-to-end in a running desktop instance — unit/integration only. - Follow-up (separate PR): `removeProject` greenfield (BR-19/D13). Conclusion: single global transaction is infeasible — conversation delete fires hooks (agent-process kill, cron clear) and removes the fs workspace dir, none of which can live in a DB tx. It will mirror `remove_team`: per-entity best-effort orchestration, localized atomicity only. - Deprecated `conversations.pinned/pinned_at` columns are left in place (not dropped); no read/write path touches them after this PR. --- Cargo.lock | 26 + Cargo.toml | 2 + crates/aionui-api-types/src/lib.rs | 5 + crates/aionui-api-types/src/sidebar.rs | 149 +++ crates/aionui-app/Cargo.toml | 1 + crates/aionui-app/src/router/routes.rs | 6 + crates/aionui-app/src/router/state.rs | 70 +- crates/aionui-app/src/services.rs | 27 +- crates/aionui-conversation/src/lib.rs | 2 + crates/aionui-conversation/src/service.rs | 18 + .../038_sidebar_ordering_and_archive.sql | 42 + crates/aionui-db/src/lib.rs | 25 +- crates/aionui-db/src/models/mod.rs | 2 + crates/aionui-db/src/models/user_order.rs | 77 ++ .../aionui-db/src/models/user_order_test.rs | 17 + crates/aionui-db/src/repository/mod.rs | 8 + crates/aionui-db/src/repository/project.rs | 6 + crates/aionui-db/src/repository/sidebar.rs | 102 ++ .../src/repository/sqlite_project.rs | 18 + .../src/repository/sqlite_sidebar.rs | 148 +++ .../src/repository/sqlite_sidebar_test.rs | 299 +++++ .../src/repository/sqlite_user_order.rs | 218 ++++ .../src/repository/sqlite_user_order_test.rs | 297 +++++ crates/aionui-db/src/repository/user_order.rs | 91 ++ crates/aionui-project/src/service.rs | 14 + crates/aionui-sidebar/Cargo.toml | 35 + crates/aionui-sidebar/src/cascade.rs | 51 + crates/aionui-sidebar/src/cascade_test.rs | 63 + crates/aionui-sidebar/src/lib.rs | 20 + crates/aionui-sidebar/src/ports.rs | 33 + crates/aionui-sidebar/src/routes.rs | 218 ++++ crates/aionui-sidebar/src/service.rs | 1070 +++++++++++++++++ crates/aionui-sidebar/src/service_test.rs | 845 +++++++++++++ crates/aionui-sidebar/src/types.rs | 259 ++++ crates/aionui-sidebar/src/types_test.rs | 186 +++ crates/aionui-team/src/service.rs | 43 +- .../tests/session_service_integration.rs | 55 + 37 files changed, 4530 insertions(+), 18 deletions(-) create mode 100644 crates/aionui-api-types/src/sidebar.rs create mode 100644 crates/aionui-db/migrations/038_sidebar_ordering_and_archive.sql create mode 100644 crates/aionui-db/src/models/user_order.rs create mode 100644 crates/aionui-db/src/models/user_order_test.rs create mode 100644 crates/aionui-db/src/repository/sidebar.rs create mode 100644 crates/aionui-db/src/repository/sqlite_sidebar.rs create mode 100644 crates/aionui-db/src/repository/sqlite_sidebar_test.rs create mode 100644 crates/aionui-db/src/repository/sqlite_user_order.rs create mode 100644 crates/aionui-db/src/repository/sqlite_user_order_test.rs create mode 100644 crates/aionui-db/src/repository/user_order.rs create mode 100644 crates/aionui-sidebar/Cargo.toml create mode 100644 crates/aionui-sidebar/src/cascade.rs create mode 100644 crates/aionui-sidebar/src/cascade_test.rs create mode 100644 crates/aionui-sidebar/src/lib.rs create mode 100644 crates/aionui-sidebar/src/ports.rs create mode 100644 crates/aionui-sidebar/src/routes.rs create mode 100644 crates/aionui-sidebar/src/service.rs create mode 100644 crates/aionui-sidebar/src/service_test.rs create mode 100644 crates/aionui-sidebar/src/types.rs create mode 100644 crates/aionui-sidebar/src/types_test.rs diff --git a/Cargo.lock b/Cargo.lock index d7991d9dc..62b4d2b7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -419,6 +419,7 @@ dependencies = [ "aionui-runtime", "aionui-session", "aionui-shell", + "aionui-sidebar", "aionui-system", "aionui-team", "aionui-team-prompts", @@ -905,6 +906,31 @@ dependencies = [ "wiremock", ] +[[package]] +name = "aionui-sidebar" +version = "0.1.64" +dependencies = [ + "aionui-api-types", + "aionui-auth", + "aionui-common", + "aionui-conversation", + "aionui-db", + "aionui-project", + "async-trait", + "axum", + "base64", + "http-body-util", + "serde", + "serde_json", + "sqlx", + "tempfile", + "thiserror 2.0.18", + "tokio", + "tower", + "tracing", + "url", +] + [[package]] name = "aionui-system" version = "0.1.64" diff --git a/Cargo.toml b/Cargo.toml index 78bb1c9b3..bf36bd5d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ members = [ "crates/aionui-team-prompts", "crates/aionui-team", "crates/aionui-project", + "crates/aionui-sidebar", "crates/aionui-cron", "crates/aionui-assistant", "crates/aionui-app", @@ -55,6 +56,7 @@ aionui-channel = { path = "crates/aionui-channel" } aionui-team-prompts = { path = "crates/aionui-team-prompts" } aionui-team = { path = "crates/aionui-team" } aionui-project = { path = "crates/aionui-project" } +aionui-sidebar = { path = "crates/aionui-sidebar" } aionui-cron = { path = "crates/aionui-cron" } aionui-assistant = { path = "crates/aionui-assistant" } aionui-app = { path = "crates/aionui-app" } diff --git a/crates/aionui-api-types/src/lib.rs b/crates/aionui-api-types/src/lib.rs index 2f3f33b08..6c30f58f1 100644 --- a/crates/aionui-api-types/src/lib.rs +++ b/crates/aionui-api-types/src/lib.rs @@ -28,6 +28,7 @@ mod remote_agent; mod response; mod runtime; mod shell; +mod sidebar; mod skill; mod system; mod team; @@ -156,6 +157,10 @@ pub use shell::{ OpenExternalRequest, OpenFileRequest, OpenFolderWithRequest, ShowItemInFolderRequest, SpeechToTextConfig, SpeechToTextProvider, SpeechToTextResult, SttStreamClientMessage, SttStreamServerMessage, ToolType, }; +pub use sidebar::{ + RemoveProjectItem, RemoveProjectItemKind, RemoveProjectResult, SidebarGroup, SidebarItem, SidebarItemsResponse, + SidebarResponse, SidebarScope, SidebarTeamItem, +}; pub use skill::{ AddExternalPathRequest, DeleteSkillRequest, ExportSkillRequest, ExternalSkillSourceResponse, ImportSkillFailureResponse, ImportSkillRequest, ImportSkillResponse, MaterializeSkillsRequest, diff --git a/crates/aionui-api-types/src/sidebar.rs b/crates/aionui-api-types/src/sidebar.rs new file mode 100644 index 000000000..78e6a20e9 --- /dev/null +++ b/crates/aionui-api-types/src/sidebar.rs @@ -0,0 +1,149 @@ +//! Sidebar read-model DTOs (`GET /api/sidebar`, `GET /api/sidebar/items`). +//! +//! One request renders the whole left panel: the backend classifies every +//! conversation/team into its group (pinned / project / pseudo-dir / chats), +//! windows each group, and hydrates items. The frontend only renders in the +//! given order — it runs no classification. See +//! `feat-project-design/temp/left-panel/api-contract-sidebar.md` §4. + +use serde::{Deserialize, Serialize}; + +use crate::conversation::ConversationResponse; + +/// Root of `GET /api/sidebar`. +/// +/// `groups` order **is** render order: `pinned → project-area (project + dir +/// interleaved) × N → chats`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SidebarResponse { + /// Rendered top to bottom in this exact order. + pub groups: Vec, + /// True when the project area exceeded the 100-group hard cap and was + /// truncated. + pub has_more_groups: bool, +} + +/// Response of `GET /api/sidebar/items` — one more window of a single group. +/// +/// Same shape as [`SidebarGroup`] minus `scope` (the caller already knows which +/// group it paged). See `api-contract-sidebar.md` §3.2. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SidebarItemsResponse { + pub items: Vec, + pub has_more: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub next_cursor: Option, +} + +/// One group (a section's window) in the sidebar. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SidebarGroup { + pub scope: SidebarScope, + pub items: Vec, + /// True when this group has items beyond the returned window (paginate via + /// `GET /api/sidebar/items?scope=&cursor=`). + pub has_more: bool, + /// Keyset cursor for the next page; `None` iff `has_more` is false. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub next_cursor: Option, +} + +/// Which section a group belongs to; the tag doubles as the group-head shape. +/// +/// The frontend has three sections (pinned / project / chats); both `Project` +/// and `Dir` render into the project area, distinguished only by group-head +/// form (a real project head carries the "+" / remove entry, a dir head does +/// not). +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum SidebarScope { + /// Pinned group. Pinned rows appear only here. + Pinned, + /// Real project group. `workspace` feeds the project-head "+" entry + /// (resolved server-side from the project source, not scanned from items). + Project { + project_id: String, + name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + workspace: Option, + }, + /// Pseudo project group (directory aggregation). `key` is the dir token used + /// for paging / `win`; `name` is the directory's last segment. + Dir { key: String, path: String, name: String }, + /// The flat "chats" group. + Chats, +} + +/// One row in a group: either a full conversation or an aggregated team row. +// `Conversation` is by far the common variant and the hot path (most sidebar +// rows are conversations); boxing it just to shrink the rarer `Team` variant +// would add a heap allocation per row for no real memory win, so the size +// disparity is accepted deliberately. +#[allow(clippy::large_enum_variant)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum SidebarItem { + /// A conversation row, reusing the shared conversation DTO. Its `pinned` + /// flag is derived from `user_order` row existence, not any table column. + Conversation { conversation: ConversationResponse }, + /// An aggregated team row (server-side aggregate; the frontend does not + /// reconstruct it from member conversations). + Team(SidebarTeamItem), +} + +/// Result of `DELETE /api/sidebar/project/{id}` (and its `dry_run` preview). +/// +/// Counts are of the units classified into the project's group (BR-19). A live +/// delete reports how many were actually removed; a `dry_run` reports how many +/// *would* be — the two agree when no concurrent deletion races in. Team-member +/// conversations are not counted separately: they are folded into their team and +/// removed by the team cascade, so only the visible rows (`teams_deleted` + +/// `conversations_deleted`) are reported. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RemoveProjectResult { + /// Teams classified into the project's group. + pub teams_deleted: i64, + /// Independent conversations classified into the project's group. + pub conversations_deleted: i64, + /// The named units in the delete set, so a `dry_run` preview can list *which* + /// items go — not just how many. Pinned members live in the top pinned group + /// (B1 double-render: a project's pinned rows are anti-joined out of its own + /// group), so the frontend cannot reconstruct project membership itself; the + /// names must come from here. Empty on a live delete (the preview already + /// showed them). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub items: Vec, +} + +/// One named unit in a [`RemoveProjectResult`] preview. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RemoveProjectItem { + pub name: String, + /// Whether the unit is currently pinned (hoisted into the top pinned group). + pub pinned: bool, + pub kind: RemoveProjectItemKind, +} + +/// Which sidebar unit a [`RemoveProjectItem`] is. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum RemoveProjectItemKind { + Conversation, + Team, +} + +/// Aggregated team row for the sidebar. +/// +/// Membership grouping is already expressed by the group the row sits in, so +/// (unlike the old draft) there is no `project` back-reference. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SidebarTeamItem { + pub team_id: String, + pub name: String, + /// `MAX(updated_at)` across active member conversations. + pub updated_at: i64, + /// Derived from a `user_order` scene=`pinned` row existing for this team. + pub pinned: bool, + /// Active member conversation ids, `created_at` ascending. + pub member_conversation_ids: Vec, +} diff --git a/crates/aionui-app/Cargo.toml b/crates/aionui-app/Cargo.toml index cf2ffe670..77ce186ad 100644 --- a/crates/aionui-app/Cargo.toml +++ b/crates/aionui-app/Cargo.toml @@ -38,6 +38,7 @@ aionui-team.workspace = true aionui-team-prompts.workspace = true aionui-cron.workspace = true aionui-project.workspace = true +aionui-sidebar.workspace = true aionui-assistant.workspace = true aionui-runtime.workspace = true aionui-process.workspace = true diff --git a/crates/aionui-app/src/router/routes.rs b/crates/aionui-app/src/router/routes.rs index 23604705e..60ca58a1b 100644 --- a/crates/aionui-app/src/router/routes.rs +++ b/crates/aionui-app/src/router/routes.rs @@ -36,6 +36,7 @@ use aionui_office::{office_proxy_routes, office_routes}; use aionui_project::project_routes; use aionui_realtime::{NoopMessageRouter, WsHandlerState, ws_upgrade_handler}; use aionui_shell::shell_routes; +use aionui_sidebar::sidebar_routes; use aionui_system::{ClientPrefService, connection_test_routes, system_routes}; use aionui_team::{TeamSessionService, team_routes}; @@ -283,6 +284,10 @@ pub fn create_router_with_all_state(services: &AppServices, states: ModuleStates let project_authenticated = project_routes(states.project).route_layer(from_fn_with_state(auth_mw_state.clone(), auth_middleware)); + // Sidebar read + ordering routes protected by auth middleware + let sidebar_authenticated = + sidebar_routes(states.sidebar).route_layer(from_fn_with_state(auth_mw_state.clone(), auth_middleware)); + // MCP routes protected by auth middleware let mcp_authenticated = mcp_routes(states.mcp).route_layer(from_fn_with_state(auth_mw_state.clone(), auth_middleware)); @@ -364,6 +369,7 @@ pub fn create_router_with_all_state(services: &AppServices, states: ModuleStates .merge(connection_test_authenticated) .merge(file_authenticated) .merge(project_authenticated) + .merge(sidebar_authenticated) .merge(mcp_authenticated) .merge(extension_authenticated) .merge(hub_authenticated) diff --git a/crates/aionui-app/src/router/state.rs b/crates/aionui-app/src/router/state.rs index 0d0b2bec9..570edae25 100644 --- a/crates/aionui-app/src/router/state.rs +++ b/crates/aionui-app/src/router/state.rs @@ -34,9 +34,10 @@ use aionui_mcp::{ McpConfigService, McpConnectionTestService, McpRouterState, McpSyncService, OpencodeAdapter, QwenAdapter, }; use aionui_office::{ConversionService, OfficeRouterState, OfficecliWatchManager, ProxyService}; -use aionui_project::ProjectRouterState; +use aionui_project::{ProjectRouterState, ProjectService}; use aionui_realtime::{MessageRouter, TokenUserResolver, WsHandlerState}; use aionui_shell::ShellRouterState; +use aionui_sidebar::{SidebarRouterState, SidebarService}; use aionui_system::{ ClientPrefService, ConnectionTestRouterState, ConnectionTestService, FeedbackDiagnosticsService, ModelFetchService, ProtocolDetectionService, ProviderService, RuntimePrepareService, SettingsService, SystemRouterState, @@ -129,6 +130,7 @@ pub struct ModuleStates { pub connection_test: ConnectionTestRouterState, pub file: FileRouterState, pub project: ProjectRouterState, + pub sidebar: SidebarRouterState, pub mcp: McpRouterState, pub extension: ExtensionRouterState, pub hub: HubRouterState, @@ -309,6 +311,7 @@ pub async fn build_module_states( connection_test: build_module_state_phase(&boot, "connection_test", build_connection_test_state), file: build_module_state_phase(&boot, "file", || build_file_state(services))?, project: build_module_state_phase(&boot, "project", || build_project_state(services)), + sidebar: build_module_state_phase(&boot, "sidebar", || build_sidebar_state(services)), mcp: build_module_state_phase(&boot, "mcp", || build_mcp_state(services)), extension: ext_state, hub: hub_state, @@ -331,6 +334,17 @@ pub async fn build_module_states( elapsed_ms = boot.elapsed().as_millis(), "startup: module state build completed" ); + // Late-inject the sidebar's remove-project ports: the team service is built + // after the sidebar state above, so the wiring cannot happen inside + // `build_sidebar_state`. `set_remove_project_ports` is set-once. + states + .sidebar + .service + .set_remove_project_ports(Arc::new(RemoveProjectAdapter { + conversation: services.conversation_service.clone(), + team: states.team.service.clone(), + project: services.project_service.clone(), + })); states .conversation .service @@ -503,6 +517,58 @@ pub fn build_project_state(services: &AppServices) -> ProjectRouterState { } } +/// Build the sidebar read/ordering router state from application services. +/// The sidebar store and ordering store share the app-wide pool; `work_dir` is +/// the conversation temp-workspace root used for path classification (must match +/// `ProjectService`'s temp root). +pub fn build_sidebar_state(services: &AppServices) -> SidebarRouterState { + let sidebar_store: Arc = + Arc::new(aionui_db::SqliteSidebarStore::new(services.database.pool().clone())); + let service = SidebarService::new( + sidebar_store, + services.user_order_store.clone(), + services.work_dir.join("conversations"), + ); + SidebarRouterState { + service: Arc::new(service), + } +} + +/// Adapts the concrete conversation / team / project services to the sidebar's +/// [`aionui_sidebar::RemoveProjectPorts`] trait so `remove_project` (BR-19) can +/// reuse the existing per-unit delete paths (`ConversationService::delete`, +/// `TeamSessionService::remove_team`, `ProjectService::delete_project`) without +/// the sidebar crate depending on any of them. +struct RemoveProjectAdapter { + conversation: ConversationService, + team: Arc, + project: ProjectService, +} + +#[async_trait::async_trait] +impl aionui_sidebar::RemoveProjectPorts for RemoveProjectAdapter { + async fn delete_conversation(&self, user_id: &str, conversation_id: &str) -> Result<(), String> { + self.conversation + .delete(user_id, conversation_id) + .await + .map_err(|err| err.to_string()) + } + + async fn remove_team(&self, user_id: &str, team_id: &str) -> Result<(), String> { + self.team + .remove_team(user_id, team_id) + .await + .map_err(|err| err.to_string()) + } + + async fn delete_project_record(&self, user_id: &str, project_id: &str) -> Result<(), String> { + self.project + .delete_project(user_id, project_id) + .await + .map_err(|err| err.to_string()) + } +} + /// Build the default `McpRouterState` from application services. pub fn build_mcp_state(services: &AppServices) -> McpRouterState { let pool = services.database.pool().clone(); @@ -762,6 +828,8 @@ pub fn build_team_state( aionui_team::TeamPromptDumpConfig::from_data_dir(&services.data_dir, services.dump_prompts), ); service.with_project_service(Arc::new(services.project_service.clone())); + // Path-2 cascade: removing a team drops its `user_order` row (sidebar §4.3). + service.with_user_order_store(services.user_order_store.clone()); TeamRouterState { service, active_leases: services.active_lease_registry.clone(), diff --git a/crates/aionui-app/src/services.rs b/crates/aionui-app/src/services.rs index c06c1b3b0..60ee9b51e 100644 --- a/crates/aionui-app/src/services.rs +++ b/crates/aionui-app/src/services.rs @@ -13,13 +13,14 @@ use aionui_common::OnConversationDelete; use aionui_conversation::{ConversationService, runtime_state::ConversationRuntimeStateService}; use aionui_db::{ Database, IAcpSessionRepository, IAgentMetadataRepository, IConversationRepository, IMcpServerRepository, - IProjectStore, ISkillRepository, IUserRepository, SqliteAcpSessionRepository, SqliteAgentMetadataRepository, - SqliteAssistantDefinitionRepository, SqliteAssistantOverlayRepository, SqliteAssistantPreferenceRepository, - SqliteConversationRepository, SqliteMcpServerRepository, SqliteProjectStore, SqliteProviderRepository, - SqliteSkillRepository, SqliteUserRepository, + IProjectStore, ISkillRepository, IUserOrderStore, IUserRepository, SqliteAcpSessionRepository, + SqliteAgentMetadataRepository, SqliteAssistantDefinitionRepository, SqliteAssistantOverlayRepository, + SqliteAssistantPreferenceRepository, SqliteConversationRepository, SqliteMcpServerRepository, SqliteProjectStore, + SqliteProviderRepository, SqliteSkillRepository, SqliteUserOrderStore, SqliteUserRepository, }; use aionui_project::ProjectService; use aionui_realtime::{BroadcastEventBus, WebSocketManager}; +use aionui_sidebar::UserOrderDeleteHook; pub struct AppServices { pub database: Database, @@ -37,6 +38,10 @@ pub struct AppServices { /// Project-bind service (project-bind side branch). Shared by conversation /// and team wiring to bind/backfill project/folder rows. Cheap to clone. pub project_service: ProjectService, + /// Sidebar ordering store (`user_order` table). Shared by the conversation + /// delete hook (path-1 cascade), the team service (path-2 cascade), and the + /// sidebar read state. Cheap to clone (Arc). See sidebar design §4. + pub user_order_store: Arc, /// Same instance as `worker_task_manager`, exposed through the /// `OnConversationDelete` trait so `ConversationService::with_delete_hook` /// can wire it up. Optional because tests construct `AppServices` with a @@ -94,6 +99,7 @@ impl AppServices { runtime_base_url: self.runtime_base_url.clone(), runtime_token_service: self.runtime_token_service.clone(), project_service: self.project_service.clone(), + user_order_store: self.user_order_store.clone(), }); self } @@ -185,6 +191,11 @@ impl AppServices { let project_store: Arc = Arc::new(SqliteProjectStore::new(database.pool().clone())); let project_service = ProjectService::new(project_store, work_dir.join("conversations")); + // Sidebar ordering store (`user_order` table). Built early so it can be + // shared by the conversation delete hook, the team service, and the + // sidebar read state. + let user_order_store: Arc = Arc::new(SqliteUserOrderStore::new(database.pool().clone())); + // Skill paths need app resource dir (for builtin rules) + data dir // (for user skills + materialized views). AcpSkillManager uses these // for first-message skill index/body loading. @@ -274,6 +285,7 @@ impl AppServices { runtime_base_url: runtime_base_url.clone(), runtime_token_service: runtime_token_service.clone(), project_service: project_service.clone(), + user_order_store: user_order_store.clone(), }); Ok(Self { @@ -291,6 +303,7 @@ impl AppServices { conversation_runtime_state, conversation_service, project_service, + user_order_store, task_manager_delete_hook: Some(task_manager_delete_hook), agent_registry, conversation_repo, @@ -325,6 +338,10 @@ struct ConversationServiceDeps<'a> { runtime_base_url: String, runtime_token_service: Arc, project_service: ProjectService, + /// Sidebar ordering store. Wired as a second delete hook so deleting a + /// conversation cascades away its `user_order` rows (sidebar design §4.3, + /// path 1). + user_order_store: Arc, } fn build_conversation_service(deps: ConversationServiceDeps<'_>) -> ConversationService { @@ -357,6 +374,8 @@ fn build_conversation_service(deps: ConversationServiceDeps<'_>) -> Conversation if let Some(hook) = deps.task_manager_delete_hook { service.with_delete_hook(hook); } + // Path-1 cascade: a deleted conversation drops its `user_order` rows. + service.with_delete_hook(Arc::new(UserOrderDeleteHook::new(deps.user_order_store))); service.with_project_service(Arc::new(deps.project_service)); service } diff --git a/crates/aionui-conversation/src/lib.rs b/crates/aionui-conversation/src/lib.rs index ce75cf73b..1ed684b50 100644 --- a/crates/aionui-conversation/src/lib.rs +++ b/crates/aionui-conversation/src/lib.rs @@ -28,10 +28,12 @@ mod turn_continuation_policy; mod turn_orchestrator; mod turn_recovery_policy; +pub use convert::row_to_response_with_extra; pub use error::ConversationError; pub use response_middleware::{MessageMiddleware, MiddlewareResult, strip_think_tags}; pub use routes::conversation_routes; pub use routes_aux::conversation_ops_routes; +pub use service::is_temp_session_workspace; pub use service::{ ConversationAgentTurnOutcome, ConversationAgentTurnRequest, ConversationAgentTurnStarted, ConversationAgentTurnStartedCallback, ConversationAgentTurnStatus, ConversationService, diff --git a/crates/aionui-conversation/src/service.rs b/crates/aionui-conversation/src/service.rs index 8e8f01dda..f6eef1992 100644 --- a/crates/aionui-conversation/src/service.rs +++ b/crates/aionui-conversation/src/service.rs @@ -4491,6 +4491,24 @@ fn is_auto_workspace_relative_path(relative: &Path) -> bool { } } +/// True when `workspace` is a backend auto-generated temp session directory +/// under `{work_dir}/conversations` — the sidebar read model's "temp path" test. +/// +/// Pure lexical prefix strip + [`is_auto_workspace_relative_path`]; performs no +/// filesystem access, so it is safe on the side-effect-free sidebar read path +/// (dead/removed workspaces classify correctly rather than failing an fs probe). +/// This is the same judgment the conversation service applies per row; it is +/// exposed only so the sidebar can classify a conversation's `extra.workspace` +/// (or a team's `workspace` column) without duplicating the rule. `work_dir` is +/// the application data directory (`ConversationService`'s `workspace_root`), +/// injected from the same source on both sides. +pub fn is_temp_session_workspace(work_dir: &Path, workspace: &Path) -> bool { + match workspace.strip_prefix(work_dir.join("conversations")) { + Ok(relative) => is_auto_workspace_relative_path(relative), + Err(_) => false, + } +} + async fn cleanup_empty_date_workspace_parents(workspace_root: &Path, workspace_path: &Path) { let Some(date_dirs) = date_workspace_parent_dirs(workspace_root, workspace_path) else { return; diff --git a/crates/aionui-db/migrations/038_sidebar_ordering_and_archive.sql b/crates/aionui-db/migrations/038_sidebar_ordering_and_archive.sql new file mode 100644 index 000000000..e7fce8859 --- /dev/null +++ b/crates/aionui-db/migrations/038_sidebar_ordering_and_archive.sql @@ -0,0 +1,42 @@ +-- Sidebar ordering foundation + archive columns (feature: left-panel grouping). +-- +-- `user_order` is the source of truth for user-defined ordering across the +-- sidebar. It is a generic ordering table keyed by (user_id, scene, item_type, +-- item_id); v1 uses only the 'pinned' scene, where a row's *existence* means +-- the item is pinned (there is no boolean pinned column — the legacy +-- conversations.pinned / pinned_at columns are deprecated and left untouched +-- from here on). order_key drives intra-scene ordering; it is deliberately NOT +-- unique per (user_id, scene): a rebalance transaction may transiently collide +-- an updated key with an as-yet-unchanged row, and cursors tie-break on the +-- full (order_key, item_type, item_id) triple, so uniqueness buys nothing. +-- +-- No pin backfill: pinned state is a user preference and is intentionally not +-- migrated from the deprecated columns or team localStorage (product decision, +-- 2026-08-11) — consistent with team pins never being migrated. +-- +-- archived_at (NULL = not archived) is added here in one shot so PR-B (archive) +-- needs no further migration. The partial indexes cover the selective +-- "archived only" read path; the default sidebar path filters archived_at IS +-- NULL, which is non-selective and needs no index. + +CREATE TABLE IF NOT EXISTS user_order ( + user_id TEXT NOT NULL, + scene TEXT NOT NULL, -- closed enum, v1 only 'pinned' + item_type TEXT NOT NULL, -- 'conversation' | 'team' + item_id TEXT NOT NULL, + order_key INTEGER NOT NULL, -- i64, not unique within a scene + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + PRIMARY KEY (user_id, scene, item_type, item_id) +); + +CREATE INDEX IF NOT EXISTS idx_user_order_scene + ON user_order(user_id, scene, order_key); + +ALTER TABLE conversations ADD COLUMN archived_at INTEGER; -- NULL = not archived +ALTER TABLE teams ADD COLUMN archived_at INTEGER; -- NULL = not archived + +CREATE INDEX IF NOT EXISTS idx_conversations_archived + ON conversations(user_id, archived_at) WHERE archived_at IS NOT NULL; +CREATE INDEX IF NOT EXISTS idx_teams_archived + ON teams(user_id, archived_at) WHERE archived_at IS NOT NULL; diff --git a/crates/aionui-db/src/lib.rs b/crates/aionui-db/src/lib.rs index b73cbe983..3f79335e4 100644 --- a/crates/aionui-db/src/lib.rs +++ b/crates/aionui-db/src/lib.rs @@ -26,11 +26,11 @@ pub use instance_lock::{DataDirInstanceGuard, instance_lock_path}; pub use models::{ AgentMetadataRow, AssistantDefinitionRow, AssistantOverlayRow, AssistantOverrideRow, AssistantPreferenceRow, AssistantRow, ConversationArtifactRow, ConversationAssistantSnapshotRow, CreateAssistantParams, - ExternalUserProjection, FolderRow, ProjectExplorerRow, ProjectKind, ProjectRow, Role, SkillImportRecordRow, - SkillRow, UpdateAgentAvailabilitySnapshotParams, UpdateAgentHandshakeParams, UpdateAssistantParams, - UpsertAgentMetadataParams, UpsertAssistantDefinitionParams, UpsertAssistantOverlayParams, - UpsertAssistantPreferenceParams, UpsertConversationAssistantSnapshotParams, UpsertOverrideParams, UserStatus, - UserType, + ExternalUserProjection, FolderRow, OrderItemType, OrderScene, ProjectExplorerRow, ProjectKind, ProjectRow, Role, + SkillImportRecordRow, SkillRow, UpdateAgentAvailabilitySnapshotParams, UpdateAgentHandshakeParams, + UpdateAssistantParams, UpsertAgentMetadataParams, UpsertAssistantDefinitionParams, UpsertAssistantOverlayParams, + UpsertAssistantPreferenceParams, UpsertConversationAssistantSnapshotParams, UpsertOverrideParams, UserOrderRow, + UserStatus, UserType, }; pub use repository::channel::UpdatePluginStatusParams; pub use repository::conversation::{ @@ -53,14 +53,15 @@ pub use repository::{ IAssistantOverrideRepository, IAssistantPreferenceRepository, IAssistantRepository, IChannelRepository, IClientPreferenceRepository, IConversationRepository, ICronRepository, IFeedbackDiagnosticsRepository, IMcpServerRepository, IOAuthTokenRepository, IProjectStore, IProviderRepository, IRemoteAgentRepository, - ISettingsRepository, ISkillRepository, ITeamRepository, IUserRepository, PageDirection, PersistedSessionState, - SaveRuntimeStateParams, SqliteAcpSessionRepository, SqliteAgentMetadataRepository, - SqliteAssistantDefinitionRepository, SqliteAssistantOverlayRepository, SqliteAssistantOverrideRepository, - SqliteAssistantPreferenceRepository, SqliteAssistantRepository, SqliteChannelRepository, - SqliteClientPreferenceRepository, SqliteConversationRepository, SqliteCronRepository, + ISettingsRepository, ISidebarStore, ISkillRepository, ITeamRepository, IUserOrderStore, IUserRepository, + OrderItemRef, PageDirection, PersistedSessionState, PinOutcome, PinnedCursor, SaveRuntimeStateParams, + SidebarConversationThin, SidebarProjectMeta, SidebarTeamThin, SqliteAcpSessionRepository, + SqliteAgentMetadataRepository, SqliteAssistantDefinitionRepository, SqliteAssistantOverlayRepository, + SqliteAssistantOverrideRepository, SqliteAssistantPreferenceRepository, SqliteAssistantRepository, + SqliteChannelRepository, SqliteClientPreferenceRepository, SqliteConversationRepository, SqliteCronRepository, SqliteFeedbackDiagnosticsRepository, SqliteMcpServerRepository, SqliteOAuthTokenRepository, SqliteProjectStore, - SqliteProviderRepository, SqliteRemoteAgentRepository, SqliteSettingsRepository, SqliteSkillRepository, - SqliteTeamRepository, SqliteUserRepository, + SqliteProviderRepository, SqliteRemoteAgentRepository, SqliteSettingsRepository, SqliteSidebarStore, + SqliteSkillRepository, SqliteTeamRepository, SqliteUserOrderStore, SqliteUserRepository, }; // Re-export sqlx pool type for downstream crates diff --git a/crates/aionui-db/src/models/mod.rs b/crates/aionui-db/src/models/mod.rs index 2c5ac5953..a29dd4a19 100644 --- a/crates/aionui-db/src/models/mod.rs +++ b/crates/aionui-db/src/models/mod.rs @@ -16,6 +16,7 @@ mod skill; mod system_settings; mod team; mod user; +mod user_order; pub use acp_session::AcpSessionRow; pub use agent_metadata::{ @@ -41,3 +42,4 @@ pub use skill::{SkillImportRecordRow, SkillRow}; pub use system_settings::SystemSettings; pub use team::{MailboxMessageRow, TeamRow, TeamTaskRow}; pub use user::{ExternalUserProjection, User, UserStatus, UserType}; +pub use user_order::{OrderItemType, OrderScene, UserOrderRow}; diff --git a/crates/aionui-db/src/models/user_order.rs b/crates/aionui-db/src/models/user_order.rs new file mode 100644 index 000000000..0bb43753b --- /dev/null +++ b/crates/aionui-db/src/models/user_order.rs @@ -0,0 +1,77 @@ +use aionui_common::TimestampMs; +use serde::{Deserialize, Serialize}; + +/// Ordering scene, stored as the TEXT `user_order.scene` column. +/// +/// A closed enum; v1 has only `pinned` (a row's existence means the item is +/// pinned). Lives in `aionui-db` so the store trait does not depend upward. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum OrderScene { + /// Pinned items, ordered by `order_key` ascending. + Pinned, +} + +impl OrderScene { + /// The canonical TEXT column value. + pub fn as_str(self) -> &'static str { + match self { + OrderScene::Pinned => "pinned", + } + } + + /// Parse a TEXT column value; `None` for unknown (out-of-enum) values. + pub fn parse(value: &str) -> Option { + match value { + "pinned" => Some(OrderScene::Pinned), + _ => None, + } + } +} + +/// Ordered item kind, stored as the TEXT `user_order.item_type` column. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum OrderItemType { + Conversation, + Team, +} + +impl OrderItemType { + /// The canonical TEXT column value. + pub fn as_str(self) -> &'static str { + match self { + OrderItemType::Conversation => "conversation", + OrderItemType::Team => "team", + } + } + + /// Parse a TEXT column value; `None` for unknown values. + pub fn parse(value: &str) -> Option { + match value { + "conversation" => Some(OrderItemType::Conversation), + "team" => Some(OrderItemType::Team), + _ => None, + } + } +} + +/// Row mapping for the `user_order` table. +/// +/// `scene` / `item_type` are TEXT enum-like columns ([`OrderScene`] / +/// [`OrderItemType`]). `order_key` is not unique within a `(user_id, scene)`; +/// callers tie-break on the full `(order_key, item_type, item_id)` triple. +#[derive(Debug, Clone, Serialize, Deserialize, sqlx::FromRow)] +pub struct UserOrderRow { + pub user_id: String, + /// One of: "pinned" (see [`OrderScene`]). + pub scene: String, + /// One of: "conversation", "team" (see [`OrderItemType`]). + pub item_type: String, + pub item_id: String, + pub order_key: i64, + pub created_at: TimestampMs, + pub updated_at: TimestampMs, +} + +#[cfg(test)] +#[path = "user_order_test.rs"] +mod user_order_test; diff --git a/crates/aionui-db/src/models/user_order_test.rs b/crates/aionui-db/src/models/user_order_test.rs new file mode 100644 index 000000000..93c4b2677 --- /dev/null +++ b/crates/aionui-db/src/models/user_order_test.rs @@ -0,0 +1,17 @@ +use super::{OrderItemType, OrderScene}; + +#[test] +fn order_scene_roundtrips_through_column_value() { + for scene in [OrderScene::Pinned] { + assert_eq!(OrderScene::parse(scene.as_str()), Some(scene)); + } + assert_eq!(OrderScene::parse("unknown"), None); +} + +#[test] +fn order_item_type_roundtrips_through_column_value() { + for item_type in [OrderItemType::Conversation, OrderItemType::Team] { + assert_eq!(OrderItemType::parse(item_type.as_str()), Some(item_type)); + } + assert_eq!(OrderItemType::parse("unknown"), None); +} diff --git a/crates/aionui-db/src/repository/mod.rs b/crates/aionui-db/src/repository/mod.rs index 09613c96d..4fa0e7664 100644 --- a/crates/aionui-db/src/repository/mod.rs +++ b/crates/aionui-db/src/repository/mod.rs @@ -13,6 +13,7 @@ pub mod project; pub mod provider; pub mod remote_agent; mod settings; +pub mod sidebar; pub mod skill; mod sqlite_acp_session; mod sqlite_agent_metadata; @@ -28,11 +29,14 @@ mod sqlite_project; mod sqlite_provider; mod sqlite_remote_agent; mod sqlite_settings; +mod sqlite_sidebar; mod sqlite_skill; mod sqlite_team; mod sqlite_user; +mod sqlite_user_order; pub mod team; mod user; +pub mod user_order; pub use acp_session::{CreateAcpSessionParams, IAcpSessionRepository, PersistedSessionState, SaveRuntimeStateParams}; pub use agent_metadata::IAgentMetadataRepository; @@ -54,6 +58,7 @@ pub use project::IProjectStore; pub use provider::IProviderRepository; pub use remote_agent::IRemoteAgentRepository; pub use settings::ISettingsRepository; +pub use sidebar::{ISidebarStore, SidebarConversationThin, SidebarProjectMeta, SidebarTeamThin}; pub use skill::ISkillRepository; pub use sqlite_acp_session::SqliteAcpSessionRepository; pub use sqlite_agent_metadata::SqliteAgentMetadataRepository; @@ -72,8 +77,11 @@ pub use sqlite_project::SqliteProjectStore; pub use sqlite_provider::SqliteProviderRepository; pub use sqlite_remote_agent::SqliteRemoteAgentRepository; pub use sqlite_settings::SqliteSettingsRepository; +pub use sqlite_sidebar::SqliteSidebarStore; pub use sqlite_skill::SqliteSkillRepository; pub use sqlite_team::SqliteTeamRepository; pub use sqlite_user::SqliteUserRepository; +pub use sqlite_user_order::SqliteUserOrderStore; pub use team::{ActivityCursor, ITeamRepository, PageDirection}; pub use user::IUserRepository; +pub use user_order::{IUserOrderStore, OrderItemRef, PinOutcome, PinnedCursor}; diff --git a/crates/aionui-db/src/repository/project.rs b/crates/aionui-db/src/repository/project.rs index e586e32c4..6e639bba3 100644 --- a/crates/aionui-db/src/repository/project.rs +++ b/crates/aionui-db/src/repository/project.rs @@ -67,6 +67,12 @@ pub trait IProjectStore: Send + Sync { ) -> Result; async fn remove_entry(&self, user_id: &str, pe_id: &str) -> Result<(), DbError>; + + /// Delete a project owned by `user_id` and all of its explorer entries in one + /// transaction. Idempotent: an absent or foreign `project_id` deletes nothing + /// and still succeeds. `folders` are global (no owner) and are never removed + /// here — a folder may still back other projects or be re-adopted later. + async fn delete_project(&self, user_id: &str, project_id: &str) -> Result<(), DbError>; async fn reorder(&self, user_id: &str, project_id: &str, ordered_pe_ids: &[String]) -> Result<(), DbError>; async fn rename_entry( &self, diff --git a/crates/aionui-db/src/repository/sidebar.rs b/crates/aionui-db/src/repository/sidebar.rs new file mode 100644 index 000000000..db73e4df5 --- /dev/null +++ b/crates/aionui-db/src/repository/sidebar.rs @@ -0,0 +1,102 @@ +//! Sidebar read-model store: thin classification rows + batch hydration. +//! +//! The sidebar renders the whole left panel from one read snapshot. To stay +//! cheap at 10k–50k conversations this store returns *thin* rows (just the +//! fields the service needs to classify and order — no message/model blobs), +//! and hydrates only the windowed items via a single `IN (...)` batch. See +//! `api-contract-sidebar.md` §5.1 (single read transaction, no N+1). +//! +//! Base exclusion (conversations): `archived_at IS NULL` + the shared +//! health-check keep-fragment (BR-23, `$.is_health_check` in `extra`). The +//! marker is legacy and has no writer anywhere in the workspace today, so the +//! predicate is a defensive no-op — but it is applied identically by the thin +//! listing and by hydration so the口径 cannot drift if a writer ever appears. +//! Teams have no probe rows, so their thin listing filters `archived_at` only. + +use crate::error::DbError; +use crate::models::ConversationRow; + +/// A conversation reduced to what sidebar classification needs. +/// +/// `workspace` / `team_id` are `json_extract`ed from `extra`; `project_id` is +/// `NULLIF`'d to fold the empty string into `None`. +#[derive(Debug, Clone)] +pub struct SidebarConversationThin { + pub id: String, + /// `conversations.project_id`, empty-string folded to `None`. + pub project_id: Option, + /// `extra.$.workspace` — a plain, un-canonicalized fs path (the write path + /// only fs-validates it; see contract §6①). Empty/absent → `None`. + pub workspace: Option, + /// Team-membership marker, `COALESCE(extra.$.team_id, extra.$.teamId)` with + /// canonical `team_id` winning (BR-22 — production still writes camelCase). + /// Present → this conversation is a team member, not an independent row + /// (contract §3.1). The referenced team may not be live/owned (orphan) — + /// the service downgrades those back to independent rows (BR-8). + pub team_id: Option, + pub updated_at: i64, + pub created_at: i64, +} + +/// A team reduced to what sidebar classification needs. The path source is the +/// `teams.workspace` column (contract §6③), never reconstructed from members. +#[derive(Debug, Clone)] +pub struct SidebarTeamThin { + pub id: String, + pub name: String, + pub project_id: Option, + /// `teams.workspace` column; empty string → `None` (no workspace set). + pub workspace: Option, + pub updated_at: i64, + pub created_at: i64, +} + +/// Project spine metadata: a project plus its workspace folder identity. +/// +/// The service uses one enumeration of the user's projects for everything: +/// `kind` distinguishes standard vs temp for bound-item classification +/// (§2 case 1/2), `workspace_canonical` drives path merge (§2 case 3), and the +/// standard subset is the group spine that surfaces even empty projects (BR-5). +#[derive(Debug, Clone)] +pub struct SidebarProjectMeta { + pub project_id: String, + pub name: String, + /// `projects.kind` — service-layer enum `standard` | `temp`. + pub kind: String, + /// Canonical URI of the workspace folder; `None` if the project has no + /// workspace entry (a bound project may legitimately lack one). + pub workspace_canonical: Option, + /// Raw workspace folder URI, for display in the project-head "+" entry. + pub workspace_uri: Option, + /// `projects.created_at` — group-order tie-break for empty standard + /// projects (BR-6: empty project groups order by created_at DESC). + pub created_at: i64, +} + +/// Read-only sidebar queries. Every method is user-scoped: conversations and +/// teams filter `user_id`, and the project enumeration filters `projects.user_id` +/// (added by migration 030). A conversation may carry a foreign `project_id`, but +/// because the project map is built only from the caller's own projects such a +/// binding resolves to nothing and degrades to a dangling id (BR-24). +#[async_trait::async_trait] +pub trait ISidebarStore: Send + Sync { + /// All active (non-archived) conversations for a user, thin. + async fn list_active_conversations_thin(&self, user_id: &str) -> Result, DbError>; + + /// All active (non-archived) teams for a user, thin. + async fn list_active_teams_thin(&self, user_id: &str) -> Result, DbError>; + + /// Every project owned by the user (standard *and* temp), each joined to its + /// workspace folder identity. This single enumeration is the service's whole + /// project picture: the group spine (standard subset, incl. zero-conversation + /// projects — BR-5), the id→kind map for bound-item classification, and the + /// canonical→project map for path merge. Projects are few per user, so no + /// windowing here — the LIMIT 100 on the project area is applied after + /// activity ordering in the service (BR-5/6). + async fn list_user_projects(&self, user_id: &str) -> Result, DbError>; + + /// Full rows for the windowed conversation ids, for response hydration. + /// Filtered to the user and to non-archived rows (a race that archives a row + /// mid-request simply drops it from the window). + async fn hydrate_conversations(&self, user_id: &str, ids: &[String]) -> Result, DbError>; +} diff --git a/crates/aionui-db/src/repository/sqlite_project.rs b/crates/aionui-db/src/repository/sqlite_project.rs index ca1f20e67..065125864 100644 --- a/crates/aionui-db/src/repository/sqlite_project.rs +++ b/crates/aionui-db/src/repository/sqlite_project.rs @@ -254,6 +254,24 @@ impl IProjectStore for SqliteProjectStore { Ok(()) } + async fn delete_project(&self, user_id: &str, project_id: &str) -> Result<(), DbError> { + // Explorer entries first, then the project row — both owner-scoped, one + // transaction. `folders` are global and intentionally left intact. + let mut tx = self.pool.begin().await?; + sqlx::query("DELETE FROM project_explorer WHERE project_id = ? AND owner_user_id = ?") + .bind(project_id) + .bind(user_id) + .execute(&mut *tx) + .await?; + sqlx::query("DELETE FROM projects WHERE project_id = ? AND user_id = ?") + .bind(project_id) + .bind(user_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) + } + async fn reorder(&self, user_id: &str, project_id: &str, ordered_pe_ids: &[String]) -> Result<(), DbError> { let now = now_ms(); let mut tx = self.pool.begin().await?; diff --git a/crates/aionui-db/src/repository/sqlite_sidebar.rs b/crates/aionui-db/src/repository/sqlite_sidebar.rs new file mode 100644 index 000000000..d2af7d7f6 --- /dev/null +++ b/crates/aionui-db/src/repository/sqlite_sidebar.rs @@ -0,0 +1,148 @@ +use sqlx::{Row, SqlitePool}; + +use crate::error::DbError; +use crate::models::ConversationRow; +use crate::repository::sidebar::{ISidebarStore, SidebarConversationThin, SidebarProjectMeta, SidebarTeamThin}; + +/// SQLite-backed implementation of [`ISidebarStore`]. +#[derive(Clone, Debug)] +pub struct SqliteSidebarStore { + pool: SqlitePool, +} + +impl SqliteSidebarStore { + pub fn new(pool: SqlitePool) -> Self { + Self { pool } + } +} + +/// Shared "keep this conversation" fragment excluding probe / health-check rows +/// (BR-23). The marker is a legacy read-only key in `extra`; the whole workspace +/// currently has no writer for it, so this exclusion is defensive (a no-op until +/// something writes `$.is_health_check`). Both the thin listing and hydration use +/// this single constant so their口径 cannot drift. +const HEALTH_CHECK_KEEP: &str = + "(json_extract(extra, '$.is_health_check') IS NULL OR json_extract(extra, '$.is_health_check') NOT IN (1, 'true'))"; + +/// Fold a stored string into `None` when it is empty or whitespace-only. +/// +/// Applies uniformly to workspace paths (empty `extra.workspace` / `teams.workspace` +/// = "no workspace") and to the `teamId` marker (matches the service-layer +/// `team_id_marker_from_extra_str` trim rule). +fn non_blank(value: Option) -> Option { + value.filter(|s| !s.trim().is_empty()) +} + +/// Build a `?,?,…` placeholder list for a dynamic `IN (...)` clause. +fn placeholders(count: usize) -> String { + std::iter::repeat_n("?", count).collect::>().join(",") +} + +#[async_trait::async_trait] +impl ISidebarStore for SqliteSidebarStore { + async fn list_active_conversations_thin(&self, user_id: &str) -> Result, DbError> { + // Team-membership marker: canonical `team_id` first, camelCase `teamId` + // fallback (BR-22 — production still writes camelCase). + let sql = format!( + "SELECT id, \ + NULLIF(project_id, '') AS project_id, \ + json_extract(extra, '$.workspace') AS workspace, \ + COALESCE(NULLIF(json_extract(extra, '$.team_id'), ''), NULLIF(json_extract(extra, '$.teamId'), '')) AS team_id, \ + updated_at, created_at \ + FROM conversations \ + WHERE user_id = ? AND archived_at IS NULL AND {HEALTH_CHECK_KEEP}" + ); + let rows = sqlx::query(&sql).bind(user_id).fetch_all(&self.pool).await?; + + Ok(rows + .into_iter() + .map(|row| SidebarConversationThin { + id: row.get("id"), + project_id: non_blank(row.get("project_id")), + workspace: non_blank(row.get("workspace")), + team_id: non_blank(row.get("team_id")), + updated_at: row.get("updated_at"), + created_at: row.get("created_at"), + }) + .collect()) + } + + async fn list_active_teams_thin(&self, user_id: &str) -> Result, DbError> { + let rows = sqlx::query( + "SELECT id, name, \ + NULLIF(project_id, '') AS project_id, \ + workspace, updated_at, created_at \ + FROM teams \ + WHERE user_id = ? AND archived_at IS NULL", + ) + .bind(user_id) + .fetch_all(&self.pool) + .await?; + + Ok(rows + .into_iter() + .map(|row| SidebarTeamThin { + id: row.get("id"), + name: row.get("name"), + project_id: non_blank(row.get("project_id")), + workspace: non_blank(row.get("workspace")), + updated_at: row.get("updated_at"), + created_at: row.get("created_at"), + }) + .collect()) + } + + async fn list_user_projects(&self, user_id: &str) -> Result, DbError> { + // One user-scoped enumeration of every project (standard + temp), each + // LEFT JOINed to its workspace-root folder. `projects.user_id` (migration + // 030) confines the result to the caller, so a foreign `project_id` carried + // by one of the caller's conversations simply finds no match here and the + // service treats it as dangling (BR-24). At most one workspace folder per + // project (idx_project_explorer_one_workspace_folder), so no row fan-out. + let rows = sqlx::query( + "SELECT p.project_id AS project_id, p.name AS name, p.kind AS kind, p.created_at AS created_at, \ + f.resource_canonical AS workspace_canonical, f.resource_uri AS workspace_uri \ + FROM projects p \ + LEFT JOIN project_explorer pe ON pe.project_id = p.project_id AND pe.role = 'workspace' \ + LEFT JOIN folders f ON f.folder_id = pe.folder_id \ + WHERE p.user_id = ?", + ) + .bind(user_id) + .fetch_all(&self.pool) + .await?; + Ok(rows.into_iter().map(map_project_meta).collect()) + } + + async fn hydrate_conversations(&self, user_id: &str, ids: &[String]) -> Result, DbError> { + if ids.is_empty() { + return Ok(Vec::new()); + } + let sql = format!( + "SELECT * FROM conversations \ + WHERE user_id = ? AND archived_at IS NULL AND {HEALTH_CHECK_KEEP} AND id IN ({})", + placeholders(ids.len()) + ); + let mut query = sqlx::query_as::<_, ConversationRow>(&sql); + query = query.bind(user_id); + for id in ids { + query = query.bind(id); + } + Ok(query.fetch_all(&self.pool).await?) + } +} + +/// Map a project spine join row into [`SidebarProjectMeta`]. +fn map_project_meta(row: sqlx::sqlite::SqliteRow) -> SidebarProjectMeta { + SidebarProjectMeta { + project_id: row.get("project_id"), + name: row.get("name"), + kind: row.get("kind"), + workspace_canonical: row.get("workspace_canonical"), + workspace_uri: row.get("workspace_uri"), + created_at: row.get("created_at"), + } +} + +#[cfg(test)] +#[path = "sqlite_sidebar_test.rs"] +mod sqlite_sidebar_test; diff --git a/crates/aionui-db/src/repository/sqlite_sidebar_test.rs b/crates/aionui-db/src/repository/sqlite_sidebar_test.rs new file mode 100644 index 000000000..54076ed3c --- /dev/null +++ b/crates/aionui-db/src/repository/sqlite_sidebar_test.rs @@ -0,0 +1,299 @@ +use super::SqliteSidebarStore; +use crate::init_database_memory; +use crate::repository::sidebar::ISidebarStore; +use sqlx::SqlitePool; + +const USER: &str = "user-1"; +const OTHER_USER: &str = "user-2"; + +async fn store() -> (SqliteSidebarStore, crate::Database) { + let db = init_database_memory().await.unwrap(); + seed_user(db.pool(), USER).await; + seed_user(db.pool(), OTHER_USER).await; + let store = SqliteSidebarStore::new(db.pool().clone()); + (store, db) +} + +async fn seed_user(pool: &SqlitePool, id: &str) { + sqlx::query("INSERT INTO users (id, username, password_hash, created_at, updated_at) VALUES (?, ?, 'x', 0, 0)") + .bind(id) + .bind(id) + .execute(pool) + .await + .unwrap(); +} + +/// Insert a conversation with explicit project_id, extra json, and archived_at. +#[allow(clippy::too_many_arguments)] +async fn insert_conv( + pool: &SqlitePool, + user: &str, + id: &str, + project_id: Option<&str>, + extra: &str, + updated_at: i64, + archived_at: Option, +) { + sqlx::query( + "INSERT INTO conversations (id, user_id, name, type, extra, project_id, archived_at, created_at, updated_at) \ + VALUES (?, ?, ?, 'chat', ?, ?, ?, ?, ?)", + ) + .bind(id) + .bind(user) + .bind(id) + .bind(extra) + .bind(project_id) + .bind(archived_at) + .bind(updated_at) + .bind(updated_at) + .execute(pool) + .await + .unwrap(); +} + +async fn insert_team( + pool: &SqlitePool, + user: &str, + id: &str, + workspace: &str, + project_id: Option<&str>, + updated_at: i64, + archived_at: Option, +) { + sqlx::query( + "INSERT INTO teams (id, user_id, name, workspace, project_id, archived_at, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + ) + .bind(id) + .bind(user) + .bind(id) + .bind(workspace) + .bind(project_id) + .bind(archived_at) + .bind(updated_at) + .bind(updated_at) + .execute(pool) + .await + .unwrap(); +} + +/// Insert a project plus its workspace-root folder + explorer entry. +#[allow(clippy::too_many_arguments)] +async fn insert_project_with_workspace( + pool: &SqlitePool, + user: &str, + project_id: &str, + name: &str, + kind: &str, + folder_id: &str, + canonical: &str, + uri: &str, +) { + sqlx::query( + "INSERT INTO projects (project_id, user_id, name, kind, created_at, updated_at) VALUES (?, ?, ?, ?, 0, 0)", + ) + .bind(project_id) + .bind(user) + .bind(name) + .bind(kind) + .execute(pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO folders (folder_id, resource_uri, resource_canonical, created_at, updated_at) VALUES (?, ?, ?, 0, 0)", + ) + .bind(folder_id) + .bind(uri) + .bind(canonical) + .execute(pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO project_explorer (pe_id, project_id, folder_id, role, order_index, created_at, updated_at) \ + VALUES (?, ?, ?, 'workspace', 0, 0, 0)", + ) + .bind(format!("pe-{project_id}")) + .bind(project_id) + .bind(folder_id) + .execute(pool) + .await + .unwrap(); +} + +#[tokio::test] +async fn thin_conversations_extract_fields_and_fold_blanks() { + let (store, db) = store().await; + let pool = db.pool(); + // Bound conversation, has workspace + teamId. + insert_conv( + pool, + USER, + "c1", + Some("proj-A"), + r#"{"workspace":"/repo/a","teamId":"t9"}"#, + 100, + None, + ) + .await; + // Unbound (empty project_id folds to None), blank workspace + whitespace teamId fold to None. + insert_conv( + pool, + USER, + "c2", + Some(""), + r#"{"workspace":"","teamId":" "}"#, + 50, + None, + ) + .await; + // No extra keys at all. + insert_conv(pool, USER, "c3", None, "{}", 10, None).await; + + let mut rows = store.list_active_conversations_thin(USER).await.unwrap(); + rows.sort_by(|a, b| a.id.cmp(&b.id)); + assert_eq!(rows.len(), 3); + + assert_eq!(rows[0].id, "c1"); + assert_eq!(rows[0].project_id.as_deref(), Some("proj-A")); + assert_eq!(rows[0].workspace.as_deref(), Some("/repo/a")); + assert_eq!(rows[0].team_id.as_deref(), Some("t9")); + assert_eq!(rows[0].updated_at, 100); + + assert_eq!(rows[1].id, "c2"); + assert_eq!(rows[1].project_id, None, "empty project_id folds to None"); + assert_eq!(rows[1].workspace, None, "empty workspace folds to None"); + assert_eq!(rows[1].team_id, None, "whitespace teamId folds to None"); + + assert_eq!(rows[2].id, "c3"); + assert_eq!(rows[2].workspace, None); + assert_eq!(rows[2].team_id, None); +} + +#[tokio::test] +async fn thin_conversations_exclude_archived_and_other_users() { + let (store, db) = store().await; + let pool = db.pool(); + insert_conv(pool, USER, "live", None, "{}", 10, None).await; + insert_conv(pool, USER, "archived", None, "{}", 20, Some(999)).await; + insert_conv(pool, OTHER_USER, "foreign", None, "{}", 30, None).await; + + let rows = store.list_active_conversations_thin(USER).await.unwrap(); + let ids: Vec<&str> = rows.iter().map(|r| r.id.as_str()).collect(); + assert_eq!(ids, vec!["live"], "archived and cross-user rows excluded"); +} + +#[tokio::test] +async fn thin_teams_fold_blank_workspace_and_exclude_archived() { + let (store, db) = store().await; + let pool = db.pool(); + insert_team(pool, USER, "t1", "/team/ws", Some("proj-A"), 100, None).await; + insert_team(pool, USER, "t2", "", None, 50, None).await; + insert_team(pool, USER, "t3", "/x", None, 10, Some(1)).await; + + let mut rows = store.list_active_teams_thin(USER).await.unwrap(); + rows.sort_by(|a, b| a.id.cmp(&b.id)); + let ids: Vec<&str> = rows.iter().map(|r| r.id.as_str()).collect(); + assert_eq!(ids, vec!["t1", "t2"], "archived team excluded"); + assert_eq!(rows[0].workspace.as_deref(), Some("/team/ws")); + assert_eq!(rows[0].project_id.as_deref(), Some("proj-A")); + assert_eq!(rows[1].workspace, None, "empty team workspace folds to None"); +} + +#[tokio::test] +async fn list_user_projects_joins_workspace_folder_and_carries_kind() { + let (store, db) = store().await; + let pool = db.pool(); + insert_project_with_workspace( + pool, + USER, + "proj-A", + "Alpha", + "standard", + "f-A", + "file:///repo/a", + "file:///repo/a", + ) + .await; + // Temp project with no workspace entry: still enumerated, canonical/uri None. + sqlx::query("INSERT INTO projects (project_id, user_id, name, kind, created_at, updated_at) VALUES ('proj-B', ?, 'Beta', 'temp', 7, 0)") + .bind(USER) + .execute(pool) + .await + .unwrap(); + + let metas = store.list_user_projects(USER).await.unwrap(); + assert_eq!(metas.len(), 2, "both the user's projects enumerated"); + + let a = metas.iter().find(|m| m.project_id == "proj-A").unwrap(); + assert_eq!(a.name, "Alpha"); + assert_eq!(a.kind, "standard"); + assert_eq!(a.workspace_canonical.as_deref(), Some("file:///repo/a")); + + let b = metas.iter().find(|m| m.project_id == "proj-B").unwrap(); + assert_eq!(b.kind, "temp", "temp kind carried for case-2 classification"); + assert_eq!(b.workspace_canonical, None, "no workspace entry => None"); + assert_eq!(b.created_at, 7, "created_at carried for empty-group ordering"); +} + +#[tokio::test] +async fn list_user_projects_is_user_scoped() { + let (store, db) = store().await; + let pool = db.pool(); + // OTHER_USER owns a project whose workspace canonical USER might also point at. + insert_project_with_workspace( + pool, + OTHER_USER, + "proj-foreign", + "Foreign", + "standard", + "f-F", + "file:///repo/shared", + "file:///repo/shared", + ) + .await; + insert_project_with_workspace( + pool, + USER, + "proj-mine", + "Mine", + "standard", + "f-M", + "file:///repo/mine", + "file:///repo/mine", + ) + .await; + + let metas = store.list_user_projects(USER).await.unwrap(); + let ids: Vec<&str> = metas.iter().map(|m| m.project_id.as_str()).collect(); + assert_eq!(ids, vec!["proj-mine"], "another user's project never surfaces (BR-24)"); +} + +#[tokio::test] +async fn hydrate_conversations_is_scoped_and_batched() { + let (store, db) = store().await; + let pool = db.pool(); + insert_conv(pool, USER, "c1", Some("proj-A"), r#"{"workspace":"/a"}"#, 100, None).await; + insert_conv(pool, USER, "c2", None, "{}", 50, None).await; + insert_conv(pool, USER, "archived", None, "{}", 40, Some(1)).await; + insert_conv(pool, OTHER_USER, "foreign", None, "{}", 30, None).await; + + let rows = store + .hydrate_conversations(USER, &["c1".into(), "c2".into(), "archived".into(), "foreign".into()]) + .await + .unwrap(); + let mut ids: Vec<&str> = rows.iter().map(|r| r.id.as_str()).collect(); + ids.sort(); + assert_eq!(ids, vec!["c1", "c2"], "archived + cross-user dropped from window"); + let c1 = rows.iter().find(|r| r.id == "c1").unwrap(); + assert_eq!(c1.project_id.as_deref(), Some("proj-A")); +} + +#[tokio::test] +async fn empty_inputs_short_circuit() { + let (store, _db) = store().await; + assert!( + store.list_user_projects(USER).await.unwrap().is_empty(), + "no projects => empty" + ); + assert!(store.hydrate_conversations(USER, &[]).await.unwrap().is_empty()); +} diff --git a/crates/aionui-db/src/repository/sqlite_user_order.rs b/crates/aionui-db/src/repository/sqlite_user_order.rs new file mode 100644 index 000000000..0447cfbad --- /dev/null +++ b/crates/aionui-db/src/repository/sqlite_user_order.rs @@ -0,0 +1,218 @@ +use aionui_common::now_ms; +use sqlx::SqlitePool; + +use crate::error::DbError; +use crate::models::{OrderItemType, OrderScene, UserOrderRow}; +use crate::repository::user_order::{IUserOrderStore, OrderItemRef, PinOutcome, PinnedCursor}; + +/// Gap between adjacent pins; a fresh top pin claims `min - PIN_GAP`. +const PIN_GAP: i64 = 1000; + +const USER_ORDER_COLS: &str = "user_id, scene, item_type, item_id, order_key, created_at, updated_at"; + +/// SQLite-backed implementation of [`IUserOrderStore`]. +#[derive(Clone, Debug)] +pub struct SqliteUserOrderStore { + pool: SqlitePool, +} + +impl SqliteUserOrderStore { + pub fn new(pool: SqlitePool) -> Self { + Self { pool } + } +} + +#[async_trait::async_trait] +impl IUserOrderStore for SqliteUserOrderStore { + async fn pin(&self, user_id: &str, scene: OrderScene, item: &OrderItemRef) -> Result { + // BEGIN IMMEDIATE claims the writer lock up front so the + // read-min-then-insert is atomic: two concurrent pins can't both read + // the same MIN and insert colliding top rows (the second queues on the + // busy handler). Same pattern as `insert_message_once`. + let mut connection = self.pool.acquire().await?; + sqlx::query("BEGIN IMMEDIATE").execute(&mut *connection).await?; + + let result: Result = async { + let exists: i64 = sqlx::query_scalar( + "SELECT EXISTS(SELECT 1 FROM user_order \ + WHERE user_id = ? AND scene = ? AND item_type = ? AND item_id = ?)", + ) + .bind(user_id) + .bind(scene.as_str()) + .bind(item.item_type.as_str()) + .bind(&item.item_id) + .fetch_one(&mut *connection) + .await?; + if exists != 0 { + return Ok(PinOutcome::AlreadyPinned); + } + + // Empty scene → NULL MIN → start at PIN_GAP; otherwise one gap above + // the current top. order_key is not unique, so no collision handling + // is needed here. + let min_key: Option = + sqlx::query_scalar("SELECT MIN(order_key) FROM user_order WHERE user_id = ? AND scene = ?") + .bind(user_id) + .bind(scene.as_str()) + .fetch_one(&mut *connection) + .await?; + let order_key = min_key.map(|min| min - PIN_GAP).unwrap_or(PIN_GAP); + + let now = now_ms(); + sqlx::query( + "INSERT INTO user_order (user_id, scene, item_type, item_id, order_key, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?)", + ) + .bind(user_id) + .bind(scene.as_str()) + .bind(item.item_type.as_str()) + .bind(&item.item_id) + .bind(order_key) + .bind(now) + .bind(now) + .execute(&mut *connection) + .await?; + Ok(PinOutcome::Inserted) + } + .await; + + match result { + Ok(outcome) => { + sqlx::query("COMMIT").execute(&mut *connection).await?; + Ok(outcome) + } + Err(error) => { + let _ = sqlx::query("ROLLBACK").execute(&mut *connection).await; + Err(error) + } + } + } + + async fn unpin(&self, user_id: &str, scene: OrderScene, item: &OrderItemRef) -> Result { + let result = sqlx::query( + "DELETE FROM user_order \ + WHERE user_id = ? AND scene = ? AND item_type = ? AND item_id = ?", + ) + .bind(user_id) + .bind(scene.as_str()) + .bind(item.item_type.as_str()) + .bind(&item.item_id) + .execute(&self.pool) + .await?; + Ok(result.rows_affected() > 0) + } + + async fn list_pinned( + &self, + user_id: &str, + scene: OrderScene, + after: Option<&PinnedCursor>, + limit: i64, + ) -> Result, DbError> { + // Keyset on (order_key, item_type, item_id). Expanded lexicographic form + // (rather than a row-value tuple) so the leading order_key range uses + // idx_user_order_scene. + let rows = match after { + None => { + sqlx::query_as::<_, UserOrderRow>(&format!( + "SELECT {USER_ORDER_COLS} FROM user_order \ + WHERE user_id = ? AND scene = ? \ + ORDER BY order_key ASC, item_type ASC, item_id ASC \ + LIMIT ?" + )) + .bind(user_id) + .bind(scene.as_str()) + .bind(limit) + .fetch_all(&self.pool) + .await? + } + Some(cursor) => { + sqlx::query_as::<_, UserOrderRow>(&format!( + "SELECT {USER_ORDER_COLS} FROM user_order \ + WHERE user_id = ? AND scene = ? AND ( \ + order_key > ? OR \ + (order_key = ? AND (item_type > ? OR (item_type = ? AND item_id > ?))) \ + ) \ + ORDER BY order_key ASC, item_type ASC, item_id ASC \ + LIMIT ?" + )) + .bind(user_id) + .bind(scene.as_str()) + .bind(cursor.order_key) + .bind(cursor.order_key) + .bind(cursor.item_type.as_str()) + .bind(cursor.item_type.as_str()) + .bind(&cursor.item_id) + .bind(limit) + .fetch_all(&self.pool) + .await? + } + }; + Ok(rows) + } + + async fn pinned_refs(&self, user_id: &str, scene: OrderScene) -> Result, DbError> { + let rows: Vec<(String, String)> = + sqlx::query_as("SELECT item_type, item_id FROM user_order WHERE user_id = ? AND scene = ?") + .bind(user_id) + .bind(scene.as_str()) + .fetch_all(&self.pool) + .await?; + // Skip rows whose item_type is out of the enum (defensive; the write + // path only ever stores known values). + Ok(rows + .into_iter() + .filter_map(|(item_type, item_id)| { + OrderItemType::parse(&item_type).map(|item_type| OrderItemRef { item_type, item_id }) + }) + .collect()) + } + + async fn remove_item(&self, user_id: &str, item: &OrderItemRef) -> Result<(), DbError> { + sqlx::query("DELETE FROM user_order WHERE user_id = ? AND item_type = ? AND item_id = ?") + .bind(user_id) + .bind(item.item_type.as_str()) + .bind(&item.item_id) + .execute(&self.pool) + .await?; + Ok(()) + } + + async fn remove_items(&self, user_id: &str, items: &[OrderItemRef]) -> Result<(), DbError> { + if items.is_empty() { + return Ok(()); + } + // One transaction so the cascade is atomic: either every referenced + // row is gone or none are (a mid-batch failure rolls back). + let mut connection = self.pool.acquire().await?; + sqlx::query("BEGIN IMMEDIATE").execute(&mut *connection).await?; + + let result: Result<(), DbError> = async { + for item in items { + sqlx::query("DELETE FROM user_order WHERE user_id = ? AND item_type = ? AND item_id = ?") + .bind(user_id) + .bind(item.item_type.as_str()) + .bind(&item.item_id) + .execute(&mut *connection) + .await?; + } + Ok(()) + } + .await; + + match result { + Ok(()) => { + sqlx::query("COMMIT").execute(&mut *connection).await?; + Ok(()) + } + Err(error) => { + let _ = sqlx::query("ROLLBACK").execute(&mut *connection).await; + Err(error) + } + } + } +} + +#[cfg(test)] +#[path = "sqlite_user_order_test.rs"] +mod sqlite_user_order_test; diff --git a/crates/aionui-db/src/repository/sqlite_user_order_test.rs b/crates/aionui-db/src/repository/sqlite_user_order_test.rs new file mode 100644 index 000000000..051f756e1 --- /dev/null +++ b/crates/aionui-db/src/repository/sqlite_user_order_test.rs @@ -0,0 +1,297 @@ +use super::{PIN_GAP, SqliteUserOrderStore}; +use crate::init_database_memory; +use crate::models::{OrderItemType, OrderScene}; +use crate::repository::user_order::{IUserOrderStore, OrderItemRef, PinOutcome, PinnedCursor}; + +const USER: &str = "user-1"; +const OTHER_USER: &str = "user-2"; + +async fn store() -> (SqliteUserOrderStore, crate::Database) { + let db = init_database_memory().await.unwrap(); + let store = SqliteUserOrderStore::new(db.pool().clone()); + (store, db) +} + +fn conv(id: &str) -> OrderItemRef { + OrderItemRef::new(OrderItemType::Conversation, id) +} + +fn team(id: &str) -> OrderItemRef { + OrderItemRef::new(OrderItemType::Team, id) +} + +#[tokio::test] +async fn pin_inserts_first_row_at_base_key() { + let (store, _db) = store().await; + let outcome = store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + assert_eq!(outcome, PinOutcome::Inserted); + + let rows = store.list_pinned(USER, OrderScene::Pinned, None, 10).await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].item_id, "c1"); + assert_eq!(rows[0].order_key, PIN_GAP); +} + +#[tokio::test] +async fn pin_stacks_newest_on_top() { + let (store, _db) = store().await; + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + store.pin(USER, OrderScene::Pinned, &conv("c2")).await.unwrap(); + store.pin(USER, OrderScene::Pinned, &team("t1")).await.unwrap(); + + // Ascending order_key => most-recently pinned first. + let rows = store.list_pinned(USER, OrderScene::Pinned, None, 10).await.unwrap(); + let ids: Vec<&str> = rows.iter().map(|r| r.item_id.as_str()).collect(); + assert_eq!(ids, vec!["t1", "c2", "c1"]); + assert_eq!(rows[0].order_key, PIN_GAP - 2 * PIN_GAP); + assert_eq!(rows[1].order_key, PIN_GAP - PIN_GAP); + assert_eq!(rows[2].order_key, PIN_GAP); +} + +#[tokio::test] +async fn pin_is_idempotent() { + let (store, _db) = store().await; + assert_eq!( + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(), + PinOutcome::Inserted + ); + assert_eq!( + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(), + PinOutcome::AlreadyPinned + ); + + let rows = store.list_pinned(USER, OrderScene::Pinned, None, 10).await.unwrap(); + assert_eq!(rows.len(), 1, "duplicate pin must not add a second row"); + assert_eq!(rows[0].order_key, PIN_GAP, "order_key preserved on no-op"); +} + +#[tokio::test] +async fn unpin_reports_removal_then_noop() { + let (store, _db) = store().await; + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + + assert!(store.unpin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap()); + assert!( + !store.unpin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(), + "second unpin is an idempotent no-op" + ); + assert!( + store + .list_pinned(USER, OrderScene::Pinned, None, 10) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test] +async fn list_pinned_paginates_by_keyset() { + let (store, _db) = store().await; + // Pin c1..c5; keys are 1000, 0, -1000, -2000, -3000 (newest lowest). + for id in ["c1", "c2", "c3", "c4", "c5"] { + store.pin(USER, OrderScene::Pinned, &conv(id)).await.unwrap(); + } + + let page1 = store.list_pinned(USER, OrderScene::Pinned, None, 2).await.unwrap(); + let ids1: Vec<&str> = page1.iter().map(|r| r.item_id.as_str()).collect(); + assert_eq!(ids1, vec!["c5", "c4"]); + + let last = page1.last().unwrap(); + let cursor = PinnedCursor { + order_key: last.order_key, + item_type: OrderItemType::parse(&last.item_type).unwrap(), + item_id: last.item_id.clone(), + }; + let page2 = store + .list_pinned(USER, OrderScene::Pinned, Some(&cursor), 2) + .await + .unwrap(); + let ids2: Vec<&str> = page2.iter().map(|r| r.item_id.as_str()).collect(); + assert_eq!( + ids2, + vec!["c3", "c2"], + "keyset resumes strictly after cursor, no repeat" + ); +} + +#[tokio::test] +async fn pinned_reads_ride_the_scene_index_no_full_scan() { + // BR-25: the hot pinned reads must ride `idx_user_order_scene` + // (user_id, scene, order_key) and never degrade into a full-table scan. + // The keyset predicate is deliberately expanded lexicographically (see the + // comment in `list_pinned`) precisely so the leading `order_key` range stays + // index-driven; this test pins that guarantee. + use sqlx::Row; + + let (store, db) = store().await; + for id in ["c1", "c2", "c3"] { + store.pin(USER, OrderScene::Pinned, &conv(id)).await.unwrap(); + } + + let plan_detail = |sql: &'static str, binds: Vec| { + let pool = db.pool().clone(); + async move { + let mut query = sqlx::query(sql); + for bind in &binds { + query = query.bind(bind); + } + let rows = query.fetch_all(&pool).await.unwrap(); + rows.iter() + .map(|row| row.get::("detail")) + .collect::>() + .join(" | ") + } + }; + + // Base (first-screen) read: WHERE user_id, scene + ORDER BY order_key. + let base = plan_detail( + "EXPLAIN QUERY PLAN \ + SELECT user_id, scene, item_type, item_id, order_key FROM user_order \ + WHERE user_id = ? AND scene = ? \ + ORDER BY order_key ASC, item_type ASC, item_id ASC \ + LIMIT ?", + vec![USER.to_owned(), OrderScene::Pinned.as_str().to_owned(), "10".to_owned()], + ) + .await; + assert!( + base.contains("idx_user_order_scene"), + "base pinned read must use idx_user_order_scene, got plan: {base}" + ); + assert!( + !base.contains("SCAN user_order"), + "base pinned read must not full-scan user_order, got plan: {base}" + ); + + // Keyset continuation read: the expanded (order_key > ? OR ...) predicate. + let keyset = plan_detail( + "EXPLAIN QUERY PLAN \ + SELECT user_id, scene, item_type, item_id, order_key FROM user_order \ + WHERE user_id = ? AND scene = ? AND ( \ + order_key > ? OR \ + (order_key = ? AND (item_type > ? OR (item_type = ? AND item_id > ?))) \ + ) \ + ORDER BY order_key ASC, item_type ASC, item_id ASC \ + LIMIT ?", + vec![ + USER.to_owned(), + OrderScene::Pinned.as_str().to_owned(), + "0".to_owned(), + "0".to_owned(), + "conversation".to_owned(), + "conversation".to_owned(), + "c2".to_owned(), + "10".to_owned(), + ], + ) + .await; + assert!( + keyset.contains("idx_user_order_scene"), + "keyset pinned read must use idx_user_order_scene, got plan: {keyset}" + ); + assert!( + !keyset.contains("SCAN user_order"), + "keyset pinned read must not full-scan user_order, got plan: {keyset}" + ); +} + +#[tokio::test] +async fn pinned_refs_returns_all_typed_refs() { + let (store, _db) = store().await; + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + store.pin(USER, OrderScene::Pinned, &team("t1")).await.unwrap(); + + let mut refs = store.pinned_refs(USER, OrderScene::Pinned).await.unwrap(); + refs.sort_by(|a, b| a.item_id.cmp(&b.item_id)); + assert_eq!(refs, vec![conv("c1"), team("t1")]); +} + +#[tokio::test] +async fn remove_item_deletes_single_ref() { + let (store, _db) = store().await; + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + store.pin(USER, OrderScene::Pinned, &conv("c2")).await.unwrap(); + + store.remove_item(USER, &conv("c1")).await.unwrap(); + let ids: Vec = store + .pinned_refs(USER, OrderScene::Pinned) + .await + .unwrap() + .into_iter() + .map(|r| r.item_id) + .collect(); + assert_eq!(ids, vec!["c2"]); + // Idempotent: removing a gone item is fine. + store.remove_item(USER, &conv("c1")).await.unwrap(); +} + +#[tokio::test] +async fn remove_items_batch_is_atomic() { + let (store, _db) = store().await; + for id in ["c1", "c2", "c3"] { + store.pin(USER, OrderScene::Pinned, &conv(id)).await.unwrap(); + } + store.pin(USER, OrderScene::Pinned, &team("t1")).await.unwrap(); + + store + .remove_items(USER, &[conv("c1"), conv("c3"), team("t1")]) + .await + .unwrap(); + let ids: Vec = store + .pinned_refs(USER, OrderScene::Pinned) + .await + .unwrap() + .into_iter() + .map(|r| r.item_id) + .collect(); + assert_eq!(ids, vec!["c2"]); + + // Empty batch is a no-op. + store.remove_items(USER, &[]).await.unwrap(); + assert_eq!(store.pinned_refs(USER, OrderScene::Pinned).await.unwrap().len(), 1); +} + +#[tokio::test] +async fn rows_are_scoped_per_user() { + let (store, _db) = store().await; + store.pin(USER, OrderScene::Pinned, &conv("c1")).await.unwrap(); + + // A different user sees nothing and cannot unpin another user's row. + assert!( + store + .list_pinned(OTHER_USER, OrderScene::Pinned, None, 10) + .await + .unwrap() + .is_empty() + ); + assert!(!store.unpin(OTHER_USER, OrderScene::Pinned, &conv("c1")).await.unwrap()); + assert_eq!( + store + .list_pinned(USER, OrderScene::Pinned, None, 10) + .await + .unwrap() + .len(), + 1 + ); +} + +#[tokio::test] +async fn concurrent_pins_serialize_into_distinct_rows() { + let (store, _db) = store().await; + // Two concurrent pins on the same empty scene. BEGIN IMMEDIATE serializes + // the read-min-then-insert, so both land as distinct rows (no lost write, + // no duplicate key) with different order_keys. + let a = { + let store = store.clone(); + tokio::spawn(async move { store.pin(USER, OrderScene::Pinned, &conv("c1")).await }) + }; + let b = { + let store = store.clone(); + tokio::spawn(async move { store.pin(USER, OrderScene::Pinned, &conv("c2")).await }) + }; + a.await.unwrap().unwrap(); + b.await.unwrap().unwrap(); + + let rows = store.list_pinned(USER, OrderScene::Pinned, None, 10).await.unwrap(); + assert_eq!(rows.len(), 2); + assert_ne!(rows[0].order_key, rows[1].order_key, "keys must not collide"); +} diff --git a/crates/aionui-db/src/repository/user_order.rs b/crates/aionui-db/src/repository/user_order.rs new file mode 100644 index 000000000..b80e42e6f --- /dev/null +++ b/crates/aionui-db/src/repository/user_order.rs @@ -0,0 +1,91 @@ +use async_trait::async_trait; + +use crate::error::DbError; +use crate::models::{OrderItemType, OrderScene, UserOrderRow}; + +/// A `(item_type, item_id)` reference into the `user_order` table, used by pin +/// writes and cascade deletes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OrderItemRef { + pub item_type: OrderItemType, + pub item_id: String, +} + +impl OrderItemRef { + pub fn new(item_type: OrderItemType, item_id: impl Into) -> Self { + Self { + item_type, + item_id: item_id.into(), + } + } +} + +/// Keyset cursor for paginating a scene's rows, ordered by +/// `(order_key, item_type, item_id)`. All three components are non-null, so +/// there is no NULL branch to handle. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PinnedCursor { + pub order_key: i64, + pub item_type: OrderItemType, + pub item_id: String, +} + +/// Result of a [`IUserOrderStore::pin`] call. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PinOutcome { + /// A new ordering row was inserted at the top of the scene. + Inserted, + /// The item was already pinned; the call was a no-op (order preserved). + AlreadyPinned, +} + +/// Access boundary for the `user_order` table. +/// +/// The store owns SQL and transactions; the `aionui-sidebar` service holds an +/// `Arc` and never opens transactions itself. Every method +/// takes the acting `user_id` and filters/writes the owner column, so a user +/// can neither see nor mutate another user's ordering. +/// +/// Pin state is row existence: [`pin`](Self::pin) inserts, [`unpin`](Self::unpin) +/// deletes. There is no boolean column. +#[async_trait] +pub trait IUserOrderStore: Send + Sync { + /// Pin `item` at the top of `scene`: insert a row with + /// `order_key = (scene min order_key) - 1000`, or `1000` when the scene is + /// empty. Idempotent — if the row already exists it is left unchanged and + /// [`PinOutcome::AlreadyPinned`] is returned. Uses `BEGIN IMMEDIATE` so the + /// read-min-then-insert is atomic under concurrent pins. + async fn pin(&self, user_id: &str, scene: OrderScene, item: &OrderItemRef) -> Result; + + /// Unpin `item` in `scene`: delete the row. Returns `true` if a row was + /// removed, `false` if it was not pinned (idempotent no-op). + async fn unpin(&self, user_id: &str, scene: OrderScene, item: &OrderItemRef) -> Result; + + /// One keyset page of a scene's rows, ordered by + /// `(order_key, item_type, item_id)` ascending. `after = None` starts at the + /// top; otherwise rows strictly after the cursor are returned. At most + /// `limit` rows. + async fn list_pinned( + &self, + user_id: &str, + scene: OrderScene, + after: Option<&PinnedCursor>, + limit: i64, + ) -> Result, DbError>; + + /// Every pinned reference in `scene` for `user_id` (unpaged). Used to derive + /// the DTO `pinned` flag and the anti-join exclusion set; the pinned set is + /// bounded by user behavior, so an unpaged read is acceptable. + async fn pinned_refs(&self, user_id: &str, scene: OrderScene) -> Result, DbError>; + + /// Delete every `user_order` row for a single item across all scenes. + /// Cascade for "the item left the sidebar" (conversation/team deletion). + /// Idempotent. + async fn remove_item(&self, user_id: &str, item: &OrderItemRef) -> Result<(), DbError>; + + /// Delete every `user_order` row for a batch of items across all scenes, in + /// one transaction (atomic all-or-nothing). Cascade for project removal, + /// where the removal set (including path-merged items) is computed by the + /// service. Idempotent; empty input is a no-op. + async fn remove_items(&self, user_id: &str, items: &[OrderItemRef]) -> Result<(), DbError>; +} diff --git a/crates/aionui-project/src/service.rs b/crates/aionui-project/src/service.rs index 792fb3e4a..5347b64d3 100644 --- a/crates/aionui-project/src/service.rs +++ b/crates/aionui-project/src/service.rs @@ -231,6 +231,20 @@ impl ProjectService { Ok(()) } + /// Delete a project and all its explorer entries (owner-scoped, one store + /// transaction). Idempotent: removing an absent/foreign project succeeds + /// silently. Folders are global and are never removed here. + /// + /// This drops only the bind-chain rows; the conversations/teams that pointed + /// at the project are removed by the caller (sidebar `remove_project` + /// orchestration, BR-19). A best-effort `repositoriesChanged` notify lets the + /// source-control actor recompute (now-empty) roots for the gone project. + pub async fn delete_project(&self, user_id: &str, project_id: &str) -> Result<(), ProjectError> { + self.store.delete_project(user_id, project_id).await?; + self.notify_roots_changed(project_id, user_id); + Ok(()) + } + pub async fn reorder( &self, user_id: &str, diff --git a/crates/aionui-sidebar/Cargo.toml b/crates/aionui-sidebar/Cargo.toml new file mode 100644 index 000000000..76870e7f6 --- /dev/null +++ b/crates/aionui-sidebar/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "aionui-sidebar" +version.workspace = true +edition.workspace = true + +[dependencies] +aionui-api-types.workspace = true +aionui-auth.workspace = true +aionui-common.workspace = true +aionui-db.workspace = true +# Read-model reuse: row_to_response_with_extra (hydration) + is_temp_session_workspace +# (temp-path classification, same predicate the write side uses). +aionui-conversation.workspace = true +# Lexical path canonicalization for the read-side path-merge match (no fs). +aionui-project.workspace = true +axum.workspace = true +base64.workspace = true +# Repeated `win` query params + percent-decoded cursors: parse the raw query with +# form_urlencoded (serde_urlencoded / axum Query collapses duplicate keys). +url.workspace = true +thiserror.workspace = true +serde.workspace = true +serde_json.workspace = true +tracing.workspace = true +tokio.workspace = true +async-trait.workspace = true + +[dev-dependencies] +tokio = { workspace = true, features = ["test-util"] } +tempfile.workspace = true +tower.workspace = true +http-body-util.workspace = true +# Service tests seed a real in-memory DB directly (init_database_memory + the +# Sqlite*Store impls), so the raw SQL seed helpers need sqlx. +sqlx.workspace = true diff --git a/crates/aionui-sidebar/src/cascade.rs b/crates/aionui-sidebar/src/cascade.rs new file mode 100644 index 000000000..b20267e75 --- /dev/null +++ b/crates/aionui-sidebar/src/cascade.rs @@ -0,0 +1,51 @@ +//! Cascade that keeps `user_order` in sync when a conversation leaves the +//! sidebar's visible domain (design §4.3, path 1). +//! +//! When a conversation is deleted — directly, or as a team member during +//! `remove_team` (which routes member deletion through +//! `ConversationService::delete`) — its ordering rows must go too, or an orphan +//! pinned row would resurface a ghost entry on the next read. This runs as an +//! `OnConversationDelete` hook, i.e. after the owning delete has already been +//! authorized, so it is best-effort: a failure here degrades to an orphan row +//! that the read side already drops during hydration (page_pinned skips ids that +//! no longer hydrate), never a failed user-facing delete. + +use std::sync::Arc; + +use aionui_common::OnConversationDelete; +use aionui_db::{IUserOrderStore, OrderItemRef, OrderItemType}; +use async_trait::async_trait; + +/// `OnConversationDelete` hook that drops a deleted conversation's `user_order` +/// rows across all scenes. Registered on `ConversationService` via +/// `with_delete_hook` in `aionui-app`. +pub struct UserOrderDeleteHook { + user_order: Arc, +} + +impl UserOrderDeleteHook { + pub fn new(user_order: Arc) -> Self { + Self { user_order } + } +} + +#[async_trait] +impl OnConversationDelete for UserOrderDeleteHook { + async fn on_conversation_deleted(&self, user_id: &str, conversation_id: &str) { + let item = OrderItemRef::new(OrderItemType::Conversation, conversation_id); + if let Err(err) = self.user_order.remove_item(user_id, &item).await { + // Best-effort: the read side drops orphan rows during hydration, so a + // failure here self-heals rather than breaking the delete. + tracing::warn!( + user_id = %user_id, + conversation_id = %conversation_id, + error = %err, + "sidebar: failed to cascade-delete user_order rows for deleted conversation" + ); + } + } +} + +#[cfg(test)] +#[path = "cascade_test.rs"] +mod cascade_test; diff --git a/crates/aionui-sidebar/src/cascade_test.rs b/crates/aionui-sidebar/src/cascade_test.rs new file mode 100644 index 000000000..1327e101d --- /dev/null +++ b/crates/aionui-sidebar/src/cascade_test.rs @@ -0,0 +1,63 @@ +//! Path-1 cascade (design §4.3): deleting a conversation drops its `user_order` +//! rows. The orphan assertion is on the table row count, not API output. + +use std::sync::Arc; + +use aionui_common::OnConversationDelete; +use aionui_db::{ + IUserOrderStore, OrderItemRef, OrderItemType, OrderScene, SqlitePool, SqliteUserOrderStore, init_database_memory, +}; + +use super::UserOrderDeleteHook; + +const USER: &str = "user-1"; +const OTHER: &str = "user-2"; + +async fn seed_user(pool: &SqlitePool, id: &str) { + sqlx::query("INSERT INTO users (id, username, password_hash, created_at, updated_at) VALUES (?, ?, 'x', 0, 0)") + .bind(id) + .bind(id) + .execute(pool) + .await + .unwrap(); +} + +async fn count_rows(pool: &SqlitePool, user: &str, item_id: &str) -> i64 { + sqlx::query_scalar("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = ?") + .bind(user) + .bind(item_id) + .fetch_one(pool) + .await + .unwrap() +} + +#[tokio::test] +async fn deleting_a_conversation_drops_its_pinned_row_and_is_user_scoped() { + let db = init_database_memory().await.unwrap(); + seed_user(db.pool(), USER).await; + seed_user(db.pool(), OTHER).await; + let store: Arc = Arc::new(SqliteUserOrderStore::new(db.pool().clone())); + + // Both users pin a conversation that happens to share the same id. + let item = OrderItemRef::new(OrderItemType::Conversation, "c1"); + store.pin(USER, OrderScene::Pinned, &item).await.unwrap(); + store.pin(OTHER, OrderScene::Pinned, &item).await.unwrap(); + assert_eq!(count_rows(db.pool(), USER, "c1").await, 1); + assert_eq!(count_rows(db.pool(), OTHER, "c1").await, 1); + + let hook = UserOrderDeleteHook::new(store.clone()); + hook.on_conversation_deleted(USER, "c1").await; + + // USER's row is gone; OTHER's identically-keyed row is untouched (BR-24). + assert_eq!( + count_rows(db.pool(), USER, "c1").await, + 0, + "path-1 cascade removed the pinned row" + ); + assert_eq!(count_rows(db.pool(), OTHER, "c1").await, 1, "cascade is user-scoped"); + + // Idempotent: deleting again (or a never-pinned id) is a silent no-op. + hook.on_conversation_deleted(USER, "c1").await; + hook.on_conversation_deleted(USER, "never-pinned").await; + assert_eq!(count_rows(db.pool(), USER, "c1").await, 0); +} diff --git a/crates/aionui-sidebar/src/lib.rs b/crates/aionui-sidebar/src/lib.rs new file mode 100644 index 000000000..6a475e1ae --- /dev/null +++ b/crates/aionui-sidebar/src/lib.rs @@ -0,0 +1,20 @@ +//! Sidebar read model: one request renders the whole left panel. +//! +//! The backend owns classification (pinned / project / pseudo-dir / chats), +//! windowing, ordering, and pin truth (a `user_order` row's existence). The +//! frontend renders in the given order and runs no classification. See +//! `feat-project-design/temp/left-panel/` (`design.md`, `api-contract-sidebar.md`, +//! `boundary-rules.md`). + +mod cascade; +mod ports; +mod service; +mod types; + +pub mod routes; + +pub use cascade::UserOrderDeleteHook; +pub use ports::RemoveProjectPorts; +pub use routes::{SidebarRouterState, sidebar_routes}; +pub use service::SidebarService; +pub use types::SidebarError; diff --git a/crates/aionui-sidebar/src/ports.rs b/crates/aionui-sidebar/src/ports.rs new file mode 100644 index 000000000..96a5acba0 --- /dev/null +++ b/crates/aionui-sidebar/src/ports.rs @@ -0,0 +1,33 @@ +//! Deletion ports for `remove_project` (BR-19 / D13 "所见即所删"). +//! +//! Removing a project deletes everything classified into its group: independent +//! conversations, whole teams, and the project record itself. Each of those is a +//! heavy, cross-crate orchestration that already lives in a domain service — +//! killing agent processes and cascading member conversations (team delete), +//! running the conversation delete hook (conversation delete), dropping the +//! bind-chain rows (project delete). The sidebar crate must not re-implement or +//! depend on those services directly, so this trait is the seam: `aionui-app` +//! injects an adapter over the concrete conversation / team / project services. +//! +//! Errors are opaque strings on purpose. The orchestration is best-effort per +//! entity (see `SidebarService::remove_project`): the sidebar only needs to know +//! "this one failed" for a warn log, not the error taxonomy of three foreign +//! crates. + +use async_trait::async_trait; + +/// The three deletion primitives `remove_project` drives, one per unit kind. +#[async_trait] +pub trait RemoveProjectPorts: Send + Sync { + /// Delete one independent (non-team-member) conversation. Its `user_order` + /// row is cascaded by the conversation delete hook. + async fn delete_conversation(&self, user_id: &str, conversation_id: &str) -> Result<(), String>; + + /// Remove a whole team: kill its agents, cascade its member conversations, + /// drop the team row and its own `user_order` row (the standalone + /// team-delete path, reused verbatim). + async fn remove_team(&self, user_id: &str, team_id: &str) -> Result<(), String>; + + /// Delete the project record and its explorer entries (owner-scoped). + async fn delete_project_record(&self, user_id: &str, project_id: &str) -> Result<(), String>; +} diff --git a/crates/aionui-sidebar/src/routes.rs b/crates/aionui-sidebar/src/routes.rs new file mode 100644 index 000000000..6e78a6e31 --- /dev/null +++ b/crates/aionui-sidebar/src/routes.rs @@ -0,0 +1,218 @@ +// `ApiError` is the intended error type at this HTTP boundary (routes map the +// crate-owned `SidebarError` to it here), so the disallowed_types lint that +// steers service code away from `ApiError` does not apply to this module. +#![allow(clippy::disallowed_types)] + +//! Sidebar read + ordering HTTP routes. +//! +//! - `GET /api/sidebar` — first screen (pinned → project area → chats), with a +//! default `limit` and repeated `win=:` per-scope overrides. +//! - `GET /api/sidebar/items` — one more window of a single group (`scope` + +//! keyset `cursor`). +//! - `PUT`/`DELETE /api/order/{scene}/{item_type}/{item_id}` — pin / unpin. +//! +//! Handlers only parse the request and shape the response; all classification +//! and ordering lives in [`SidebarService`]. [`SidebarError`] maps to `ApiError` +//! with stable machine codes (`sidebar_bad_request`, `sidebar_scope_gone`, +//! `sidebar_internal`); DB/project detail never leaks to the client. + +use std::sync::Arc; + +use aionui_api_types::{ApiResponse, RemoveProjectResult, SidebarItemsResponse, SidebarResponse}; +use aionui_auth::CurrentUser; +use aionui_common::ApiError; +use axum::extract::{Path, RawQuery, State}; +use axum::http::StatusCode; +use axum::routing::{delete, get, put}; +use axum::{Extension, Json, Router}; + +use crate::service::SidebarService; +use crate::types::{SidebarError, parse_win, validate_limit}; + +/// Shared state for sidebar route handlers. +#[derive(Clone)] +pub struct SidebarRouterState { + pub service: Arc, +} + +/// Build the sidebar router. All routes require authentication (applied by the +/// caller, matching the project/team route wiring in `aionui-app`). +pub fn sidebar_routes(state: SidebarRouterState) -> Router { + Router::new() + .route("/api/sidebar", get(get_sidebar)) + .route("/api/sidebar/items", get(get_items)) + .route( + "/api/order/{scene}/{item_type}/{item_id}", + put(put_order).delete(delete_order), + ) + .route("/api/sidebar/project/{project_id}", delete(delete_project)) + .with_state(state) +} + +/// `GET /api/sidebar?limit=5&win=project:P1:10&win=chats:20` — first screen. +async fn get_sidebar( + State(state): State, + Extension(user): Extension, + RawQuery(query): RawQuery, +) -> Result>, ApiError> { + let params = QueryParams::parse(query.as_deref()); + let limit = params.limit().map_err(to_api_error)?; + if let Some(limit) = limit { + validate_limit(limit).map_err(to_api_error)?; + } + let win = parse_win(¶ms.multi("win")).map_err(to_api_error)?; + + let response = state + .service + .first_screen(&user.id, limit, &win) + .await + .map_err(to_api_error)?; + Ok(Json(ApiResponse::ok(response))) +} + +/// `GET /api/sidebar/items?scope=&cursor=&limit=10` — page one group. +async fn get_items( + State(state): State, + Extension(user): Extension, + RawQuery(query): RawQuery, +) -> Result>, ApiError> { + let params = QueryParams::parse(query.as_deref()); + let scope = params + .single("scope") + .ok_or_else(|| to_api_error(SidebarError::BadRequest("missing scope".into())))?; + let cursor = params.single("cursor"); + let limit = params.limit().map_err(to_api_error)?; + + let response = state + .service + .items(&user.id, &scope, cursor.as_deref(), limit) + .await + .map_err(to_api_error)?; + Ok(Json(ApiResponse::ok(response))) +} + +/// `PUT /api/order/{scene}/{item_type}/{item_id}` — pin (idempotent). +async fn put_order( + State(state): State, + Extension(user): Extension, + Path((scene, item_type, item_id)): Path<(String, String, String)>, +) -> Result>, ApiError> { + state + .service + .pin(&user.id, &scene, &item_type, &item_id) + .await + .map_err(to_api_error)?; + Ok(Json(ApiResponse::ok(()))) +} + +/// `DELETE /api/order/{scene}/{item_type}/{item_id}` — unpin (idempotent). +async fn delete_order( + State(state): State, + Extension(user): Extension, + Path((scene, item_type, item_id)): Path<(String, String, String)>, +) -> Result>, ApiError> { + state + .service + .unpin(&user.id, &scene, &item_type, &item_id) + .await + .map_err(to_api_error)?; + Ok(Json(ApiResponse::ok(()))) +} + +/// `DELETE /api/sidebar/project/{project_id}?dry_run=true` — remove a project +/// and everything classified into its group (BR-19 "所见即所删"). +/// +/// With `dry_run=true` nothing is deleted; the response reports the counts that +/// *would* be removed so the frontend can render a confirmation. A missing or +/// non-standard `project_id` maps to `ScopeGone` → 404. +async fn delete_project( + State(state): State, + Extension(user): Extension, + Path(project_id): Path, + RawQuery(query): RawQuery, +) -> Result>, ApiError> { + let params = QueryParams::parse(query.as_deref()); + let dry_run = params.flag("dry_run"); + + let result = state + .service + .remove_project(&user.id, &project_id, dry_run) + .await + .map_err(to_api_error)?; + Ok(Json(ApiResponse::ok(result))) +} + +/// Parsed query string: percent-decoded `(key, value)` pairs. `axum::Query` +/// (serde_urlencoded) collapses duplicate keys, so `win` — which repeats — is +/// parsed from the raw query instead. +struct QueryParams { + pairs: Vec<(String, String)>, +} + +impl QueryParams { + fn parse(query: Option<&str>) -> Self { + let pairs = query + .map(|q| { + url::form_urlencoded::parse(q.as_bytes()) + .map(|(k, v)| (k.into_owned(), v.into_owned())) + .collect() + }) + .unwrap_or_default(); + Self { pairs } + } + + fn single(&self, key: &str) -> Option { + self.pairs.iter().find(|(k, _)| k == key).map(|(_, v)| v.clone()) + } + + fn multi(&self, key: &str) -> Vec { + self.pairs + .iter() + .filter(|(k, _)| k == key) + .map(|(_, v)| v.clone()) + .collect() + } + + /// A boolean query flag: present with no value / `true` / `1` → true; + /// absent or any other value → false. + fn flag(&self, key: &str) -> bool { + match self.single(key) { + None => false, + Some(v) => v.is_empty() || v == "true" || v == "1", + } + } + + /// `limit` as an `i64`; a present-but-non-numeric value is a 400. + fn limit(&self) -> Result, SidebarError> { + match self.single("limit") { + None => Ok(None), + Some(raw) => raw + .parse::() + .map(Some) + .map_err(|_| SidebarError::BadRequest(format!("limit is not a number: {raw}"))), + } + } +} + +/// Map the crate error to an `ApiError` with a stable code. Internal causes +/// (DB / project / unexpected) are logged and flattened so no detail leaks. +fn to_api_error(err: SidebarError) -> ApiError { + match err { + SidebarError::BadRequest(msg) => ApiError::coded(StatusCode::BAD_REQUEST, "sidebar_bad_request", msg, None), + SidebarError::ScopeGone => ApiError::coded( + StatusCode::NOT_FOUND, + "sidebar_scope_gone", + "scope no longer exists", + None, + ), + SidebarError::Db(_) | SidebarError::Project(_) | SidebarError::Internal(_) => { + tracing::error!(error = %err, "sidebar: internal error"); + ApiError::coded( + StatusCode::INTERNAL_SERVER_ERROR, + "sidebar_internal", + "internal error", + None, + ) + } + } +} diff --git a/crates/aionui-sidebar/src/service.rs b/crates/aionui-sidebar/src/service.rs new file mode 100644 index 000000000..9a0fc56c5 --- /dev/null +++ b/crates/aionui-sidebar/src/service.rs @@ -0,0 +1,1070 @@ +//! Sidebar classification and assembly engine. +//! +//! One request renders the whole left panel. The service loads a thin read +//! snapshot (conversations / teams / projects / pinned refs), classifies every +//! non-pinned unit into its group (5-case matrix, `api-contract-sidebar.md` §2), +//! windows each group, and hydrates only the windowed conversations in one batch +//! (BR-16, no N+1). It opens no transactions — the stores own SQL/txns — and has +//! zero side effects (BR-17/27): path merge and pseudo-dir grouping are +//! display-only, computed by lexical canonicalization with no filesystem access. +//! +//! Pin truth is a `user_order` row's existence: a unit is excluded from its +//! natural group when it appears in the pinned set (anti-join, BR-7), and shows +//! up only in the pinned group; the DTO `pinned` flag is overridden accordingly +//! (never read from the deprecated `conversations.pinned` column, BR — B1). + +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, OnceLock}; + +use aionui_api_types::{ + ConversationResponse, RemoveProjectItem, RemoveProjectItemKind, RemoveProjectResult, SidebarGroup, SidebarItem, + SidebarItemsResponse, SidebarResponse, SidebarScope, SidebarTeamItem, +}; +use aionui_conversation::{is_temp_session_workspace, row_to_response_with_extra}; +use aionui_db::models::ConversationRow; +use aionui_db::{ + ISidebarStore, IUserOrderStore, OrderItemRef, OrderItemType, OrderScene, PinnedCursor, SidebarConversationThin, + SidebarProjectMeta, SidebarTeamThin, +}; +use aionui_project::canonical; + +use crate::ports::RemoveProjectPorts; +use crate::types::{ + Cursor, DEFAULT_ITEMS_LIMIT, DEFAULT_LIMIT, ScopeToken, SidebarError, canonical_to_dir_key, validate_limit, +}; + +/// Project-area hard cap: the interleaved (project + pseudo-dir) group list is +/// truncated to this many groups, ordered by activity (BR-5). +const MAX_PROJECT_GROUPS: usize = 100; + +/// Standard-project kind marker (`projects.kind`). +const KIND_STANDARD: &str = "standard"; + +/// The sidebar read/assembly service. +pub struct SidebarService { + sidebar: Arc, + user_order: Arc, + /// Backend-managed data dir (conversation workspace root). Used both to + /// detect temp workspaces (classification) and as the hydration `data_dir`. + work_dir: PathBuf, + /// Deletion ports for `remove_project`, injected once after startup (the + /// team service they wrap is built after this service — see + /// `build_module_states`). Empty in the read/pin paths, which never touch it. + remove_project_ports: Arc>>, +} + +impl SidebarService { + pub fn new(sidebar: Arc, user_order: Arc, work_dir: PathBuf) -> Self { + Self { + sidebar, + user_order, + work_dir, + remove_project_ports: Arc::new(OnceLock::new()), + } + } + + /// Install the deletion ports `remove_project` drives. Called once, after the + /// conversation / team / project services exist; later calls are ignored + /// (set-once). Because the field is shared behind `Arc`, setting it on any + /// clone makes it visible everywhere. + pub fn set_remove_project_ports(&self, ports: Arc) { + let _ = self.remove_project_ports.set(ports); + } + + // -- Write side (pin / unpin) -------------------------------------------- + + /// Pin an item at the top of a scene. Idempotent. `scene` / `item_type` are + /// closed enums — an unknown value is a 400, never a silent no-op. + pub async fn pin(&self, user_id: &str, scene: &str, item_type: &str, item_id: &str) -> Result<(), SidebarError> { + let scene = parse_scene(scene)?; + let item = OrderItemRef::new(parse_item_type(item_type)?, item_id.to_owned()); + self.user_order.pin(user_id, scene, &item).await?; + Ok(()) + } + + /// Unpin an item. Idempotent (unpinning an unpinned item is a no-op). + pub async fn unpin(&self, user_id: &str, scene: &str, item_type: &str, item_id: &str) -> Result<(), SidebarError> { + let scene = parse_scene(scene)?; + let item = OrderItemRef::new(parse_item_type(item_type)?, item_id.to_owned()); + self.user_order.unpin(user_id, scene, &item).await?; + Ok(()) + } + + // -- Remove project (BR-19 / D13 "所见即所删") --------------------------- + + /// Remove a standard project: delete every unit that renders into its group, + /// then the project record itself. When `dry_run` is set, nothing is deleted + /// and the returned counts are the preview (what *would* be removed). + /// + /// # Delete set + /// The set is computed with the **same** classifier the renderer uses + /// ([`classify_unit`](Self::classify_unit)) over the active universe: a unit + /// is deleted iff it classifies into `Project(project_id)`. This is why + /// "所见即所删" holds — the delete set *is* the render construct, so it also + /// catches path-merged unbound items (a conversation whose workspace + /// canonicalizes onto this project's root, case 3) that carry no `project_id`. + /// Pinned units are included: pinning only hoists a row into the pinned group + /// for display, it does not change which project the row belongs to. + /// + /// Team-member conversations are not enumerated here — they are folded into + /// their team and removed by the team cascade, "无论成员自身 project_id 为何"; + /// a member whose *team* lands in another project is therefore not touched. + /// + /// # Atomicity + /// Deletion is **best-effort per entity**, mirroring the standalone team + /// delete: killing agent processes, dropping filesystem dirs, and running + /// cross-service delete hooks cannot share one DB transaction, so D13's + /// "单事务原子 + 中途失败全回滚" is not literally achievable. A failed unit is + /// logged and skipped; the project record is dropped last, so a mid-way + /// failure leaves a smaller (self-consistent) project rather than orphaning + /// its contents. Only the archived-inclusion refinement is deferred to PR-B; + /// PR-A has no archiving, so the active universe is the whole universe. + pub async fn remove_project( + &self, + user_id: &str, + project_id: &str, + dry_run: bool, + ) -> Result { + let convs = self.sidebar.list_active_conversations_thin(user_id).await?; + let teams = self.sidebar.list_active_teams_thin(user_id).await?; + let projects = self.sidebar.list_user_projects(user_id).await?; + + // Same project maps `classify` builds: id→meta and standard-canonical→id. + let by_id: HashMap<&str, &SidebarProjectMeta> = projects.iter().map(|p| (p.project_id.as_str(), p)).collect(); + let std_canon: HashMap<&str, &str> = projects + .iter() + .filter(|p| p.kind == KIND_STANDARD) + .filter_map(|p| p.workspace_canonical.as_deref().map(|c| (c, p.project_id.as_str()))) + .collect(); + + // The target must be an owned standard project. Temp projects and + // pseudo-dir groups are not removable through this path → 404. + match by_id.get(project_id) { + Some(meta) if meta.kind == KIND_STANDARD => {} + _ => return Err(SidebarError::ScopeGone), + } + + let (team_by_id, independents) = aggregate_teams(convs, teams.clone()); + let target = GroupKey::Project(project_id.to_owned()); + + let mut team_ids: Vec = Vec::new(); + for team in &teams { + // Skip teams that dropped out of the aggregate (none currently, but + // keeps parity with `classify`'s guard). + if !team_by_id.contains_key(&team.id) { + continue; + } + let key = self.classify_unit( + team.project_id.as_deref(), + team.workspace.as_deref(), + &by_id, + &std_canon, + ); + if key == target { + team_ids.push(team.id.clone()); + } + } + + let mut conv_ids: Vec = Vec::new(); + for conv in &independents { + let key = self.classify_unit( + conv.project_id.as_deref(), + conv.workspace.as_deref(), + &by_id, + &std_canon, + ); + if key == target { + conv_ids.push(conv.id.clone()); + } + } + + if dry_run { + // Name the delete set so the confirm dialog can list *which* items go. + // Pinned members were hoisted into the top pinned group (B1 anti-join), + // so the frontend can't reconstruct project membership — the names and + // pinned flags must come from here. + let pinned_refs = self.user_order.pinned_refs(user_id, OrderScene::Pinned).await?; + let pinned_set: HashSet<(String, String)> = pinned_refs + .iter() + .map(|r| (r.item_type.as_str().to_owned(), r.item_id.clone())) + .collect(); + + let team_name: HashMap<&str, &str> = teams.iter().map(|t| (t.id.as_str(), t.name.as_str())).collect(); + let conv_resp = self.hydrate(user_id, &conv_ids).await?; + + let mut items: Vec = Vec::with_capacity(team_ids.len() + conv_ids.len()); + for team_id in &team_ids { + items.push(RemoveProjectItem { + name: team_name.get(team_id.as_str()).copied().unwrap_or_default().to_owned(), + pinned: pinned_set.contains(&(OrderItemType::Team.as_str().to_owned(), team_id.clone())), + kind: RemoveProjectItemKind::Team, + }); + } + for conv_id in &conv_ids { + items.push(RemoveProjectItem { + name: conv_resp.get(conv_id).map(|c| c.name.clone()).unwrap_or_default(), + pinned: pinned_set.contains(&(OrderItemType::Conversation.as_str().to_owned(), conv_id.clone())), + kind: RemoveProjectItemKind::Conversation, + }); + } + + return Ok(RemoveProjectResult { + teams_deleted: team_ids.len() as i64, + conversations_deleted: conv_ids.len() as i64, + items, + }); + } + + let ports = self + .remove_project_ports + .get() + .ok_or_else(|| SidebarError::Internal("remove_project ports not wired".into()))?; + + let mut teams_deleted = 0i64; + for team_id in &team_ids { + match ports.remove_team(user_id, team_id).await { + Ok(()) => teams_deleted += 1, + Err(err) => { + tracing::warn!(team_id = %team_id, error = %err, "remove_project: team delete failed") + } + } + } + + let mut conversations_deleted = 0i64; + for conv_id in &conv_ids { + match ports.delete_conversation(user_id, conv_id).await { + Ok(()) => conversations_deleted += 1, + Err(err) => { + tracing::warn!(conversation_id = %conv_id, error = %err, "remove_project: conversation delete failed") + } + } + } + + // Project record last: its contents are already gone, so a failure here + // leaves only an empty shell (which a retry finishes off). + if let Err(err) = ports.delete_project_record(user_id, project_id).await { + tracing::warn!(project_id = %project_id, error = %err, "remove_project: project record delete failed"); + } + + tracing::info!( + project_id = %project_id, + teams_deleted, + conversations_deleted, + "Project removed (best-effort)" + ); + + Ok(RemoveProjectResult { + teams_deleted, + conversations_deleted, + items: Vec::new(), + }) + } + + // -- Read side (first screen / paging) ----------------------------------- + + /// First screen: `pinned → project-area (project + dir interleaved) → chats`. + /// `win` is the already-parsed per-scope window overrides; `limit` is the + /// default window for any scope not named in `win`. + pub async fn first_screen( + &self, + user_id: &str, + limit: Option, + win: &[(String, i64)], + ) -> Result { + let default_limit = limit.unwrap_or(DEFAULT_LIMIT); + let win_map: HashMap<&str, i64> = win.iter().map(|(t, l)| (t.as_str(), *l)).collect(); + let snapshot = self.classify(user_id).await?; + + let mut groups: Vec = Vec::new(); + + // Pinned group (only rendered when non-empty). + let pinned_limit = win_map.get("pinned").copied().unwrap_or(default_limit); + let pinned = self + .page_pinned(user_id, None, pinned_limit, &snapshot.team_by_id) + .await?; + if !pinned.items.is_empty() { + groups.push(SidebarGroup { + scope: SidebarScope::Pinned, + items: pinned.items, + has_more: pinned.has_more, + next_cursor: pinned.next_cursor, + }); + } + + // Natural groups: project area (already ordered) then chats. Collect the + // windowed conversation ids across all of them for a single hydration. + let mut natural: Vec<&NaturalGroup> = snapshot.project_area.iter().collect(); + natural.push(&snapshot.chats); + + let mut conv_ids: Vec = Vec::new(); + let mut windows: Vec<(&NaturalGroup, usize, bool)> = Vec::with_capacity(natural.len()); + for group in natural { + let lim = win_map.get(group.token_str.as_str()).copied().unwrap_or(default_limit) as usize; + let has_more = group.items.len() > lim; + let take = lim.min(group.items.len()); + for item in &group.items[..take] { + if let GroupItemRef::Conv { id, .. } = item { + conv_ids.push(id.clone()); + } + } + windows.push((group, take, has_more)); + } + + let hydrated = self.hydrate(user_id, &conv_ids).await?; + + for (group, take, has_more) in windows { + let items = assemble_items(&group.items[..take], &hydrated, &snapshot.team_by_id, false); + let next_cursor = has_more + .then(|| { + group + .items + .get(take - 1) + .map(|i| i.activity_cursor().encode(&group.token)) + }) + .flatten(); + groups.push(SidebarGroup { + scope: group.scope.clone(), + items, + has_more, + next_cursor, + }); + } + + Ok(SidebarResponse { + groups, + has_more_groups: snapshot.has_more_groups, + }) + } + + /// Page one more window of a single group. + pub async fn items( + &self, + user_id: &str, + scope: &str, + cursor: Option<&str>, + limit: Option, + ) -> Result { + let token = + ScopeToken::parse(scope).ok_or_else(|| SidebarError::BadRequest(format!("unknown scope: {scope}")))?; + let limit = limit.unwrap_or(DEFAULT_ITEMS_LIMIT); + validate_limit(limit)?; + + if let ScopeToken::Pinned = token { + let after = match cursor { + Some(raw) => Some(to_pinned_cursor(Cursor::decode(raw, &token)?)?), + None => None, + }; + let team_by_id = self.load_team_aggregates(user_id).await?; + let page = self.page_pinned(user_id, after.as_ref(), limit, &team_by_id).await?; + return Ok(SidebarItemsResponse { + items: page.items, + has_more: page.has_more, + next_cursor: page.next_cursor, + }); + } + + // Natural scope: classify, find the named group (stale → 404), page it. + let snapshot = self.classify(user_id).await?; + let group = match &token { + ScopeToken::Chats => &snapshot.chats, + _ => snapshot + .project_area + .iter() + .find(|g| g.token == token) + .ok_or(SidebarError::ScopeGone)?, + }; + let cursor = match cursor { + Some(raw) => Some(Cursor::decode(raw, &token)?), + None => None, + }; + self.page_natural(user_id, group, cursor.as_ref(), limit).await + } + + /// Window a natural group after `cursor`, hydrate, and assemble. + async fn page_natural( + &self, + user_id: &str, + group: &NaturalGroup, + cursor: Option<&Cursor>, + limit: i64, + ) -> Result { + // Items are sorted later-first, so the "after cursor" set is a suffix. + let after: Vec<&GroupItemRef> = group.items.iter().filter(|i| i.is_after(cursor)).collect(); + let take = (limit as usize).min(after.len()); + let has_more = after.len() > limit as usize; + + let conv_ids: Vec = after[..take] + .iter() + .filter_map(|i| match i { + GroupItemRef::Conv { id, .. } => Some(id.clone()), + GroupItemRef::Team { .. } => None, + }) + .collect(); + let hydrated = self.hydrate(user_id, &conv_ids).await?; + + let window: Vec = after[..take].iter().map(|i| (*i).clone()).collect(); + let items = assemble_items(&window, &hydrated, &group.team_by_id_ref(), false); + let next_cursor = has_more + .then(|| window.last().map(|i| i.activity_cursor().encode(&group.token))) + .flatten(); + Ok(SidebarItemsResponse { + items, + has_more, + next_cursor, + }) + } + + /// Window the pinned scene after `cursor` (order_key ascending) and assemble. + async fn page_pinned( + &self, + user_id: &str, + after: Option<&PinnedCursor>, + limit: i64, + team_by_id: &HashMap, + ) -> Result { + let rows = self + .user_order + .list_pinned(user_id, OrderScene::Pinned, after, limit + 1) + .await?; + let has_more = rows.len() as i64 > limit; + let window = &rows[..(limit as usize).min(rows.len())]; + + // Path-4 read-side defense (design §4.3): a live team member never has an + // independent row — it is folded into its team. The only teamId writer + // (`build_team_extra`) creates fresh conversations, so a member cannot + // carry a pre-existing pinned row through normal flows; this guard closes + // the residual dirty-data / future-write hole and self-heals, so no + // write-side chokepoint is needed. + let member_ids: HashSet<&str> = team_by_id + .values() + .flat_map(|t| t.member_ids.iter()) + .map(String::as_str) + .collect(); + + let conv_ids: Vec = window + .iter() + .filter(|r| r.item_type == OrderItemType::Conversation.as_str() && !member_ids.contains(r.item_id.as_str())) + .map(|r| r.item_id.clone()) + .collect(); + let hydrated = self.hydrate(user_id, &conv_ids).await?; + + let mut items: Vec = Vec::with_capacity(window.len()); + for row in window { + if row.item_type == OrderItemType::Conversation.as_str() { + if let Some(resp) = hydrated.get(&row.item_id) { + items.push(conversation_item(resp.clone(), true)); + } + } else if let Some(agg) = team_by_id.get(&row.item_id) { + items.push(team_item(agg, true)); + } + } + + let next_cursor = if has_more { + window.last().and_then(|row| { + OrderItemType::parse(&row.item_type).map(|t| { + Cursor::Pinned { + order_key: row.order_key, + item_type: t.as_str().to_owned(), + item_id: row.item_id.clone(), + } + .encode(&ScopeToken::Pinned) + }) + }) + } else { + None + }; + + Ok(PinnedPage { + items, + has_more, + next_cursor, + }) + } + + /// One batched hydration of windowed conversations into response DTOs. A row + /// with unparsable `extra` is skipped (warn) rather than failing the request. + async fn hydrate( + &self, + user_id: &str, + ids: &[String], + ) -> Result, SidebarError> { + if ids.is_empty() { + return Ok(HashMap::new()); + } + let rows = self.sidebar.hydrate_conversations(user_id, ids).await?; + let mut out = HashMap::with_capacity(rows.len()); + for row in rows { + let id = row.id.clone(); + match parse_row(row, &self.work_dir) { + Ok(resp) => { + out.insert(id, resp); + } + Err(err) => { + tracing::warn!(conversation_id = %id, error = %err, "sidebar: skipping unhydratable conversation") + } + } + } + Ok(out) + } + + // -- Classification ------------------------------------------------------ + + /// Build team aggregates only (used by the pinned items path, which needs no + /// group classification). Members are folded into their team; orphan members + /// downgrade to independents (BR-8) but independents are discarded here. + async fn load_team_aggregates(&self, user_id: &str) -> Result, SidebarError> { + let convs = self.sidebar.list_active_conversations_thin(user_id).await?; + let teams = self.sidebar.list_active_teams_thin(user_id).await?; + Ok(aggregate_teams(convs, teams).0) + } + + /// Full classification snapshot: team aggregates, independents + teams sorted + /// into groups (5-case matrix), the ordered/truncated project area, and the + /// chats group. + async fn classify(&self, user_id: &str) -> Result { + let convs = self.sidebar.list_active_conversations_thin(user_id).await?; + let teams = self.sidebar.list_active_teams_thin(user_id).await?; + let projects = self.sidebar.list_user_projects(user_id).await?; + let pinned_refs = self.user_order.pinned_refs(user_id, OrderScene::Pinned).await?; + let pinned_set: HashSet<(String, String)> = pinned_refs + .iter() + .map(|r| (r.item_type.as_str().to_owned(), r.item_id.clone())) + .collect(); + + // Project maps: id→meta and standard-canonical→id (path merge, BR case 3). + let by_id: HashMap<&str, &SidebarProjectMeta> = projects.iter().map(|p| (p.project_id.as_str(), p)).collect(); + let std_canon: HashMap<&str, &str> = projects + .iter() + .filter(|p| p.kind == KIND_STANDARD) + .filter_map(|p| p.workspace_canonical.as_deref().map(|c| (c, p.project_id.as_str()))) + .collect(); + + let (team_by_id, independents) = aggregate_teams(convs, teams.clone()); + + let mut builder = GroupBuilder::new(); + + // Teams as units. + for team in &teams { + let agg = match team_by_id.get(&team.id) { + Some(a) => a, + None => continue, + }; + let key = self.classify_unit( + team.project_id.as_deref(), + team.workspace.as_deref(), + &by_id, + &std_canon, + ); + let pinned = pinned_set.contains(&(OrderItemType::Team.as_str().to_owned(), team.id.clone())); + builder.push( + key, + &by_id, + agg.updated_at, + pinned, + GroupItemRef::Team { + team_id: team.id.clone(), + updated_at: agg.updated_at, + }, + ); + } + + // Independent conversations as units. + for conv in &independents { + let key = self.classify_unit( + conv.project_id.as_deref(), + conv.workspace.as_deref(), + &by_id, + &std_canon, + ); + let pinned = pinned_set.contains(&(OrderItemType::Conversation.as_str().to_owned(), conv.id.clone())); + builder.push( + key, + &by_id, + conv.updated_at, + pinned, + GroupItemRef::Conv { + id: conv.id.clone(), + updated_at: conv.updated_at, + }, + ); + } + + // Spine: every standard project surfaces even with zero units (BR-5). + for project in projects.iter().filter(|p| p.kind == KIND_STANDARD) { + builder.ensure_standard_spine(project); + } + + let (project_area, chats, has_more_groups) = builder.finish(team_by_id.clone()); + Ok(Snapshot { + project_area, + chats, + has_more_groups, + team_by_id, + }) + } + + /// Classify one unit (`project_id`, `workspace`) into its group key. Pure and + /// filesystem-free: dangling project ids fall through to the path branch, and + /// any path we cannot lexically canonicalize degrades to chats. + fn classify_unit( + &self, + project_id: Option<&str>, + workspace: Option<&str>, + by_id: &HashMap<&str, &SidebarProjectMeta>, + std_canon: &HashMap<&str, &str>, + ) -> GroupKey { + if let Some(pid) = project_id + && let Some(meta) = by_id.get(pid) + { + return if meta.kind == KIND_STANDARD { + GroupKey::Project(pid.to_owned()) + } else { + GroupKey::Chats + }; + } + // No project_id, or a dangling id (foreign / deleted project): fall + // through to path-based classification. + let ws = match workspace { + Some(ws) if !ws.is_empty() => ws, + _ => return GroupKey::Chats, + }; + let ws_path = Path::new(ws); + if is_temp_session_workspace(&self.work_dir, ws_path) { + return GroupKey::Chats; + } + let uri = match canonical::to_file_uri(ws_path) { + Ok(u) => u, + Err(_) => return GroupKey::Chats, + }; + let canon = match canonical::canonicalize(&uri) { + Ok(c) => c, + Err(_) => return GroupKey::Chats, + }; + match std_canon.get(canon.as_str()) { + Some(pid) => GroupKey::Project((*pid).to_owned()), + None => GroupKey::Dir(canon.as_str().to_owned()), + } + } +} + +// -- Snapshot / group model -------------------------------------------------- + +struct Snapshot { + project_area: Vec, + chats: NaturalGroup, + has_more_groups: bool, + team_by_id: HashMap, +} + +/// A team collapsed into one sidebar row. +#[derive(Clone)] +struct TeamAgg { + team_id: String, + name: String, + /// `MAX(updated_at)` over active members, else the team's own `updated_at`. + updated_at: i64, + /// Active member conversation ids, `created_at` ascending. + member_ids: Vec, +} + +/// A finished natural group: ordered non-pinned item refs plus the DTO scope. +struct NaturalGroup { + scope: SidebarScope, + token: ScopeToken, + token_str: String, + /// Sorted later-first (updated_at DESC, then item-type ASC, then id ASC). + items: Vec, + /// Shared team aggregates (set at finish); assembly reads names/members here. + team_by_id: Arc>, +} + +impl NaturalGroup { + fn team_by_id_ref(&self) -> Arc> { + self.team_by_id.clone() + } +} + +#[derive(Clone)] +enum GroupItemRef { + Conv { id: String, updated_at: i64 }, + Team { team_id: String, updated_at: i64 }, +} + +impl GroupItemRef { + fn updated_at(&self) -> i64 { + match self { + GroupItemRef::Conv { updated_at, .. } | GroupItemRef::Team { updated_at, .. } => *updated_at, + } + } + + fn type_ord(&self) -> u8 { + match self { + GroupItemRef::Conv { .. } => 0, + GroupItemRef::Team { .. } => 1, + } + } + + fn item_id(&self) -> &str { + match self { + GroupItemRef::Conv { id, .. } => id, + GroupItemRef::Team { team_id, .. } => team_id, + } + } + + fn item_type_str(&self) -> &'static str { + match self { + GroupItemRef::Conv { .. } => OrderItemType::Conversation.as_str(), + GroupItemRef::Team { .. } => OrderItemType::Team.as_str(), + } + } + + fn activity_cursor(&self) -> Cursor { + Cursor::Activity { + updated_at: self.updated_at(), + item_type: self.item_type_str().to_owned(), + item_id: self.item_id().to_owned(), + } + } + + /// Is this item strictly after `cursor` in later-first order? `None` cursor + /// means "from the top" (everything qualifies). + /// + /// Later-first: a comes before b iff `a.updated_at > b.updated_at`, or on a + /// tie iff `(a.type_ord, a.id) < (b.type_ord, b.id)`. So "after the cursor" + /// is: smaller `updated_at`, or equal `updated_at` with a larger + /// `(type_ord, id)` pair. + fn is_after(&self, cursor: Option<&Cursor>) -> bool { + let Some(Cursor::Activity { + updated_at, + item_type, + item_id, + }) = cursor + else { + return true; + }; + let cur = (type_ord_of(item_type), item_id.as_str()); + let this = (self.type_ord(), self.item_id()); + self.updated_at() < *updated_at || (self.updated_at() == *updated_at && this > cur) + } +} + +/// Group key used only during classification (before scope DTOs are built). +#[derive(Clone, PartialEq, Eq, Hash)] +enum GroupKey { + Project(String), + Dir(String), + Chats, +} + +/// Accumulates units into groups, tracking activity for ordering. +struct GroupBuilder { + map: HashMap, +} + +struct GroupAccum { + items: Vec, + /// MAX(updated_at) over ALL units incl pinned (group order, BR-6). + latest_activity: Option, + /// created_at fallback for empty standard-project groups. + created_at: i64, + /// Cached DTO scope + tokens (filled when the group is first created). + scope: SidebarScope, + token: ScopeToken, + token_str: String, +} + +impl GroupBuilder { + fn new() -> Self { + Self { map: HashMap::new() } + } + + fn push( + &mut self, + key: GroupKey, + by_id: &HashMap<&str, &SidebarProjectMeta>, + updated_at: i64, + pinned: bool, + item: GroupItemRef, + ) { + let accum = self + .map + .entry(key.clone()) + .or_insert_with(|| GroupAccum::new(&key, by_id)); + accum.latest_activity = Some(accum.latest_activity.map_or(updated_at, |a| a.max(updated_at))); + if !pinned { + accum.items.push(item); + } + } + + fn ensure_standard_spine(&mut self, project: &SidebarProjectMeta) { + let key = GroupKey::Project(project.project_id.clone()); + self.map.entry(key).or_insert_with(|| GroupAccum { + items: Vec::new(), + latest_activity: None, + created_at: project.created_at, + scope: project_scope(project), + token: ScopeToken::Project(project.project_id.clone()), + token_str: format!("project:{}", project.project_id), + }); + } + + /// Split into ordered project area (truncated to MAX_PROJECT_GROUPS) + chats. + fn finish(self, team_by_id: HashMap) -> (Vec, NaturalGroup, bool) { + let shared = Arc::new(team_by_id); + let mut chats: Option = None; + let mut area: Vec = Vec::new(); + for (key, accum) in self.map { + match key { + GroupKey::Chats => chats = Some(accum), + _ => area.push(accum), + } + } + + area.sort_by(cmp_group); + let has_more_groups = area.len() > MAX_PROJECT_GROUPS; + area.truncate(MAX_PROJECT_GROUPS); + + let project_area: Vec = area.into_iter().map(|a| a.into_group(shared.clone())).collect(); + let chats = chats + .unwrap_or_else(|| GroupAccum { + items: Vec::new(), + latest_activity: None, + created_at: 0, + scope: SidebarScope::Chats, + token: ScopeToken::Chats, + token_str: "chats".to_owned(), + }) + .into_group(shared); + (project_area, chats, has_more_groups) + } +} + +impl GroupAccum { + fn new(key: &GroupKey, by_id: &HashMap<&str, &SidebarProjectMeta>) -> Self { + match key { + GroupKey::Project(pid) => { + // Invariant: a Project key only arises from a resolved standard + // project (direct binding or path merge), so meta is present. + let meta = by_id.get(pid.as_str()).expect("project key without meta"); + GroupAccum { + items: Vec::new(), + latest_activity: None, + created_at: meta.created_at, + scope: project_scope(meta), + token: ScopeToken::Project(pid.clone()), + token_str: format!("project:{pid}"), + } + } + GroupKey::Dir(canonical) => { + let (path, name) = dir_display(canonical); + GroupAccum { + items: Vec::new(), + latest_activity: None, + created_at: 0, + scope: SidebarScope::Dir { + key: canonical_to_dir_key(canonical), + path, + name, + }, + token: ScopeToken::Dir(canonical.clone()), + token_str: format!("dir:{}", canonical_to_dir_key(canonical)), + } + } + GroupKey::Chats => GroupAccum { + items: Vec::new(), + latest_activity: None, + created_at: 0, + scope: SidebarScope::Chats, + token: ScopeToken::Chats, + token_str: "chats".to_owned(), + }, + } + } + + fn into_group(mut self, team_by_id: Arc>) -> NaturalGroup { + self.items.sort_by(cmp_item); + NaturalGroup { + scope: self.scope, + token: self.token, + token_str: self.token_str, + items: self.items, + team_by_id, + } + } +} + +// -- Free helpers ------------------------------------------------------------ + +/// Fold conversations into their live team; return (team aggregates, independents). +/// A member whose `team_id` names no live team is an orphan and downgrades to an +/// independent row (BR-8). +fn aggregate_teams( + convs: Vec, + teams: Vec, +) -> (HashMap, Vec) { + let live: HashSet<&str> = teams.iter().map(|t| t.id.as_str()).collect(); + let mut members: HashMap> = HashMap::new(); + let mut independents: Vec = Vec::new(); + for conv in convs { + match &conv.team_id { + Some(tid) if live.contains(tid.as_str()) => members.entry(tid.clone()).or_default().push(conv), + _ => independents.push(conv), + } + } + + let mut team_by_id = HashMap::with_capacity(teams.len()); + for team in teams { + let mut mem = members.remove(&team.id).unwrap_or_default(); + mem.sort_by(|a, b| a.created_at.cmp(&b.created_at).then_with(|| a.id.cmp(&b.id))); + let updated_at = mem.iter().map(|c| c.updated_at).max().unwrap_or(team.updated_at); + let member_ids = mem.into_iter().map(|c| c.id).collect(); + team_by_id.insert( + team.id.clone(), + TeamAgg { + team_id: team.id, + name: team.name, + updated_at, + member_ids, + }, + ); + } + (team_by_id, independents) +} + +/// Group order (BR-6): active groups by latest_activity DESC; empty standard +/// projects sink below and order by created_at DESC; ties break by token DESC. +fn cmp_group(a: &GroupAccum, b: &GroupAccum) -> std::cmp::Ordering { + use std::cmp::Ordering; + match (a.latest_activity, b.latest_activity) { + (Some(x), Some(y)) => y.cmp(&x).then_with(|| b.token_str.cmp(&a.token_str)), + (Some(_), None) => Ordering::Less, + (None, Some(_)) => Ordering::Greater, + (None, None) => b + .created_at + .cmp(&a.created_at) + .then_with(|| b.token_str.cmp(&a.token_str)), + } +} + +/// Item order within a group: later-first (updated_at DESC), tie-break +/// (item_type ASC, id ASC) — matching the keyset cursor's "strictly after". +fn cmp_item(a: &GroupItemRef, b: &GroupItemRef) -> std::cmp::Ordering { + b.updated_at() + .cmp(&a.updated_at()) + .then_with(|| a.type_ord().cmp(&b.type_ord())) + .then_with(|| a.item_id().cmp(b.item_id())) +} + +fn type_ord_of(item_type: &str) -> u8 { + match item_type { + t if t == OrderItemType::Conversation.as_str() => 0, + t if t == OrderItemType::Team.as_str() => 1, + _ => u8::MAX, + } +} + +fn project_scope(meta: &SidebarProjectMeta) -> SidebarScope { + SidebarScope::Project { + project_id: meta.project_id.clone(), + name: meta.name.clone(), + workspace: meta.workspace_uri.clone(), + } +} + +/// Human display path + last-segment name for a pseudo-dir canonical URI. +fn dir_display(canonical: &str) -> (String, String) { + match canonical::uri_to_path(canonical) { + Ok(path) => { + let name = path + .file_name() + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_else(|| path.display().to_string()); + (path.display().to_string(), name) + } + Err(_) => (canonical.to_owned(), canonical.to_owned()), + } +} + +fn assemble_items( + window: &[GroupItemRef], + hydrated: &HashMap, + team_by_id: &HashMap, + pinned: bool, +) -> Vec { + let mut out = Vec::with_capacity(window.len()); + for item in window { + match item { + GroupItemRef::Conv { id, .. } => { + if let Some(resp) = hydrated.get(id) { + out.push(conversation_item(resp.clone(), pinned)); + } + } + GroupItemRef::Team { team_id, .. } => { + if let Some(agg) = team_by_id.get(team_id) { + out.push(team_item(agg, pinned)); + } + } + } + } + out +} + +fn conversation_item(mut resp: ConversationResponse, pinned: bool) -> SidebarItem { + resp.pinned = pinned; + resp.pinned_at = None; + SidebarItem::Conversation { conversation: resp } +} + +fn team_item(agg: &TeamAgg, pinned: bool) -> SidebarItem { + SidebarItem::Team(SidebarTeamItem { + team_id: agg.team_id.clone(), + name: agg.name.clone(), + updated_at: agg.updated_at, + pinned, + member_conversation_ids: agg.member_ids.clone(), + }) +} + +fn parse_row(row: ConversationRow, work_dir: &Path) -> Result { + let extra: serde_json::Value = + serde_json::from_str(&row.extra).map_err(|e| SidebarError::Internal(format!("invalid extra JSON: {e}")))?; + row_to_response_with_extra(row, extra, work_dir).map_err(|e| SidebarError::Internal(e.to_string())) +} + +fn parse_scene(scene: &str) -> Result { + OrderScene::parse(scene).ok_or_else(|| SidebarError::BadRequest(format!("unknown scene: {scene}"))) +} + +fn parse_item_type(item_type: &str) -> Result { + OrderItemType::parse(item_type).ok_or_else(|| SidebarError::BadRequest(format!("unknown item type: {item_type}"))) +} + +fn to_pinned_cursor(cursor: Cursor) -> Result { + match cursor { + Cursor::Pinned { + order_key, + item_type, + item_id, + } => { + let item_type = OrderItemType::parse(&item_type) + .ok_or_else(|| SidebarError::BadRequest(format!("unknown cursor item type: {item_type}")))?; + Ok(PinnedCursor { + order_key, + item_type, + item_id, + }) + } + Cursor::Activity { .. } => Err(SidebarError::BadRequest("expected a pinned cursor".into())), + } +} + +struct PinnedPage { + items: Vec, + has_more: bool, + next_cursor: Option, +} + +#[cfg(test)] +#[path = "service_test.rs"] +mod service_test; diff --git a/crates/aionui-sidebar/src/service_test.rs b/crates/aionui-sidebar/src/service_test.rs new file mode 100644 index 000000000..8cf200a29 --- /dev/null +++ b/crates/aionui-sidebar/src/service_test.rs @@ -0,0 +1,845 @@ +//! Service-level tests over a real in-memory DB (no mocks): the D11 5-case +//! classification matrix, team folding + orphan downgrade (BR-8), display-only +//! path merge (case 3, asserts no DB write), the pinned anti-join / B1 +//! double-render regression, cross-user scoping (BR-24), dangling project ids, +//! keyset paging continuity, and the ScopeGone 404. + +use std::collections::HashSet; +use std::path::Path; +use std::sync::{Arc, Mutex}; + +use aionui_api_types::{SidebarItem, SidebarScope}; +use aionui_db::{ + Database, ISidebarStore, IUserOrderStore, OrderItemRef, OrderItemType, SqlitePool, SqliteSidebarStore, + SqliteUserOrderStore, init_database_memory, +}; +use aionui_project::canonical; +use async_trait::async_trait; +use tempfile::TempDir; + +use super::{SidebarError, SidebarService}; +use crate::ports::RemoveProjectPorts; + +const USER: &str = "user-1"; +const OTHER: &str = "user-2"; + +// -- Fixture ----------------------------------------------------------------- + +struct Fixture { + db: Database, + _tmp: TempDir, + service: SidebarService, +} + +impl Fixture { + fn pool(&self) -> &SqlitePool { + self.db.pool() + } + + /// A temp-session workspace path under `work_dir/conversations/` — the + /// same shape the write side auto-assigns, which classifies to chats (case 5). + fn temp_workspace(&self, leaf: &str) -> String { + self._tmp.path().join("conversations").join(leaf).display().to_string() + } +} + +async fn fixture() -> Fixture { + let db = init_database_memory().await.unwrap(); + seed_user(db.pool(), USER).await; + seed_user(db.pool(), OTHER).await; + let tmp = tempfile::tempdir().unwrap(); + let sidebar: Arc = Arc::new(SqliteSidebarStore::new(db.pool().clone())); + let user_order: Arc = Arc::new(SqliteUserOrderStore::new(db.pool().clone())); + let service = SidebarService::new(sidebar, user_order, tmp.path().to_path_buf()); + Fixture { db, _tmp: tmp, service } +} + +// -- Seed helpers (raw SQL; mirrors sqlite_sidebar_test.rs) ------------------- + +async fn seed_user(pool: &SqlitePool, id: &str) { + sqlx::query("INSERT INTO users (id, username, password_hash, created_at, updated_at) VALUES (?, ?, 'x', 0, 0)") + .bind(id) + .bind(id) + .execute(pool) + .await + .unwrap(); +} + +#[allow(clippy::too_many_arguments)] +async fn insert_conv(pool: &SqlitePool, user: &str, id: &str, project_id: Option<&str>, extra: &str, updated_at: i64) { + sqlx::query( + "INSERT INTO conversations (id, user_id, name, type, extra, project_id, archived_at, created_at, updated_at) \ + VALUES (?, ?, ?, 'acp', ?, ?, NULL, ?, ?)", + ) + .bind(id) + .bind(user) + .bind(id) + .bind(extra) + .bind(project_id) + .bind(updated_at) + .bind(updated_at) + .execute(pool) + .await + .unwrap(); +} + +/// A conversation whose workspace lives in `extra.$.workspace`. +async fn insert_conv_ws(pool: &SqlitePool, user: &str, id: &str, workspace: &str, updated_at: i64) { + let extra = serde_json::json!({ "workspace": workspace }).to_string(); + insert_conv(pool, user, id, None, &extra, updated_at).await; +} + +/// A conversation that is a team member (`extra.$.teamId`). +async fn insert_member(pool: &SqlitePool, user: &str, id: &str, team_id: &str, updated_at: i64) { + let extra = serde_json::json!({ "teamId": team_id }).to_string(); + insert_conv(pool, user, id, None, &extra, updated_at).await; +} + +async fn insert_team( + pool: &SqlitePool, + user: &str, + id: &str, + workspace: &str, + project_id: Option<&str>, + updated_at: i64, +) { + sqlx::query( + "INSERT INTO teams (id, user_id, name, workspace, project_id, archived_at, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, NULL, ?, ?)", + ) + .bind(id) + .bind(user) + .bind(id) + .bind(workspace) + .bind(project_id) + .bind(updated_at) + .bind(updated_at) + .execute(pool) + .await + .unwrap(); +} + +/// Standard project with a workspace-root folder whose canonical is `canon`. +async fn insert_std_project(pool: &SqlitePool, user: &str, project_id: &str, name: &str, canon: &str, uri: &str) { + sqlx::query("INSERT INTO projects (project_id, user_id, name, kind, created_at, updated_at) VALUES (?, ?, ?, 'standard', 0, 0)") + .bind(project_id) + .bind(user) + .bind(name) + .execute(pool) + .await + .unwrap(); + let folder_id = format!("f-{project_id}"); + sqlx::query("INSERT INTO folders (folder_id, resource_uri, resource_canonical, created_at, updated_at) VALUES (?, ?, ?, 0, 0)") + .bind(&folder_id) + .bind(uri) + .bind(canon) + .execute(pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO project_explorer (pe_id, project_id, folder_id, role, order_index, created_at, updated_at) \ + VALUES (?, ?, ?, 'workspace', 0, 0, 0)", + ) + .bind(format!("pe-{project_id}")) + .bind(project_id) + .bind(&folder_id) + .execute(pool) + .await + .unwrap(); +} + +async fn insert_temp_project(pool: &SqlitePool, user: &str, project_id: &str) { + sqlx::query("INSERT INTO projects (project_id, user_id, name, kind, created_at, updated_at) VALUES (?, ?, 'Temp', 'temp', 0, 0)") + .bind(project_id) + .bind(user) + .execute(pool) + .await + .unwrap(); +} + +// -- Assertion helpers ------------------------------------------------------- + +fn canon_of(path: &str) -> String { + let uri = canonical::to_file_uri(Path::new(path)).unwrap(); + canonical::canonicalize(&uri).unwrap().as_str().to_owned() +} + +fn file_uri(path: &str) -> String { + canonical::to_file_uri(Path::new(path)).unwrap() +} + +fn conv_ids(items: &[SidebarItem]) -> Vec { + let mut ids: Vec = items + .iter() + .filter_map(|i| match i { + SidebarItem::Conversation { conversation } => Some(conversation.id.clone()), + SidebarItem::Team(_) => None, + }) + .collect(); + ids.sort(); + ids +} + +fn team_ids(items: &[SidebarItem]) -> Vec { + let mut ids: Vec = items + .iter() + .filter_map(|i| match i { + SidebarItem::Team(t) => Some(t.team_id.clone()), + SidebarItem::Conversation { .. } => None, + }) + .collect(); + ids.sort(); + ids +} + +fn find_project<'a>( + resp: &'a aionui_api_types::SidebarResponse, + project_id: &str, +) -> &'a aionui_api_types::SidebarGroup { + resp.groups + .iter() + .find(|g| matches!(&g.scope, SidebarScope::Project { project_id: p, .. } if p == project_id)) + .unwrap_or_else(|| panic!("no project group {project_id}")) +} + +fn find_chats(resp: &aionui_api_types::SidebarResponse) -> &aionui_api_types::SidebarGroup { + resp.groups + .iter() + .find(|g| matches!(g.scope, SidebarScope::Chats)) + .expect("no chats group") +} + +fn find_dir<'a>(resp: &'a aionui_api_types::SidebarResponse, name: &str) -> Option<&'a aionui_api_types::SidebarGroup> { + resp.groups + .iter() + .find(|g| matches!(&g.scope, SidebarScope::Dir { name: n, .. } if n == name)) +} + +fn find_pinned(resp: &aionui_api_types::SidebarResponse) -> Option<&aionui_api_types::SidebarGroup> { + resp.groups.iter().find(|g| matches!(g.scope, SidebarScope::Pinned)) +} + +// -- D11 classification matrix (conversations) ------------------------------- + +#[tokio::test] +async fn classifies_five_cases_for_conversations() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + insert_temp_project(pool, USER, "proj-temp").await; + + // case 1: bound to standard project. + insert_conv(pool, USER, "c1", Some("proj-std"), "{}", 100).await; + // case 2: bound to temp project => chats. + insert_conv(pool, USER, "c2", Some("proj-temp"), "{}", 90).await; + // case 3: unbound, workspace canonicalizes onto the standard project root. + insert_conv_ws(pool, USER, "c3", "/repo/std", 80).await; + // case 4: unbound, non-temp path with no matching project => pseudo-dir. + insert_conv_ws(pool, USER, "c4", "/repo/other", 70).await; + // case 5: unbound, temp-session workspace => chats. + insert_conv_ws(pool, USER, "c5", &fx.temp_workspace("sess-a"), 60).await; + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + + // case 1 + case 3 land in the same standard-project group (path merge). + assert_eq!(conv_ids(&find_project(&resp, "proj-std").items), vec!["c1", "c3"]); + // case 4 -> its own dir group. + assert_eq!( + conv_ids(&find_dir(&resp, "other").expect("dir group").items), + vec!["c4"] + ); + // case 2 + case 5 -> chats. + assert_eq!(conv_ids(&find_chats(&resp).items), vec!["c2", "c5"]); + // temp project never becomes its own project group. + assert!( + !resp + .groups + .iter() + .any(|g| matches!(&g.scope, SidebarScope::Project { project_id, .. } if project_id == "proj-temp")) + ); +} + +#[tokio::test] +async fn path_merge_does_not_write_the_db() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + insert_conv_ws(pool, USER, "c3", "/repo/std", 80).await; + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + assert_eq!( + conv_ids(&find_project(&resp, "proj-std").items), + vec!["c3"], + "displayed under the project" + ); + + // BR-17/27: the merge is display-only; project_id stays NULL on disk. + let pid: Option = sqlx::query_scalar("SELECT project_id FROM conversations WHERE id = 'c3'") + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(pid, None, "path merge must not persist a project binding"); +} + +#[tokio::test] +async fn dangling_project_id_falls_through_to_path() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + // project_id points at a project that does not exist -> treat as NULL, then + // classify by workspace path (which merges onto proj-std). + let extra = serde_json::json!({ "workspace": "/repo/std" }).to_string(); + insert_conv(pool, USER, "ghost", Some("no-such-proj"), &extra, 80).await; + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + assert_eq!(conv_ids(&find_project(&resp, "proj-std").items), vec!["ghost"]); +} + +// -- Teams: isomorphic classification + folding + orphan downgrade ----------- + +#[tokio::test] +async fn teams_classify_and_fold_members_orphans_downgrade() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + + // Live team bound to the standard project. + insert_team(pool, USER, "T1", "", Some("proj-std"), 200).await; + // Two members of T1: folded into the team row, not independent. + insert_member(pool, USER, "m1", "T1", 150).await; + insert_member(pool, USER, "m2", "T1", 160).await; + // Orphan member: teamId names no live team -> downgrades to an independent + // conversation, classified by its own (temp) workspace => chats (BR-8). + let orphan_extra = serde_json::json!({ "teamId": "ghost-team", "workspace": fx.temp_workspace("o") }).to_string(); + insert_conv(pool, USER, "orphan", None, &orphan_extra, 140).await; + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + + let proj = find_project(&resp, "proj-std"); + assert_eq!(team_ids(&proj.items), vec!["T1"], "team lands in the project group"); + assert!( + conv_ids(&proj.items).is_empty(), + "folded members are not independent rows" + ); + + // T1 aggregates its members (created_at asc); orphan is not a member. + let team = proj + .items + .iter() + .find_map(|i| match i { + SidebarItem::Team(t) => Some(t), + _ => None, + }) + .unwrap(); + assert_eq!(team.member_conversation_ids, vec!["m1", "m2"]); + + // Orphan surfaces as an independent conversation in chats. + assert_eq!(conv_ids(&find_chats(&resp).items), vec!["orphan"]); +} + +// -- Pinned anti-join / B1 double-render ------------------------------------- + +#[tokio::test] +async fn pinned_item_leaves_its_natural_group() { + let fx = fixture().await; + let pool = fx.pool(); + insert_conv_ws(pool, USER, "c4", "/repo/other", 70).await; + + // Before pin: c4 lives in its dir group, no pinned group. + let before = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + assert!(find_pinned(&before).is_none()); + assert_eq!(conv_ids(&find_dir(&before, "other").unwrap().items), vec!["c4"]); + + fx.service.pin(USER, "pinned", "conversation", "c4").await.unwrap(); + + let after = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + // Pinned group carries c4 with pinned=true. + let pinned = find_pinned(&after).expect("pinned group present"); + assert_eq!(conv_ids(&pinned.items), vec!["c4"]); + let is_pinned = pinned.items.iter().any( + |i| matches!(i, SidebarItem::Conversation { conversation } if conversation.id == "c4" && conversation.pinned), + ); + assert!(is_pinned, "DTO pinned flag overridden to true"); + // B1: c4 no longer double-renders in its natural (dir) group. The dir group + // may still surface (its pinned member keeps contributing latest_activity), + // but must not re-list c4 among its items. + let dir_convs = find_dir(&after, "other") + .map(|g| conv_ids(&g.items)) + .unwrap_or_default(); + assert!( + !dir_convs.contains(&"c4".to_owned()), + "pinned item must not appear in its natural group" + ); +} + +#[tokio::test] +async fn pin_unpin_is_idempotent_and_validated() { + let fx = fixture().await; + insert_conv_ws(fx.pool(), USER, "c1", "/repo/other", 70).await; + + // Unknown scene / item_type -> 400, never a silent no-op. + assert!(matches!( + fx.service.pin(USER, "bogus", "conversation", "c1").await, + Err(SidebarError::BadRequest(_)) + )); + assert!(matches!( + fx.service.pin(USER, "pinned", "bogus", "c1").await, + Err(SidebarError::BadRequest(_)) + )); + + // Two pins collapse to one row. + fx.service.pin(USER, "pinned", "conversation", "c1").await.unwrap(); + fx.service.pin(USER, "pinned", "conversation", "c1").await.unwrap(); + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = 'c1'") + .bind(USER) + .fetch_one(fx.pool()) + .await + .unwrap(); + assert_eq!(count, 1, "pin is idempotent"); + + // Unpin removes; unpinning again is a no-op. + fx.service.unpin(USER, "pinned", "conversation", "c1").await.unwrap(); + fx.service.unpin(USER, "pinned", "conversation", "c1").await.unwrap(); + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = 'c1'") + .bind(USER) + .fetch_one(fx.pool()) + .await + .unwrap(); + assert_eq!(count, 0); +} + +#[tokio::test] +async fn pinned_group_hides_a_conversation_that_became_a_live_team_member() { + // Path-4 read-side defense (design §4.3): if a pinned conversation is a live + // team member (folded into its team), it must not also surface as an + // independent row in the pinned group. The pinned row still exists on disk — + // this is read-side defense, not a table-level cascade. + let fx = fixture().await; + let pool = fx.pool(); + insert_team(pool, USER, "T1", "", None, 200).await; + insert_member(pool, USER, "m1", "T1", 150).await; + + // Pin the member conversation directly (simulating dirty data / a future + // write path — the normal UI never exposes a member as an independent row). + fx.service.pin(USER, "pinned", "conversation", "m1").await.unwrap(); + let row_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = 'm1'") + .bind(USER) + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(row_count, 1, "the pinned row is present on disk"); + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + let pinned = find_pinned(&resp); + let pinned_convs = pinned.map(|g| conv_ids(&g.items)).unwrap_or_default(); + assert!( + !pinned_convs.contains(&"m1".to_owned()), + "a live team member must not render as an independent pinned row" + ); +} + +// -- Cross-user scoping (BR-24) ---------------------------------------------- + +#[tokio::test] +async fn first_screen_is_user_scoped() { + let fx = fixture().await; + let pool = fx.pool(); + // Another user owns a project + conversation on a workspace USER also uses. + insert_std_project( + pool, + OTHER, + "proj-foreign", + "Foreign", + &canon_of("/repo/shared"), + &file_uri("/repo/shared"), + ) + .await; + insert_conv(pool, OTHER, "foreign-c", Some("proj-foreign"), "{}", 100).await; + // USER's own single conversation. + insert_conv_ws(pool, USER, "mine", "/repo/mine", 50).await; + + let resp = fx.service.first_screen(USER, Some(50), &[]).await.unwrap(); + assert!( + !resp + .groups + .iter() + .any(|g| matches!(&g.scope, SidebarScope::Project { project_id, .. } if project_id == "proj-foreign")) + ); + let all_conv: Vec = resp.groups.iter().flat_map(|g| conv_ids(&g.items)).collect(); + assert_eq!(all_conv, vec!["mine"], "no cross-user leakage"); +} + +// -- Keyset paging continuity ------------------------------------------------ + +#[tokio::test] +async fn chats_paging_is_continuous_no_dup_no_miss() { + let fx = fixture().await; + let pool = fx.pool(); + // Five chats conversations (temp workspaces), distinct updated_at. + for (i, ts) in [("a", 50), ("b", 40), ("c", 30), ("d", 20), ("e", 10)] { + insert_conv_ws(pool, USER, i, &fx.temp_workspace(i), ts).await; + } + + // First screen with a window of 2 over chats. + let resp = fx.service.first_screen(USER, Some(2), &[]).await.unwrap(); + let chats = find_chats(&resp); + assert!(chats.has_more); + let mut seen = conv_ids(&chats.items); + assert_eq!(seen.len(), 2); + let mut cursor = chats.next_cursor.clone(); + + // Page the rest via the items endpoint. + while let Some(c) = cursor { + let page = fx.service.items(USER, "chats", Some(&c), Some(2)).await.unwrap(); + seen.extend(conv_ids(&page.items)); + cursor = page.next_cursor; + if !page.has_more { + break; + } + } + seen.sort(); + assert_eq!(seen, vec!["a", "b", "c", "d", "e"], "every chat seen exactly once"); +} + +// -- ScopeGone 404 ----------------------------------------------------------- + +#[tokio::test] +async fn items_on_missing_scope_is_scope_gone() { + let fx = fixture().await; + insert_conv_ws(fx.pool(), USER, "c1", "/repo/other", 70).await; + + let err = fx + .service + .items(USER, "project:no-such", None, Some(10)) + .await + .unwrap_err(); + assert!(matches!(err, SidebarError::ScopeGone), "stale project scope -> 404"); + + // A syntactically bad scope is a 400, not a 404. + let err = fx.service.items(USER, "bogus", None, Some(10)).await.unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +// -- remove_project (BR-19 / D13 "所见即所删") -------------------------------- + +/// A `RemoveProjectPorts` that performs the same real deletes production does +/// (conversation row + its `user_order` rows via the same store; team + folded +/// members; project record), so a test can assert both the returned counts and +/// the on-disk orphan cleanup. `fail` ids/records make a single port call error +/// to exercise best-effort orchestration. +struct FakePorts { + pool: SqlitePool, + user_order: Arc, + fail: HashSet, + deleted_convs: Mutex>, + deleted_teams: Mutex>, + project_deleted: Mutex, +} + +impl FakePorts { + fn new(pool: SqlitePool, user_order: Arc, fail: &[&str]) -> Arc { + Arc::new(Self { + pool, + user_order, + fail: fail.iter().map(|s| s.to_string()).collect(), + deleted_convs: Mutex::new(Vec::new()), + deleted_teams: Mutex::new(Vec::new()), + project_deleted: Mutex::new(false), + }) + } +} + +#[async_trait] +impl RemoveProjectPorts for FakePorts { + async fn delete_conversation(&self, user_id: &str, conversation_id: &str) -> Result<(), String> { + if self.fail.contains(conversation_id) { + return Err(format!("forced failure deleting {conversation_id}")); + } + sqlx::query("DELETE FROM conversations WHERE id = ? AND user_id = ?") + .bind(conversation_id) + .bind(user_id) + .execute(&self.pool) + .await + .unwrap(); + // Mirror the production path-1 cascade so orphan rows are cleaned up. + self.user_order + .remove_item( + user_id, + &OrderItemRef::new(OrderItemType::Conversation, conversation_id), + ) + .await + .unwrap(); + self.deleted_convs.lock().unwrap().push(conversation_id.to_owned()); + Ok(()) + } + + async fn remove_team(&self, user_id: &str, team_id: &str) -> Result<(), String> { + if self.fail.contains(team_id) { + return Err(format!("forced failure removing team {team_id}")); + } + // Member conversations are folded into the team and go with it. + let members: Vec = sqlx::query_scalar( + "SELECT id FROM conversations WHERE user_id = ? AND json_extract(extra, '$.teamId') = ?", + ) + .bind(user_id) + .bind(team_id) + .fetch_all(&self.pool) + .await + .unwrap(); + for member in &members { + sqlx::query("DELETE FROM conversations WHERE id = ?") + .bind(member) + .execute(&self.pool) + .await + .unwrap(); + self.user_order + .remove_item(user_id, &OrderItemRef::new(OrderItemType::Conversation, member)) + .await + .unwrap(); + } + sqlx::query("DELETE FROM teams WHERE id = ? AND user_id = ?") + .bind(team_id) + .bind(user_id) + .execute(&self.pool) + .await + .unwrap(); + self.user_order + .remove_item(user_id, &OrderItemRef::new(OrderItemType::Team, team_id)) + .await + .unwrap(); + self.deleted_teams.lock().unwrap().push(team_id.to_owned()); + Ok(()) + } + + async fn delete_project_record(&self, user_id: &str, project_id: &str) -> Result<(), String> { + if self.fail.contains(project_id) { + return Err(format!("forced failure deleting project {project_id}")); + } + sqlx::query("DELETE FROM project_explorer WHERE project_id = ?") + .bind(project_id) + .execute(&self.pool) + .await + .unwrap(); + sqlx::query("DELETE FROM projects WHERE project_id = ? AND user_id = ?") + .bind(project_id) + .bind(user_id) + .execute(&self.pool) + .await + .unwrap(); + *self.project_deleted.lock().unwrap() = true; + Ok(()) + } +} + +/// A `user_order` store over the same pool the service uses — `SqliteUserOrderStore` +/// is stateless over the pool, so a fresh instance behaves identically. +fn uo_store(pool: &SqlitePool) -> Arc { + Arc::new(SqliteUserOrderStore::new(pool.clone())) +} + +async fn conv_exists(pool: &SqlitePool, id: &str) -> bool { + let n: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM conversations WHERE id = ?") + .bind(id) + .fetch_one(pool) + .await + .unwrap(); + n > 0 +} + +async fn user_order_count(pool: &SqlitePool, user: &str, item_id: &str) -> i64 { + sqlx::query_scalar("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = ?") + .bind(user) + .bind(item_id) + .fetch_one(pool) + .await + .unwrap() +} + +/// The delete set is the render construct: the bound conversation, the +/// path-merged unbound conversation, and the bound team (with its folded +/// members) all go; unrelated dir/chats rows stay. Pinned rows are included +/// (pinning only hoists display) and their `user_order` rows are cleaned up. +#[tokio::test] +async fn remove_project_deletes_the_visible_construct_and_orphan_rows() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + + insert_conv(pool, USER, "c1", Some("proj-std"), "{}", 100).await; // bound (case 1) + insert_conv_ws(pool, USER, "c3", "/repo/std", 90).await; // path-merged (case 3) + insert_team(pool, USER, "T1", "", Some("proj-std"), 200).await; // bound team + insert_member(pool, USER, "m1", "T1", 150).await; + insert_member(pool, USER, "m2", "T1", 160).await; + insert_conv_ws(pool, USER, "c4", "/repo/other", 70).await; // dir group -> keep + insert_conv_ws(pool, USER, "c5", &fx.temp_workspace("s"), 60).await; // chats -> keep + + // Pin a conversation and the team so we can assert orphan cleanup. + fx.service.pin(USER, "pinned", "conversation", "c1").await.unwrap(); + fx.service.pin(USER, "pinned", "team", "T1").await.unwrap(); + + let ports = FakePorts::new(pool.clone(), uo_store(pool), &[]); + fx.service.set_remove_project_ports(ports.clone()); + + // Dry run reports the set without touching anything. + let preview = fx.service.remove_project(USER, "proj-std", true).await.unwrap(); + assert_eq!(preview.teams_deleted, 1); + assert_eq!(preview.conversations_deleted, 2, "c1 + c3"); + assert!(conv_exists(pool, "c1").await, "dry run must not delete"); + assert_eq!(ports.deleted_convs.lock().unwrap().len(), 0); + + // The preview names the delete set with pinned flags so the confirm dialog can + // list *which* items go. Pinned members (c1, T1) were hoisted into the top + // pinned group (B1 anti-join) — the frontend cannot reconstruct them, so the + // names must ride the preview. + let team_items: Vec<_> = preview + .items + .iter() + .filter(|i| i.kind == aionui_api_types::RemoveProjectItemKind::Team) + .collect(); + assert_eq!(team_items.len(), 1); + assert_eq!(team_items[0].name, "T1"); + assert!(team_items[0].pinned, "T1 was pinned"); + + let mut conv_items: Vec<_> = preview + .items + .iter() + .filter(|i| i.kind == aionui_api_types::RemoveProjectItemKind::Conversation) + .collect(); + conv_items.sort_by(|a, b| a.name.cmp(&b.name)); + assert_eq!( + conv_items.iter().map(|i| i.name.as_str()).collect::>(), + ["c1", "c3"] + ); + let c1 = conv_items.iter().find(|i| i.name == "c1").unwrap(); + let c3 = conv_items.iter().find(|i| i.name == "c3").unwrap(); + assert!(c1.pinned, "c1 was pinned"); + assert!(!c3.pinned, "c3 was not pinned"); + + // Live delete matches the preview exactly. + let result = fx.service.remove_project(USER, "proj-std", false).await.unwrap(); + assert_eq!(result.teams_deleted, preview.teams_deleted); + assert_eq!(result.conversations_deleted, preview.conversations_deleted); + assert!( + result.items.is_empty(), + "live delete omits the name list (preview already showed it)" + ); + + // The whole visible construct is gone: bound conv, merged conv, team, members. + assert!(!conv_exists(pool, "c1").await); + assert!(!conv_exists(pool, "c3").await); + assert!(!conv_exists(pool, "m1").await); + assert!(!conv_exists(pool, "m2").await); + let team_left: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM teams WHERE id = 'T1'") + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(team_left, 0); + let proj_left: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM projects WHERE project_id = 'proj-std'") + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(proj_left, 0, "project record removed"); + + // Unrelated rows survive. + assert!(conv_exists(pool, "c4").await, "dir-group conv untouched"); + assert!(conv_exists(pool, "c5").await, "chats conv untouched"); + + // Orphan cleanup: no dangling user_order rows for the removed conv/team. + assert_eq!(user_order_count(pool, USER, "c1").await, 0, "pinned conv row gone"); + assert_eq!(user_order_count(pool, USER, "T1").await, 0, "pinned team row gone"); +} + +/// A failing port call does not abort the sweep: siblings still delete and the +/// reported count reflects only the successes. +#[tokio::test] +async fn remove_project_is_best_effort_on_port_failure() { + let fx = fixture().await; + let pool = fx.pool(); + insert_std_project( + pool, + USER, + "proj-std", + "Std", + &canon_of("/repo/std"), + &file_uri("/repo/std"), + ) + .await; + insert_conv(pool, USER, "c1", Some("proj-std"), "{}", 100).await; + insert_conv(pool, USER, "c2", Some("proj-std"), "{}", 90).await; + + // Deleting c1 fails; c2 must still be removed. + let ports = FakePorts::new(pool.clone(), uo_store(pool), &["c1"]); + fx.service.set_remove_project_ports(ports.clone()); + + let result = fx.service.remove_project(USER, "proj-std", false).await.unwrap(); + assert_eq!(result.conversations_deleted, 1, "only c2 counted"); + assert!(conv_exists(pool, "c1").await, "failed delete left c1 in place"); + assert!(!conv_exists(pool, "c2").await, "sibling delete still ran"); + assert!( + *ports.project_deleted.lock().unwrap(), + "project record delete still attempted" + ); +} + +/// A missing or non-standard target is a 404 (`ScopeGone`), before any port is +/// touched. +#[tokio::test] +async fn remove_project_on_missing_or_nonstandard_scope_is_scope_gone() { + let fx = fixture().await; + let pool = fx.pool(); + insert_temp_project(pool, USER, "proj-temp").await; + + let ports = FakePorts::new(pool.clone(), uo_store(pool), &[]); + fx.service.set_remove_project_ports(ports.clone()); + + // Unknown project id. + let err = fx.service.remove_project(USER, "no-such", false).await.unwrap_err(); + assert!(matches!(err, SidebarError::ScopeGone)); + // A temp project is not a removable standard-project scope. + let err = fx.service.remove_project(USER, "proj-temp", false).await.unwrap_err(); + assert!(matches!(err, SidebarError::ScopeGone)); + + // Neither attempt invoked a port. + assert!(ports.deleted_convs.lock().unwrap().is_empty()); + assert!(!*ports.project_deleted.lock().unwrap()); +} diff --git a/crates/aionui-sidebar/src/types.rs b/crates/aionui-sidebar/src/types.rs new file mode 100644 index 000000000..39f5f3465 --- /dev/null +++ b/crates/aionui-sidebar/src/types.rs @@ -0,0 +1,259 @@ +//! Wire-format helpers and the crate error type. +//! +//! Everything the two endpoints must parse/format lives here so the service can +//! stay focused on classification: the crate error ([`SidebarError`]), the +//! scope-token grammar ([`ScopeToken`]), the pseudo-dir key codec, the keyset +//! [`Cursor`], and the `win` list parser. See `api-contract-sidebar.md` §3. + +use base64::Engine; +use base64::engine::general_purpose::{STANDARD as B64, URL_SAFE_NO_PAD as B64_URL}; +use serde::{Deserialize, Serialize}; + +use aionui_db::DbError; +use aionui_project::ProjectError; + +/// Cursor / group-ordering version. Bumped when the keyset ordering changes so a +/// stale client cursor is rejected (400) instead of silently mis-paging. +pub const CURSOR_VERSION: u32 = 1; + +/// Default per-group window when the request omits `limit` / a `win` entry. +pub const DEFAULT_LIMIT: i64 = 5; +/// Default `items` window when the request omits `limit`. +pub const DEFAULT_ITEMS_LIMIT: i64 = 10; +/// Per-window hard cap (both `limit` and any `win` entry). +pub const MAX_LIMIT: i64 = 100; +/// Cap on the number of `win` entries in one `GET /api/sidebar` request. +pub const MAX_WIN_ENTRIES: usize = 100; + +/// Errors surfaced by the sidebar service. Route handlers map these to +/// `ApiError` with stable machine codes; DB/project detail never leaks out. +#[derive(Debug, thiserror::Error)] +pub enum SidebarError { + /// Malformed request input (bad `win`, bad cursor, unknown scope/item type, + /// out-of-range limit). Maps to 400. + #[error("bad request: {0}")] + BadRequest(String), + /// The paged scope no longer exists (project removed, pseudo-dir emptied). + /// Maps to 404 so the frontend drops that group. + #[error("scope no longer exists")] + ScopeGone, + #[error(transparent)] + Db(#[from] DbError), + #[error(transparent)] + Project(#[from] ProjectError), + #[error("internal: {0}")] + Internal(String), +} + +/// A parsed `scope` token (`api-contract-sidebar.md` §3.3). +/// +/// The pseudo-dir variant carries the *decoded* canonical path, not the raw +/// base64url key — decoding happens at parse time so the service compares plain +/// canonical strings. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ScopeToken { + Pinned, + Project(String), + /// Decoded canonical path of the pseudo-dir group. + Dir(String), + Chats, +} + +impl ScopeToken { + /// Parse a `scope` token. Unknown prefixes / undecodable dir keys → `None` + /// (the caller turns that into a 400). + pub fn parse(token: &str) -> Option { + if token == "pinned" { + return Some(ScopeToken::Pinned); + } + if token == "chats" { + return Some(ScopeToken::Chats); + } + if let Some(id) = token.strip_prefix("project:") { + return (!id.is_empty()).then(|| ScopeToken::Project(id.to_owned())); + } + if let Some(key) = token.strip_prefix("dir:") { + return dir_key_to_canonical(key).map(ScopeToken::Dir); + } + None + } + + /// Format back to the wire token (the inverse of [`parse`](Self::parse)). + pub fn to_token(&self) -> String { + match self { + ScopeToken::Pinned => "pinned".to_owned(), + ScopeToken::Chats => "chats".to_owned(), + ScopeToken::Project(id) => format!("project:{id}"), + ScopeToken::Dir(canonical) => format!("dir:{}", canonical_to_dir_key(canonical)), + } + } +} + +/// Encode a pseudo-dir canonical path into its scope-token key. +/// +/// Canonical paths contain `:` (the `file:` scheme), so they cannot be passed +/// raw in a `:` token — base64url (no padding) keeps the token a +/// single opaque segment. +pub fn canonical_to_dir_key(canonical: &str) -> String { + B64_URL.encode(canonical.as_bytes()) +} + +/// Decode a pseudo-dir scope-token key back to its canonical path. `None` when +/// the key is not valid base64url / not UTF-8. +pub fn dir_key_to_canonical(key: &str) -> Option { + let bytes = B64_URL.decode(key.as_bytes()).ok()?; + String::from_utf8(bytes).ok() +} + +/// Keyset cursor position within a group. `Pinned` scopes page by `order_key` +/// ascending; every other scope pages by `updated_at` descending. Both tie-break +/// on the full `(item_type, item_id)` pair, so no position is ambiguous. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Cursor { + Pinned { + order_key: i64, + item_type: String, + item_id: String, + }, + Activity { + updated_at: i64, + item_type: String, + item_id: String, + }, +} + +/// Serialized cursor payload (base64 of this JSON). `scope` binds the cursor to +/// the group it was minted for; a cursor replayed against another scope is +/// rejected rather than silently mis-applied. +#[derive(Debug, Serialize, Deserialize)] +struct CursorPayload { + v: u32, + scope: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + order_key: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + updated_at: Option, + item_type: String, + item_id: String, +} + +impl Cursor { + /// Encode this cursor for the given scope token: `base64(JSON)`. + pub fn encode(&self, scope: &ScopeToken) -> String { + let payload = match self { + Cursor::Pinned { + order_key, + item_type, + item_id, + } => CursorPayload { + v: CURSOR_VERSION, + scope: scope.to_token(), + order_key: Some(*order_key), + updated_at: None, + item_type: item_type.clone(), + item_id: item_id.clone(), + }, + Cursor::Activity { + updated_at, + item_type, + item_id, + } => CursorPayload { + v: CURSOR_VERSION, + scope: scope.to_token(), + order_key: None, + updated_at: Some(*updated_at), + item_type: item_type.clone(), + item_id: item_id.clone(), + }, + }; + // Serialization of a plain struct cannot fail; fall back to empty rather + // than panicking on the impossible branch. + B64.encode(serde_json::to_vec(&payload).unwrap_or_default()) + } + + /// Decode a cursor and verify it was minted for `scope` at this ordering + /// version. Bad base64 / bad JSON / wrong version / cross-scope / missing + /// keyset field → `BadRequest` (never a silent reset to the first page). + pub fn decode(raw: &str, scope: &ScopeToken) -> Result { + let bytes = B64 + .decode(raw.as_bytes()) + .map_err(|_| SidebarError::BadRequest("cursor is not valid base64".into()))?; + let payload: CursorPayload = + serde_json::from_slice(&bytes).map_err(|_| SidebarError::BadRequest("cursor JSON is malformed".into()))?; + if payload.v != CURSOR_VERSION { + return Err(SidebarError::BadRequest("cursor version mismatch".into())); + } + if payload.scope != scope.to_token() { + return Err(SidebarError::BadRequest("cursor belongs to a different scope".into())); + } + match scope { + ScopeToken::Pinned => { + let order_key = payload + .order_key + .ok_or_else(|| SidebarError::BadRequest("pinned cursor missing order_key".into()))?; + Ok(Cursor::Pinned { + order_key, + item_type: payload.item_type, + item_id: payload.item_id, + }) + } + _ => { + let updated_at = payload + .updated_at + .ok_or_else(|| SidebarError::BadRequest("activity cursor missing updated_at".into()))?; + Ok(Cursor::Activity { + updated_at, + item_type: payload.item_type, + item_id: payload.item_id, + }) + } + } + } +} + +/// Parse the repeated `win=:` query params into a +/// scope-token → window-size map. +/// +/// The scope token itself contains `:` (`project:P1`, `dir:`), so the limit +/// is taken from after the *last* `:`. Duplicate tokens, malformed entries, +/// out-of-range limits, or more than [`MAX_WIN_ENTRIES`] entries → `BadRequest`. +/// Whether a named project/dir currently exists is *not* checked here — a syntax- +/// valid but stale window is tolerated by the service (BR-15), not rejected. +pub fn parse_win(entries: &[String]) -> Result, SidebarError> { + if entries.len() > MAX_WIN_ENTRIES { + return Err(SidebarError::BadRequest("too many win entries".into())); + } + let mut out: Vec<(String, i64)> = Vec::with_capacity(entries.len()); + for entry in entries { + let (token, limit_str) = entry + .rsplit_once(':') + .ok_or_else(|| SidebarError::BadRequest(format!("malformed win entry: {entry}")))?; + if ScopeToken::parse(token).is_none() { + return Err(SidebarError::BadRequest(format!("unknown win scope: {token}"))); + } + let limit: i64 = limit_str + .parse() + .map_err(|_| SidebarError::BadRequest(format!("win limit is not a number: {entry}")))?; + validate_limit(limit)?; + if out.iter().any(|(t, _)| t == token) { + return Err(SidebarError::BadRequest(format!("duplicate win scope: {token}"))); + } + out.push((token.to_owned(), limit)); + } + Ok(out) +} + +/// Enforce the per-window `[1, MAX_LIMIT]` bound shared by `limit` and `win`. +pub fn validate_limit(limit: i64) -> Result<(), SidebarError> { + if (1..=MAX_LIMIT).contains(&limit) { + Ok(()) + } else { + Err(SidebarError::BadRequest(format!( + "limit out of range [1,{MAX_LIMIT}]: {limit}" + ))) + } +} + +#[cfg(test)] +#[path = "types_test.rs"] +mod types_test; diff --git a/crates/aionui-sidebar/src/types_test.rs b/crates/aionui-sidebar/src/types_test.rs new file mode 100644 index 000000000..2e2a8df03 --- /dev/null +++ b/crates/aionui-sidebar/src/types_test.rs @@ -0,0 +1,186 @@ +//! Wire-format unit tests: scope-token grammar, dir-key codec, keyset cursor +//! round-trip + the 400 rejection matrix, and the `win` parser. + +use super::*; + +// -- ScopeToken -------------------------------------------------------------- + +#[test] +fn scope_token_parses_each_variant() { + assert_eq!(ScopeToken::parse("pinned"), Some(ScopeToken::Pinned)); + assert_eq!(ScopeToken::parse("chats"), Some(ScopeToken::Chats)); + assert_eq!(ScopeToken::parse("project:P1"), Some(ScopeToken::Project("P1".into()))); + + let key = canonical_to_dir_key("file:///work/a"); + let dir = ScopeToken::parse(&format!("dir:{key}")); + assert_eq!(dir, Some(ScopeToken::Dir("file:///work/a".into()))); +} + +#[test] +fn scope_token_rejects_unknown_and_empty() { + assert_eq!(ScopeToken::parse("bogus"), None); + assert_eq!(ScopeToken::parse("project:"), None); + assert_eq!(ScopeToken::parse("dir:@@not-base64@@"), None); +} + +#[test] +fn scope_token_round_trips_through_token() { + for token in ["pinned", "chats", "project:abc"] { + let parsed = ScopeToken::parse(token).unwrap(); + assert_eq!(parsed.to_token(), token); + } + // Dir round-trips via canonical, not the raw key string. + let dir = ScopeToken::Dir("file:///x/y".into()); + assert_eq!(ScopeToken::parse(&dir.to_token()), Some(dir)); +} + +// -- dir key codec ----------------------------------------------------------- + +#[test] +fn dir_key_codec_round_trips_paths_with_colons() { + let canonical = "file:///Users/me/Projects/a-b_c"; + let key = canonical_to_dir_key(canonical); + // URL-safe: no '/', '+', or '=' that would break a query token. + assert!(!key.contains('/') && !key.contains('+') && !key.contains('=')); + assert_eq!(dir_key_to_canonical(&key).as_deref(), Some(canonical)); +} + +#[test] +fn dir_key_rejects_garbage() { + assert_eq!(dir_key_to_canonical("*not*base64*"), None); +} + +// -- Cursor ------------------------------------------------------------------ + +#[test] +fn activity_cursor_round_trips() { + let scope = ScopeToken::Project("P1".into()); + let cursor = Cursor::Activity { + updated_at: 42, + item_type: "conversation".into(), + item_id: "c1".into(), + }; + let encoded = cursor.encode(&scope); + assert_eq!(Cursor::decode(&encoded, &scope).unwrap(), cursor); +} + +#[test] +fn pinned_cursor_round_trips() { + let scope = ScopeToken::Pinned; + let cursor = Cursor::Pinned { + order_key: -1000, + item_type: "team".into(), + item_id: "t1".into(), + }; + let encoded = cursor.encode(&scope); + assert_eq!(Cursor::decode(&encoded, &scope).unwrap(), cursor); +} + +#[test] +fn cursor_rejects_bad_base64() { + let err = Cursor::decode("*** not base64 ***", &ScopeToken::Pinned).unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +#[test] +fn cursor_rejects_bad_json() { + let raw = B64.encode(b"{not json"); + let err = Cursor::decode(&raw, &ScopeToken::Pinned).unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +#[test] +fn cursor_rejects_version_mismatch() { + let raw = B64.encode( + serde_json::to_vec(&serde_json::json!({ + "v": CURSOR_VERSION + 1, "scope": "pinned", "order_key": 1, "item_type": "team", "item_id": "t1" + })) + .unwrap(), + ); + let err = Cursor::decode(&raw, &ScopeToken::Pinned).unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +#[test] +fn cursor_rejects_cross_scope_replay() { + // Minted for project:P1, replayed against project:P2. + let minted = Cursor::Activity { + updated_at: 1, + item_type: "conversation".into(), + item_id: "c1".into(), + } + .encode(&ScopeToken::Project("P1".into())); + let err = Cursor::decode(&minted, &ScopeToken::Project("P2".into())).unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +#[test] +fn cursor_rejects_missing_keyset_field() { + // A pinned scope needs order_key; an activity payload lacks it. + let raw = B64.encode( + serde_json::to_vec(&serde_json::json!({ + "v": CURSOR_VERSION, "scope": "pinned", "updated_at": 5, "item_type": "team", "item_id": "t1" + })) + .unwrap(), + ); + let err = Cursor::decode(&raw, &ScopeToken::Pinned).unwrap_err(); + assert!(matches!(err, SidebarError::BadRequest(_))); +} + +// -- parse_win --------------------------------------------------------------- + +#[test] +fn parse_win_reads_token_and_limit() { + let dir_key = canonical_to_dir_key("file:///w/a"); + let entries = vec![ + "pinned:5".to_owned(), + "project:P1:10".to_owned(), + format!("dir:{dir_key}:3"), + "chats:20".to_owned(), + ]; + let parsed = parse_win(&entries).unwrap(); + assert_eq!(parsed.len(), 4); + assert!(parsed.contains(&("project:P1".to_owned(), 10))); + assert!(parsed.contains(&(format!("dir:{dir_key}"), 3))); +} + +#[test] +fn parse_win_rejects_bad_entries() { + assert!(matches!( + parse_win(&["nolimit".to_owned()]), + Err(SidebarError::BadRequest(_)) + )); + assert!(matches!( + parse_win(&["pinned:abc".to_owned()]), + Err(SidebarError::BadRequest(_)) + )); + assert!(matches!( + parse_win(&["bogus:5".to_owned()]), + Err(SidebarError::BadRequest(_)) + )); + assert!(matches!( + parse_win(&["pinned:0".to_owned()]), + Err(SidebarError::BadRequest(_)) + )); + assert!(matches!( + parse_win(&["pinned:5".to_owned(), "pinned:6".to_owned()]), + Err(SidebarError::BadRequest(_)) + )); +} + +#[test] +fn parse_win_rejects_too_many_entries() { + let entries: Vec = (0..(MAX_WIN_ENTRIES + 1)).map(|i| format!("project:P{i}:5")).collect(); + assert!(matches!(parse_win(&entries), Err(SidebarError::BadRequest(_)))); +} + +// -- validate_limit ---------------------------------------------------------- + +#[test] +fn validate_limit_enforces_bounds() { + assert!(validate_limit(1).is_ok()); + assert!(validate_limit(MAX_LIMIT).is_ok()); + assert!(validate_limit(0).is_err()); + assert!(validate_limit(MAX_LIMIT + 1).is_err()); + assert!(validate_limit(-3).is_err()); +} diff --git a/crates/aionui-team/src/service.rs b/crates/aionui-team/src/service.rs index 97076fb75..08f106f37 100644 --- a/crates/aionui-team/src/service.rs +++ b/crates/aionui-team/src/service.rs @@ -19,7 +19,8 @@ use aionui_common::{AgentKillReason, ConversationStatus, TimestampMs, generate_i use aionui_db::models::TeamRow; use aionui_db::{ ActivityCursor, IAgentMetadataRepository, IAssistantDefinitionRepository, IAssistantOverlayRepository, - IProviderRepository, ITeamRepository, PageDirection, UpdateTeamParams, + IProviderRepository, ITeamRepository, IUserOrderStore, OrderItemRef, OrderItemType, PageDirection, + UpdateTeamParams, }; use aionui_project::{ProjectService, canonical}; use aionui_realtime::EventBroadcaster; @@ -143,6 +144,10 @@ pub struct TeamSessionService { /// Project-bind side branch (optional). `None` → team binding is a no-op, /// so team create/read behaves exactly as before. project_service: Arc>>>, + /// Sidebar ordering store (optional). Set → `remove_team` cascade-deletes the + /// team's `user_order` rows (design §4.3, path 2). `None` → no-op, so team + /// deletion behaves exactly as before. + user_order: Arc>>>, /// Back-pointer used by [`TeamSession::spawn_agent`] to reach DB-facing /// orchestration without threading the service through every session method. /// Stored as `Weak` so the session map does not create a strong cycle with @@ -225,6 +230,7 @@ impl TeamSessionService { add_agent_locks: Arc::new(DashMap::new()), ensure_session_locks: Arc::new(DashMap::new()), project_service: Arc::new(RwLock::new(None)), + user_order: Arc::new(RwLock::new(None)), self_ref: weak.clone(), }) } @@ -247,6 +253,34 @@ impl TeamSessionService { } } + /// Inject the sidebar ordering store so `remove_team` cascade-deletes the + /// team's `user_order` rows (design §4.3, path 2). When unset, the cascade is + /// a no-op. Member conversations are handled separately by the conversation + /// delete hook (they route through `ConversationService::delete`). + pub fn with_user_order_store(&self, user_order: Arc) { + if let Ok(mut guard) = self.user_order.write() { + *guard = Some(user_order); + } + } + + /// Best-effort cascade of a removed team's `user_order` row (design §4.3, + /// path 2). Store unset → no-op. An error is logged, not propagated: an + /// orphan `team` row self-heals on read (the pinned group only emits teams + /// present in the live aggregate), so it must never block team deletion. + async fn remove_team_order_row(&self, user_id: &str, team_id: &str) { + let store = self.user_order.read().ok().and_then(|guard| guard.clone()); + let Some(store) = store else { return }; + let item = OrderItemRef::new(OrderItemType::Team, team_id); + if let Err(err) = store.remove_item(user_id, &item).await { + warn!( + user_id = %user_id, + team_id = %team_id, + error = %err, + "sidebar: failed to cascade-delete user_order row for removed team" + ); + } + } + /// Resolve a team workspace into `(project_id, folder_id)`. Best-effort: /// missing service / empty workspace / bad URI / resolve error → `(None, None)`, /// logged at `warn`. Never affects team create/read. @@ -684,6 +718,13 @@ impl TeamSessionService { self.repo.delete_tasks_by_team(user_id, team_id).await?; self.repo.delete_team(user_id, team_id).await?; + // Cascade the team's sidebar ordering row (design §4.3, path 2). Members' + // conversation rows are dropped by the conversation delete hook via the + // `delete_team_conversation` calls above. Best-effort: an orphan `team` + // row self-heals on read (the pinned group only emits teams present in + // the live aggregate), so it never blocks deletion. + self.remove_team_order_row(user_id, team_id).await; + self.add_agent_locks.remove(team_id); info!(team_id = %team_id, "Team removed"); diff --git a/crates/aionui-team/tests/session_service_integration.rs b/crates/aionui-team/tests/session_service_integration.rs index 2f676b922..37b24fd5d 100644 --- a/crates/aionui-team/tests/session_service_integration.rs +++ b/crates/aionui-team/tests/session_service_integration.rs @@ -3793,6 +3793,61 @@ async fn td6_delete_nonexistent_returns_error() { assert!(result.is_err()); } +// Path 2 cascade (design §4.3): removing a team drops its `user_order` row. +// The store is injected independently; the assertion is on the table row count, +// not any API output. +#[tokio::test] +async fn td_remove_team_cascades_its_pinned_order_row() { + let svc = setup(); + let db = aionui_db::init_database_memory().await.unwrap(); + let store: Arc = Arc::new(aionui_db::SqliteUserOrderStore::new(db.pool().clone())); + svc.with_user_order_store(store.clone()); + + let created = svc + .create_team( + "user1", + CreateTeamRequest { + name: "T".into(), + agents: two_agent_input(), + workspace: None, + }, + ) + .await + .unwrap(); + + // Pin the team, then pin an unrelated team for a second user (BR-24 scope). + let team_item = aionui_db::OrderItemRef::new(aionui_db::OrderItemType::Team, &created.id); + store + .pin("user1", aionui_db::OrderScene::Pinned, &team_item) + .await + .unwrap(); + let other = aionui_db::OrderItemRef::new(aionui_db::OrderItemType::Team, &created.id); + store.pin("user2", aionui_db::OrderScene::Pinned, &other).await.unwrap(); + + let count = |user: &'static str, id: String| { + let pool = db.pool().clone(); + async move { + sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM user_order WHERE user_id = ? AND item_id = ?") + .bind(user) + .bind(id) + .fetch_one(&pool) + .await + .unwrap() + } + }; + assert_eq!(count("user1", created.id.clone()).await, 1); + + svc.remove_team("user1", &created.id).await.unwrap(); + + // Owner's team row is gone; the other user's identically-keyed row stands. + assert_eq!( + count("user1", created.id.clone()).await, + 0, + "path-2 cascade removed the team's pinned row" + ); + assert_eq!(count("user2", created.id.clone()).await, 1, "cascade is user-scoped"); +} + // -- Rename team -------------------------------------------------------------- #[tokio::test] From 12269bba8c65b224faeed0d9a9822b946aac33bc Mon Sep 17 00:00:00 2001 From: Boii Date: Wed, 12 Aug 2026 10:24:31 +0800 Subject: [PATCH 2/6] fix(conversation): make temp-workspace check root-agnostic (#825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Make the "is this a temporary/auto-provisioned session workspace?" check **root-agnostic**, fixing a historical-debt bug where long-time users — whose conversation directories were migrated across data-dir layouts — had their temporary sessions wrongly rendered as **projects** in the sidebar. ### Root cause Both backends that decide "is this a temp workspace?" anchored on the *current* `data_dir` / `work_dir` root: 1. `aionui-conversation/src/service.rs` `is_temp_session_workspace` did `workspace.strip_prefix(work_dir.join("conversations"))` — after a migration `extra.workspace` holds an **absolute path under the old root**, so the strip failed and it returned `false`. 2. `aionui-conversation/src/convert.rs` badge `is_temporary_workspace` did `Path::new(ws).starts_with(data_dir)` — same current-root anchor, same false negative for old-root workspaces. Symptom chain (new sidebar `classify_unit`): a temp session with no `project_id` → path branch → `is_temp_session_workspace` false → not folded into Chats → `canonicalize` misses (old dir may still exist physically) → `GroupKey::Dir` → **rendered as a project**. ### Fix The auto/temp directory leaf has carried a `-temp-` marker across **every** historical layout (`{agent}-temp-{ts}`, dated `YYYY/MM/DD/{label}-temp-{id}`, `team-temp-{team_id}`). That marker is root-agnostic, so: - `is_temp_session_workspace` now scans the path components for the **last** `conversations` segment and matches the relative tail, dropping the `work_dir` prefix dependency. Each `is_auto_workspace_relative_path` arm is **tightened** to additionally require `leaf.contains("-temp-")`, so a real user project like `/x/conversations/myproj` is not misclassified. - The `convert.rs` badge now delegates to the same `is_temp_session_workspace` predicate instead of a raw `starts_with(data_dir)`. - The sidebar call site drops the now-removed `work_dir` argument. This covers the migration case with negligible false-positive risk (user projects almost never have a `-temp-` leaf) and keeps the heuristic as the foundation — no schema migration / backfill needed. ## Testing ```bash cargo fmt --all cargo test -p aionui-conversation -p aionui-sidebar ``` Added regression tests deliberately mixing an **old-root temp workspace** with a **user project**, per our verification discipline: - `is_temp_session_workspace`: current-root temp → true (no regression); **migrated old-root** temp → true (the fix); team temp → true; legacy bare leaf → true; user project `/home/me/conversations/myproj` → **false**; no `conversations` segment → false; bad date → false. - `convert.rs` badge: old-root workspace → `is_temporary_workspace=true`; user project under data_dir → false. - sidebar `classify_unit`: no `project_id` + old-root temp workspace → `GroupKey::Chats`, no longer `GroupKey::Dir`. All green. ## Cross-platform Path handling uses component iteration (no hardcoded separators); tests exercise Unix-style absolute paths. No platform-specific branches introduced. --- crates/aionui-conversation/src/convert.rs | 57 +++++++++++++-- crates/aionui-conversation/src/service.rs | 56 ++++++++++----- .../aionui-conversation/src/service_test.rs | 72 +++++++++++++++++++ crates/aionui-sidebar/src/service.rs | 2 +- crates/aionui-sidebar/src/service_test.rs | 40 ++++++++++- 5 files changed, 198 insertions(+), 29 deletions(-) diff --git a/crates/aionui-conversation/src/convert.rs b/crates/aionui-conversation/src/convert.rs index 772a3968c..d67f0582a 100644 --- a/crates/aionui-conversation/src/convert.rs +++ b/crates/aionui-conversation/src/convert.rs @@ -8,6 +8,7 @@ use aionui_db::MessageSearchRow; use aionui_db::models::{ConversationArtifactRow, ConversationRow, MessageRow}; use crate::ConversationError; +use crate::service::is_temp_session_workspace; pub(crate) const TOOL_CONTENT_COMPACT_THRESHOLD_BYTES: usize = 64 * 1024; const TOOL_CONTENT_PREVIEW_CHARS: usize = 4096; @@ -29,19 +30,26 @@ pub fn row_to_response(row: ConversationRow, data_dir: &Path) -> Result Result { let is_temporary_workspace = { let ws = extra.get("workspace").and_then(|v| v.as_str()).unwrap_or(""); - !ws.is_empty() && Path::new(ws).starts_with(data_dir) + !ws.is_empty() && is_temp_session_workspace(Path::new(ws)) }; if let Some(obj) = extra.as_object_mut() { obj.remove("preset_context"); @@ -499,6 +507,41 @@ mod tests { assert_eq!(resp.extra["is_temporary_workspace"], false); } + // Historical-debt regression: a temp workspace persisted under a PREVIOUS + // data-dir root (user migrated their conversation directory across + // releases) must still be flagged temporary, even though it no longer sits + // under the current `data_dir`. The `-temp-` leaf marker is what makes this + // root-agnostic recognition safe. + #[test] + fn row_to_response_marks_migrated_root_temp_workspace_as_temporary() { + let row = make_row( + "acp", + "pending", + Some("aionui"), + None, + r#"{"workspace":"/old-data/aionui/conversations/users/u1/2025/01/02/acp-temp-conv-1"}"#, + ); + // Current data_dir differs from the old root the workspace lives under. + let resp = row_to_response(row, Path::new("/srv/aionui-data")).unwrap(); + assert_eq!(resp.extra["is_temporary_workspace"], true); + } + + // Negative guard for the root-agnostic match: a user project that merely + // happens to sit under some `conversations/` ancestor must NOT be flagged + // temporary, because its leaf carries no `-temp-` marker. + #[test] + fn row_to_response_does_not_mark_user_project_under_conversations_as_temporary() { + let row = make_row( + "acp", + "pending", + Some("aionui"), + None, + r#"{"workspace":"/home/me/conversations/myproj"}"#, + ); + let resp = row_to_response(row, Path::new("/srv/aionui-data")).unwrap(); + assert_eq!(resp.extra["is_temporary_workspace"], false); + } + #[test] fn row_with_pinned_at() { let row = ConversationRow { diff --git a/crates/aionui-conversation/src/service.rs b/crates/aionui-conversation/src/service.rs index f6eef1992..97a979270 100644 --- a/crates/aionui-conversation/src/service.rs +++ b/crates/aionui-conversation/src/service.rs @@ -4481,32 +4481,52 @@ fn is_auto_workspace_relative_path(relative: &Path) -> bool { && day.chars().all(|ch| ch.is_ascii_digit()) }; + // Auto/temp workspace leaves are always `{label}-temp-{id}` (conversations) + // or `team-temp-{team_id}` (teams); the `-temp-` marker has been stable + // across every historical layout. Requiring it keeps the root-agnostic + // match (see [`is_temp_session_workspace`]) from misclassifying a user + // directory that merely sits under some `conversations/` ancestor. + let is_temp_leaf = |leaf: &str| leaf.contains("-temp-"); + match parts.as_slice() { // legacy: bare leaf, or {Y}/{M}/{D}/leaf - [_file_name] => true, - [year, month, day, _file_name] => dated(year, month, day), + [leaf] => is_temp_leaf(leaf), + [year, month, day, leaf] => dated(year, month, day) && is_temp_leaf(leaf), // per-user, type-first: users/{user_dir}/{Y}/{M}/{D}/leaf - ["users", _user_dir, year, month, day, _file_name] => dated(year, month, day), + ["users", _user_dir, year, month, day, leaf] => dated(year, month, day) && is_temp_leaf(leaf), _ => false, } } -/// True when `workspace` is a backend auto-generated temp session directory -/// under `{work_dir}/conversations` — the sidebar read model's "temp path" test. +/// True when `workspace` is a backend auto-generated temp session directory — +/// the sidebar read model's "temp path" test. /// -/// Pure lexical prefix strip + [`is_auto_workspace_relative_path`]; performs no -/// filesystem access, so it is safe on the side-effect-free sidebar read path -/// (dead/removed workspaces classify correctly rather than failing an fs probe). -/// This is the same judgment the conversation service applies per row; it is -/// exposed only so the sidebar can classify a conversation's `extra.workspace` -/// (or a team's `workspace` column) without duplicating the rule. `work_dir` is -/// the application data directory (`ConversationService`'s `workspace_root`), -/// injected from the same source on both sides. -pub fn is_temp_session_workspace(work_dir: &Path, workspace: &Path) -> bool { - match workspace.strip_prefix(work_dir.join("conversations")) { - Ok(relative) => is_auto_workspace_relative_path(relative), - Err(_) => false, - } +/// Root-agnostic: matches the auto-workspace layout after the LAST +/// `conversations` path segment, regardless of which data-dir root precedes +/// it. This is deliberate. Users who migrated their conversation directory +/// across releases carry `extra.workspace` values baked under a *previous* +/// root; anchoring on the current `work_dir` would strip-fail on those and +/// misclassify historical temp sessions as projects. The layout after +/// `conversations/` has always ended in a `-temp-` leaf, so +/// [`is_auto_workspace_relative_path`] keys on that marker rather than the +/// (mutable) root prefix. +/// +/// Pure lexical (no filesystem access), so it is safe on the side-effect-free +/// sidebar read path — dead/removed workspaces classify correctly rather than +/// failing an fs probe. Exposed so the sidebar can classify a conversation's +/// `extra.workspace` (or a team's `workspace` column) without duplicating the +/// rule. +pub fn is_temp_session_workspace(workspace: &Path) -> bool { + let parts = workspace.iter().map(|part| part.to_str()).collect::>>(); + let Some(parts) = parts else { + return false; + }; + // Classify the tail after the LAST `conversations` segment (root-agnostic). + let Some(idx) = parts.iter().rposition(|part| *part == "conversations") else { + return false; + }; + let relative: PathBuf = parts[idx + 1..].iter().collect(); + is_auto_workspace_relative_path(&relative) } async fn cleanup_empty_date_workspace_parents(workspace_root: &Path, workspace_path: &Path) { diff --git a/crates/aionui-conversation/src/service_test.rs b/crates/aionui-conversation/src/service_test.rs index a659199cf..b1a892c27 100644 --- a/crates/aionui-conversation/src/service_test.rs +++ b/crates/aionui-conversation/src/service_test.rs @@ -51,6 +51,7 @@ use serde_json::json; use tokio::sync::{Notify, broadcast}; use crate::service::ConversationService; +use crate::service::is_temp_session_workspace; use crate::skill_resolver::{FixedSkillResolver, ResolvedAgentSkill, SkillResolver}; use crate::{ConversationAgentTurnRequest, ConversationAgentTurnStatus, ConversationError}; @@ -8439,3 +8440,74 @@ async fn a_deferred_cancel_does_not_leak_into_a_later_turn() { // Consumed exactly once. assert!(!svc.runtime_state().take_deferred_cancel(&conv.id, "turn_old")); } + +// --- is_temp_session_workspace: root-agnostic temp-directory judgment --- +// +// These lock the historical-debt fix: a temp workspace is recognized by the +// auto-workspace layout after the LAST `conversations` segment plus a `-temp-` +// leaf marker, regardless of which data-dir root precedes it. Users who +// migrated their conversation directory across releases carry workspaces baked +// under a *previous* root; those must still classify as temp. + +#[test] +fn temp_workspace_current_root_is_recognized() { + // No regression for workspaces under the live root. + assert!(is_temp_session_workspace(Path::new( + "/srv/aionui-data/conversations/users/u1/2026/08/11/acp-temp-conv-1" + ))); +} + +#[test] +fn temp_workspace_migrated_old_root_is_recognized() { + // Core fix: a different (older) root prefix must not defeat the judgment. + assert!(is_temp_session_workspace(Path::new( + "/old-data/aionui/conversations/users/u1/2025/01/02/acp-temp-conv-1" + ))); +} + +#[test] +fn temp_workspace_team_leaf_is_recognized() { + assert!(is_temp_session_workspace(Path::new( + "/srv/aionui-data/conversations/users/u1/2026/08/11/team-temp-team_1" + ))); +} + +#[test] +fn temp_workspace_legacy_bare_leaf_is_recognized() { + // Earliest layout: no dated dirs, just a `-temp-` leaf under conversations. + assert!(is_temp_session_workspace(Path::new( + "/old/conversations/claude-temp-xyz" + ))); +} + +#[test] +fn temp_workspace_legacy_dated_leaf_is_recognized() { + assert!(is_temp_session_workspace(Path::new( + "/old/conversations/2025/01/02/acp-temp-conv-1" + ))); +} + +#[test] +fn user_project_under_conversations_is_not_temp() { + // Negative guard: a user directory that merely sits under some + // `conversations/` ancestor lacks the `-temp-` marker → not temp. + assert!(!is_temp_session_workspace(Path::new("/home/me/conversations/myproj"))); +} + +#[test] +fn workspace_without_conversations_segment_is_not_temp() { + assert!(!is_temp_session_workspace(Path::new("/home/me/work/proj"))); +} + +#[test] +fn temp_workspace_with_bad_date_is_not_temp() { + // Dated arm still requires numeric YYYY/MM/DD. + assert!(!is_temp_session_workspace(Path::new( + "/srv/aionui-data/conversations/users/u1/YY/MM/DD/acp-temp-1" + ))); +} + +#[test] +fn bare_conversations_dir_is_not_temp() { + assert!(!is_temp_session_workspace(Path::new("/srv/aionui-data/conversations"))); +} diff --git a/crates/aionui-sidebar/src/service.rs b/crates/aionui-sidebar/src/service.rs index 9a0fc56c5..082e668e7 100644 --- a/crates/aionui-sidebar/src/service.rs +++ b/crates/aionui-sidebar/src/service.rs @@ -630,7 +630,7 @@ impl SidebarService { _ => return GroupKey::Chats, }; let ws_path = Path::new(ws); - if is_temp_session_workspace(&self.work_dir, ws_path) { + if is_temp_session_workspace(ws_path) { return GroupKey::Chats; } let uri = match canonical::to_file_uri(ws_path) { diff --git a/crates/aionui-sidebar/src/service_test.rs b/crates/aionui-sidebar/src/service_test.rs index 8cf200a29..601d14153 100644 --- a/crates/aionui-sidebar/src/service_test.rs +++ b/crates/aionui-sidebar/src/service_test.rs @@ -36,10 +36,18 @@ impl Fixture { self.db.pool() } - /// A temp-session workspace path under `work_dir/conversations/` — the - /// same shape the write side auto-assigns, which classifies to chats (case 5). + /// A temp-session workspace path under + /// `work_dir/conversations/