diff --git a/docs/advanced.md b/docs/advanced.md index d91bd67..b898ae6 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 72c1f0f..64cd86b 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 0000000..7c1e774 --- /dev/null +++ b/docs/shared/hosted-review-phase-2-pr.md @@ -0,0 +1,44 @@ +# 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. +- Corrects Windows TUI system-root test fixtures to use drive-qualified absolute paths. + +## 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` +- `cargo test -p claurst-tui --lib windows_system_root --quiet` +- `cargo test --workspace --quiet` + +## Full-suite status + +- `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 + +- 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 e0f3b6f..90233f5 100644 --- a/src-rust/crates/commands/src/lib.rs +++ b/src-rust/crates/commands/src/lib.rs @@ -10363,6 +10363,7 @@ 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, @@ -10377,6 +10378,7 @@ 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(), @@ -10386,6 +10388,7 @@ 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 { @@ -10418,14 +10421,18 @@ pub(crate) mod test_env { Some(value) => std::env::set_var("HOME", value), None => std::env::remove_var("HOME"), } - match &self.old_coven_home { - Some(value) => std::env::set_var("COVEN_HOME", value), - None => std::env::remove_var("COVEN_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"), + } match &self.old_user { Some(value) => std::env::set_var("USER", value), None => std::env::remove_var("USER"), diff --git a/src-rust/crates/core/src/git_utils.rs b/src-rust/crates/core/src/git_utils.rs index 624551a..f44165b 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 560875d..0bce7dc 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/memdir.rs b/src-rust/crates/core/src/memdir.rs index 335e51b..daf234a 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/session_storage.rs b/src-rust/crates/core/src/session_storage.rs index e3d4c89..2aed0f1 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")) } } @@ -833,7 +835,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( @@ -847,6 +851,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 9e0ca0e..a1eacca 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 855989a..785946d 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]