From a11391340b0f7d7cab64066ddd4b7c50c55dec85 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 14:55:22 -0400 Subject: [PATCH 1/2] feat(core): scope hosted review identity Fixes #98. Fixes #99. Fixes #104. Fixes #110. - add hosted tenant/installation/repo/domain scope types - key hosted memory, transcripts, settings sync, and team memory by scoped identity - add canonical repo parsing and Windows test isolation for home-derived paths Signed-off-by: Timothy Wayne Gregg --- docs/advanced.md | 6 +- docs/configuration.md | 7 +- docs/shared/hosted-review-phase-2-pr.md | 43 +++ src-rust/crates/commands/src/lib.rs | 14 + .../crates/commands/src/named_commands.rs | 4 +- .../crates/core/src/anthropic_cli_import.rs | 13 +- src-rust/crates/core/src/coven_daemon.rs | 3 + src-rust/crates/core/src/git_utils.rs | 40 +++ src-rust/crates/core/src/hosted_review.rs | 282 +++++++++++++++++- src-rust/crates/core/src/import_config.rs | 27 +- src-rust/crates/core/src/lib.rs | 21 ++ src-rust/crates/core/src/memdir.rs | 69 ++++- src-rust/crates/core/src/roster_reset.rs | 20 +- src-rust/crates/core/src/session_storage.rs | 67 ++++- src-rust/crates/core/src/settings_sync.rs | 37 +++ src-rust/crates/core/src/team_memory_sync.rs | 53 ++++ 16 files changed, 682 insertions(+), 24 deletions(-) create mode 100644 docs/shared/hosted-review-phase-2-pr.md diff --git a/docs/advanced.md b/docs/advanced.md index d91bd675..b898ae61 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -531,8 +531,10 @@ Hosted review mode is enabled with `--hosted-review`, Coven Code does not load user-scope memory by default. The prompt records that hosted review mode is active and lists the AGENTS.md scopes that were loaded. Durable hosted memory and transcript namespaces are separated under a hosted -review path and require tenant scope plus a canonical repository identity before -they can be resolved for persistence. +review path and require tenant scope, GitHub App installation id, stable repo +id, and canonical repository identity before they can be resolved for +persistence. Hosted namespaces include a domain component so default branch, +branch, pull-request, release, and security-private memory stay separated. Hosted review mode also disables write/execute-capable tools, configured MCP servers, plugins, user memory, and managed rules by default. Trusted hosted diff --git a/docs/configuration.md b/docs/configuration.md index 72c1f0fa..64cd86ba 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -129,8 +129,11 @@ Or in `settings.json`: When hosted review mode is active, Coven Code skips user-scope memory (`~/.coven-code/AGENTS.md` and `~/.coven-code/CLAUDE.md`) by default, marks new session artifacts as hosted review artifacts, and requires a tenant plus -canonical repository identity before resolving hosted durable memory paths. -Local-personal mode remains the default and continues to load user memory. +GitHub App installation id, stable repository id, and canonical repository +identity before resolving hosted durable memory paths. Hosted memory and +transcripts are also split by memory domain, such as default branch, named +branch, pull request, release, or security-private review. Local-personal mode +remains the default and continues to load user memory. Hosted review also disables write/execute-capable tools, configured MCP servers, and plugins by default. A trusted hosted policy can opt individual diff --git a/docs/shared/hosted-review-phase-2-pr.md b/docs/shared/hosted-review-phase-2-pr.md new file mode 100644 index 00000000..6c6517b9 --- /dev/null +++ b/docs/shared/hosted-review-phase-2-pr.md @@ -0,0 +1,43 @@ +# Hosted Review Phase 2 PR Notes + +## Linked issues + +Fixes #98. +Fixes #99. +Fixes #104. +Fixes #110. + +## Summary + +- Expands hosted review scope to include tenant id, GitHub App installation id, stable repo id, repo full name, canonical repo identity, and memory domain. +- Moves hosted memory and transcript paths from local-path identity to tenant/installation/repo/domain namespaces. +- Adds canonical GitHub repo identity parsing for HTTPS and SSH remotes plus deterministic local project ids. +- Adds hosted-derived settings sync project keys and hosted team-memory repo keys so hosted callers do not pass arbitrary project ids. +- Splits hosted memory domains for default branch, branch, release, pull request, and security-private review contexts. +- Hardens Windows test isolation for home-derived paths and gates Unix-socket daemon tests to Unix. + +## Test evidence + +- `cargo fmt --all` +- `cargo check --workspace` +- `cargo clippy --workspace --all-targets -- -D warnings` +- `cargo test -p claurst-core --lib hosted -- --nocapture` +- `cargo test -p claurst-core --lib local_project_id -- --nocapture` +- `cargo test -p claurst-core --lib sync_keys -- --nocapture` +- `cargo test -p claurst-core --lib team_memory_key -- --nocapture` +- `cargo test -p claurst-commands --lib named_commands::tests::test_agents_reset_removes_saved_roster_state -- --nocapture` +- `cargo test -p claurst-core --lib roster_reset -- --nocapture` +- `cargo test -p claurst-core --lib build_import_preview_maps_settings_and_doc -- --nocapture` +- `cargo test -p claurst-core --lib test_imported_anthropic_cli_token_resolves_without_coven_oauth_client -- --nocapture` + +## Full-suite status + +- `cargo test --workspace` progressed through CLI, ACP, API, bridge, buddy, commands, and core tests, then Smart App Control blocked an unsigned freshly built test binary. +- Latest block: `C:\dev-cargo-target\coven-code\debug\deps\claurst_tools-f5b8d284b5de1d1b.exe`, OS error 4551. +- Code Integrity event: Smart App Control reported the binary did not meet Enterprise signing level requirements under policy `{0283ac0f-fff1-49ae-ada1-8a933130cad6}`. + +## Risk notes + +- Local mode pathing remains available through existing local APIs. +- Hosted durable state now requires explicit scope to avoid path-derived cross-tenant collisions. +- Security-private domains are represented and are excluded from public review loading unless explicitly allowed by policy. diff --git a/src-rust/crates/commands/src/lib.rs b/src-rust/crates/commands/src/lib.rs index cb24161a..90233f59 100644 --- a/src-rust/crates/commands/src/lib.rs +++ b/src-rust/crates/commands/src/lib.rs @@ -10363,6 +10363,8 @@ pub(crate) mod test_env { pub(crate) struct CommandEnvGuard { old_home: Option, + old_test_home: Option, + old_userprofile: Option, old_coven_home: Option, old_user: Option, old_username: Option, @@ -10376,6 +10378,8 @@ pub(crate) mod test_env { let lock = ENV_LOCK.lock().unwrap_or_else(|err| err.into_inner()); let guard = Self { old_home: std::env::var("HOME").ok(), + old_test_home: std::env::var("COVEN_CODE_TEST_HOME").ok(), + old_userprofile: std::env::var("USERPROFILE").ok(), old_coven_home: std::env::var("COVEN_HOME").ok(), old_user: std::env::var("USER").ok(), old_username: std::env::var("USERNAME").ok(), @@ -10384,6 +10388,8 @@ pub(crate) mod test_env { _lock: lock, }; std::env::set_var("HOME", home); + std::env::set_var("COVEN_CODE_TEST_HOME", home); + std::env::set_var("USERPROFILE", home); std::env::set_var("COVEN_HOME", coven_home); match user { Some(value) => std::env::set_var("USER", value), @@ -10415,6 +10421,14 @@ pub(crate) mod test_env { Some(value) => std::env::set_var("HOME", value), None => std::env::remove_var("HOME"), } + match &self.old_test_home { + Some(value) => std::env::set_var("COVEN_CODE_TEST_HOME", value), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + match &self.old_userprofile { + Some(value) => std::env::set_var("USERPROFILE", value), + None => std::env::remove_var("USERPROFILE"), + } match &self.old_coven_home { Some(value) => std::env::set_var("COVEN_HOME", value), None => std::env::remove_var("COVEN_HOME"), diff --git a/src-rust/crates/commands/src/named_commands.rs b/src-rust/crates/commands/src/named_commands.rs index c16c28d4..0bdbe921 100644 --- a/src-rust/crates/commands/src/named_commands.rs +++ b/src-rust/crates/commands/src/named_commands.rs @@ -1012,12 +1012,12 @@ mod tests { let home = temp.path().join("home"); let coven_home = temp.path().join("coven"); let project = temp.path().join("project"); - let global_agents = home.join(".coven-code").join("agents"); let project_agents = project.join(".coven-code").join("agents"); - std::fs::create_dir_all(&global_agents).expect("global agents dir"); std::fs::create_dir_all(&project_agents).expect("project agents dir"); std::fs::create_dir_all(&coven_home).expect("coven home"); let _guard = CommandEnvGuard::set(&home, &coven_home, None); + let global_agents = claurst_core::Settings::config_dir().join("agents"); + std::fs::create_dir_all(&global_agents).expect("global agents dir"); let global_agent = global_agents.join("global.md"); let project_agent = project_agents.join("project.md"); diff --git a/src-rust/crates/core/src/anthropic_cli_import.rs b/src-rust/crates/core/src/anthropic_cli_import.rs index 9eaddd93..36428bd7 100644 --- a/src-rust/crates/core/src/anthropic_cli_import.rs +++ b/src-rust/crates/core/src/anthropic_cli_import.rs @@ -47,7 +47,18 @@ pub struct DiscoveredCredential { // --------------------------------------------------------------------------- fn claude_code_credentials_path() -> Option { - Some(dirs::home_dir()?.join(".claude").join(".credentials.json")) + Some(cli_home_dir()?.join(".claude").join(".credentials.json")) +} + +fn cli_home_dir() -> Option { + #[cfg(test)] + if let Ok(home) = std::env::var("COVEN_CODE_TEST_HOME") { + if !home.is_empty() { + return Some(PathBuf::from(home)); + } + } + + dirs::home_dir() } fn ant_credentials_dir() -> Option { diff --git a/src-rust/crates/core/src/coven_daemon.rs b/src-rust/crates/core/src/coven_daemon.rs index da03183e..28231f10 100644 --- a/src-rust/crates/core/src/coven_daemon.rs +++ b/src-rust/crates/core/src/coven_daemon.rs @@ -771,6 +771,7 @@ fn url_quote(input: &str) -> String { mod tests { use super::*; use crate::coven_shared::COVEN_HOME_ENV_LOCK; + #[cfg(unix)] use std::fs; /// Guard that temporarily sets `COVEN_HOME` and restores it on drop. @@ -805,6 +806,7 @@ mod tests { assert!(DaemonClient::new().is_none()); } + #[cfg(unix)] #[test] fn new_returns_some_when_sock_present() { let _lock = COVEN_HOME_ENV_LOCK @@ -873,6 +875,7 @@ mod tests { assert_eq!(s1.active_sessions, 0); } + #[cfg(unix)] #[test] fn familiar_statuses_returns_offline_when_connect_fails() { let _lock = COVEN_HOME_ENV_LOCK diff --git a/src-rust/crates/core/src/git_utils.rs b/src-rust/crates/core/src/git_utils.rs index 624551aa..f44165b1 100644 --- a/src-rust/crates/core/src/git_utils.rs +++ b/src-rust/crates/core/src/git_utils.rs @@ -1,6 +1,8 @@ //! Git utilities for Coven Code. //! Mirrors src/utils/git.ts (926 lines) and src/utils/git/ subdirectory. +use crate::hosted_review::CanonicalRepoIdentity; +use sha2::{Digest, Sha256}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -49,6 +51,28 @@ pub fn get_current_branch(repo_root: &Path) -> String { } } +pub fn get_origin_remote_url(repo_root: &Path) -> Option { + let remote = git_output(repo_root, &["remote", "get-url", "origin"]); + (!remote.is_empty()).then_some(remote) +} + +pub fn canonical_repo_identity_from_origin(repo_root: &Path) -> Option { + let remote = get_origin_remote_url(repo_root)?; + CanonicalRepoIdentity::from_git_remote_url(&remote) +} + +pub fn local_project_id_from_identity(identity: &CanonicalRepoIdentity) -> String { + let mut hasher = Sha256::new(); + hasher.update(identity.canonical_string().as_bytes()); + let digest = hex::encode(hasher.finalize()); + format!("local-git-{}", &digest[..16]) +} + +pub fn local_project_id_from_origin(repo_root: &Path) -> Option { + canonical_repo_identity_from_origin(repo_root) + .map(|identity| local_project_id_from_identity(&identity)) +} + /// Return list of files modified (staged or unstaged). pub fn list_modified_files(repo_root: &Path) -> Vec { let output = git_output(repo_root, &["diff", "--name-only", "HEAD"]); @@ -216,4 +240,20 @@ mod tests { let commits = get_commit_history(Path::new("."), 0); assert!(commits.is_empty()); } + + #[test] + fn local_project_id_normalizes_equivalent_https_and_ssh_remotes() { + let https = CanonicalRepoIdentity::from_git_remote_url( + "https://github.com/OpenCoven/coven-code.git", + ) + .unwrap(); + let ssh = + CanonicalRepoIdentity::from_git_remote_url("git@github.com:OpenCoven/coven-code.git") + .unwrap(); + + assert_eq!( + local_project_id_from_identity(&https), + local_project_id_from_identity(&ssh) + ); + } } diff --git a/src-rust/crates/core/src/hosted_review.rs b/src-rust/crates/core/src/hosted_review.rs index 560875d0..0bce7dc6 100644 --- a/src-rust/crates/core/src/hosted_review.rs +++ b/src-rust/crates/core/src/hosted_review.rs @@ -1,5 +1,8 @@ use serde::{Deserialize, Serialize}; +const DEFAULT_PROVIDER: &str = "github"; +const DEFAULT_HOST: &str = "github.com"; + /// Runtime isolation mode for a Coven Code session. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "kebab-case")] @@ -44,21 +47,186 @@ impl HostedReviewConfig { } } +/// Canonical repository identity supplied by the hosted control plane or +/// derived from a git remote for local diagnostics. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CanonicalRepoIdentity { + pub provider: String, + pub host: String, + pub owner: String, + pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub repo_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub node_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default_branch: Option, +} + +impl CanonicalRepoIdentity { + pub fn github( + host: impl Into, + owner: impl Into, + name: impl Into, + ) -> Self { + Self { + provider: DEFAULT_PROVIDER.to_string(), + host: host.into(), + owner: owner.into(), + name: name.into(), + repo_id: None, + node_id: None, + default_branch: None, + } + } + + pub fn with_repo_id(mut self, repo_id: impl Into) -> Self { + self.repo_id = Some(repo_id.into()); + self + } + + pub fn full_name(&self) -> String { + format!("{}/{}", self.owner, self.name) + } + + pub fn stable_repo_key(&self) -> String { + self.repo_id + .as_deref() + .map(safe_component) + .unwrap_or_else(|| { + safe_component(&format!( + "{}_{}_{}_{}", + self.provider, self.host, self.owner, self.name + )) + }) + } + + pub fn canonical_string(&self) -> String { + format!( + "{}/{}/{}/{}", + self.provider, self.host, self.owner, self.name + ) + } + + pub fn from_git_remote_url(remote_url: &str) -> Option { + parse_url_remote(remote_url).or_else(|| parse_scp_remote(remote_url)) + } +} + +/// Hosted memory domain. Domains are intentionally part of durable hosted +/// storage keys so branch, PR, and security-private memory cannot collide. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "kebab-case", tag = "type", content = "value")] +pub enum MemoryDomain { + #[default] + DefaultBranch, + Branch(String), + Release(String), + PullRequest(u64), + SecurityPrivate, +} + +impl MemoryDomain { + pub fn path_component(&self) -> String { + match self { + Self::DefaultBranch => "default-branch".to_string(), + Self::Branch(name) => format!("branch-{}", safe_component(name)), + Self::Release(name) => format!("release-{}", safe_component(name)), + Self::PullRequest(number) => format!("pr-{number}"), + Self::SecurityPrivate => "security-private".to_string(), + } + } + + pub fn can_load_in_public_review(&self, allow_security_private: bool) -> bool { + !matches!(self, Self::SecurityPrivate) || allow_security_private + } +} + /// Tenant/repository identity required before hosted mode may persist /// durable memory or transcript artifacts into hosted namespaces. #[derive(Debug, Clone, PartialEq, Eq)] pub struct HostedReviewScope { pub tenant_id: String, + pub installation_id: String, + pub repo_id: String, + pub repo_full_name: String, pub canonical_repo_identity: String, + pub memory_domain: MemoryDomain, } impl HostedReviewScope { - pub fn new(tenant_id: String, canonical_repo_identity: String) -> Self { + pub fn new( + tenant_id: String, + installation_id: String, + repo_id: String, + repo_full_name: String, + ) -> Self { + let canonical_repo_identity = format!("{DEFAULT_PROVIDER}/{DEFAULT_HOST}/{repo_full_name}"); Self { tenant_id, + installation_id, + repo_id, + repo_full_name, canonical_repo_identity, + memory_domain: MemoryDomain::DefaultBranch, + } + } + + pub fn from_identity( + tenant_id: String, + installation_id: String, + identity: CanonicalRepoIdentity, + ) -> Self { + Self { + tenant_id, + installation_id, + repo_id: identity.stable_repo_key(), + repo_full_name: identity.full_name(), + canonical_repo_identity: identity.canonical_string(), + memory_domain: MemoryDomain::DefaultBranch, } } + + pub fn with_domain(mut self, memory_domain: MemoryDomain) -> Self { + self.memory_domain = memory_domain; + self + } + + pub fn tenant_component(&self) -> String { + safe_component(&self.tenant_id) + } + + pub fn installation_component(&self) -> String { + safe_component(&self.installation_id) + } + + pub fn repo_component(&self) -> String { + safe_component(&self.repo_id) + } + + pub fn domain_component(&self) -> String { + self.memory_domain.path_component() + } +} + +pub fn hosted_project_id(scope: &HostedReviewScope) -> String { + format!( + "hosted-tenant-{}-installation-{}-repo-{}", + scope.tenant_component(), + scope.installation_component(), + scope.repo_component() + ) +} + +pub fn hosted_team_memory_repo_key(scope: &HostedReviewScope) -> String { + format!( + "tenants/{}/installations/{}/repos/{}/domains/{}", + scope.tenant_component(), + scope.installation_component(), + scope.repo_component(), + scope.domain_component() + ) } pub fn env_enables_hosted_review() -> bool { @@ -77,3 +245,115 @@ fn is_truthy(value: &str) -> bool { fn is_false(value: &bool) -> bool { !*value } + +fn parse_url_remote(remote_url: &str) -> Option { + let url = url::Url::parse(remote_url).ok()?; + let host = url.host_str()?.to_ascii_lowercase(); + let mut segments = url.path_segments()?; + let owner = segments.next()?.to_string(); + let name = segments.next()?.trim_end_matches(".git").to_string(); + if owner.is_empty() || name.is_empty() { + return None; + } + + Some(CanonicalRepoIdentity::github(host, owner, name)) +} + +fn parse_scp_remote(remote_url: &str) -> Option { + let (host_part, path_part) = remote_url.split_once(':')?; + let host = host_part + .rsplit_once('@') + .map(|(_, host)| host) + .unwrap_or(host_part) + .to_ascii_lowercase(); + let mut pieces = path_part.split('/'); + let owner = pieces.next()?.to_string(); + let name = pieces.next()?.trim_end_matches(".git").to_string(); + if host.is_empty() || owner.is_empty() || name.is_empty() { + return None; + } + + Some(CanonicalRepoIdentity::github(host, owner, name)) +} + +fn safe_component(value: &str) -> String { + let mut out = String::with_capacity(value.len()); + for ch in value.chars() { + if ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.') { + out.push(ch); + } else { + out.push('_'); + } + } + if out.is_empty() { + "unknown".to_string() + } else { + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hosted_project_id_uses_installation_and_repo_id() { + let scope = HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-99".to_string(), + "OpenCoven/coven-code".to_string(), + ); + + assert_eq!( + hosted_project_id(&scope), + "hosted-tenant-tenant-a-installation-install-1-repo-repo-99" + ); + } + + #[test] + fn hosted_team_memory_key_includes_domain() { + let scope = HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-99".to_string(), + "OpenCoven/coven-code".to_string(), + ) + .with_domain(MemoryDomain::PullRequest(42)); + + assert_eq!( + hosted_team_memory_repo_key(&scope), + "tenants/tenant-a/installations/install-1/repos/repo-99/domains/pr-42" + ); + } + + #[test] + fn security_private_domain_requires_explicit_public_review_allowance() { + assert!(!MemoryDomain::SecurityPrivate.can_load_in_public_review(false)); + assert!(MemoryDomain::SecurityPrivate.can_load_in_public_review(true)); + assert!(MemoryDomain::DefaultBranch.can_load_in_public_review(false)); + } + + #[test] + fn parses_https_git_remote() { + let identity = CanonicalRepoIdentity::from_git_remote_url( + "https://github.com/OpenCoven/coven-code.git", + ) + .unwrap(); + + assert_eq!(identity.host, "github.com"); + assert_eq!(identity.owner, "OpenCoven"); + assert_eq!(identity.name, "coven-code"); + } + + #[test] + fn parses_ssh_git_remote() { + let identity = + CanonicalRepoIdentity::from_git_remote_url("git@github.com:OpenCoven/coven-code.git") + .unwrap(); + + assert_eq!(identity.host, "github.com"); + assert_eq!(identity.owner, "OpenCoven"); + assert_eq!(identity.name, "coven-code"); + } +} diff --git a/src-rust/crates/core/src/import_config.rs b/src-rust/crates/core/src/import_config.rs index 6f58ffff..a95a4080 100644 --- a/src-rust/crates/core/src/import_config.rs +++ b/src-rust/crates/core/src/import_config.rs @@ -30,7 +30,7 @@ pub struct ImportPaths { impl ImportPaths { pub fn detect() -> Self { - let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); + let home = import_home_dir(); let claude_dir = home.join(".claude"); let claurst_dir = Settings::config_dir(); Self { @@ -42,6 +42,17 @@ impl ImportPaths { } } +fn import_home_dir() -> PathBuf { + #[cfg(test)] + if let Ok(home) = std::env::var("COVEN_CODE_TEST_HOME") { + if !home.is_empty() { + return PathBuf::from(home); + } + } + + dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")) +} + #[derive(Debug, Clone)] pub struct FilePlan { pub source_path: PathBuf, @@ -654,7 +665,11 @@ mod tests { .unwrap(); let old_home = std::env::var("HOME").ok(); + let old_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + let old_userprofile = std::env::var("USERPROFILE").ok(); std::env::set_var("HOME", home); + std::env::set_var("COVEN_CODE_TEST_HOME", home); + std::env::set_var("USERPROFILE", home); let preview = build_import_preview(ImportSelection::Both).unwrap(); assert!(preview.claude_md.is_some()); @@ -682,6 +697,16 @@ mod tests { } else { std::env::remove_var("HOME"); } + if let Some(old) = old_test_home { + std::env::set_var("COVEN_CODE_TEST_HOME", old); + } else { + std::env::remove_var("COVEN_CODE_TEST_HOME"); + } + if let Some(old) = old_userprofile { + std::env::set_var("USERPROFILE", old); + } else { + std::env::remove_var("USERPROFILE"); + } } #[test] diff --git a/src-rust/crates/core/src/lib.rs b/src-rust/crates/core/src/lib.rs index aad70ba2..99dc98f8 100644 --- a/src-rust/crates/core/src/lib.rs +++ b/src-rust/crates/core/src/lib.rs @@ -1471,6 +1471,13 @@ pub mod config { impl Settings { /// The per-user configuration directory (`~/.coven-code`). pub fn config_dir() -> PathBuf { + #[cfg(test)] + if let Ok(home) = std::env::var("COVEN_CODE_TEST_HOME") { + if !home.is_empty() { + return PathBuf::from(home).join(".coven-code"); + } + } + dirs::home_dir() .unwrap_or_else(|| PathBuf::from(".")) .join(".coven-code") @@ -4969,6 +4976,8 @@ mod tests { fn test_imported_anthropic_cli_token_resolves_without_coven_oauth_client() { struct EnvRestore { home: Option, + test_home: Option, + userprofile: Option, api_key: Option, client_id: Option, } @@ -4979,6 +4988,14 @@ mod tests { Some(value) => std::env::set_var("HOME", value), None => std::env::remove_var("HOME"), } + match self.test_home.take() { + Some(value) => std::env::set_var("COVEN_CODE_TEST_HOME", value), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + match self.userprofile.take() { + Some(value) => std::env::set_var("USERPROFILE", value), + None => std::env::remove_var("USERPROFILE"), + } match self.api_key.take() { Some(value) => std::env::set_var("ANTHROPIC_API_KEY", value), None => std::env::remove_var("ANTHROPIC_API_KEY"), @@ -5003,10 +5020,14 @@ mod tests { let temp_home = tempfile::tempdir().expect("temp home"); let _restore = EnvRestore { home: std::env::var("HOME").ok(), + test_home: std::env::var("COVEN_CODE_TEST_HOME").ok(), + userprofile: std::env::var("USERPROFILE").ok(), api_key: std::env::var("ANTHROPIC_API_KEY").ok(), client_id: std::env::var(crate::oauth::CLIENT_ID_ENV).ok(), }; std::env::set_var("HOME", temp_home.path()); + std::env::set_var("COVEN_CODE_TEST_HOME", temp_home.path()); + std::env::set_var("USERPROFILE", temp_home.path()); std::env::remove_var("ANTHROPIC_API_KEY"); std::env::remove_var(crate::oauth::CLIENT_ID_ENV); diff --git a/src-rust/crates/core/src/memdir.rs b/src-rust/crates/core/src/memdir.rs index 335e51b0..daf234a6 100644 --- a/src-rust/crates/core/src/memdir.rs +++ b/src-rust/crates/core/src/memdir.rs @@ -377,7 +377,7 @@ pub fn auto_memory_path_for_mode( RuntimeMode::HostedReview => { let scope = scope.ok_or_else(|| { crate::ClaudeError::Config( - "hosted review memory persistence requires tenant scope and canonical repo identity" + "hosted review memory persistence requires tenant scope, installation scope, and canonical repo identity" .to_string(), ) })?; @@ -400,9 +400,13 @@ fn hosted_memory_path(scope: &HostedReviewScope) -> PathBuf { crate::config::Settings::config_dir() .join("hosted-review") .join("tenants") - .join(sanitize_path_component(&scope.tenant_id)) + .join(scope.tenant_component()) + .join("installations") + .join(scope.installation_component()) .join("repos") - .join(sanitize_path_component(&scope.canonical_repo_identity)) + .join(scope.repo_component()) + .join("domains") + .join(scope.domain_component()) .join("memory") } @@ -956,7 +960,9 @@ mod tests { let project = PathBuf::from("/tmp/repo"); let scope = crate::hosted_review::HostedReviewScope::new( "tenant-a".to_string(), - "github.com/OpenCoven/coven-code".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), ); let local = auto_memory_path(&project); @@ -970,8 +976,57 @@ mod tests { assert_ne!(hosted, local); assert!(hosted.to_string_lossy().contains("hosted-review")); assert!(hosted.to_string_lossy().contains("tenant-a")); - assert!(hosted - .to_string_lossy() - .contains("github.com_OpenCoven_coven-code")); + assert!(hosted.to_string_lossy().contains("install-1")); + assert!(hosted.to_string_lossy().contains("repo-1")); + assert!(hosted.to_string_lossy().contains("default-branch")); + } + + #[test] + fn hosted_memory_path_splits_installations_and_domains() { + let project = PathBuf::from("/tmp/repo"); + let first_install = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let second_install = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-2".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let branch_domain = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ) + .with_domain(crate::hosted_review::MemoryDomain::Branch( + "feature/review".to_string(), + )); + + let first = auto_memory_path_for_mode( + &project, + crate::hosted_review::RuntimeMode::HostedReview, + Some(&first_install), + ) + .unwrap(); + let second = auto_memory_path_for_mode( + &project, + crate::hosted_review::RuntimeMode::HostedReview, + Some(&second_install), + ) + .unwrap(); + let branch = auto_memory_path_for_mode( + &project, + crate::hosted_review::RuntimeMode::HostedReview, + Some(&branch_domain), + ) + .unwrap(); + + assert_ne!(first, second); + assert_ne!(first, branch); + assert!(branch.to_string_lossy().contains("branch-feature_review")); } } diff --git a/src-rust/crates/core/src/roster_reset.rs b/src-rust/crates/core/src/roster_reset.rs index bfe22f53..2324b2eb 100644 --- a/src-rust/crates/core/src/roster_reset.rs +++ b/src-rust/crates/core/src/roster_reset.rs @@ -139,17 +139,25 @@ mod tests { struct HomeGuard { old_home: Option, + old_test_home: Option, + old_userprofile: Option, old_coven_home: Option, } impl HomeGuard { fn set(home: &Path, coven_home: &Path) -> Self { let old_home = std::env::var("HOME").ok(); + let old_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + let old_userprofile = std::env::var("USERPROFILE").ok(); let old_coven_home = std::env::var("COVEN_HOME").ok(); std::env::set_var("HOME", home); + std::env::set_var("COVEN_CODE_TEST_HOME", home); + std::env::set_var("USERPROFILE", home); std::env::set_var("COVEN_HOME", coven_home); Self { old_home, + old_test_home, + old_userprofile, old_coven_home, } } @@ -161,6 +169,14 @@ mod tests { Some(value) => std::env::set_var("HOME", value), None => std::env::remove_var("HOME"), } + match &self.old_test_home { + Some(value) => std::env::set_var("COVEN_CODE_TEST_HOME", value), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + match &self.old_userprofile { + Some(value) => std::env::set_var("USERPROFILE", value), + None => std::env::remove_var("USERPROFILE"), + } match &self.old_coven_home { Some(value) => std::env::set_var("COVEN_HOME", value), None => std::env::remove_var("COVEN_HOME"), @@ -204,12 +220,12 @@ mod tests { let home = temp.path().join("home"); let coven_home = temp.path().join("coven"); let project = temp.path().join("project"); - let global_agents = home.join(".coven-code").join("agents"); let project_agents = project.join(".coven-code").join("agents"); - std::fs::create_dir_all(&global_agents).expect("global agents dir"); std::fs::create_dir_all(&project_agents).expect("project agents dir"); std::fs::create_dir_all(&coven_home).expect("coven home"); let _guard = HomeGuard::set(&home, &coven_home); + let global_agents = Settings::config_dir().join("agents"); + std::fs::create_dir_all(&global_agents).expect("global agents dir"); std::fs::write(global_agents.join("global.md"), "global").expect("global agent"); std::fs::write(global_agents.join("README.txt"), "keep").expect("global keep"); diff --git a/src-rust/crates/core/src/session_storage.rs b/src-rust/crates/core/src/session_storage.rs index d5df6f93..867be529 100644 --- a/src-rust/crates/core/src/session_storage.rs +++ b/src-rust/crates/core/src/session_storage.rs @@ -253,18 +253,20 @@ pub fn transcript_dir_for_mode( RuntimeMode::HostedReview => { let scope = scope.ok_or_else(|| { crate::ClaudeError::Config( - "hosted review transcript persistence requires tenant scope and canonical repo identity" + "hosted review transcript persistence requires tenant scope, installation scope, and canonical repo identity" .to_string(), ) })?; Ok(projects_dir() .join("hosted-review") .join("tenants") - .join(crate::memdir::sanitize_path_component(&scope.tenant_id)) + .join(scope.tenant_component()) + .join("installations") + .join(scope.installation_component()) .join("repos") - .join(crate::memdir::sanitize_path_component( - &scope.canonical_repo_identity, - )) + .join(scope.repo_component()) + .join("domains") + .join(scope.domain_component()) .join("transcripts")) } } @@ -832,7 +834,9 @@ mod tests { fn hosted_transcript_path_uses_separate_namespace() { let scope = crate::hosted_review::HostedReviewScope::new( "tenant-a".to_string(), - "github.com/OpenCoven/coven-code".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), ); let local = transcript_path(Path::new("/tmp/repo"), "sess-1"); let hosted = transcript_path_for_mode( @@ -846,6 +850,57 @@ mod tests { assert_ne!(hosted, local); assert!(hosted.to_string_lossy().contains("hosted-review")); assert!(hosted.to_string_lossy().contains("tenant-a")); + assert!(hosted.to_string_lossy().contains("install-1")); + assert!(hosted.to_string_lossy().contains("repo-1")); + } + + #[test] + fn hosted_transcript_path_splits_repos_and_domains() { + let repo_one = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let repo_two = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-2".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let security_domain = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-1".to_string(), + "OpenCoven/coven-code".to_string(), + ) + .with_domain(crate::hosted_review::MemoryDomain::SecurityPrivate); + + let first = transcript_path_for_mode( + Path::new("/tmp/repo"), + "sess-1", + crate::hosted_review::RuntimeMode::HostedReview, + Some(&repo_one), + ) + .unwrap(); + let second = transcript_path_for_mode( + Path::new("/tmp/repo"), + "sess-1", + crate::hosted_review::RuntimeMode::HostedReview, + Some(&repo_two), + ) + .unwrap(); + let security = transcript_path_for_mode( + Path::new("/tmp/repo"), + "sess-1", + crate::hosted_review::RuntimeMode::HostedReview, + Some(&security_domain), + ) + .unwrap(); + + assert_ne!(first, second); + assert_ne!(first, security); + assert!(security.to_string_lossy().contains("security-private")); } #[test] diff --git a/src-rust/crates/core/src/settings_sync.rs b/src-rust/crates/core/src/settings_sync.rs index 9e0ca0e0..a1eaccae 100644 --- a/src-rust/crates/core/src/settings_sync.rs +++ b/src-rust/crates/core/src/settings_sync.rs @@ -13,6 +13,7 @@ // The sync API stores a flat key→value map where keys are canonical file paths // and values are the UTF-8 file contents (JSON or Markdown). +use crate::hosted_review::{hosted_project_id, HostedReviewScope}; use anyhow::Result; use serde::Deserialize; use serde_json::Value; @@ -50,6 +51,18 @@ pub fn sync_key_project_memory(project_id: &str) -> String { format!("projects/{project_id}/AGENTS.local.md") } +/// Canonical hosted project settings key. The project id is derived from +/// tenant, installation, and repo id rather than accepted from a caller. +pub fn sync_key_hosted_project_settings(scope: &HostedReviewScope) -> String { + sync_key_project_settings(&hosted_project_id(scope)) +} + +/// Canonical hosted project memory key. The project id is derived from +/// tenant, installation, and repo id rather than accepted from a caller. +pub fn sync_key_hosted_project_memory(scope: &HostedReviewScope) -> String { + sync_key_project_memory(&hosted_project_id(scope)) +} + // --------------------------------------------------------------------------- // API wire types // --------------------------------------------------------------------------- @@ -410,6 +423,11 @@ pub async fn collect_local_entries(project_id: Option<&str>) -> HashMap HashMap { + let project_id = hosted_project_id(scope); + collect_local_entries(Some(&project_id)).await +} + /// Try to read a file, applying the 500 KB size limit. /// Returns `None` if the file doesn't exist, is empty, or exceeds the limit. async fn try_read_for_sync(path: &PathBuf) -> Option { @@ -472,6 +490,25 @@ mod tests { ); } + #[test] + fn hosted_sync_keys_derive_project_id_from_scope() { + let scope = HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-99".to_string(), + "OpenCoven/coven-code".to_string(), + ); + + assert_eq!( + sync_key_hosted_project_settings(&scope), + "projects/hosted-tenant-tenant-a-installation-install-1-repo-repo-99/.coven-code/settings.local.json" + ); + assert_eq!( + sync_key_hosted_project_memory(&scope), + "projects/hosted-tenant-tenant-a-installation-install-1-repo-repo-99/AGENTS.local.md" + ); + } + #[test] fn test_entries_to_synced_data_settings_parsed() { let mut entries = HashMap::new(); diff --git a/src-rust/crates/core/src/team_memory_sync.rs b/src-rust/crates/core/src/team_memory_sync.rs index 855989af..785946dd 100644 --- a/src-rust/crates/core/src/team_memory_sync.rs +++ b/src-rust/crates/core/src/team_memory_sync.rs @@ -6,6 +6,7 @@ //! //! Pull is server-wins: remote content overwrites local files unconditionally. +use crate::hosted_review::{hosted_team_memory_repo_key, HostedReviewScope}; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -134,6 +135,24 @@ impl TeamMemorySync { } } + pub fn hosted( + api_base: String, + scope: &HostedReviewScope, + token: String, + team_dir: PathBuf, + ) -> Self { + Self::new( + api_base, + hosted_team_memory_repo_key(scope), + token, + team_dir, + ) + } + + pub fn repo_key(&self) -> &str { + &self.repo + } + // ----------------------------------------------------------------------- // Pull // ----------------------------------------------------------------------- @@ -499,6 +518,40 @@ mod tests { assert_ne!(content_checksum("foo"), content_checksum("bar")); } + #[test] + fn hosted_team_memory_key_splits_installations_for_same_repo_name() { + let tmp = TempDir::new().unwrap(); + let first = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-1".to_string(), + "repo-99".to_string(), + "OpenCoven/coven-code".to_string(), + ); + let second = crate::hosted_review::HostedReviewScope::new( + "tenant-a".to_string(), + "install-2".to_string(), + "repo-99".to_string(), + "OpenCoven/coven-code".to_string(), + ); + + let first_sync = TeamMemorySync::hosted( + "https://example.com".to_string(), + &first, + "token".to_string(), + tmp.path().to_path_buf(), + ); + let second_sync = TeamMemorySync::hosted( + "https://example.com".to_string(), + &second, + "token".to_string(), + tmp.path().to_path_buf(), + ); + + assert_ne!(first_sync.repo_key(), second_sync.repo_key()); + assert!(first_sync.repo_key().contains("installations/install-1")); + assert!(second_sync.repo_key().contains("installations/install-2")); + } + // --- validate_memory_path --- #[test] From 7573a67045170713cebfbf39b180a0f14635501c Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 16:01:25 -0400 Subject: [PATCH 2/2] test(tui): use Windows absolute system root fixtures (#98 #99 #104 #110) Refs #98. Refs #99. Refs #104. Refs #110. - keep hosted identity/scoping tests portable on Windows - preserve Phase 2 full-suite test coverage Signed-off-by: Timothy Wayne Gregg --- docs/shared/hosted-review-phase-2-pr.md | 7 ++++--- src-rust/crates/tui/src/app.rs | 10 +++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/shared/hosted-review-phase-2-pr.md b/docs/shared/hosted-review-phase-2-pr.md index 6c6517b9..7c1e774a 100644 --- a/docs/shared/hosted-review-phase-2-pr.md +++ b/docs/shared/hosted-review-phase-2-pr.md @@ -15,6 +15,7 @@ Fixes #110. - Adds hosted-derived settings sync project keys and hosted team-memory repo keys so hosted callers do not pass arbitrary project ids. - Splits hosted memory domains for default branch, branch, release, pull request, and security-private review contexts. - Hardens Windows test isolation for home-derived paths and gates Unix-socket daemon tests to Unix. +- Corrects Windows TUI system-root test fixtures to use drive-qualified absolute paths. ## Test evidence @@ -29,12 +30,12 @@ Fixes #110. - `cargo test -p claurst-core --lib roster_reset -- --nocapture` - `cargo test -p claurst-core --lib build_import_preview_maps_settings_and_doc -- --nocapture` - `cargo test -p claurst-core --lib test_imported_anthropic_cli_token_resolves_without_coven_oauth_client -- --nocapture` +- `cargo test -p claurst-tui --lib windows_system_root --quiet` +- `cargo test --workspace --quiet` ## Full-suite status -- `cargo test --workspace` progressed through CLI, ACP, API, bridge, buddy, commands, and core tests, then Smart App Control blocked an unsigned freshly built test binary. -- Latest block: `C:\dev-cargo-target\coven-code\debug\deps\claurst_tools-f5b8d284b5de1d1b.exe`, OS error 4551. -- Code Integrity event: Smart App Control reported the binary did not meet Enterprise signing level requirements under policy `{0283ac0f-fff1-49ae-ada1-8a933130cad6}`. +- `cargo test --workspace --quiet` passes on Windows with `CARGO_TARGET_DIR=C:\dev-cargo-target\coven-code` after Smart App Control was disabled locally. ## Risk notes diff --git a/src-rust/crates/tui/src/app.rs b/src-rust/crates/tui/src/app.rs index c53f661c..d0184952 100644 --- a/src-rust/crates/tui/src/app.rs +++ b/src-rust/crates/tui/src/app.rs @@ -7421,21 +7421,21 @@ role = "Research" #[test] fn windows_system_root_prefers_absolute_system_root() { let root = windows_system_root_from_env( - Some(std::ffi::OsString::from("/windows")), - Some(std::ffi::OsString::from("/windir")), + Some(std::ffi::OsString::from(r"C:\Windows")), + Some(std::ffi::OsString::from(r"D:\WinDir")), ); - assert_eq!(root, std::ffi::OsString::from("/windows")); + assert_eq!(root, std::ffi::OsString::from(r"C:\Windows")); } #[test] fn windows_system_root_falls_back_to_absolute_windir() { let root = windows_system_root_from_env( Some(std::ffi::OsString::from("relative-system-root")), - Some(std::ffi::OsString::from("/windir")), + Some(std::ffi::OsString::from(r"D:\WinDir")), ); - assert_eq!(root, std::ffi::OsString::from("/windir")); + assert_eq!(root, std::ffi::OsString::from(r"D:\WinDir")); } #[test]