diff --git a/crates/command-contract/src/facets.rs b/crates/command-contract/src/facets.rs index c5efbf61cf..4e384a6f41 100644 --- a/crates/command-contract/src/facets.rs +++ b/crates/command-contract/src/facets.rs @@ -112,6 +112,7 @@ pub trait CommandMediaContext { fn attach_media(&mut self, resolved_path: &Path) -> Result; } +// --------------------------------------------------------------------------- // Project (FEAT-021 D1/D2/D3/D4) // --------------------------------------------------------------------------- @@ -206,6 +207,128 @@ pub trait CommandProjectContext { fn goal_state(&self) -> ProjectGoalState; } +// --------------------------------------------------------------------------- +// Memory (FEAT-019 D1/D2/D8/D9) +// --------------------------------------------------------------------------- + +/// Portable semantic hit for a native-memory search or get result. +/// +/// Carries only the typed location and text the handler consumes for +/// formatting; the TUI-owned `NativeMemoryHit` never crosses the boundary. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryHit { + pub source: PathBuf, + pub line_start: usize, + pub line_end: usize, + pub text: String, +} + +/// Portable native-memory location summary (status operation). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryStatus { + pub root: PathBuf, + pub source: PathBuf, + pub index: PathBuf, +} + +/// Portable result of a successful remember operation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryRemembered { + pub source: PathBuf, + pub line_start: usize, +} + +/// Portable import outcome: imported (with destination) or skipped. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MemoryImportOutcome { + Imported { destination: PathBuf }, + Skipped, +} + +/// Portable get outcome: found hit or explicit not-found. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MemoryGetOutcome { + Found(MemoryHit), + NotFound, +} + +/// Portable export payload — the exported memory document itself, never a +/// preformatted command response. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryExport { + pub content: String, +} + +/// Portable reindex entry count. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MemoryReindex { + pub entry_count: usize, +} + +/// Zero-field success value for delete operations (D2): the handler already +/// owns the selected scope and needs no additional success data. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct MemoryDelete; + +/// Typed remember target (D9): the handler resolves workspace identity through +/// the workspace facet and passes the resulting typed ID here. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MemoryRememberTarget { + Global, + Workspace { workspace_id: String }, +} + +/// Typed delete scope for the non-workspace delete method (D8/D9). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MemoryDeleteScope { + /// Delete every memory entry (global and all workspace scopes). + All, + /// Delete only the global scope entries. + Global, +} + +/// Host memory data for the memory command group (FEAT-019 D1). +/// +/// Exposes the resolved user-memory file path, the enablement flag, and one +/// typed method per exposed native-memory operation. All results are +/// contract-owned portable values; implementation errors cross as safe text. +/// Workspace-scoped operations take the borrowed workspace path as their first +/// argument (D8); non-workspace operations never receive workspace authority +/// and the facet never captures or retains workspace state internally. +pub trait CommandMemoryContext { + /// The resolved user-memory file path. + fn memory_path(&self) -> PathBuf; + /// Whether the `[memory] enabled` / `DEEPSEEK_MEMORY=on` flag is set. + fn memory_enabled(&self) -> bool; + /// Native-memory root, global source, and index paths. + fn status(&self) -> Result; + /// The native-memory root path. + fn path(&self) -> Result; + /// Workspace identity for the given workspace path. + fn workspace_id(&self, workspace: &Path) -> Result; + /// Workspace-scoped search over the native-memory store. + fn search(&self, workspace: &Path, query: &str, limit: usize) + -> Result, String>; + /// Append a reviewed note to the typed global or workspace target. + fn remember( + &self, + target: MemoryRememberTarget, + note: &str, + ) -> Result; + /// Import legacy memory; distinguishes imported from skipped. + fn import(&self) -> Result; + /// Workspace-scoped get by entry id; not-found is a typed outcome. + fn get(&self, workspace: &Path, id: i64) -> Result; + /// Export the native-memory document content. + fn export(&self) -> Result; + /// Reindex the native-memory store; returns the indexed entry count. + fn reindex(&self) -> Result; + /// Delete all or global scope; never receives workspace authority. + fn delete(&self, scope: MemoryDeleteScope) -> Result; + /// Delete the given workspace scope; workspace path is the first argument. + fn delete_workspace(&self, workspace: &Path) -> Result; +} + // --------------------------------------------------------------------------- // Skill group (FEAT-022 D1) // --------------------------------------------------------------------------- diff --git a/crates/command-contract/src/handler.rs b/crates/command-contract/src/handler.rs index 592a297368..36cf62d36e 100644 --- a/crates/command-contract/src/handler.rs +++ b/crates/command-contract/src/handler.rs @@ -5,16 +5,67 @@ //! `CommandHandler`. use crate::facets::{ - CommandCostContext, CommandMediaContext, CommandModePolicyContext, CommandModelContext, - CommandPresentationContext, CommandProjectContext, CommandSessionContext, + CommandCostContext, CommandMediaContext, CommandMemoryContext, CommandModePolicyContext, + CommandModelContext, CommandPresentationContext, CommandProjectContext, CommandSessionContext, CommandSkillGroupContext, CommandSkillsContext, CommandSystemPromptContext, CommandWorkspaceContext, }; + +/// Exact host capabilities exposed to one contextual command handler. +/// +/// The set lives in the external contract crate so command registrations can +/// declare least authority without naming the TUI host. The dispatcher uses +/// the declaration to populate only those slots in [`CommandContexts`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct CommandCapabilities(u16); + +impl CommandCapabilities { + pub const NONE: Self = Self(0); + pub const SESSION: Self = Self(1 << 0); + pub const MODEL: Self = Self(1 << 1); + pub const COST: Self = Self(1 << 2); + pub const MODE_POLICY: Self = Self(1 << 3); + pub const SYSTEM_PROMPT: Self = Self(1 << 4); + pub const SKILLS: Self = Self(1 << 5); + pub const WORKSPACE: Self = Self(1 << 6); + pub const PRESENTATION: Self = Self(1 << 7); + pub const MEDIA: Self = Self(1 << 8); + /// Memory-group host data (FEAT-019 D1). + pub const MEMORY: Self = Self(1 << 9); + /// Project-group host data (FEAT-021 D1). + pub const PROJECT: Self = Self(1 << 10); + /// Skills-group host data (FEAT-022 D1). + pub const SKILL_GROUP: Self = Self(1 << 11); + + pub const fn union(self, other: Self) -> Self { + Self(self.0 | other.0) + } + + pub const fn contains(self, capability: Self) -> bool { + !capability.is_empty() && self.0 & capability.0 == capability.0 + } + + pub const fn is_empty(self) -> bool { + self.0 == 0 + } +} + +impl std::ops::BitOr for CommandCapabilities { + type Output = Self; + + fn bitor(self, rhs: Self) -> Self::Output { + self.union(rhs) + } +} + /// A command handler that is either argument-only or capability-scoped. #[derive(Clone, Copy)] pub enum CommandHandler { Pure(fn(Option<&str>) -> R), - Contextual(fn(CommandContexts<'_>, Option<&str>) -> R), + Contextual { + capabilities: CommandCapabilities, + handler: fn(CommandContexts<'_>, Option<&str>) -> R, + }, } /// Transport envelope with one independently optional facet slot. @@ -28,6 +79,7 @@ pub struct CommandContexts<'a> { workspace: Option<&'a mut dyn CommandWorkspaceContext>, presentation: Option<&'a mut dyn CommandPresentationContext>, media: Option<&'a mut dyn CommandMediaContext>, + memory: Option<&'a mut dyn CommandMemoryContext>, project: Option<&'a mut dyn CommandProjectContext>, skill_group: Option<&'a mut dyn CommandSkillGroupContext>, } @@ -43,6 +95,7 @@ pub struct ContextParts<'a> { pub workspace: Option<&'a mut dyn CommandWorkspaceContext>, pub presentation: Option<&'a mut dyn CommandPresentationContext>, pub media: Option<&'a mut dyn CommandMediaContext>, + pub memory: Option<&'a mut dyn CommandMemoryContext>, pub project: Option<&'a mut dyn CommandProjectContext>, pub skill_group: Option<&'a mut dyn CommandSkillGroupContext>, } @@ -59,6 +112,7 @@ impl<'a> CommandContexts<'a> { workspace: None, presentation: None, media: None, + memory: None, project: None, skill_group: None, } @@ -75,6 +129,7 @@ impl<'a> CommandContexts<'a> { workspace: self.workspace, presentation: self.presentation, media: self.media, + memory: self.memory, project: self.project, skill_group: self.skill_group, } @@ -149,6 +204,14 @@ impl<'a> CommandContexts<'a> { self } + pub fn with_memory(mut self, value: &'a mut dyn CommandMemoryContext) -> Self { + assert!( + self.memory.replace(value).is_none(), + "memory facet already set" + ); + self + } + pub fn with_project(mut self, value: &'a mut dyn CommandProjectContext) -> Self { assert!( self.project.replace(value).is_none(), @@ -160,7 +223,7 @@ impl<'a> CommandContexts<'a> { pub fn with_skill_group(mut self, value: &'a mut dyn CommandSkillGroupContext) -> Self { assert!( self.skill_group.replace(value).is_none(), - "skill_group facet already set" + "skill-group facet already set" ); self } diff --git a/crates/command-contract/src/lib.rs b/crates/command-contract/src/lib.rs index 40439e84a2..eb8136f1fe 100644 --- a/crates/command-contract/src/lib.rs +++ b/crates/command-contract/src/lib.rs @@ -12,7 +12,7 @@ pub mod metadata; pub mod types; pub use facets::*; -pub use handler::{CommandContexts, CommandHandler, ContextParts}; +pub use handler::{CommandCapabilities, CommandContexts, CommandHandler, ContextParts}; pub use metadata::{CommandDiscovery, CommandInfo, RegisterCommand}; pub use types::*; diff --git a/crates/command-contract/src/tests.rs b/crates/command-contract/src/tests.rs index 918f9b66f7..52546f3ab8 100644 --- a/crates/command-contract/src/tests.rs +++ b/crates/command-contract/src/tests.rs @@ -156,13 +156,20 @@ fn contextual(_contexts: CommandContexts<'_>, value: Option<&str>) -> String { #[test] fn handlers_are_plain_function_pointers() { let pure_handler = CommandHandler::Pure(pure); - let contextual_handler = CommandHandler::Contextual(contextual); + let contextual_handler = CommandHandler::Contextual { + capabilities: CommandCapabilities::NONE, + handler: contextual, + }; match pure_handler { CommandHandler::Pure(handler) => assert_eq!(handler(Some("x")), "x"), _ => unreachable!(), } match contextual_handler { - CommandHandler::Contextual(handler) => { + CommandHandler::Contextual { + capabilities, + handler, + } => { + assert!(capabilities.is_empty()); assert_eq!(handler(CommandContexts::empty(), Some("y")), "y") } _ => unreachable!(), @@ -345,6 +352,7 @@ fn envelope_rejects_duplicate_new_slots_deterministically() { assert!(result.is_err(), "duplicate media slot must assert"); } +// --------------------------------------------------------------------------- // Project facet (FEAT-021 D1/D4) // --------------------------------------------------------------------------- @@ -405,6 +413,222 @@ impl CommandProjectContext for FakeProject { } } +// --------------------------------------------------------------------------- +// FEAT-019: memory capability, typed outcomes, and workspace scoping (D1-D9) +// --------------------------------------------------------------------------- + +/// Deterministic fake memory facet over portable values only. Tracks the +/// workspace argument discipline (D8): only workspace-scoped methods receive +/// the workspace path. +struct FakeMemory { + hits: Vec, + remembered_result: Option, + workspace_id_result: Result, +} + +impl FakeMemory { + fn new() -> Self { + Self { + hits: vec![MemoryHit { + source: PathBuf::from("/mem/source.md"), + line_start: 3, + line_end: 5, + text: "reviewed note".to_string(), + }], + remembered_result: Some(MemoryRemembered { + source: PathBuf::from("/mem/global.md"), + line_start: 7, + }), + workspace_id_result: Ok("owner/repo".to_string()), + } + } +} + +impl CommandMemoryContext for FakeMemory { + fn memory_path(&self) -> PathBuf { + PathBuf::from("/mem/user-memory.md") + } + + fn memory_enabled(&self) -> bool { + true + } + + fn status(&self) -> Result { + Ok(MemoryStatus { + root: PathBuf::from("/mem/memory"), + source: PathBuf::from("/mem/memory/global/global.md"), + index: PathBuf::from("/mem/memory/index.db"), + }) + } + + fn path(&self) -> Result { + Ok(PathBuf::from("/mem/memory")) + } + + fn workspace_id(&self, _workspace: &Path) -> Result { + self.workspace_id_result.clone() + } + + fn search( + &self, + _workspace: &Path, + query: &str, + limit: usize, + ) -> Result, String> { + if query.is_empty() { + return Ok(Vec::new()); + } + Ok(self.hits.iter().take(limit).cloned().collect()) + } + + fn remember( + &self, + _target: MemoryRememberTarget, + note: &str, + ) -> Result { + if note.is_empty() { + return Err("empty note".to_string()); + } + Ok(self.remembered_result.clone().unwrap_or(MemoryRemembered { + source: PathBuf::from("/mem/global.md"), + line_start: 1, + })) + } + + fn import(&self) -> Result { + Ok(MemoryImportOutcome::Skipped) + } + + fn get(&self, _workspace: &Path, id: i64) -> Result { + if id == 42 { + Ok(MemoryGetOutcome::Found(self.hits[0].clone())) + } else { + Ok(MemoryGetOutcome::NotFound) + } + } + + fn export(&self) -> Result { + Ok(MemoryExport { + content: "# memory\n\n- bullet".to_string(), + }) + } + + fn reindex(&self) -> Result { + Ok(MemoryReindex { entry_count: 3 }) + } + + fn delete(&self, scope: MemoryDeleteScope) -> Result { + match scope { + MemoryDeleteScope::All => Ok(MemoryDelete), + MemoryDeleteScope::Global => Ok(MemoryDelete), + } + } + + fn delete_workspace(&self, _workspace: &Path) -> Result { + Ok(MemoryDelete) + } +} + +/// Recording fake that captures remember targets and delete scopes to prove +/// the typed target/scope discipline (D2/D8/D9). Interior mutability lets the +/// contract-level test assert exactly which operations the handler drives. +#[derive(Default)] +struct RecordingMemory { + remembered_targets: std::cell::RefCell>, + delete_scopes: std::cell::RefCell>, + workspace_deletes: std::cell::Cell, +} + +impl RecordingMemory { + fn new() -> Self { + Self::default() + } + + fn recorded_targets(&self) -> Vec { + self.remembered_targets.borrow().clone() + } + + fn recorded_delete_scopes(&self) -> Vec { + self.delete_scopes.borrow().clone() + } + + fn recorded_workspace_deletes(&self) -> usize { + self.workspace_deletes.get() + } +} + +impl CommandMemoryContext for RecordingMemory { + fn memory_path(&self) -> PathBuf { + PathBuf::from("/mem/user-memory.md") + } + + fn memory_enabled(&self) -> bool { + true + } + + fn status(&self) -> Result { + unreachable!("recording fake") + } + + fn path(&self) -> Result { + unreachable!("recording fake") + } + + fn workspace_id(&self, _workspace: &Path) -> Result { + Ok("owner/repo".to_string()) + } + + fn search( + &self, + _workspace: &Path, + _query: &str, + _limit: usize, + ) -> Result, String> { + unreachable!("recording fake") + } + + fn remember( + &self, + target: MemoryRememberTarget, + _note: &str, + ) -> Result { + self.remembered_targets.borrow_mut().push(target); + Ok(MemoryRemembered { + source: PathBuf::from("/mem/global.md"), + line_start: 1, + }) + } + + fn import(&self) -> Result { + unreachable!("recording fake") + } + + fn get(&self, _workspace: &Path, _id: i64) -> Result { + unreachable!("recording fake") + } + + fn export(&self) -> Result { + unreachable!("recording fake") + } + + fn reindex(&self) -> Result { + unreachable!("recording fake") + } + + fn delete(&self, scope: MemoryDeleteScope) -> Result { + self.delete_scopes.borrow_mut().push(match scope { + MemoryDeleteScope::All => "all".to_string(), + MemoryDeleteScope::Global => "global".to_string(), + }); + Ok(MemoryDelete) + } + + fn delete_workspace(&self, _workspace: &Path) -> Result { + self.workspace_deletes.set(self.workspace_deletes.get() + 1); + Ok(MemoryDelete) + } +} + #[test] fn project_facet_is_object_safe_and_typed() { fn project(_: &dyn CommandProjectContext) {} @@ -499,7 +723,179 @@ fn envelope_rejects_duplicate_project_slot_deterministically() { assert!(result.is_err(), "duplicate project slot must assert"); } -// --------------------------------------------------------------------------- +#[test] +fn memory_facet_is_object_safe_and_typed() { + fn memory(_: &dyn CommandMemoryContext) {} + let fake = FakeMemory::new(); + memory(&fake); + + assert_eq!(fake.memory_path(), PathBuf::from("/mem/user-memory.md")); + assert!(fake.memory_enabled()); + let status = fake.status().expect("status"); + assert_eq!(status.root, PathBuf::from("/mem/memory")); + assert_eq!(status.source, PathBuf::from("/mem/memory/global/global.md")); + assert_eq!(status.index, PathBuf::from("/mem/memory/index.db")); +} + +#[test] +fn memory_typed_results_preserve_semantic_distinctions() { + let fake = FakeMemory::new(); + + // Search returns semantic hits, never preformatted messages. + let hits = fake.search(Path::new("/ws"), "note", 10).expect("search"); + assert_eq!(hits.len(), 1); + assert_eq!(hits[0].source, PathBuf::from("/mem/source.md")); + assert_eq!(hits[0].line_start, 3); + assert_eq!(hits[0].line_end, 5); + assert_eq!(hits[0].text, "reviewed note"); + assert!( + fake.search(Path::new("/ws"), "", 10) + .expect("empty") + .is_empty() + ); + + // Get distinguishes found from not-found without an error string. + assert!(matches!( + fake.get(Path::new("/ws"), 42), + Ok(MemoryGetOutcome::Found(_)) + )); + assert_eq!( + fake.get(Path::new("/ws"), 1).expect("get"), + MemoryGetOutcome::NotFound + ); + + // Export carries the raw document, not a command response. + let exported = fake.export().expect("export"); + assert_eq!(exported.content, "# memory\n\n- bullet"); + + // Reindex carries the typed count. + assert_eq!(fake.reindex().expect("reindex").entry_count, 3); + + // Remember distinguishes global from workspace via the typed target. + let global = fake + .remember(MemoryRememberTarget::Global, "note") + .expect("global remember"); + assert_eq!(global.source, PathBuf::from("/mem/global.md")); + assert_eq!(global.line_start, 7); + let workspace = fake + .remember( + MemoryRememberTarget::Workspace { + workspace_id: "owner/repo".to_string(), + }, + "note", + ) + .expect("workspace remember"); + assert_eq!(workspace.source, PathBuf::from("/mem/global.md")); + + // Import distinguishes imported from skipped. + assert_eq!(fake.import().expect("import"), MemoryImportOutcome::Skipped); + assert_eq!( + MemoryImportOutcome::Imported { + destination: PathBuf::from("/mem/global.md") + }, + MemoryImportOutcome::Imported { + destination: PathBuf::from("/mem/global.md") + } + ); + + // Remember rejects empty notes with a safe error, never a panic. + assert!(fake.remember(MemoryRememberTarget::Global, "").is_err()); + + // Zero-field delete outcome stays distinguishable. + assert_eq!(fake.delete(MemoryDeleteScope::All), Ok(MemoryDelete)); +} + +#[test] +fn memory_delete_and_remember_targets_are_typed_and_scoped() { + let memory = RecordingMemory::new(); + let _ = memory.delete(MemoryDeleteScope::All); + let _ = memory.delete(MemoryDeleteScope::Global); + let _ = memory.delete_workspace(Path::new("/ws")); + let _ = memory.remember(MemoryRememberTarget::Global, "a"); + let _ = memory.remember( + MemoryRememberTarget::Workspace { + workspace_id: "owner/repo".to_string(), + }, + "b", + ); + + // The non-workspace delete method receives exactly the all/global scopes; + // workspace deletion goes through the distinct typed method (D8/D9). + assert_eq!(memory.recorded_delete_scopes(), vec!["all", "global"]); + assert_eq!(memory.recorded_workspace_deletes(), 1); + + // Remember targets preserve the typed global/workspace distinction. + assert_eq!( + memory.recorded_targets(), + vec![ + MemoryRememberTarget::Global, + MemoryRememberTarget::Workspace { + workspace_id: "owner/repo".to_string(), + }, + ] + ); +} + +#[test] +fn capabilities_declare_exact_memory_authority() { + let workspace = CommandCapabilities::WORKSPACE; + let memory = CommandCapabilities::MEMORY; + let workspace_memory = workspace.union(memory); + + assert_eq!( + workspace_memory, + CommandCapabilities::WORKSPACE | CommandCapabilities::MEMORY + ); + assert_ne!(workspace_memory, workspace); + assert_ne!(workspace_memory, memory); + assert!(workspace_memory.contains(CommandCapabilities::WORKSPACE)); + assert!(workspace_memory.contains(CommandCapabilities::MEMORY)); + assert!(!workspace.contains(CommandCapabilities::MEMORY)); + assert!(!memory.contains(CommandCapabilities::WORKSPACE)); + assert!(CommandCapabilities::NONE.is_empty()); + assert!(!workspace_memory.contains(CommandCapabilities::NONE)); + assert!(!CommandCapabilities::NONE.contains(CommandCapabilities::NONE)); + // No presentation or media authority is declared for the memory group. + assert!(!workspace_memory.contains(CommandCapabilities::PRESENTATION)); + assert!(!workspace_memory.contains(CommandCapabilities::MEDIA)); + // Existing capability identities stay stable. + assert_ne!(CommandCapabilities::SESSION, CommandCapabilities::MODEL); +} + +#[test] +fn memory_facet_transports_through_envelope_when_declared() { + let mut memory = FakeMemory::new(); + let parts = CommandContexts::empty() + .with_memory(&mut memory) + .into_parts(); + assert!(parts.memory.is_some()); + assert!(parts.session.is_none()); + assert!(parts.workspace.is_none()); + + // Undeclared slots stay absent when the memory facet is carried alone. + let mut workspace = Workspace; + let parts = CommandContexts::empty() + .with_memory(&mut memory) + .with_workspace(&mut workspace) + .into_parts(); + assert!(parts.memory.is_some()); + assert!(parts.workspace.is_some()); + assert!(parts.presentation.is_none()); + assert!(parts.media.is_none()); +} + +#[test] +fn envelope_rejects_duplicate_memory_slot_deterministically() { + let mut a = FakeMemory::new(); + let mut b = FakeMemory::new(); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + CommandContexts::empty() + .with_memory(&mut a) + .with_memory(&mut b); + })); + assert!(result.is_err(), "duplicate memory slot must assert"); +} + // FEAT-022: skill-group facet (CommandSkillGroupContext) // --------------------------------------------------------------------------- diff --git a/crates/tui/src/commands/contract.rs b/crates/tui/src/commands/contract.rs index 054c6c840d..777cf82941 100644 --- a/crates/tui/src/commands/contract.rs +++ b/crates/tui/src/commands/contract.rs @@ -14,7 +14,7 @@ //! //! ## Authoritative host-proxy design (D1) //! -//! `CommandContexts` holds seven independently borrowed facet objects, while +//! `CommandContexts` holds eleven independently borrowed facet objects, while //! important behavior (mode transitions, model invalidation, cost accounting, //! skill refresh) is authoritative on `App`. The adapters therefore share a //! synchronous TUI-owned host proxy. Each trait call borrows `App` only for the @@ -32,18 +32,20 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use codewhale_command_contract::facets::{ - CommandApprovalState, CommandCostContext, CommandMediaContext, CommandModePolicyContext, - CommandModelContext, CommandPresentationContext, CommandProjectContext, CommandSessionContext, - CommandSkillGroupContext, CommandSkillsContext, CommandSystemPromptContext, - CommandWorkspaceContext, MediaAttachmentReceipt, ProjectGoalState, ProjectGoalStatus, - ProjectShareProjection, RemoteRegistryOutcome, RemoteSkillEntry, ReviewOutcome, - SkillActivationError, SkillActivationOutcome, SkillBundledTier, SkillEntry, + CommandApprovalState, CommandCostContext, CommandMediaContext, CommandMemoryContext, + CommandModePolicyContext, CommandModelContext, CommandPresentationContext, + CommandProjectContext, CommandSessionContext, CommandSkillGroupContext, CommandSkillsContext, + CommandSystemPromptContext, CommandWorkspaceContext, MediaAttachmentReceipt, MemoryDelete, + MemoryDeleteScope, MemoryExport, MemoryGetOutcome, MemoryHit, MemoryImportOutcome, + MemoryReindex, MemoryRememberTarget, MemoryRemembered, MemoryStatus, ProjectGoalState, + ProjectGoalStatus, ProjectShareProjection, RemoteRegistryOutcome, RemoteSkillEntry, + ReviewOutcome, SkillActivationError, SkillActivationOutcome, SkillBundledTier, SkillEntry, SkillMutationOutcome, SkillMutationReceipt, SkillRecommendation, SkillRegistryProjection, SkillSourceKind, SkillSyncEntry, SkillSyncOutcome, SkillTargetScope, SnapshotEntry, }; -use codewhale_command_contract::handler::CommandContexts; #[cfg(test)] use codewhale_command_contract::handler::ContextParts; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts}; use codewhale_command_contract::types::{ CommandApprovalMode, CommandCurrency, CommandMode, CommandProviderId, CommandReasoningEffort, }; @@ -70,8 +72,7 @@ use crate::tui::history::HistoryCell; /// (`scripts/check-command-migration-manifest.py`) reads this exact /// declaration by source regex and the Rust frontier tests assert it. #[allow(dead_code)] -pub(crate) const PENDING_GROUPS: &[&str] = - &["config", "core", "debug", "memory", "plugins", "session"]; +pub(crate) const PENDING_GROUPS: &[&str] = &["config", "core", "debug", "plugins", "session"]; // --------------------------------------------------------------------------- // Boundary-value mappings (D8) @@ -262,7 +263,7 @@ pub(crate) fn key_to_message_id(key: &'static str) -> Option { /// Shared TUI host hidden behind the portable command facets. /// -/// The envelope needs seven independently borrowed facet objects, while the +/// The envelope needs ten independently borrowed facet objects, while the /// authoritative mutation methods live on `App`. Each adapter therefore owns /// an `Rc` clone of this synchronous host proxy. Trait calls borrow `App` only /// for the duration of one method, delegate to the real TUI authority, and @@ -646,6 +647,191 @@ fn media_kind(path: &Path) -> Option<&'static str> { } } +/// Memory host-data adapter (FEAT-019 D1). +/// +/// Derives the authoritative native store exactly like the legacy `/memory` +/// handler (`from_global_path` on the app memory path, falling back to a +/// `memory` root beside it) and converts every host value/error to a portable +/// contract value before it crosses the boundary. All methods are `&self` and +/// borrow `App` only for the duration of one call; workspace state is passed +/// per call and never retained by the facet (D8). +pub(crate) struct MemoryAdapter<'a> { + host: SharedCommandHost<'a>, +} + +/// Derive the authoritative native-memory store from the resolved user-memory +/// file path, mirroring the pre-migration `/memory` handler exactly. +fn native_store_from_memory_path(memory_path: &Path) -> crate::native_memory::NativeMemoryStore { + if let Some(store) = crate::native_memory::NativeMemoryStore::from_global_path(memory_path) { + return store; + } + let root = memory_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("memory"); + crate::native_memory::NativeMemoryStore::new(root) +} + +/// Convert a TUI-owned native hit into the portable contract hit. Only the +/// semantic fields the handler consumes for rendering cross the boundary (D2). +fn portable_hit(hit: crate::native_memory::MemoryHit) -> MemoryHit { + MemoryHit { + source: hit.source, + line_start: hit.line_start, + line_end: hit.line_end, + text: hit.text, + } +} + +impl CommandMemoryContext for MemoryAdapter<'_> { + fn memory_path(&self) -> PathBuf { + self.host.app.borrow().memory_path.clone() + } + + fn memory_enabled(&self) -> bool { + self.host.app.borrow().use_memory + } + + fn status(&self) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + Ok(MemoryStatus { + root: store.root().to_path_buf(), + source: store.global_path(), + index: store.index_path(), + }) + } + + fn path(&self) -> Result { + let app = self.host.app.borrow(); + Ok(native_store_from_memory_path(&app.memory_path) + .root() + .to_path_buf()) + } + + fn workspace_id(&self, workspace: &Path) -> Result { + match crate::native_memory::NativeMemoryStore::workspace_id(workspace) { + Ok(Some(id)) => Ok(id), + Ok(None) => { + Err("workspace memory requires a git repository with an origin".to_string()) + } + Err(err) => Err(format!("failed to resolve workspace identity: {err}")), + } + } + + fn search( + &self, + workspace: &Path, + query: &str, + limit: usize, + ) -> Result, String> { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + match store.search_for_workspace(workspace, query, limit) { + Ok(hits) => Ok(hits.into_iter().map(portable_hit).collect()), + Err(err) => Err(err.to_string()), + } + } + + fn remember( + &self, + target: MemoryRememberTarget, + note: &str, + ) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + let (scope, workspace_id) = match target { + MemoryRememberTarget::Global => (crate::native_memory::MemoryScope::Global, None), + MemoryRememberTarget::Workspace { workspace_id } => ( + crate::native_memory::MemoryScope::Workspace, + Some(workspace_id), + ), + }; + match store.remember(scope, workspace_id.as_deref(), note) { + Ok(hit) => Ok(MemoryRemembered { + source: hit.source, + line_start: hit.line_start, + }), + Err(err) => Err(err.to_string()), + } + } + + fn import(&self) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + let legacy_path = store + .root() + .parent() + .map(|parent| parent.join("memory.md")) + .unwrap_or_else(|| app.memory_path.clone()); + match store.import_legacy(&legacy_path) { + Ok(true) => Ok(MemoryImportOutcome::Imported { + destination: store.global_path(), + }), + Ok(false) => Ok(MemoryImportOutcome::Skipped), + Err(err) => Err(err.to_string()), + } + } + + fn get(&self, workspace: &Path, id: i64) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + match store.get_for_workspace(workspace, id) { + Ok(Some(hit)) => Ok(MemoryGetOutcome::Found(portable_hit(hit))), + Ok(None) => Ok(MemoryGetOutcome::NotFound), + Err(err) => Err(err.to_string()), + } + } + + fn export(&self) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + match store.export() { + Ok(content) => Ok(MemoryExport { content }), + Err(err) => Err(err.to_string()), + } + } + + fn reindex(&self) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + match store.reindex() { + Ok(entry_count) => Ok(MemoryReindex { entry_count }), + Err(err) => Err(err.to_string()), + } + } + + fn delete(&self, scope: MemoryDeleteScope) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + let result = match scope { + MemoryDeleteScope::All => store.delete_all(None, None), + MemoryDeleteScope::Global => { + store.delete_all(Some(crate::native_memory::MemoryScope::Global), None) + } + }; + result.map(|()| MemoryDelete).map_err(|err| err.to_string()) + } + + fn delete_workspace(&self, workspace: &Path) -> Result { + let app = self.host.app.borrow(); + let store = native_store_from_memory_path(&app.memory_path); + match crate::native_memory::NativeMemoryStore::workspace_id(workspace) { + Ok(Some(id)) => store + .delete_all( + Some(crate::native_memory::MemoryScope::Workspace), + Some(&id), + ) + .map(|()| MemoryDelete) + .map_err(|err| err.to_string()), + Ok(None) => { + Err("workspace memory requires a git repository with an origin".to_string()) + } + Err(err) => Err(format!("failed to resolve workspace identity: {err}")), + } + } +} + // --------------------------------------------------------------------------- // Project host adapter (FEAT-021 D1/D3) // --------------------------------------------------------------------------- @@ -1368,7 +1554,6 @@ impl CommandSkillGroupContext for SkillGroupAdapter<'_> { /// be called sequentially without exposing TUI types across the boundary. pub(crate) struct CommandContextBundle<'a> { session: SessionAdapter<'a>, - skill_group: SkillGroupAdapter<'a>, model: ModelAdapter<'a>, cost: CostAdapter<'a>, mode_policy: ModePolicyAdapter<'a>, @@ -1378,28 +1563,69 @@ pub(crate) struct CommandContextBundle<'a> { presentation: PresentationAdapter<'a>, media: MediaAdapter<'a>, project: ProjectAdapter<'a>, + memory: MemoryAdapter<'a>, + skill_group: SkillGroupAdapter<'a>, } impl<'a> CommandContextBundle<'a> { - pub(crate) fn contexts(&mut self) -> CommandContexts<'_> { - CommandContexts::empty() - .with_session(&mut self.session) - .with_model(&mut self.model) - .with_cost(&mut self.cost) - .with_mode_policy(&mut self.mode_policy) - .with_system_prompt(&mut self.system_prompt) - .with_skills(&mut self.skills) - .with_workspace(&mut self.workspace) - .with_presentation(&mut self.presentation) - .with_media(&mut self.media) - .with_project(&mut self.project) - .with_skill_group(&mut self.skill_group) + /// Expose exactly the capabilities declared by the command registration. + pub(crate) fn contexts(&mut self, capabilities: CommandCapabilities) -> CommandContexts<'_> { + let mut contexts = CommandContexts::empty(); + if capabilities.contains(CommandCapabilities::SESSION) { + contexts = contexts.with_session(&mut self.session); + } + if capabilities.contains(CommandCapabilities::MODEL) { + contexts = contexts.with_model(&mut self.model); + } + if capabilities.contains(CommandCapabilities::COST) { + contexts = contexts.with_cost(&mut self.cost); + } + if capabilities.contains(CommandCapabilities::MODE_POLICY) { + contexts = contexts.with_mode_policy(&mut self.mode_policy); + } + if capabilities.contains(CommandCapabilities::SYSTEM_PROMPT) { + contexts = contexts.with_system_prompt(&mut self.system_prompt); + } + if capabilities.contains(CommandCapabilities::SKILLS) { + contexts = contexts.with_skills(&mut self.skills); + } + if capabilities.contains(CommandCapabilities::WORKSPACE) { + contexts = contexts.with_workspace(&mut self.workspace); + } + if capabilities.contains(CommandCapabilities::PRESENTATION) { + contexts = contexts.with_presentation(&mut self.presentation); + } + if capabilities.contains(CommandCapabilities::MEDIA) { + contexts = contexts.with_media(&mut self.media); + } + if capabilities.contains(CommandCapabilities::MEMORY) { + contexts = contexts.with_memory(&mut self.memory); + } + if capabilities.contains(CommandCapabilities::PROJECT) { + contexts = contexts.with_project(&mut self.project); + } + if capabilities.contains(CommandCapabilities::SKILL_GROUP) { + contexts = contexts.with_skill_group(&mut self.skill_group); + } + contexts } /// Test-only: consume the bundle into independent facet parts. #[cfg(test)] pub(crate) fn parts(&mut self) -> ContextParts<'_> { - self.contexts().into_parts() + let all_test_capabilities = CommandCapabilities::SESSION + .union(CommandCapabilities::MODEL) + .union(CommandCapabilities::COST) + .union(CommandCapabilities::MODE_POLICY) + .union(CommandCapabilities::SYSTEM_PROMPT) + .union(CommandCapabilities::SKILLS) + .union(CommandCapabilities::WORKSPACE) + .union(CommandCapabilities::PRESENTATION) + .union(CommandCapabilities::MEDIA) + .union(CommandCapabilities::MEMORY) + .union(CommandCapabilities::PROJECT) + .union(CommandCapabilities::SKILL_GROUP); + self.contexts(all_test_capabilities).into_parts() } } @@ -1419,8 +1645,9 @@ impl App { skills: SkillsAdapter { host: host.clone() }, workspace: WorkspaceAdapter { host: host.clone() }, presentation: PresentationAdapter { host: host.clone() }, - project: ProjectAdapter { host: host.clone() }, media: MediaAdapter { host: host.clone() }, + project: ProjectAdapter { host: host.clone() }, + memory: MemoryAdapter { host: host.clone() }, skill_group: SkillGroupAdapter { host }, } } @@ -1890,11 +2117,11 @@ mod tests { // perform capability work; the adapters only run on method calls. let _ = parts.media.is_some(); let _ = parts.presentation.is_some(); + let _ = parts.memory.is_some(); + let _ = parts.project.is_some(); } assert_eq!(app.input, input_before, "no eager composer mutation"); } - - // --------------------------------------------------------------------- // FEAT-021 project adapter tests // --------------------------------------------------------------------- @@ -1939,6 +2166,100 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // FEAT-019: memory adapter mappings (D6/D9) + // ----------------------------------------------------------------------- + + /// App with an isolated temp memory file; memory feature enabled or not. + fn memory_test_app(tmpdir: &TempDir, use_memory: bool) -> App { + let options = crate::test_support::test_tui_options(tmpdir.path()); + let options = crate::tui::app::TuiOptions { + memory_path: tmpdir.path().join("memory.md"), + use_memory, + ..options + }; + crate::test_support::test_app_with_options(options) + } + + /// Give a temp workspace a git origin so workspace identity resolves. + fn git_origin(workspace: &Path) { + let init = std::process::Command::new("git") + .arg("-C") + .arg(workspace) + .args(["init", "-q"]) + .status() + .unwrap(); + assert!(init.success(), "git init must succeed"); + let remote = std::process::Command::new("git") + .arg("-C") + .arg(workspace) + .args(["remote", "add", "origin", "https://example.test/repo.git"]) + .status() + .unwrap(); + assert!(remote.success(), "git remote add must succeed"); + } + + #[test] + fn memory_adapter_maps_path_and_enablement() { + let tmp = TempDir::new().unwrap(); + let mut enabled = memory_test_app(&tmp, true); + let mut bundle = enabled.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet must be present"); + assert_eq!(memory.memory_path(), tmp.path().join("memory.md")); + assert!(memory.memory_enabled()); + + let mut disabled = memory_test_app(&tmp, false); + let mut bundle = disabled.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet must be present"); + assert!(!memory.memory_enabled()); + } + + #[test] + fn memory_adapter_status_and_path_map_native_store() { + let tmp = TempDir::new().unwrap(); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + + // Fallback root derivation mirrors the legacy handler: a plain + // `memory.md` file is not a native global source, so the root is the + // sibling `memory` directory. + let status = memory.status().expect("status"); + assert_eq!(status.root, tmp.path().join("memory")); + assert_eq!( + status.source, + tmp.path().join("memory").join("global").join("MEMORY.md") + ); + assert_eq!( + status.index, + tmp.path().join("memory").join("index.sqlite3") + ); + assert_eq!(memory.path().expect("path"), tmp.path().join("memory")); + } + + #[test] + fn memory_adapter_workspace_identity_resolves_and_preserves_errors() { + let tmp = TempDir::new().unwrap(); + git_origin(tmp.path()); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + // A git origin resolves to a stable workspace identity (sha256 digest). + let id = memory.workspace_id(tmp.path()).expect("workspace id"); + assert!(!id.is_empty()); + assert_eq!(id, memory.workspace_id(tmp.path()).expect("stable id")); + + // A plain directory without git origin preserves the established error. + let plain = TempDir::new().unwrap(); + let err = memory + .workspace_id(plain.path()) + .expect_err("missing origin"); + assert_eq!( + err, + "workspace memory requires a git repository with an origin" + ); + } + #[test] fn project_adapter_maps_lsp_state() { let mut app = test_app(); @@ -2081,6 +2402,207 @@ mod tests { assert!(parts.presentation.is_some()); } + #[test] + fn memory_adapter_search_remember_get_export_reindex_work() { + let tmp = TempDir::new().unwrap(); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + + // Global remember produces a portable remembered location. + let remembered = memory + .remember(MemoryRememberTarget::Global, "alpha note") + .expect("remember global"); + assert!(remembered.source.ends_with("global/MEMORY.md")); + assert_eq!(remembered.line_start, 2); + + // Workspace remember targets the workspace scope with the typed id. + git_origin(tmp.path()); + let workspace_id = memory.workspace_id(tmp.path()).expect("id"); + let workspace_note = memory + .remember( + MemoryRememberTarget::Workspace { workspace_id }, + "workspace-only note", + ) + .expect("remember workspace"); + assert!( + workspace_note + .source + .to_string_lossy() + .contains("workspace") + ); + + // Search finds workspace-scoped content only for the given workspace. + let hits = memory + .search(tmp.path(), "workspace-only", 10) + .expect("search"); + assert_eq!(hits.len(), 1); + assert!(hits[0].text.contains("workspace-only note")); + assert_eq!(hits[0].line_start, 2); + // Empty results stay a typed empty vec, never an error. + assert!( + memory + .search(tmp.path(), "zzz-no-match", 10) + .expect("empty search") + .is_empty() + ); + + // Get distinguishes found from not-found (first rowid is 1). + match memory.get(tmp.path(), 1) { + Ok(MemoryGetOutcome::Found(hit)) => assert!(!hit.text.is_empty()), + other => panic!("expected found entry, got {other:?}"), + } + assert_eq!( + memory.get(tmp.path(), 999_999).expect("get"), + MemoryGetOutcome::NotFound + ); + + // Export carries the document; reindex reports the typed count. + let exported = memory.export().expect("export"); + assert!(exported.content.contains("alpha note")); + assert!(exported.content.contains("workspace-only note")); + assert!(memory.reindex().expect("reindex").entry_count >= 1); + } + + #[test] + fn memory_adapter_import_distinguishes_imported_from_skipped() { + let tmp = TempDir::new().unwrap(); + let legacy = tmp.path().join("memory.md"); + std::fs::write(&legacy, "# legacy\n\n- imported line").unwrap(); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + + let imported = memory.import().expect("import"); + let MemoryImportOutcome::Imported { destination } = imported else { + panic!("first import must be imported"); + }; + assert!(destination.ends_with("global/MEMORY.md")); + + // Idempotent: an existing global source reports skipped. + assert_eq!( + memory.import().expect("second"), + MemoryImportOutcome::Skipped + ); + } + + #[test] + fn memory_adapter_deletes_are_scoped_and_preserve_other_memory() { + let tmp = TempDir::new().unwrap(); + git_origin(tmp.path()); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + + memory + .remember(MemoryRememberTarget::Global, "keep global") + .expect("global"); + let workspace_id = memory.workspace_id(tmp.path()).expect("id"); + memory + .remember( + MemoryRememberTarget::Workspace { workspace_id }, + "remove workspace", + ) + .expect("workspace"); + + // Workspace deletion removes only the workspace scope. + memory + .delete_workspace(tmp.path()) + .expect("workspace delete"); + assert!( + memory + .search(tmp.path(), "remove workspace", 10) + .expect("search") + .is_empty() + ); + assert_eq!( + memory.search(tmp.path(), "keep global", 10).unwrap().len(), + 1 + ); + + // Global deletion removes the global scope but keeps the workspace one. + memory + .remember( + MemoryRememberTarget::Workspace { + workspace_id: memory.workspace_id(tmp.path()).expect("id"), + }, + "workspace survivor", + ) + .expect("workspace again"); + memory + .delete(MemoryDeleteScope::Global) + .expect("global delete"); + assert!( + memory + .search(tmp.path(), "keep global", 10) + .expect("search") + .is_empty() + ); + assert_eq!( + memory + .search(tmp.path(), "workspace survivor", 10) + .unwrap() + .len(), + 1 + ); + + // All deletion removes every scope. + memory.delete(MemoryDeleteScope::All).expect("all delete"); + assert!( + memory + .search(tmp.path(), "workspace survivor", 10) + .expect("search") + .is_empty() + ); + } + + #[test] + fn memory_adapter_preserves_workspace_delete_error_text() { + let tmp = TempDir::new().unwrap(); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + let memory = bundle.parts().memory.expect("memory facet"); + let err = memory + .delete_workspace(tmp.path()) + .expect_err("missing origin"); + assert_eq!( + err, + "workspace memory requires a git repository with an origin" + ); + } + + #[test] + fn envelope_exposes_only_declared_capabilities() { + let tmp = TempDir::new().unwrap(); + let mut app = memory_test_app(&tmp, true); + let mut bundle = app.command_contexts(); + + // Memory-only: memory present, workspace/session absent. + let parts = bundle.contexts(CommandCapabilities::MEMORY).into_parts(); + assert!(parts.memory.is_some()); + assert!(parts.workspace.is_none()); + assert!(parts.session.is_none()); + + // Workspace-only: memory absent. + let parts = bundle.contexts(CommandCapabilities::WORKSPACE).into_parts(); + assert!(parts.workspace.is_some()); + assert!(parts.memory.is_none()); + + // Workspace | MEMORY: both present, presentation/media absent. + let parts = bundle + .contexts(CommandCapabilities::WORKSPACE.union(CommandCapabilities::MEMORY)) + .into_parts(); + assert!(parts.workspace.is_some()); + assert!(parts.memory.is_some()); + assert!(parts.presentation.is_none()); + assert!(parts.media.is_none()); + + // Unrelated capability: memory absent. + let parts = bundle.contexts(CommandCapabilities::SESSION).into_parts(); + assert!(parts.session.is_some()); + assert!(parts.memory.is_none()); + } + // ─── FEAT-022 skill-group adapter tests ─────────────────────────────────── /// Pins HOME to a tempdir for the duration of the test under the diff --git a/crates/tui/src/commands/groups/memory/memory.rs b/crates/tui/src/commands/groups/memory/memory.rs index 3485781384..3caa5da159 100644 --- a/crates/tui/src/commands/groups/memory/memory.rs +++ b/crates/tui/src/commands/groups/memory/memory.rs @@ -20,8 +20,14 @@ use std::fs; use std::path::Path; +use codewhale_command_contract::facets::{ + CommandMemoryContext, MemoryDeleteScope, MemoryGetOutcome, MemoryImportOutcome, + MemoryRememberTarget, +}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; +use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; + use crate::commands::CommandResult; -use crate::tui::app::App; const MEMORY_USAGE: &str = "/memory [show|path|clear|edit|native ...|help]"; @@ -44,21 +50,11 @@ fn memory_help(path: &Path) -> String { ) } -fn native_store(app: &App) -> crate::native_memory::NativeMemoryStore { - if let Some(store) = crate::native_memory::NativeMemoryStore::from_global_path(&app.memory_path) - { - return store; - } - let root = app - .memory_path - .parent() - .unwrap_or_else(|| Path::new(".")) - .join("memory"); - crate::native_memory::NativeMemoryStore::new(root) -} - -fn native_command(app: &App, input: &str) -> CommandResult { - let store = native_store(app); +fn native_command( + workspace: &Path, + memory: &dyn CommandMemoryContext, + input: &str, +) -> CommandResult { let mut parts = input.splitn(2, char::is_whitespace); let command = parts.next().unwrap_or("status"); let arg = parts @@ -66,18 +62,24 @@ fn native_command(app: &App, input: &str) -> CommandResult { .map(str::trim) .filter(|value| !value.is_empty()); match command { - "status" => CommandResult::message(format!( - "native memory: {}\nsource: {}\nindex: {}", - store.root().display(), - store.global_path().display(), - store.index_path().display() - )), - "path" => CommandResult::message(store.root().display().to_string()), + "status" => match memory.status() { + Ok(status) => CommandResult::message(format!( + "native memory: {}\nsource: {}\nindex: {}", + status.root.display(), + status.source.display(), + status.index.display() + )), + Err(err) => CommandResult::error(format!("native memory status failed: {err}")), + }, + "path" => match memory.path() { + Ok(root) => CommandResult::message(root.display().to_string()), + Err(err) => CommandResult::error(format!("native memory path failed: {err}")), + }, "search" => { let Some(query) = arg else { return CommandResult::error("Usage: /memory native search "); }; - match store.search_for_workspace(&app.workspace, query, 10) { + match memory.search(workspace, query, 10) { Ok(hits) if hits.is_empty() => CommandResult::message("No native memory matches."), Ok(hits) => CommandResult::message( hits.into_iter() @@ -108,25 +110,16 @@ fn native_command(app: &App, input: &str) -> CommandResult { let Some(note) = words.next() else { return CommandResult::error("Usage: /memory native remember workspace "); }; - let workspace_id = - match crate::native_memory::NativeMemoryStore::workspace_id(&app.workspace) { - Ok(Some(id)) => id, - Ok(None) => { - return CommandResult::error( - "workspace memory requires a git repository with an origin", - ); - } - Err(err) => { - return CommandResult::error(format!( - "failed to resolve workspace identity: {err}" - )); - } - }; - match store.remember( - crate::native_memory::MemoryScope::Workspace, - Some(&workspace_id), - note, - ) { + let remembered = match memory.workspace_id(workspace) { + Ok(workspace_id) => { + memory.remember(MemoryRememberTarget::Workspace { workspace_id }, note) + } + Err(err) => { + // The adapter preserves the established identity text. + return CommandResult::error(err); + } + }; + match remembered { Ok(hit) => CommandResult::message(format!( "native memory remembered at {}:{}", hit.source.display(), @@ -135,7 +128,7 @@ fn native_command(app: &App, input: &str) -> CommandResult { Err(err) => CommandResult::error(format!("native memory write failed: {err}")), } } else { - match store.remember(crate::native_memory::MemoryScope::Global, None, input) { + match memory.remember(MemoryRememberTarget::Global, input) { Ok(hit) => CommandResult::message(format!( "native memory remembered at {}:{}", hit.source.display(), @@ -145,67 +138,54 @@ fn native_command(app: &App, input: &str) -> CommandResult { } } } - "import" => { - let legacy_path = store - .root() - .parent() - .map(|parent| parent.join("memory.md")) - .unwrap_or_else(|| app.memory_path.clone()); - match store.import_legacy(&legacy_path) { - Ok(true) => CommandResult::message(format!( - "legacy memory imported non-destructively into {}", - store.global_path().display() - )), - Ok(false) => { - CommandResult::message("legacy memory was already imported or is empty") - } - Err(err) => CommandResult::error(format!("legacy memory import failed: {err}")), + "import" => match memory.import() { + Ok(MemoryImportOutcome::Imported { destination }) => CommandResult::message(format!( + "legacy memory imported non-destructively into {}", + destination.display() + )), + Ok(MemoryImportOutcome::Skipped) => { + CommandResult::message("legacy memory was already imported or is empty") } - } + Err(err) => CommandResult::error(format!("legacy memory import failed: {err}")), + }, "get" => { let Some(id) = arg.and_then(|value| value.parse::().ok()) else { return CommandResult::error("Usage: /memory native get "); }; - match store.get_for_workspace(&app.workspace, id) { - Ok(Some(hit)) => CommandResult::message(format!( + match memory.get(workspace, id) { + Ok(MemoryGetOutcome::Found(hit)) => CommandResult::message(format!( "{}:{}-{}\n{}", hit.source.display(), hit.line_start, hit.line_end, hit.text )), - Ok(None) => CommandResult::error(format!("native memory entry {id} not found")), + Ok(MemoryGetOutcome::NotFound) => { + CommandResult::error(format!("native memory entry {id} not found")) + } Err(err) => CommandResult::error(format!("native memory get failed: {err}")), } } - "export" => match store.export() { - Ok(export) if export.is_empty() => CommandResult::message("Native memory is empty."), - Ok(export) => CommandResult::message(export), + "export" => match memory.export() { + Ok(export) if export.content.is_empty() => { + CommandResult::message("Native memory is empty.") + } + Ok(export) => CommandResult::message(export.content), Err(err) => CommandResult::error(format!("native memory export failed: {err}")), }, - "reindex" => match store.reindex() { - Ok(count) => { - CommandResult::message(format!("native memory reindexed: {count} entries")) - } + "reindex" => match memory.reindex() { + Ok(result) => CommandResult::message(format!( + "native memory reindexed: {} entries", + result.entry_count + )), Err(err) => CommandResult::error(format!("native memory reindex failed: {err}")), }, "delete" | "clear" => { let scope = arg.unwrap_or("all"); let result = match scope { - "all" => store.delete_all(None, None), - "global" => store.delete_all(Some(crate::native_memory::MemoryScope::Global), None), - "workspace" => { - match crate::native_memory::NativeMemoryStore::workspace_id(&app.workspace) { - Ok(Some(id)) => store.delete_all( - Some(crate::native_memory::MemoryScope::Workspace), - Some(&id), - ), - Ok(None) => Err(anyhow::anyhow!( - "workspace memory requires a git repository with an origin" - )), - Err(err) => Err(err), - } - } + "all" => memory.delete(MemoryDeleteScope::All), + "global" => memory.delete(MemoryDeleteScope::Global), + "workspace" => memory.delete_workspace(workspace), _ => { return CommandResult::error( "Usage: /memory native delete [all|global|workspace]", @@ -213,7 +193,7 @@ fn native_command(app: &App, input: &str) -> CommandResult { } }; match result { - Ok(()) => CommandResult::message(format!("native memory {scope} deleted")), + Ok(_) => CommandResult::message(format!("native memory {scope} deleted")), Err(err) => CommandResult::error(format!("native memory delete failed: {err}")), } } @@ -223,18 +203,18 @@ fn native_command(app: &App, input: &str) -> CommandResult { } } -fn memory(app: &mut App, arg: Option<&str>) -> CommandResult { - if !app.use_memory { +fn memory(workspace: &Path, memory: &dyn CommandMemoryContext, arg: Option<&str>) -> CommandResult { + if !memory.memory_enabled() { return CommandResult::error( "user memory is disabled. Enable with `[memory] enabled = true` in `~/.codewhale/config.toml` or `DEEPSEEK_MEMORY=on` in your environment, then restart the TUI.", ); } - let path = app.memory_path.clone(); + let path = memory.memory_path(); let sub = arg.unwrap_or("show").trim(); if let Some(native_arg) = sub.strip_prefix("native").map(str::trim) { - return native_command(app, native_arg); + return native_command(workspace, memory, native_arg); } match sub { @@ -269,67 +249,227 @@ fn memory(app: &mut App, arg: Option<&str>) -> CommandResult { } } -pub(in crate::commands) const COMMAND_INFO: crate::commands::traits::CommandInfo = - crate::commands::traits::CommandInfo { - name: "memory", - aliases: &[], - usage: "/memory [show|path|clear|edit|help]", - description_id: crate::localization::MessageId::CmdMemoryDescription, - }; +pub(in crate::commands) const COMMAND_INFO: CommandInfo = CommandInfo { + name: "memory", + aliases: &[], + usage: "/memory [show|path|clear|edit|help]", + description_key: "cmd_memory_description", +}; pub(in crate::commands) struct MemoryCmd; -impl crate::commands::traits::RegisterCommand for MemoryCmd { - fn info() -> &'static crate::commands::traits::CommandInfo { +impl RegisterCommand for MemoryCmd { + fn info() -> &'static CommandInfo { &COMMAND_INFO } - fn execute( - app: &mut crate::tui::app::App, - arg: Option<&str>, - ) -> crate::commands::CommandResult { - memory(app, arg) + fn handler() -> CommandHandler { + CommandHandler::Contextual { + capabilities: CommandCapabilities::WORKSPACE.union(CommandCapabilities::MEMORY), + handler: memory_contextual, + } } } +fn memory_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { + let parts = contexts.into_parts(); + let Some(workspace) = parts.workspace.as_deref() else { + return CommandResult::error("Command capability unavailable: workspace"); + }; + let Some(memory_ctx) = parts.memory.as_deref() else { + return CommandResult::error("Command capability unavailable: memory"); + }; + memory(&workspace.workspace(), memory_ctx, arg) +} + #[cfg(test)] mod tests { use super::*; - use crate::config::Config; - use crate::tui::app::{App, TuiOptions}; + use std::path::PathBuf; use tempfile::TempDir; - fn create_test_app_with_memory(tmpdir: &TempDir, use_memory: bool) -> App { - let options = TuiOptions { - skills_dir: tmpdir.path().join("skills"), - memory_path: tmpdir.path().join("memory.md"), - notes_path: tmpdir.path().join("notes.txt"), - mcp_config_path: tmpdir.path().join("mcp.json"), - use_memory, - ..crate::test_support::test_tui_options(tmpdir.path()) - }; - App::new(options, &Config::default()) + use codewhale_command_contract::facets::{ + CommandWorkspaceContext, MemoryDelete, MemoryExport, MemoryHit, MemoryReindex, + MemoryRemembered, MemoryStatus, + }; + + struct FakeWorkspace { + path: PathBuf, + } + + impl CommandWorkspaceContext for FakeWorkspace { + fn workspace(&self) -> PathBuf { + self.path.clone() + } + + fn work_state_snapshot(&self) -> Result, String> { + Ok(None) + } + + fn operation_digest(&mut self) -> Result { + Ok("No active operations or to-do items.".to_string()) + } + } + + /// Programmable fake memory facet driving every handler branch. + struct FakeMemory { + enabled: bool, + path: PathBuf, + status: Result, + root: Result, + workspace_id: Result, + search: Result, String>, + remember: Result, + import: Result, + get: Result, + export: Result, + reindex: Result, + delete: Result<(), String>, + delete_workspace: Result<(), String>, + } + + impl Default for FakeMemory { + fn default() -> Self { + Self { + enabled: true, + path: PathBuf::from("/mem/user-memory.md"), + status: Ok(MemoryStatus { + root: PathBuf::from("/mem/root"), + source: PathBuf::from("/mem/root/global/MEMORY.md"), + index: PathBuf::from("/mem/root/index.sqlite3"), + }), + root: Ok(PathBuf::from("/mem/root")), + workspace_id: Ok("owner/repo".to_string()), + search: Ok(vec![MemoryHit { + source: PathBuf::from("/mem/root/global/MEMORY.md"), + line_start: 2, + line_end: 2, + text: "alpha hit".to_string(), + }]), + remember: Ok(MemoryRemembered { + source: PathBuf::from("/mem/root/global/MEMORY.md"), + line_start: 3, + }), + import: Ok(MemoryImportOutcome::Skipped), + get: Ok(MemoryGetOutcome::Found(MemoryHit { + source: PathBuf::from("/mem/root/global/MEMORY.md"), + line_start: 2, + line_end: 2, + text: "found entry".to_string(), + })), + export: Ok(MemoryExport { + content: "# memory\n\n- bullet".to_string(), + }), + reindex: Ok(MemoryReindex { entry_count: 4 }), + delete: Ok(()), + delete_workspace: Ok(()), + } + } } + impl CommandMemoryContext for FakeMemory { + fn memory_path(&self) -> PathBuf { + self.path.clone() + } + + fn memory_enabled(&self) -> bool { + self.enabled + } + + fn status(&self) -> Result { + self.status.clone() + } + + fn path(&self) -> Result { + self.root.clone() + } + + fn workspace_id(&self, _workspace: &Path) -> Result { + self.workspace_id.clone() + } + + fn search( + &self, + _workspace: &Path, + _query: &str, + _limit: usize, + ) -> Result, String> { + self.search.clone() + } + + fn remember( + &self, + _target: MemoryRememberTarget, + _note: &str, + ) -> Result { + self.remember.clone() + } + + fn import(&self) -> Result { + self.import.clone() + } + + fn get(&self, _workspace: &Path, _id: i64) -> Result { + self.get.clone() + } + + fn export(&self) -> Result { + self.export.clone() + } + + fn reindex(&self) -> Result { + self.reindex.clone() + } + + fn delete(&self, _scope: MemoryDeleteScope) -> Result { + self.delete.clone().map(|()| MemoryDelete) + } + + fn delete_workspace(&self, _workspace: &Path) -> Result { + self.delete_workspace.clone().map(|()| MemoryDelete) + } + } + + fn fake_workspace(tmpdir: &TempDir) -> FakeWorkspace { + FakeWorkspace { + path: tmpdir.path().to_path_buf(), + } + } + + fn message(result: CommandResult) -> String { + result.message.expect("command message") + } + + fn error(result: CommandResult) -> String { + result + .message + .expect("command error") + .strip_prefix("Error: ") + .unwrap_or_default() + .to_string() + } + + // --- Existing 3 tests ported to fake facets (D6) --- + #[test] fn memory_help_lists_subcommands_and_resolved_path() { let tmpdir = TempDir::new().expect("tempdir"); - let mut app = create_test_app_with_memory(&tmpdir, true); - let result = memory(&mut app, Some("help")); - let msg = result.message.expect("help should return text"); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let result = memory(&workspace.path, &fake, Some("help")); + let msg = message(result); assert!(msg.contains("Usage: /memory [show|path|clear|edit|native ...|help]")); assert!(msg.contains("/memory edit")); - assert!(msg.contains(app.memory_path.to_string_lossy().as_ref())); + assert!(msg.contains("/mem/user-memory.md")); } #[test] fn memory_unknown_subcommand_points_to_help() { let tmpdir = TempDir::new().expect("tempdir"); - let mut app = create_test_app_with_memory(&tmpdir, true); - let result = memory(&mut app, Some("wat")); - let msg = result - .message - .expect("unknown subcommand should return text"); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let result = memory(&workspace.path, &fake, Some("wat")); + let msg = message(result); assert!(msg.contains("Try `/memory help`")); assert!(msg.contains("/memory clear")); } @@ -337,10 +477,320 @@ mod tests { #[test] fn memory_disabled_returns_enablement_hint() { let tmpdir = TempDir::new().expect("tempdir"); - let mut app = create_test_app_with_memory(&tmpdir, false); - let result = memory(&mut app, None); - let msg = result.message.expect("disabled memory should return text"); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory { + enabled: false, + ..FakeMemory::default() + }; + let result = memory(&workspace.path, &fake, None); + let msg = message(result); assert!(msg.contains("user memory is disabled")); assert!(msg.contains("DEEPSEEK_MEMORY=on")); } + + // --- Native operation matrix (D6/D9) --- + + #[test] + fn native_status_renders_root_source_and_index() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let msg = message(memory(&workspace.path, &fake, Some("native status"))); + assert!(msg.contains("native memory: /mem/root")); + assert!(msg.contains("source: /mem/root/global/MEMORY.md")); + assert!(msg.contains("index: /mem/root/index.sqlite3")); + } + + #[test] + fn native_path_renders_root() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let msg = message(memory(&workspace.path, &fake, Some("native path"))); + assert_eq!(msg, "/mem/root"); + } + + #[test] + fn native_search_renders_hits_empty_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let msg = message(memory(&workspace.path, &fake, Some("native search alpha"))); + assert_eq!(msg, "/mem/root/global/MEMORY.md:2-2 alpha hit"); + + let empty = FakeMemory { + search: Ok(Vec::new()), + ..FakeMemory::default() + }; + let msg = message(memory(&workspace.path, &empty, Some("native search zzz"))); + assert_eq!(msg, "No native memory matches."); + + let failing = FakeMemory { + search: Err("index corrupt".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native search zzz"))); + assert!(err.contains("native memory search failed: index corrupt")); + + let usage = memory(&workspace.path, &fake, Some("native search")); + assert!(error(usage).contains("Usage: /memory native search ")); + } + + #[test] + fn native_remember_global_and_workspace() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + + let global = message(memory( + &workspace.path, + &fake, + Some("native remember global hello"), + )); + assert_eq!( + global, + "native memory remembered at /mem/root/global/MEMORY.md:3" + ); + + let workspace_note = message(memory( + &workspace.path, + &fake, + Some("native remember workspace hello"), + )); + assert_eq!( + workspace_note, + "native memory remembered at /mem/root/global/MEMORY.md:3" + ); + + let missing_origin = FakeMemory { + workspace_id: Err( + "workspace memory requires a git repository with an origin".to_string() + ), + ..FakeMemory::default() + }; + let err = error(memory( + &workspace.path, + &missing_origin, + Some("native remember workspace hello"), + )); + assert_eq!( + err, + "workspace memory requires a git repository with an origin" + ); + + let failing = FakeMemory { + remember: Err("disk full".to_string()), + ..FakeMemory::default() + }; + let err = error(memory( + &workspace.path, + &failing, + Some("native remember global hello"), + )); + assert_eq!(err, "native memory write failed: disk full"); + + let usage = memory(&workspace.path, &fake, Some("native remember")); + assert!(error(usage).contains("Usage: /memory native remember")); + } + + #[test] + fn native_import_imported_skipped_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + + let imported = FakeMemory { + import: Ok(MemoryImportOutcome::Imported { + destination: PathBuf::from("/mem/root/global/MEMORY.md"), + }), + ..FakeMemory::default() + }; + let msg = message(memory(&workspace.path, &imported, Some("native import"))); + assert_eq!( + msg, + "legacy memory imported non-destructively into /mem/root/global/MEMORY.md" + ); + + let msg = message(memory( + &workspace.path, + &FakeMemory::default(), + Some("native import"), + )); + assert_eq!(msg, "legacy memory was already imported or is empty"); + + let failing = FakeMemory { + import: Err("read failed".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native import"))); + assert_eq!(err, "legacy memory import failed: read failed"); + } + + #[test] + fn native_get_found_not_found_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + + let msg = message(memory(&workspace.path, &fake, Some("native get 5"))); + assert_eq!(msg, "/mem/root/global/MEMORY.md:2-2\nfound entry"); + + let not_found = FakeMemory { + get: Ok(MemoryGetOutcome::NotFound), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, ¬_found, Some("native get 5"))); + assert_eq!(err, "native memory entry 5 not found"); + + let failing = FakeMemory { + get: Err("db error".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native get 5"))); + assert_eq!(err, "native memory get failed: db error"); + + let usage = memory(&workspace.path, &fake, Some("native get abc")); + assert!(error(usage).contains("Usage: /memory native get ")); + } + + #[test] + fn native_export_empty_content_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + + let msg = message(memory(&workspace.path, &fake, Some("native export"))); + assert_eq!(msg, "# memory\n\n- bullet"); + + let empty = FakeMemory { + export: Ok(MemoryExport { + content: String::new(), + }), + ..FakeMemory::default() + }; + let msg = message(memory(&workspace.path, &empty, Some("native export"))); + assert_eq!(msg, "Native memory is empty."); + + let failing = FakeMemory { + export: Err("locked".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native export"))); + assert_eq!(err, "native memory export failed: locked"); + } + + #[test] + fn native_reindex_count_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let msg = message(memory(&workspace.path, &fake, Some("native reindex"))); + assert_eq!(msg, "native memory reindexed: 4 entries"); + + let failing = FakeMemory { + reindex: Err("lock".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native reindex"))); + assert_eq!(err, "native memory reindex failed: lock"); + } + + #[test] + fn native_delete_scopes_and_errors() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + + for scope in ["all", "global", "workspace"] { + let msg = message(memory( + &workspace.path, + &fake, + Some(&format!("native delete {scope}")), + )); + assert_eq!(msg, format!("native memory {scope} deleted")); + } + + // Missing origin preserves the established identity text under the + // delete prefix, matching the pre-migration behavior. + let missing_origin = FakeMemory { + workspace_id: Err( + "workspace memory requires a git repository with an origin".to_string() + ), + delete_workspace: Err( + "workspace memory requires a git repository with an origin".to_string() + ), + ..FakeMemory::default() + }; + let err = error(memory( + &workspace.path, + &missing_origin, + Some("native delete workspace"), + )); + assert_eq!( + err, + "native memory delete failed: workspace memory requires a git repository with an origin" + ); + + let failing = FakeMemory { + delete: Err("busy".to_string()), + ..FakeMemory::default() + }; + let err = error(memory(&workspace.path, &failing, Some("native delete all"))); + assert_eq!(err, "native memory delete failed: busy"); + + let usage = memory(&workspace.path, &fake, Some("native delete bogus")); + assert!(error(usage).contains("Usage: /memory native delete [all|global|workspace]")); + } + + #[test] + fn native_unknown_subcommand_returns_usage() { + let tmpdir = TempDir::new().unwrap(); + let workspace = fake_workspace(&tmpdir); + let fake = FakeMemory::default(); + let err = error(memory(&workspace.path, &fake, Some("native bogus"))); + assert!(err.contains("Usage: /memory native")); + } + + #[test] + fn memory_registration_declares_exactly_workspace_and_memory() { + let CommandHandler::Contextual { + capabilities, + handler, + } = MemoryCmd::handler() + else { + panic!("memory must be contextual"); + }; + assert_eq!( + capabilities, + CommandCapabilities::WORKSPACE.union(CommandCapabilities::MEMORY) + ); + assert!(!capabilities.contains(CommandCapabilities::PRESENTATION)); + assert!(!capabilities.contains(CommandCapabilities::MEDIA)); + + // Missing facets fail safely instead of panicking. + let missing = handler(CommandContexts::empty(), Some("help")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); + assert_eq!(MemoryCmd::info().description_key, "cmd_memory_description"); + assert_eq!(MemoryCmd::info().name, "memory"); + assert_eq!(MemoryCmd::info().aliases, &[] as &[&str]); + } + + #[test] + fn memory_missing_memory_facet_fails_safely() { + // An envelope carrying WORKSPACE but no MEMORY must fail safely with + // the memory-capability error (never panic). + let mut workspace = FakeWorkspace { + path: PathBuf::from("/ws"), + }; + let contexts = CommandContexts::empty().with_workspace(&mut workspace); + let result = memory_contextual(contexts, Some("help")); + assert!(result.is_error); + assert_eq!( + result.message.as_deref(), + Some("Error: Command capability unavailable: memory") + ); + } } diff --git a/crates/tui/src/commands/groups/memory/mod.rs b/crates/tui/src/commands/groups/memory/mod.rs index 87cb462f9a..753a852541 100644 --- a/crates/tui/src/commands/groups/memory/mod.rs +++ b/crates/tui/src/commands/groups/memory/mod.rs @@ -7,21 +7,20 @@ mod memory; mod note; -use crate::commands::traits::{Command, CommandGroup, FunctionCommand, RegisterCommand}; +use crate::commands::traits::{CommandGroup, ContextualCommand}; pub struct MemoryCommands; impl CommandGroup for MemoryCommands { - fn commands(&self) -> &'static [Box] { + fn commands(&self) -> &'static [Box] { cached_command_list!(vec![ - Box::new(FunctionCommand::new( - note::NoteCmd::info(), - note::NoteCmd::execute, - )), - Box::new(FunctionCommand::new( - memory::MemoryCmd::info(), - memory::MemoryCmd::execute, - )), + Box::new( + ContextualCommand::from_contract::().expect("note registration"), + ), + Box::new( + ContextualCommand::from_contract::() + .expect("memory registration"), + ), ]) } } diff --git a/crates/tui/src/commands/groups/memory/note.rs b/crates/tui/src/commands/groups/memory/note.rs index d3d84484c6..15c35f206c 100644 --- a/crates/tui/src/commands/groups/memory/note.rs +++ b/crates/tui/src/commands/groups/memory/note.rs @@ -1,16 +1,18 @@ //! Note command: manage persistent workspace notes. -use crate::tui::app::App; use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; +use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; + use crate::commands::CommandResult; const USAGE: &str = "/note | /note add | /note list | /note show | /note edit | /note remove | /note clear | /note path"; /// Manage the persistent workspace notes file. -fn note(app: &mut App, content: Option<&str>) -> CommandResult { +fn note(workspace: &Path, content: Option<&str>) -> CommandResult { let input = match content { Some(c) => c.trim(), None => { @@ -22,7 +24,7 @@ fn note(app: &mut App, content: Option<&str>) -> CommandResult { return CommandResult::error("Note content cannot be empty"); } - let notes_path = notes_path(app); + let notes_path = notes_path(workspace); let (command, rest) = split_command(input); match command.to_ascii_lowercase().as_str() { @@ -38,12 +40,15 @@ fn note(app: &mut App, content: Option<&str>) -> CommandResult { } } -fn notes_path(app: &App) -> PathBuf { - let primary = app.workspace.join(".codewhale").join("notes.md"); +/// Resolve the notes file. An existing `.codewhale` notes file is preferred; +/// otherwise the `.deepseek` notes path is used (D3 — the fallback stays +/// handler-owned through standard filesystem operations). +fn notes_path(workspace: &Path) -> PathBuf { + let primary = workspace.join(".codewhale").join("notes.md"); if primary.exists() { return primary; } - app.workspace.join(".deepseek").join("notes.md") + workspace.join(".deepseek").join("notes.md") } fn split_command(input: &str) -> (&str, Option<&str>) { @@ -266,46 +271,66 @@ fn parse_note_index(rest: Option<&str>, note_count: usize, usage: &str) -> Resul Ok(index - 1) } -pub(in crate::commands) const COMMAND_INFO: crate::commands::traits::CommandInfo = - crate::commands::traits::CommandInfo { - name: "note", - aliases: &[], - usage: "/note [add|list|show|edit|remove|clear|path]", - description_id: crate::localization::MessageId::CmdNoteDescription, - }; +pub(in crate::commands) const COMMAND_INFO: CommandInfo = CommandInfo { + name: "note", + aliases: &[], + usage: "/note [add|list|show|edit|remove|clear|path]", + description_key: "cmd_note_description", +}; pub(in crate::commands) struct NoteCmd; -impl crate::commands::traits::RegisterCommand for NoteCmd { - fn info() -> &'static crate::commands::traits::CommandInfo { +impl RegisterCommand for NoteCmd { + fn info() -> &'static CommandInfo { &COMMAND_INFO } - fn execute( - app: &mut crate::tui::app::App, - arg: Option<&str>, - ) -> crate::commands::CommandResult { - note(app, arg) + fn handler() -> CommandHandler { + CommandHandler::Contextual { + capabilities: CommandCapabilities::WORKSPACE, + handler: note_contextual, + } } } +fn note_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { + let parts = contexts.into_parts(); + let Some(workspace) = parts.workspace.as_deref() else { + return CommandResult::error("Command capability unavailable: workspace"); + }; + note(&workspace.workspace(), arg) +} + #[cfg(test)] mod tests { use super::*; - use crate::config::Config; - use crate::tui::app::{App, TuiOptions}; use std::path::PathBuf; use tempfile::TempDir; - fn create_test_app_with_tmpdir(tmpdir: &TempDir) -> App { - let options = TuiOptions { - skills_dir: tmpdir.path().join("skills"), - memory_path: tmpdir.path().join("memory.md"), - notes_path: tmpdir.path().join("notes.txt"), - mcp_config_path: tmpdir.path().join("mcp.json"), - ..crate::test_support::test_tui_options(tmpdir.path()) - }; - App::new(options, &Config::default()) + use codewhale_command_contract::facets::CommandWorkspaceContext; + + struct FakeWorkspace { + path: PathBuf, + } + + impl CommandWorkspaceContext for FakeWorkspace { + fn workspace(&self) -> PathBuf { + self.path.clone() + } + + fn work_state_snapshot(&self) -> Result, String> { + Ok(None) + } + + fn operation_digest(&mut self) -> Result { + Ok("No active operations or to-do items.".to_string()) + } + } + + fn fake_workspace(tmpdir: &TempDir) -> FakeWorkspace { + FakeWorkspace { + path: tmpdir.path().to_path_buf(), + } } fn notes_path(tmpdir: &TempDir) -> PathBuf { @@ -319,8 +344,8 @@ mod tests { #[test] fn test_note_without_content_returns_error() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - let result = note(&mut app, None); + let workspace = fake_workspace(&tmpdir); + let result = note(&workspace.path, None); assert!(result.message.is_some()); assert!(result.message.unwrap().contains("Usage: /note")); } @@ -328,8 +353,8 @@ mod tests { #[test] fn test_note_with_empty_content_returns_error() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - let result = note(&mut app, Some(" ")); + let workspace = fake_workspace(&tmpdir); + let result = note(&workspace.path, Some(" ")); assert!(result.message.is_some()); assert!(result.message.unwrap().contains("cannot be empty")); } @@ -337,8 +362,8 @@ mod tests { #[test] fn test_note_appends_to_file() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - let result = note(&mut app, Some("Test note content")); + let workspace = fake_workspace(&tmpdir); + let result = note(&workspace.path, Some("Test note content")); assert!(result.message.is_some()); let msg = message(result); assert!(msg.contains("Note appended to")); @@ -352,9 +377,9 @@ mod tests { #[test] fn test_note_multiple_appends() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("First note")); - note(&mut app, Some("Second note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("First note")); + note(&workspace.path, Some("Second note")); let notes_path = notes_path(&tmpdir); let content = std::fs::read_to_string(¬es_path).unwrap(); @@ -367,11 +392,11 @@ mod tests { #[test] fn test_note_list_numbers_entries_without_storing_numbers() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("Alpha note")); - note(&mut app, Some("Beta note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("Alpha note")); + note(&workspace.path, Some("Beta note")); - let listed = message(note(&mut app, Some("list"))); + let listed = message(note(&workspace.path, Some("list"))); assert!(listed.contains("1. Alpha note")); assert!(listed.contains("2. Beta note")); @@ -383,10 +408,10 @@ mod tests { #[test] fn test_note_show_displays_full_multiline_note() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("add first line\nsecond line")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("add first line\nsecond line")); - let shown = message(note(&mut app, Some("show 1"))); + let shown = message(note(&workspace.path, Some("show 1"))); assert!(shown.contains("Note 1:")); assert!(shown.contains("first line\nsecond line")); } @@ -394,11 +419,11 @@ mod tests { #[test] fn test_note_edit_updates_numbered_entry() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("First note")); - note(&mut app, Some("Second note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("First note")); + note(&workspace.path, Some("Second note")); - let edited = message(note(&mut app, Some("edit 2 Updated second note"))); + let edited = message(note(&workspace.path, Some("edit 2 Updated second note"))); assert!(edited.contains("Note 2 updated")); let content = std::fs::read_to_string(notes_path(&tmpdir)).unwrap(); @@ -410,15 +435,15 @@ mod tests { #[test] fn test_note_remove_renumbers_remaining_entries() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("First note")); - note(&mut app, Some("Second note")); - note(&mut app, Some("Third note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("First note")); + note(&workspace.path, Some("Second note")); + note(&workspace.path, Some("Third note")); - let removed = message(note(&mut app, Some("remove 2"))); + let removed = message(note(&workspace.path, Some("remove 2"))); assert!(removed.contains("Note 2 removed")); - let listed = message(note(&mut app, Some("list"))); + let listed = message(note(&workspace.path, Some("list"))); assert!(listed.contains("1. First note")); assert!(listed.contains("2. Third note")); assert!(!listed.contains("Second note")); @@ -427,10 +452,10 @@ mod tests { #[test] fn test_note_clear_empties_file() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("First note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("First note")); - let cleared = message(note(&mut app, Some("clear"))); + let cleared = message(note(&workspace.path, Some("clear"))); assert!(cleared.contains("Notes cleared")); assert_eq!(std::fs::read_to_string(notes_path(&tmpdir)).unwrap(), ""); } @@ -438,20 +463,35 @@ mod tests { #[test] fn test_note_path_prints_workspace_notes_file() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); + let workspace = fake_workspace(&tmpdir); - let path = message(note(&mut app, Some("path"))); + let path = message(note(&workspace.path, Some("path"))); assert!(path.contains(".deepseek")); assert!(path.contains("notes.md")); } + #[test] + fn test_note_prefers_existing_codewhale_notes_file() { + let tmpdir = TempDir::new().unwrap(); + let codewhale_dir = tmpdir.path().join(".codewhale"); + std::fs::create_dir_all(&codewhale_dir).unwrap(); + let codewhale_notes = codewhale_dir.join("notes.md"); + std::fs::write(&codewhale_notes, "---\nexisting codewhale note").unwrap(); + + let workspace = fake_workspace(&tmpdir); + let path = message(note(&workspace.path, Some("path"))); + assert!(path.contains(".codewhale")); + assert!(path.contains("notes.md")); + assert!(!path.contains(".deepseek")); + } + #[test] fn test_note_rejects_out_of_range_index() { let tmpdir = TempDir::new().unwrap(); - let mut app = create_test_app_with_tmpdir(&tmpdir); - note(&mut app, Some("Only note")); + let workspace = fake_workspace(&tmpdir); + note(&workspace.path, Some("Only note")); - let result = note(&mut app, Some("show 2")); + let result = note(&workspace.path, Some("show 2")); assert!(result.message.unwrap().contains("out of range")); } @@ -460,4 +500,30 @@ mod tests { let parsed = parse_notes("plain note\n---\nseparated note"); assert_eq!(parsed, vec!["plain note", "separated note"]); } + + #[test] + fn note_registration_declares_exactly_workspace() { + let CommandHandler::Contextual { + capabilities, + handler, + } = NoteCmd::handler() + else { + panic!("note must be contextual"); + }; + assert_eq!(capabilities, CommandCapabilities::WORKSPACE); + assert!(!capabilities.contains(CommandCapabilities::MEMORY)); + assert!(!capabilities.contains(CommandCapabilities::PRESENTATION)); + assert!(!capabilities.contains(CommandCapabilities::MEDIA)); + + // Missing WORKSPACE fails safely instead of panicking. + let missing = handler(CommandContexts::empty(), Some("list")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); + assert_eq!(NoteCmd::info().description_key, "cmd_note_description"); + assert_eq!(NoteCmd::info().name, "note"); + assert_eq!(NoteCmd::info().aliases, &[] as &[&str]); + } } diff --git a/crates/tui/src/commands/groups/project/goal.rs b/crates/tui/src/commands/groups/project/goal.rs index 52c72a1d04..ff0ae84f6b 100644 --- a/crates/tui/src/commands/groups/project/goal.rs +++ b/crates/tui/src/commands/groups/project/goal.rs @@ -289,7 +289,11 @@ impl RegisterCommand for GoalCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(goal_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::PROJECT + .union(codewhale_command_contract::handler::CommandCapabilities::PRESENTATION), + handler: goal_contextual, + } } } diff --git a/crates/tui/src/commands/groups/project/init.rs b/crates/tui/src/commands/groups/project/init.rs index e188285add..22c0a1cf7f 100644 --- a/crates/tui/src/commands/groups/project/init.rs +++ b/crates/tui/src/commands/groups/project/init.rs @@ -821,7 +821,10 @@ impl codewhale_command_contract::metadata::RegisterCommand codewhale_command_contract::handler::CommandHandler { - codewhale_command_contract::handler::CommandHandler::Contextual(init_contextual) + codewhale_command_contract::handler::CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::WORKSPACE, + handler: init_contextual, + } } } diff --git a/crates/tui/src/commands/groups/project/lsp.rs b/crates/tui/src/commands/groups/project/lsp.rs index f2ad5eee6d..296159eb46 100644 --- a/crates/tui/src/commands/groups/project/lsp.rs +++ b/crates/tui/src/commands/groups/project/lsp.rs @@ -25,7 +25,10 @@ impl RegisterCommand for LspCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(lsp_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::PROJECT, + handler: lsp_contextual, + } } } diff --git a/crates/tui/src/commands/groups/project/share.rs b/crates/tui/src/commands/groups/project/share.rs index 391b51df0c..07b84e521c 100644 --- a/crates/tui/src/commands/groups/project/share.rs +++ b/crates/tui/src/commands/groups/project/share.rs @@ -205,7 +205,10 @@ impl RegisterCommand for ShareCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(share_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::PROJECT, + handler: share_contextual, + } } } diff --git a/crates/tui/src/commands/groups/skills/restore.rs b/crates/tui/src/commands/groups/skills/restore.rs index b41c84827e..e77223987d 100644 --- a/crates/tui/src/commands/groups/skills/restore.rs +++ b/crates/tui/src/commands/groups/skills/restore.rs @@ -39,7 +39,10 @@ impl RegisterCommand for RestoreCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(restore_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::SKILL_GROUP, + handler: restore_contextual, + } } } diff --git a/crates/tui/src/commands/groups/skills/review.rs b/crates/tui/src/commands/groups/skills/review.rs index 9b0adf8319..62543d3f74 100644 --- a/crates/tui/src/commands/groups/skills/review.rs +++ b/crates/tui/src/commands/groups/skills/review.rs @@ -35,7 +35,10 @@ impl RegisterCommand for ReviewCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(review_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::SKILL_GROUP, + handler: review_contextual, + } } } diff --git a/crates/tui/src/commands/groups/skills/skills.rs b/crates/tui/src/commands/groups/skills/skills.rs index f7ffa0dc97..80c95074dc 100644 --- a/crates/tui/src/commands/groups/skills/skills.rs +++ b/crates/tui/src/commands/groups/skills/skills.rs @@ -254,7 +254,10 @@ impl RegisterCommand for SkillsCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(skills_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::SKILL_GROUP, + handler: skills_contextual, + } } } @@ -621,7 +624,11 @@ impl RegisterCommand for SkillCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(skill_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::SKILL_GROUP + .union(codewhale_command_contract::handler::CommandCapabilities::SKILLS), + handler: skill_contextual, + } } } diff --git a/crates/tui/src/commands/groups/utility/attachment.rs b/crates/tui/src/commands/groups/utility/attachment.rs index 9e80b95f98..7f4509cf66 100644 --- a/crates/tui/src/commands/groups/utility/attachment.rs +++ b/crates/tui/src/commands/groups/utility/attachment.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use codewhale_command_contract::facets::CommandMediaContext; -use codewhale_command_contract::handler::{CommandContexts, CommandHandler}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; use crate::commands::CommandResult; @@ -23,14 +23,21 @@ impl RegisterCommand for AttachCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(attach_contextual) + CommandHandler::Contextual { + capabilities: CommandCapabilities::WORKSPACE.union(CommandCapabilities::MEDIA), + handler: attach_contextual, + } } } fn attach_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { let mut parts = contexts.into_parts(); - let workspace = parts.workspace.as_deref().expect("workspace facet"); - let media = parts.media.as_deref_mut().expect("media facet"); + let Some(workspace) = parts.workspace.as_deref() else { + return CommandResult::error("Command capability unavailable: workspace"); + }; + let Some(media) = parts.media.as_deref_mut() else { + return CommandResult::error("Command capability unavailable: media"); + }; attach(workspace.workspace(), media, arg) } @@ -163,10 +170,23 @@ mod tests { #[test] fn handler_is_contextual() { - assert!(matches!( - AttachCmd::handler(), - CommandHandler::Contextual(_) - )); + let CommandHandler::Contextual { + capabilities, + handler, + } = AttachCmd::handler() + else { + panic!("attach must be contextual"); + }; + assert_eq!( + capabilities, + CommandCapabilities::WORKSPACE.union(CommandCapabilities::MEDIA) + ); + let missing = handler(CommandContexts::empty(), Some("photo.png")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); assert_eq!(AttachCmd::info().description_key, "cmd_attach_description"); assert_eq!(AttachCmd::info().aliases, &["image", "media", "fujian"]); } diff --git a/crates/tui/src/commands/groups/utility/automation.rs b/crates/tui/src/commands/groups/utility/automation.rs index aa8d83a35a..6e54d753c6 100644 --- a/crates/tui/src/commands/groups/utility/automation.rs +++ b/crates/tui/src/commands/groups/utility/automation.rs @@ -1,7 +1,7 @@ //! Operator controls for durable scheduled automations. use codewhale_command_contract::facets::CommandPresentationContext; -use codewhale_command_contract::handler::{CommandContexts, CommandHandler}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; use crate::commands::CommandResult; @@ -22,16 +22,18 @@ impl RegisterCommand for AutomationCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(automation_contextual) + CommandHandler::Contextual { + capabilities: CommandCapabilities::PRESENTATION, + handler: automation_contextual, + } } } fn automation_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { let mut parts = contexts.into_parts(); - let presentation = parts - .presentation - .as_deref_mut() - .expect("presentation facet"); + let Some(presentation) = parts.presentation.as_deref_mut() else { + return CommandResult::error("Command capability unavailable: presentation"); + }; automation(presentation, arg) } @@ -207,10 +209,20 @@ mod tests { #[test] fn handler_is_contextual_and_requests_presentation_facet() { - assert!(matches!( - AutomationCmd::handler(), - CommandHandler::Contextual(_) - )); + let CommandHandler::Contextual { + capabilities, + handler, + } = AutomationCmd::handler() + else { + panic!("automation must be contextual"); + }; + assert_eq!(capabilities, CommandCapabilities::PRESENTATION); + let missing = handler(CommandContexts::empty(), Some("list")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: presentation") + ); assert_eq!( AutomationCmd::info().description_key, "cmd_automation_description" diff --git a/crates/tui/src/commands/groups/utility/dispatch.rs b/crates/tui/src/commands/groups/utility/dispatch.rs index 23279c0a78..d7e531372c 100644 --- a/crates/tui/src/commands/groups/utility/dispatch.rs +++ b/crates/tui/src/commands/groups/utility/dispatch.rs @@ -27,13 +27,18 @@ impl RegisterCommand for DispatchCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(dispatch_contextual) + CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::WORKSPACE, + handler: dispatch_contextual, + } } } fn dispatch_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { let mut parts = contexts.into_parts(); - let workspace = parts.workspace.as_deref_mut().expect("workspace facet"); + let Some(workspace) = parts.workspace.as_deref_mut() else { + return CommandResult::error("Command capability unavailable: workspace"); + }; dispatch(workspace, arg) } @@ -213,7 +218,7 @@ mod tests { fn handler_is_contextual_and_argument_aware() { assert!(matches!( DispatchCmd::handler(), - CommandHandler::Contextual(_) + CommandHandler::Contextual { .. } )); assert_eq!( DispatchCmd::info().description_key, @@ -226,6 +231,17 @@ mod tests { assert!(DispatchCmd::info().usage.starts_with("/dispatch")); } + #[test] + fn missing_workspace_facet_fails_safely() { + let result = dispatch_contextual(CommandContexts::empty(), None); + assert!(result.is_error, "{result:?}"); + assert_eq!( + result.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); + assert!(result.action.is_none()); + } + #[test] fn bare_dispatch_is_a_status_card_not_a_silent_launch() { let mut workspace = FakeWorkspace(PathBuf::from(".")); diff --git a/crates/tui/src/commands/groups/utility/mcp.rs b/crates/tui/src/commands/groups/utility/mcp.rs index 0c9a37372e..c3186b84de 100644 --- a/crates/tui/src/commands/groups/utility/mcp.rs +++ b/crates/tui/src/commands/groups/utility/mcp.rs @@ -1,7 +1,7 @@ //! In-TUI MCP manager command parser. use codewhale_command_contract::facets::CommandPresentationContext; -use codewhale_command_contract::handler::{CommandContexts, CommandHandler}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; use crate::commands::CommandResult; @@ -29,16 +29,18 @@ impl RegisterCommand for McpCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(mcp_contextual) + CommandHandler::Contextual { + capabilities: CommandCapabilities::PRESENTATION, + handler: mcp_contextual, + } } } fn mcp_contextual(contexts: CommandContexts<'_>, args: Option<&str>) -> CommandResult { let mut parts = contexts.into_parts(); - let presentation = parts - .presentation - .as_deref_mut() - .expect("presentation facet"); + let Some(presentation) = parts.presentation.as_deref_mut() else { + return CommandResult::error("Command capability unavailable: presentation"); + }; mcp(presentation, args) } @@ -610,7 +612,20 @@ mod tests { #[test] fn handler_is_contextual_and_requests_presentation_facet() { - assert!(matches!(McpCmd::handler(), CommandHandler::Contextual(_))); + let CommandHandler::Contextual { + capabilities, + handler, + } = McpCmd::handler() + else { + panic!("mcp must be contextual"); + }; + assert_eq!(capabilities, CommandCapabilities::PRESENTATION); + let missing = handler(CommandContexts::empty(), Some("list")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: presentation") + ); assert_eq!(McpCmd::info().description_key, "cmd_mcp_description"); assert_eq!(McpCmd::info().aliases, &[] as &[&str]); } diff --git a/crates/tui/src/commands/groups/utility/task.rs b/crates/tui/src/commands/groups/utility/task.rs index 6ae1ddd761..fa4bb7a955 100644 --- a/crates/tui/src/commands/groups/utility/task.rs +++ b/crates/tui/src/commands/groups/utility/task.rs @@ -1,6 +1,6 @@ //! Task commands: add/list/show/cancel -use codewhale_command_contract::handler::{CommandContexts, CommandHandler}; +use codewhale_command_contract::handler::{CommandCapabilities, CommandContexts, CommandHandler}; use codewhale_command_contract::metadata::{CommandInfo, RegisterCommand}; use crate::commands::CommandResult; @@ -21,13 +21,18 @@ impl RegisterCommand for TaskCmd { } fn handler() -> CommandHandler { - CommandHandler::Contextual(task_contextual) + CommandHandler::Contextual { + capabilities: CommandCapabilities::WORKSPACE, + handler: task_contextual, + } } } fn task_contextual(contexts: CommandContexts<'_>, arg: Option<&str>) -> CommandResult { let mut parts = contexts.into_parts(); - let workspace = parts.workspace.as_deref_mut().expect("workspace facet"); + let Some(workspace) = parts.workspace.as_deref_mut() else { + return CommandResult::error("Command capability unavailable: workspace"); + }; task(workspace, arg) } @@ -149,7 +154,20 @@ mod tests { #[test] fn handler_is_contextual() { - assert!(matches!(TaskCmd::handler(), CommandHandler::Contextual(_))); + let CommandHandler::Contextual { + capabilities, + handler, + } = TaskCmd::handler() + else { + panic!("task must be contextual"); + }; + assert_eq!(capabilities, CommandCapabilities::WORKSPACE); + let missing = handler(CommandContexts::empty(), Some("list")); + assert!(missing.is_error); + assert_eq!( + missing.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); assert_eq!(TaskCmd::info().description_key, "cmd_task_description"); assert_eq!(TaskCmd::info().aliases, &["tasks"]); } diff --git a/crates/tui/src/commands/mod.rs b/crates/tui/src/commands/mod.rs index 8d8083553b..051cfcf8d0 100644 --- a/crates/tui/src/commands/mod.rs +++ b/crates/tui/src/commands/mod.rs @@ -133,7 +133,12 @@ impl codewhale_command_contract::metadata::RegisterCommand for Fe } fn handler() -> codewhale_command_contract::handler::CommandHandler { - codewhale_command_contract::handler::CommandHandler::Contextual(feat015_contextual) + codewhale_command_contract::handler::CommandHandler::Contextual { + capabilities: codewhale_command_contract::handler::CommandCapabilities::WORKSPACE + .union(codewhale_command_contract::handler::CommandCapabilities::MODE_POLICY) + .union(codewhale_command_contract::handler::CommandCapabilities::COST), + handler: feat015_contextual, + } } } @@ -147,9 +152,18 @@ fn feat015_contextual( ) -> CommandResult { use codewhale_command_contract::handler::ContextParts; let parts: ContextParts<'_> = contexts.into_parts(); - let workspace = parts.workspace.expect("workspace facet").workspace(); - let mode = parts.mode_policy.expect("mode-policy facet").mode(); - let currency = parts.cost.expect("cost facet").display_currency(); + let Some(workspace) = parts.workspace else { + return CommandResult::error("Command capability unavailable: workspace"); + }; + let Some(mode_policy) = parts.mode_policy else { + return CommandResult::error("Command capability unavailable: mode-policy"); + }; + let Some(cost) = parts.cost else { + return CommandResult::error("Command capability unavailable: cost"); + }; + let workspace = workspace.workspace(); + let mode = mode_policy.mode(); + let currency = cost.display_currency(); let normalized = arg.unwrap_or(""); CommandResult::message(format!( "feat015ctx workspace={} mode={:?} currency={:?} arg={}", @@ -265,17 +279,21 @@ pub fn execute(cmd: &str, app: &mut App) -> CommandResult { }; // FEAT-015 dual-path seam (D2): a migrated entry with a // capability-scoped handler receives the envelope built from `app`; - // everything else keeps the legacy `execute(app, args)` path. No - // production entry is migrated in FEAT-015, so the contextual branch - // is only reachable by the test-only fixture (D6). + // everything else keeps the legacy `execute(app, args)` path. The + // envelope is populated only with the capabilities the registration + // declared (FEAT-019 D1/D3); production groups such as utility and + // memory dispatch through this contextual branch. if let Some(handler) = command_object.contextual_handler() { - let mut bundle = app.command_contexts(); return match handler { codewhale_command_contract::handler::CommandHandler::Pure(pure_fn) => { pure_fn(command_arg) } - codewhale_command_contract::handler::CommandHandler::Contextual(contextual) => { - contextual(bundle.contexts(), command_arg) + codewhale_command_contract::handler::CommandHandler::Contextual { + capabilities, + handler: contextual, + } => { + let mut bundle = app.command_contexts(); + contextual(bundle.contexts(capabilities), command_arg) } }; } @@ -1917,6 +1935,20 @@ mod tests { assert!(result.action.is_none()); } + #[test] + fn feat015_contextual_command_fails_safely_without_declared_facets() { + let result = feat015_contextual( + codewhale_command_contract::handler::CommandContexts::empty(), + None, + ); + assert!(result.is_error, "{result:?}"); + assert_eq!( + result.message.as_deref(), + Some("Error: Command capability unavailable: workspace") + ); + assert!(result.action.is_none()); + } + #[test] fn feat015_contextual_command_is_registered_only_in_test_builds() { // The fixture entry is present in the test-build registry with a @@ -1935,11 +1967,9 @@ mod tests { fn feat015_all_production_entries_remain_legacy() { // FEAT-015 shipped no production contextual command, so the assertion // below used to exclude nothing. FEAT-018 migrates the utility group; - // the remaining non-fixture commands must still use the legacy - // concrete-App path. The migrated groups (FEAT-018 utility seven plus - // `/dispatch`, and the FEAT-021 project four) are asserted separately - // by their public-dispatch and inventory tests. + // FEAT-019 migrates the memory group; FEAT-021 migrates the project group. const MIGRATED_GROUPS: &[&str] = &[ + // FEAT-018 utility group. "attach", "automation", "dispatch", @@ -1953,6 +1983,9 @@ mod tests { "lsp", "share", "goal", + // FEAT-019 memory group. + "note", + "memory", // FEAT-022 skills group. "skills", "skill", @@ -2074,12 +2107,17 @@ mod tests { #[test] fn feat021_project_entries_register_through_portable_bridge() { - // main's model carries no capability bitmask: each project command - // must register through the portable bridge as a contextual handler - // and dispatch safely through the public seam. Exact facet - // destructuring (D4) is proven by the handler tests and the adapter - // exposure test. - for name in ["init", "lsp", "share", "goal"] { + use codewhale_command_contract::handler::{CommandCapabilities, CommandHandler}; + + for (name, expected) in [ + ("init", CommandCapabilities::WORKSPACE), + ("lsp", CommandCapabilities::PROJECT), + ("share", CommandCapabilities::PROJECT), + ( + "goal", + CommandCapabilities::PROJECT.union(CommandCapabilities::PRESENTATION), + ), + ] { assert!( registry().has_contextual_handler(name), "/{name} must register through the portable bridge" @@ -2089,13 +2127,10 @@ mod tests { .expect("entry") .contextual_handler() .expect("contextual handler"); - assert!( - matches!( - handler, - codewhale_command_contract::handler::CommandHandler::Contextual(_) - ), - "/{name} must be contextual" - ); + let CommandHandler::Contextual { capabilities, .. } = handler else { + panic!("/{name} must be contextual"); + }; + assert_eq!(capabilities, expected, "/{name} exact capability set"); } } @@ -2169,6 +2204,63 @@ mod tests { ); } } + + // --------------------------------------------------------------------- + // FEAT-019: public memory registration/dispatch and exact capability + // declarations (Task 6.2). Tests enter through the registry and the + // public `execute` seam and prove the memory group's portable entries. + // --------------------------------------------------------------------- + + /// App with an isolated temp workspace and memory enabled. + fn memory_test_app(tmpdir: &tempfile::TempDir) -> App { + let options = TuiOptions { + memory_path: tmpdir.path().join("memory.md"), + use_memory: true, + ..crate::test_support::test_tui_options(tmpdir.path()) + }; + App::new(options, &Config::default()) + } + + #[test] + fn feat019_memory_entries_are_registered_with_exact_capabilities() { + for (name, expected) in [ + ( + "note", + codewhale_command_contract::handler::CommandCapabilities::WORKSPACE, + ), + ( + "memory", + codewhale_command_contract::handler::CommandCapabilities::WORKSPACE + .union(codewhale_command_contract::handler::CommandCapabilities::MEMORY), + ), + ] { + assert!( + registry().has_contextual_handler(name), + "/{name} must register through the portable bridge" + ); + let handler = registry() + .get(name) + .expect("entry") + .contextual_handler() + .expect("contextual handler"); + let codewhale_command_contract::handler::CommandHandler::Contextual { + capabilities, + .. + } = handler + else { + panic!("/{name} must be contextual"); + }; + assert_eq!(capabilities, expected, "/{name} exact capability set"); + assert!( + !capabilities.contains( + codewhale_command_contract::handler::CommandCapabilities::PRESENTATION + ) && !capabilities + .contains(codewhale_command_contract::handler::CommandCapabilities::MEDIA), + "/{name} must not declare presentation or media" + ); + } + } + // --------------------------------------------------------------------- // FEAT-022: skills group registration + public dispatch (Task 6.2). // All four commands register through the portable bridge; frontier state @@ -2225,20 +2317,35 @@ mod tests { #[test] fn feat022_all_four_skills_entries_are_registered_with_portable_handlers() { - for (name, alias) in [ - ("skills", Some("jinengliebiao")), - ("skill", Some("jineng")), - ("review", Some("shencha")), - ("restore", None), + use codewhale_command_contract::handler::{CommandCapabilities, CommandHandler}; + + for (name, alias, expected) in [ + ( + "skills", + Some("jinengliebiao"), + CommandCapabilities::SKILL_GROUP, + ), + ( + "skill", + Some("jineng"), + CommandCapabilities::SKILL_GROUP.union(CommandCapabilities::SKILLS), + ), + ("review", Some("shencha"), CommandCapabilities::SKILL_GROUP), + ("restore", None, CommandCapabilities::SKILL_GROUP), ] { let info = registry() .get_info(name) .unwrap_or_else(|| panic!("/{name} must be registered")); assert_eq!(info.name, name, "canonical name"); - assert!( - registry().has_contextual_handler(name), - "/{name} must carry a portable handler" - ); + let handler = registry() + .get(name) + .expect("entry") + .contextual_handler() + .expect("contextual handler"); + let CommandHandler::Contextual { capabilities, .. } = handler else { + panic!("/{name} must be contextual"); + }; + assert_eq!(capabilities, expected, "/{name} exact capability set"); if let Some(alias) = alias { assert!( registry().get_info(alias).is_some(), @@ -2248,6 +2355,75 @@ mod tests { } } + #[test] + fn feat019_note_dispatches_through_public_seam() { + let tmpdir = tempfile::TempDir::new().unwrap(); + let mut app = memory_test_app(&tmpdir); + + let appended = execute("/note hello from dispatch", &mut app); + assert!(!appended.is_error, "{appended:?}"); + assert!( + appended + .message + .as_deref() + .is_some_and(|msg| msg.contains("Note appended to")), + "{appended:?}" + ); + let notes = tmpdir.path().join(".deepseek").join("notes.md"); + assert!(notes.exists(), "notes file written under the workspace"); + let content = std::fs::read_to_string(¬es).unwrap(); + assert!(content.contains("hello from dispatch")); + + // Metadata bridges to the TUI localization id. + let info = registry().get_info("note").expect("note info"); + assert_eq!( + info.description_id, + crate::localization::MessageId::CmdNoteDescription + ); + } + + #[test] + fn feat019_memory_dispatches_through_public_seam() { + let tmpdir = tempfile::TempDir::new().unwrap(); + let mut app = memory_test_app(&tmpdir); + + let path = execute("/memory path", &mut app); + assert!(!path.is_error, "{path:?}"); + assert_eq!( + path.message.as_deref(), + Some(tmpdir.path().join("memory.md").to_str().unwrap()) + ); + + // Native status reaches the real adapter through the public seam. + let status = execute("/memory native status", &mut app); + assert!(!status.is_error, "{status:?}"); + let msg = status.message.expect("status message"); + assert!(msg.contains("native memory:"), "{msg}"); + + let info = registry().get_info("memory").expect("memory info"); + assert_eq!( + info.description_id, + crate::localization::MessageId::CmdMemoryDescription + ); + } + + #[test] + fn feat019_public_dispatch_never_panics_on_memory_commands() { + let tmpdir = tempfile::TempDir::new().unwrap(); + let mut app = memory_test_app(&tmpdir); + for command in [ + "/note", + "/note ", + "/memory", + "/memory native bogus", + "/memory wat", + ] { + let result = execute(command, &mut app); + // Every path returns a result; none may panic. + assert!(result.message.is_some(), "{command}: {result:?}"); + } + } + #[test] fn feat022_skills_commands_dispatch_through_public_seam() { let tmp = tempfile::TempDir::new().unwrap(); @@ -2314,9 +2490,10 @@ mod tests { #[test] fn feat022_context_exposure_is_exact_per_d4() { - // The envelope exposes every adapter (main's model); the handlers - // consume exactly their required facets. skills/review/restore consume - // only skill_group; skill also consumes skills for cache refreshes. + // The test-only full envelope exposes every adapter; production + // dispatch exposes only each handler's declared facets. + // skills/review/restore consume only skill_group; skill also consumes + // skills for cache refreshes. let tmp = tempfile::TempDir::new().unwrap(); let _home = feat022_scoped_home(&tmp); let mut app = feat022_test_app(&tmp); diff --git a/scripts/command-migration-topology.json b/scripts/command-migration-topology.json index 327ef17063..a0a17f0b1b 100644 --- a/scripts/command-migration-topology.json +++ b/scripts/command-migration-topology.json @@ -282,7 +282,6 @@ "config", "core", "debug", - "memory", "plugins", "session" ] diff --git a/scripts/test_check_command_migration_manifest.py b/scripts/test_check_command_migration_manifest.py index acb0a917ca..191147ef86 100644 --- a/scripts/test_check_command_migration_manifest.py +++ b/scripts/test_check_command_migration_manifest.py @@ -362,12 +362,11 @@ def test_topology_artifact_is_sorted_unique(self) -> None: frontier = doc["frontier"] self.assertEqual(frontier, sorted(frontier)) self.assertEqual(len(frontier), len(set(frontier))) - # FEAT-018 removed the utility group from the frontier (Stage B first - # slice); FEAT-021 removed the project group; the remaining seven - # groups stay pending. + # FEAT-018 removed utility, FEAT-019 removed memory, FEAT-021 removed project, + # and FEAT-022 removed skills; the remaining five groups stay pending. self.assertEqual( set(frontier), - {"memory", "plugins", "session", "config", "debug", "core"}, + {"plugins", "session", "config", "debug", "core"}, )