From b8b614d0f103c9680c727f93bf6f080898f8fe1b Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Tue, 7 Jul 2026 12:57:17 -0400 Subject: [PATCH 1/3] Add memory lifecycle controls Signed-off-by: Timothy Wayne Gregg --- docs/configuration.md | 35 ++ .../crates/commands/src/named_commands.rs | 537 ++++++++++++++++++ 2 files changed, 572 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index 87cc383..0e40ccc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -402,6 +402,41 @@ conflict record is written for operator review instead of overwriting local memory. Hosted team-memory sync also sends tenant, installation, repo, and domain scope metadata so the server can authorize the full tuple. +### Memory lifecycle operations + +Operators can inspect and apply memory lifecycle controls with the named +`memory` command: + +```bash +coven-code memory list +coven-code memory delete policy.md --reason "user requested deletion" +coven-code memory redact policy.md --reason "security incident" +coven-code memory redact policy.md --team --reason "security incident" +coven-code memory conflicts +coven-code memory resolve-conflict MEMORY.md +``` + +`delete` and `redact` accept only repository-relative memory keys, reject path +traversal, and never print the original memory body. Deleting a synced team +memory file writes a `deleted_at` tombstone instead of removing the file, so the +next sync can propagate the deletion and avoid resurrecting stale content. +Redaction writes a `redacted_at` audit stub and removes the original body. + +Hosted memory namespace deletion requires the full hosted scope: + +```bash +coven-code memory hosted-delete \ + --tenant tenant-a \ + --installation install-1 \ + --repo-id repo-99 \ + --repo OpenCoven/coven-code \ + --domain pr:42 +``` + +All hosted delete inputs are explicit; Coven Code does not infer or broaden the +scope. Supported domains are `default`, `security-private`, `pr:`, +`branch:`, and `release:`. + ### @include directives AGENTS.md files support `@include` to pull in content from other files: diff --git a/src-rust/crates/commands/src/named_commands.rs b/src-rust/crates/commands/src/named_commands.rs index 0bdbe92..1801131 100644 --- a/src-rust/crates/commands/src/named_commands.rs +++ b/src-rust/crates/commands/src/named_commands.rs @@ -18,6 +18,7 @@ use crate::{CommandContext, CommandResult}; use once_cell::sync::Lazy; +use std::path::{Path, PathBuf}; // `open` crate: used by StickersCommand to launch the browser. // --------------------------------------------------------------------------- @@ -387,6 +388,282 @@ impl NamedCommand for AddDirCommand { } } +// --------------------------------------------------------------------------- +// memory +// --------------------------------------------------------------------------- + +pub struct MemoryCommand; + +impl NamedCommand for MemoryCommand { + fn name(&self) -> &str { + "memory" + } + fn description(&self) -> &str { + "Inspect and manage lifecycle controls for local, team, and hosted memory" + } + fn usage(&self) -> &str { + "coven-code memory [list|delete|redact|conflicts|resolve-conflict|hosted-delete] ..." + } + + fn execute_named(&self, args: &[&str], ctx: &CommandContext) -> CommandResult { + match args.first().copied().unwrap_or("list") { + "list" | "status" => memory_list(ctx), + "delete" => memory_delete_or_redact(args, ctx, MemoryLifecycleAction::Delete), + "redact" => memory_delete_or_redact(args, ctx, MemoryLifecycleAction::Redact), + "conflicts" => memory_conflicts(ctx), + "resolve-conflict" => memory_resolve_conflict(args, ctx), + "hosted-delete" => memory_hosted_delete(args), + "--help" | "-h" | "help" => CommandResult::Message(memory_usage()), + sub => CommandResult::Error(format!( + "Unknown memory subcommand: '{sub}'\n{}", + memory_usage() + )), + } + } +} + +#[derive(Clone, Copy)] +enum MemoryLifecycleAction { + Delete, + Redact, +} + +fn memory_usage() -> String { + "Usage: coven-code memory \n\n\ + Subcommands:\n\ + list\n\ + delete --reason [--team]\n\ + redact --reason [--team]\n\ + conflicts\n\ + resolve-conflict \n\ + hosted-delete --tenant --installation --repo-id --repo [--domain ]\n\n\ + Domains: default, security-private, pr:, branch:, release:" + .to_string() +} + +fn memory_root(ctx: &CommandContext) -> PathBuf { + claurst_core::memdir::auto_memory_path(&ctx.working_dir) +} + +fn team_memory_root(ctx: &CommandContext) -> PathBuf { + claurst_core::memdir::team_memory_path(&memory_root(ctx)) +} + +fn memory_list(ctx: &CommandContext) -> CommandResult { + let memory_dir = memory_root(ctx); + let team_dir = claurst_core::memdir::team_memory_path(&memory_dir); + let memories = claurst_core::memdir::scan_memory_dir(&memory_dir); + let conflicts = claurst_core::team_memory_sync::pending_conflicts(&team_dir); + + let mut out = String::new(); + out.push_str("Memory lifecycle status\n"); + out.push_str(&format!(" Local memory: {}\n", memory_dir.display())); + out.push_str(&format!(" Team memory: {}\n", team_dir.display())); + out.push_str(&format!(" Memory files: {}\n", memories.len())); + for memory in memories.iter().take(50) { + out.push_str(&format!(" {}\n", memory.filename)); + } + if memories.len() > 50 { + out.push_str(&format!(" ... {} more\n", memories.len() - 50)); + } + out.push_str(&format!(" Pending team conflicts: {}\n", conflicts.len())); + + CommandResult::Message(out) +} + +fn memory_delete_or_redact( + args: &[&str], + ctx: &CommandContext, + action: MemoryLifecycleAction, +) -> CommandResult { + let key = match args.get(1).copied() { + Some(value) if !value.starts_with("--") => value, + _ => { + let verb = match action { + MemoryLifecycleAction::Delete => "delete", + MemoryLifecycleAction::Redact => "redact", + }; + return CommandResult::Error(format!( + "Usage: coven-code memory {verb} --reason [--team]" + )); + } + }; + let reason = match flag_value(args, "--reason") { + Some(value) if !value.trim().is_empty() => value, + _ => return CommandResult::Error("--reason is required".to_string()), + }; + + let root = if has_flag(args, "--team") { + team_memory_root(ctx) + } else { + memory_root(ctx) + }; + let path = match memory_file_path(&root, key) { + Ok(path) => path, + Err(message) => return CommandResult::Error(message), + }; + if !path.is_file() { + return CommandResult::Error(format!("Memory file not found: {key}")); + } + + let result = match action { + MemoryLifecycleAction::Delete => claurst_core::memdir::delete_memory_file(&path, reason), + MemoryLifecycleAction::Redact => claurst_core::memdir::redact_memory_file(&path, reason), + }; + match result { + Ok(()) => { + let verb = match action { + MemoryLifecycleAction::Delete => "Deleted", + MemoryLifecycleAction::Redact => "Redacted", + }; + CommandResult::Message(format!("{verb} memory file: {key}")) + } + Err(err) => CommandResult::Error(format!("Failed to update memory file: {err}")), + } +} + +fn memory_conflicts(ctx: &CommandContext) -> CommandResult { + let team_dir = team_memory_root(ctx); + let conflicts = claurst_core::team_memory_sync::pending_conflicts(&team_dir); + if conflicts.is_empty() { + return CommandResult::Message(format!( + "No pending team-memory conflicts.\nTeam memory: {}", + team_dir.display() + )); + } + + let mut out = format!("Pending team-memory conflicts ({})\n", conflicts.len()); + for conflict in conflicts { + out.push_str(&format!( + " {} [{:?}]: {}\n", + conflict.key, conflict.kind, conflict.reason + )); + } + CommandResult::Message(out) +} + +fn memory_resolve_conflict(args: &[&str], ctx: &CommandContext) -> CommandResult { + let key = match args.get(1).copied() { + Some(value) if !value.starts_with("--") => value, + _ => { + return CommandResult::Error( + "Usage: coven-code memory resolve-conflict ".to_string(), + ) + } + }; + if let Err(err) = claurst_core::team_memory_sync::validate_memory_path(key) { + return CommandResult::Error(format!("Invalid memory key: {err}")); + } + + let team_dir = team_memory_root(ctx); + match claurst_core::team_memory_sync::resolve_conflict(&team_dir, key) { + Ok(true) => CommandResult::Message(format!("Resolved team-memory conflict: {key}")), + Ok(false) => CommandResult::Error(format!("No pending conflict for key: {key}")), + Err(err) => CommandResult::Error(format!("Failed to resolve conflict: {err}")), + } +} + +fn memory_hosted_delete(args: &[&str]) -> CommandResult { + let scope = match hosted_scope_from_args(args) { + Ok(scope) => scope, + Err(message) => return CommandResult::Error(message), + }; + let path = claurst_core::memdir::hosted_memory_path_for_scope(&scope); + match claurst_core::memdir::delete_hosted_memory_for_scope(&scope) { + Ok(()) => CommandResult::Message(format!( + "Deleted hosted memory namespace: {}", + path.display() + )), + Err(err) => CommandResult::Error(format!("Failed to delete hosted memory: {err}")), + } +} + +fn hosted_scope_from_args( + args: &[&str], +) -> Result { + let tenant = required_flag(args, "--tenant")?; + let installation = required_flag(args, "--installation")?; + let repo_id = required_flag(args, "--repo-id")?; + let repo = required_flag(args, "--repo")?; + let mut scope = claurst_core::hosted_review::HostedReviewScope::new( + tenant.to_string(), + installation.to_string(), + repo_id.to_string(), + repo.to_string(), + ); + if let Some(domain) = flag_value(args, "--domain") { + scope = scope.with_domain(parse_memory_domain(domain)?); + } + scope + .validate() + .map_err(|err| format!("Invalid hosted scope: {err}"))?; + Ok(scope) +} + +fn parse_memory_domain(raw: &str) -> Result { + if raw == "default" || raw == "default-branch" { + return Ok(claurst_core::hosted_review::MemoryDomain::DefaultBranch); + } + if raw == "security-private" { + return Ok(claurst_core::hosted_review::MemoryDomain::SecurityPrivate); + } + if let Some(value) = raw.strip_prefix("pr:") { + let number = value + .parse::() + .map_err(|_| format!("Invalid pull request domain: {raw}"))?; + return Ok(claurst_core::hosted_review::MemoryDomain::PullRequest( + number, + )); + } + if let Some(value) = raw.strip_prefix("branch:") { + if value.trim().is_empty() { + return Err("branch domain requires a branch name".to_string()); + } + return Ok(claurst_core::hosted_review::MemoryDomain::Branch( + value.to_string(), + )); + } + if let Some(value) = raw.strip_prefix("release:") { + if value.trim().is_empty() { + return Err("release domain requires a release name".to_string()); + } + return Ok(claurst_core::hosted_review::MemoryDomain::Release( + value.to_string(), + )); + } + Err(format!("Unsupported hosted memory domain: {raw}")) +} + +fn memory_file_path(root: &Path, key: &str) -> Result { + if key.trim().is_empty() { + return Err("Memory key is required".to_string()); + } + claurst_core::team_memory_sync::validate_memory_path(key) + .map_err(|err| format!("Invalid memory key: {err}"))?; + Ok(root.join(key)) +} + +fn required_flag<'a>(args: &'a [&str], name: &str) -> Result<&'a str, String> { + match flag_value(args, name) { + Some(value) if !value.trim().is_empty() => Ok(value), + _ => Err(format!("{name} is required")), + } +} + +fn flag_value<'a>(args: &'a [&str], name: &str) -> Option<&'a str> { + args.windows(2).find_map(|window| { + if window[0] == name && !window[1].starts_with("--") { + Some(window[1]) + } else { + None + } + }) +} + +fn has_flag(args: &[&str], name: &str) -> bool { + args.contains(&name) +} + // --------------------------------------------------------------------------- // branch // --------------------------------------------------------------------------- @@ -899,6 +1176,7 @@ static NAMED_COMMANDS: Lazy>> = Lazy::new(|| { Box::new(AgentsCommand), Box::new(AgentCommand), Box::new(AddDirCommand), + Box::new(MemoryCommand), Box::new(BranchCommand), Box::new(TagCommand), Box::new(PrCommentsCommand), @@ -952,6 +1230,58 @@ mod tests { } } + struct RemoteMemoryEnvGuard { + old_remote_memory_dir: Option, + } + + impl RemoteMemoryEnvGuard { + fn set(path: &std::path::Path) -> Self { + let guard = Self { + old_remote_memory_dir: std::env::var("COVEN_CODE_REMOTE_MEMORY_DIR").ok(), + }; + std::env::set_var("COVEN_CODE_REMOTE_MEMORY_DIR", path); + guard + } + } + + impl Drop for RemoteMemoryEnvGuard { + fn drop(&mut self) { + match &self.old_remote_memory_dir { + Some(value) => std::env::set_var("COVEN_CODE_REMOTE_MEMORY_DIR", value), + None => std::env::remove_var("COVEN_CODE_REMOTE_MEMORY_DIR"), + } + } + } + + struct MemoryTestEnv { + _remote_memory: RemoteMemoryEnvGuard, + _command_env: CommandEnvGuard, + _tmp: tempfile::TempDir, + } + + fn memory_test_ctx() -> (CommandContext, MemoryTestEnv) { + let tmp = tempfile::tempdir().expect("tempdir"); + let home = tmp.path().join("home"); + let coven_home = tmp.path().join("coven"); + let project = tmp.path().join("project"); + let memory_base = tmp.path().join("remote-memory"); + std::fs::create_dir_all(&home).expect("home dir"); + std::fs::create_dir_all(&coven_home).expect("coven home dir"); + std::fs::create_dir_all(&project).expect("project dir"); + std::fs::create_dir_all(&memory_base).expect("memory base"); + let command_env = CommandEnvGuard::set(&home, &coven_home, None); + let remote_memory = RemoteMemoryEnvGuard::set(&memory_base); + let ctx = make_ctx_at(project); + ( + ctx, + MemoryTestEnv { + _remote_memory: remote_memory, + _command_env: command_env, + _tmp: tmp, + }, + ) + } + #[test] fn test_all_named_commands_non_empty() { assert!(!all_named_commands().is_empty()); @@ -973,6 +1303,7 @@ mod tests { fn test_find_named_command_found() { assert!(find_named_command("agents").is_some()); assert!(find_named_command("branch").is_some()); + assert!(find_named_command("memory").is_some()); } #[test] @@ -1090,6 +1421,212 @@ mod tests { assert!(matches!(result, CommandResult::Error(_))); } + #[test] + fn test_memory_list_shows_paths_without_body() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + std::fs::write(memory_dir.join("policy.md"), "secret body").expect("memory file"); + + let result = MemoryCommand.execute_named(&["list"], &ctx); + + let CommandResult::Message(msg) = result else { + panic!("Expected Message"); + }; + assert!(msg.contains("policy.md")); + assert!(msg.contains("Team memory:")); + assert!(!msg.contains("secret body")); + } + + #[test] + fn test_memory_delete_writes_tombstone_without_original_body() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + let path = memory_dir.join("policy.md"); + std::fs::write(&path, "sensitive fact").expect("memory file"); + + let result = + MemoryCommand.execute_named(&["delete", "policy.md", "--reason", "operator"], &ctx); + + assert!(matches!(result, CommandResult::Message(_))); + let content = std::fs::read_to_string(path).expect("updated memory"); + assert!(content.contains("deleted_at:")); + assert!(content.contains("[DELETED: operator]")); + assert!(!content.contains("sensitive fact")); + } + + #[test] + fn test_memory_redact_team_file_writes_stub_without_original_body() { + let (ctx, _env) = memory_test_ctx(); + let team_dir = team_memory_root(&ctx); + std::fs::create_dir_all(&team_dir).expect("team dir"); + let path = team_dir.join("team.md"); + std::fs::write(&path, "security incident detail").expect("team memory"); + + let result = MemoryCommand.execute_named( + &["redact", "team.md", "--reason", "request", "--team"], + &ctx, + ); + + assert!(matches!(result, CommandResult::Message(_))); + let content = std::fs::read_to_string(path).expect("updated team memory"); + assert!(content.contains("redacted_at:")); + assert!(content.contains("[REDACTED: request]")); + assert!(!content.contains("security incident detail")); + } + + #[test] + fn test_memory_rejects_path_traversal() { + let (ctx, _env) = memory_test_ctx(); + + let result = + MemoryCommand.execute_named(&["delete", "../secret.md", "--reason", "operator"], &ctx); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Invalid memory key")); + } + + #[test] + fn test_memory_rejects_missing_flag_values() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + std::fs::write(memory_dir.join("policy.md"), "body").expect("memory file"); + + let result = + MemoryCommand.execute_named(&["delete", "policy.md", "--reason", "--team"], &ctx); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("--reason is required")); + } + + #[test] + fn test_memory_conflicts_lists_pending_records_without_content() { + let (ctx, _env) = memory_test_ctx(); + let team_dir = team_memory_root(&ctx); + let conflict_dir = team_dir.join(".conflicts"); + std::fs::create_dir_all(&conflict_dir).expect("conflict dir"); + let conflict = claurst_core::team_memory_sync::TeamMemoryPullConflict { + key: "MEMORY.md".to_string(), + kind: claurst_core::team_memory_sync::PullConflictKind::BothChanged, + local_checksum: Some("local".to_string()), + base_checksum: Some("base".to_string()), + remote_checksum: Some("remote".to_string()), + reason: "local and remote changed".to_string(), + }; + let record = serde_json::json!({ + "conflict": conflict, + "local": "do not print local body", + "remote": { "key": "MEMORY.md", "content": "do not print remote body" } + }); + std::fs::write( + conflict_dir.join("MEMORY.md.json"), + serde_json::to_string(&record).expect("conflict json"), + ) + .expect("conflict write"); + + let result = MemoryCommand.execute_named(&["conflicts"], &ctx); + + let CommandResult::Message(msg) = result else { + panic!("Expected Message"); + }; + assert!(msg.contains("MEMORY.md")); + assert!(msg.contains("BothChanged")); + assert!(!msg.contains("do not print")); + } + + #[test] + fn test_memory_resolve_conflict_removes_only_matching_record() { + let (ctx, _env) = memory_test_ctx(); + let team_dir = team_memory_root(&ctx); + let conflict_dir = team_dir.join(".conflicts"); + std::fs::create_dir_all(&conflict_dir).expect("conflict dir"); + std::fs::write(conflict_dir.join("MEMORY.md.json"), "{}").expect("first conflict"); + std::fs::write(conflict_dir.join("other.md.json"), "{}").expect("second conflict"); + + let result = MemoryCommand.execute_named(&["resolve-conflict", "MEMORY.md"], &ctx); + + assert!(matches!(result, CommandResult::Message(_))); + assert!(!conflict_dir.join("MEMORY.md.json").exists()); + assert!(conflict_dir.join("other.md.json").exists()); + } + + #[test] + fn test_memory_hosted_delete_targets_only_requested_namespace() { + let tmp = tempfile::tempdir().expect("tempdir"); + let home = tmp.path().join("home"); + let coven_home = tmp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home"); + std::fs::create_dir_all(&coven_home).expect("coven home"); + let _env_guard = CommandEnvGuard::set(&home, &coven_home, None); + let first = claurst_core::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let second = claurst_core::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-2".to_string(), + "OpenCoven/other".to_string(), + ); + let first_dir = claurst_core::memdir::hosted_memory_path_for_scope(&first); + let second_dir = claurst_core::memdir::hosted_memory_path_for_scope(&second); + std::fs::create_dir_all(&first_dir).expect("first hosted dir"); + std::fs::create_dir_all(&second_dir).expect("second hosted dir"); + std::fs::write(first_dir.join("MEMORY.md"), "first").expect("first memory"); + std::fs::write(second_dir.join("MEMORY.md"), "second").expect("second memory"); + + let result = MemoryCommand.execute_named( + &[ + "hosted-delete", + "--tenant", + "tenant-a", + "--installation", + "install-1", + "--repo-id", + "repo-1", + "--repo", + "OpenCoven/coven-code", + ], + &make_ctx(), + ); + + assert!(matches!(result, CommandResult::Message(_))); + assert!(!first_dir.exists()); + assert!(second_dir.exists()); + } + + #[test] + fn test_memory_hosted_delete_rejects_missing_scope_value() { + let ctx = make_ctx(); + + let result = MemoryCommand.execute_named( + &[ + "hosted-delete", + "--tenant", + "--installation", + "install-1", + "--repo-id", + "repo-1", + "--repo", + "OpenCoven/coven-code", + ], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("--tenant is required")); + } + #[test] fn test_pr_comments_no_gh_returns_error() { // Without `gh` installed or outside a git repo with an open PR, From 3dfb59e49b316a66e62bc4a292d6d583c257bc6d Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Tue, 7 Jul 2026 13:39:25 -0400 Subject: [PATCH 2/3] Harden memory lifecycle command parsing Fixes #109 Signed-off-by: Timothy Wayne Gregg --- .../crates/commands/src/named_commands.rs | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/src-rust/crates/commands/src/named_commands.rs b/src-rust/crates/commands/src/named_commands.rs index 1801131..f50961a 100644 --- a/src-rust/crates/commands/src/named_commands.rs +++ b/src-rust/crates/commands/src/named_commands.rs @@ -488,6 +488,9 @@ fn memory_delete_or_redact( )); } }; + if let Err(message) = validate_memory_args(args, 1, &["--reason"], &["--team"]) { + return CommandResult::Error(message); + } let reason = match flag_value(args, "--reason") { Some(value) if !value.trim().is_empty() => value, _ => return CommandResult::Error("--reason is required".to_string()), @@ -551,6 +554,12 @@ fn memory_resolve_conflict(args: &[&str], ctx: &CommandContext) -> CommandResult ) } }; + if let Err(message) = validate_memory_args(args, 1, &[], &[]) { + return CommandResult::Error(message); + } + if key.trim().is_empty() { + return CommandResult::Error("Memory key is required".to_string()); + } if let Err(err) = claurst_core::team_memory_sync::validate_memory_path(key) { return CommandResult::Error(format!("Invalid memory key: {err}")); } @@ -564,6 +573,20 @@ fn memory_resolve_conflict(args: &[&str], ctx: &CommandContext) -> CommandResult } fn memory_hosted_delete(args: &[&str]) -> CommandResult { + if let Err(message) = validate_memory_args( + args, + 0, + &[ + "--tenant", + "--installation", + "--repo-id", + "--repo", + "--domain", + ], + &[], + ) { + return CommandResult::Error(message); + } let scope = match hosted_scope_from_args(args) { Ok(scope) => scope, Err(message) => return CommandResult::Error(message), @@ -643,6 +666,36 @@ fn memory_file_path(root: &Path, key: &str) -> Result { Ok(root.join(key)) } +fn validate_memory_args( + args: &[&str], + positional_count: usize, + value_flags: &[&str], + bool_flags: &[&str], +) -> Result<(), String> { + let mut index = 1 + positional_count; + while index < args.len() { + let arg = args[index]; + if !arg.starts_with("--") { + return Err(format!("Unexpected argument: {arg}")); + } + if bool_flags.contains(&arg) { + index += 1; + continue; + } + if value_flags.contains(&arg) { + match args.get(index + 1).copied() { + Some(value) if !value.starts_with("--") && !value.trim().is_empty() => { + index += 2; + continue; + } + _ => return Err(format!("{arg} is required")), + } + } + return Err(format!("Unknown flag: {arg}")); + } + Ok(()) +} + fn required_flag<'a>(args: &'a [&str], name: &str) -> Result<&'a str, String> { match flag_value(args, name) { Some(value) if !value.trim().is_empty() => Ok(value), @@ -1505,6 +1558,42 @@ mod tests { assert!(msg.contains("--reason is required")); } + #[test] + fn test_memory_rejects_unknown_destructive_flags() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + std::fs::write(memory_dir.join("policy.md"), "body").expect("memory file"); + + let result = MemoryCommand.execute_named( + &["delete", "policy.md", "--reason", "operator", "--unknown"], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Unknown flag: --unknown")); + } + + #[test] + fn test_memory_rejects_extra_destructive_positionals() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + std::fs::write(memory_dir.join("policy.md"), "body").expect("memory file"); + + let result = MemoryCommand.execute_named( + &["redact", "policy.md", "extra.md", "--reason", "operator"], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Unexpected argument: extra.md")); + } + #[test] fn test_memory_conflicts_lists_pending_records_without_content() { let (ctx, _env) = memory_test_ctx(); @@ -1556,6 +1645,19 @@ mod tests { assert!(conflict_dir.join("other.md.json").exists()); } + #[test] + fn test_memory_resolve_conflict_rejects_extra_args() { + let (ctx, _env) = memory_test_ctx(); + + let result = + MemoryCommand.execute_named(&["resolve-conflict", "MEMORY.md", "--force"], &ctx); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Unknown flag: --force")); + } + #[test] fn test_memory_hosted_delete_targets_only_requested_namespace() { let tmp = tempfile::tempdir().expect("tempdir"); @@ -1627,6 +1729,32 @@ mod tests { assert!(msg.contains("--tenant is required")); } + #[test] + fn test_memory_hosted_delete_rejects_unknown_flags() { + let ctx = make_ctx(); + + let result = MemoryCommand.execute_named( + &[ + "hosted-delete", + "--tenant", + "tenant-a", + "--installation", + "install-1", + "--repo-id", + "repo-1", + "--repo", + "OpenCoven/coven-code", + "--all", + ], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Unknown flag: --all")); + } + #[test] fn test_pr_comments_no_gh_returns_error() { // Without `gh` installed or outside a git repo with an open PR, From c5d89b27375c146c31a12295b17969faf51418d8 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Tue, 7 Jul 2026 13:52:26 -0400 Subject: [PATCH 3/3] Clarify team memory lifecycle targeting Fixes #109 Signed-off-by: Timothy Wayne Gregg --- docs/configuration.md | 4 +- .../crates/commands/src/named_commands.rs | 117 +++++++++++++++++- 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 0e40ccc..9cabfed 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -420,7 +420,9 @@ coven-code memory resolve-conflict MEMORY.md traversal, and never print the original memory body. Deleting a synced team memory file writes a `deleted_at` tombstone instead of removing the file, so the next sync can propagate the deletion and avoid resurrecting stale content. -Redaction writes a `redacted_at` audit stub and removes the original body. +Redaction writes a `redacted_at` audit stub and removes the original body. Keys +under the reserved `team/` directory require `--team`; with `--team`, keys are +resolved relative to the team-memory root. Hosted memory namespace deletion requires the full hosted scope: diff --git a/src-rust/crates/commands/src/named_commands.rs b/src-rust/crates/commands/src/named_commands.rs index f50961a..3bdbc72 100644 --- a/src-rust/crates/commands/src/named_commands.rs +++ b/src-rust/crates/commands/src/named_commands.rs @@ -496,7 +496,15 @@ fn memory_delete_or_redact( _ => return CommandResult::Error("--reason is required".to_string()), }; - let root = if has_flag(args, "--team") { + let targets_team = has_flag(args, "--team"); + if !targets_team && is_team_memory_key(key) { + return CommandResult::Error( + "Team memory keys require --team and must be relative to the team memory root" + .to_string(), + ); + } + + let root = if targets_team { team_memory_root(ctx) } else { memory_root(ctx) @@ -666,6 +674,12 @@ fn memory_file_path(root: &Path, key: &str) -> Result { Ok(root.join(key)) } +fn is_team_memory_key(key: &str) -> bool { + key.split('/') + .next() + .is_some_and(|component| component.eq_ignore_ascii_case("team")) +} + fn validate_memory_args( args: &[&str], positional_count: usize, @@ -673,11 +687,15 @@ fn validate_memory_args( bool_flags: &[&str], ) -> Result<(), String> { let mut index = 1 + positional_count; + let mut seen_flags = std::collections::HashSet::new(); while index < args.len() { let arg = args[index]; if !arg.starts_with("--") { return Err(format!("Unexpected argument: {arg}")); } + if !seen_flags.insert(arg) { + return Err(format!("Duplicate flag: {arg}")); + } if bool_flags.contains(&arg) { index += 1; continue; @@ -1576,6 +1594,31 @@ mod tests { assert!(msg.contains("Unknown flag: --unknown")); } + #[test] + fn test_memory_rejects_duplicate_destructive_flags() { + let (ctx, _env) = memory_test_ctx(); + let memory_dir = memory_root(&ctx); + std::fs::create_dir_all(&memory_dir).expect("memory dir"); + std::fs::write(memory_dir.join("policy.md"), "body").expect("memory file"); + + let result = MemoryCommand.execute_named( + &[ + "redact", + "policy.md", + "--reason", + "first", + "--reason", + "second", + ], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Duplicate flag: --reason")); + } + #[test] fn test_memory_rejects_extra_destructive_positionals() { let (ctx, _env) = memory_test_ctx(); @@ -1594,6 +1637,51 @@ mod tests { assert!(msg.contains("Unexpected argument: extra.md")); } + #[test] + fn test_memory_requires_team_flag_for_team_directory_keys() { + let (ctx, _env) = memory_test_ctx(); + let team_dir = team_memory_root(&ctx); + std::fs::create_dir_all(&team_dir).expect("team dir"); + std::fs::write(team_dir.join("policy.md"), "team body").expect("team memory"); + + let result = MemoryCommand + .execute_named(&["delete", "team/policy.md", "--reason", "operator"], &ctx); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Team memory keys require --team")); + let content = std::fs::read_to_string(team_dir.join("policy.md")).expect("team memory"); + assert_eq!(content, "team body"); + + let result = MemoryCommand + .execute_named(&["delete", "TEAM/policy.md", "--reason", "operator"], &ctx); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Team memory keys require --team")); + } + + #[test] + fn test_memory_team_flag_treats_key_as_relative_to_team_root() { + let (ctx, _env) = memory_test_ctx(); + let team_dir = team_memory_root(&ctx); + std::fs::create_dir_all(team_dir.join("team")).expect("nested team dir"); + let path = team_dir.join("team").join("policy.md"); + std::fs::write(&path, "nested team body").expect("nested team memory"); + + let result = MemoryCommand.execute_named( + &["delete", "team/policy.md", "--reason", "operator", "--team"], + &ctx, + ); + + assert!(matches!(result, CommandResult::Message(_))); + let content = std::fs::read_to_string(path).expect("updated team memory"); + assert!(content.contains("deleted_at:")); + assert!(!content.contains("nested team body")); + } + #[test] fn test_memory_conflicts_lists_pending_records_without_content() { let (ctx, _env) = memory_test_ctx(); @@ -1755,6 +1843,33 @@ mod tests { assert!(msg.contains("Unknown flag: --all")); } + #[test] + fn test_memory_hosted_delete_rejects_duplicate_scope_flags() { + let ctx = make_ctx(); + + let result = MemoryCommand.execute_named( + &[ + "hosted-delete", + "--tenant", + "tenant-a", + "--tenant", + "tenant-b", + "--installation", + "install-1", + "--repo-id", + "repo-1", + "--repo", + "OpenCoven/coven-code", + ], + &ctx, + ); + + let CommandResult::Error(msg) = result else { + panic!("Expected Error"); + }; + assert!(msg.contains("Duplicate flag: --tenant")); + } + #[test] fn test_pr_comments_no_gh_returns_error() { // Without `gh` installed or outside a git repo with an open PR,