diff --git a/COVEN.md b/COVEN.md index 8d470b49..19283110 100644 --- a/COVEN.md +++ b/COVEN.md @@ -27,6 +27,9 @@ are rebranded. This boundary is explicit and documented below. | Binary name | `src-rust/crates/cli/src/main.rs`, `src-rust/crates/cli/src/bin/coven-cave.rs` | `coven-code`; `coven-cave` alias | | npm package | `npm/package.json` | `@opencoven/coven-code` | | Data/cache dirs | `src-rust/crates/core/src/snapshot/`, `skill_discovery.rs`, `update_check.rs`, `app.rs` | `coven-code/` | +| Engine home (standalone) | `src-rust/crates/core/src/lib.rs` (`config_home`) | `~/.coven-code/` | +| Engine home (under coven CLI) | `src-rust/crates/core/src/lib.rs` (`config_home`) | `~/.coven/code/` — migrated in-place from `~/.coven-code/` on first launch; legacy path is symlinked to new location | +| Shared config layer | `src-rust/crates/core/src/lib.rs` (`SharedSettings`, `load_hierarchical`) | `~/.coven/settings.json` — cross-tool defaults layered UNDER the engine and project settings | | Env var prefix | throughout `src-rust/` | `COVEN_CODE_*` | | User-Agent | `src-rust/crates/tools/src/web_search.rs`, `update_check.rs` | `CovenCode/x.y` | | System prompt identity | `src-rust/crates/core/src/system_prompt.rs` | "You are Coven Code…" | @@ -100,6 +103,57 @@ Add Coven-specific tools (e.g. `coven_session_tool.rs`) and register in `tools/s --- +## Shared settings (`~/.coven/settings.json`) + +When running under the unified `coven` CLI, a small whitelist of +cross-tool defaults can be placed at `~/.coven/settings.json`. +This is the **lowest-precedence** layer: values are used only when the +engine-global settings (`~/.coven/code/settings.json`) and project +settings (`.coven-code/settings.json`) do not override them. + +### Load order (lowest → highest) + +1. `~/.coven/settings.json` — shared, whitelisted keys only +2. `~/.coven/code/settings.json` — engine-global +3. `/.coven-code/settings.json` — project + +When there is no `~/.coven/` directory (standalone mode, no coven CLI), +the shared layer is absent and coven-code behaves exactly as before +(engine + project only). + +### Whitelisted keys + +| Key | Type | Description | +|---|---|---| +| `model` | string | Default model (e.g. `"claude-opus-4-8"`) | +| `theme` | string | UI theme: `"default"`, `"dark"`, `"light"`, `"deuteranopia"`, or a custom string | +| `permission_mode` | string | Permission posture: `"default"`, `"acceptEdits"`, `"bypassPermissions"`, `"plan"` | + +Any other keys in `~/.coven/settings.json` are silently ignored by +coven-code, so future Coven tools can extend the schema without breaking +older engine versions. + +### Example + +```json +{ + "model": "claude-sonnet-4-6", + "theme": "dark", + "permission_mode": "acceptEdits" +} +``` + +### Implementation + +`SharedSettings` struct in `src-rust/crates/core/src/lib.rs` (inside +`pub mod config`). `SharedSettings::load()` reads the file via +`coven_shared::coven_home()`. `SharedSettings::apply_to(&mut Settings)` +fills whitelisted fields only when the engine-global still has the +built-in default value (for `Option` fields: `None`; for enum fields: +the `Default` variant). + +--- + ## Engine contract coven-code is the engine behind the unified `coven` CLI. The exact CLI/env/ diff --git a/docs/superpowers/plans/2026-07-12-phase4-state-unification.md b/docs/superpowers/plans/2026-07-12-phase4-state-unification.md new file mode 100644 index 00000000..b9c7d48d --- /dev/null +++ b/docs/superpowers/plans/2026-07-12-phase4-state-unification.md @@ -0,0 +1,85 @@ +# Phase 4 — State, Config & Auth Unification under `~/.coven` + +> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:subagent-driven-development. Steps use `- [ ]`. Phase 4 sub-plan of `2026-07-12-coven-cli-unification.md`. + +**Goal:** one state root (`~/.coven`), layered config, no user-visible `~/.coven-code` on fresh installs, one auth story surfaced by one doctor, and search that spans engine TUI sessions. + +**Architecture:** Two tracks. **Track C (coven-code, HIGH RISK — moves user data):** consolidate the 8 scattered `.coven-code` path sites into one `config_home()` helper (safe refactor first), then add env-override precedence + a first-run migration to `~/.coven/code/` with a compatibility symlink, then config layering. **Track D (coven):** a unified "Credentials" doctor panel and cross-session FTS that ingests external-session transcripts. + +**Repos:** `[coven-code]` base `main` (b3a35a0); `[coven]` base `feat/engine-ledger` (Phase 3 Track B #349). + +--- + +## Grounding facts (verified 2026-07-12) + +**`[coven-code]` — path construction is SCATTERED across 8 sites (NOT centralized):** +- Central-ish: `config::config_dir()` (`core/src/lib.rs:1484-1495`) → `~/.coven-code`; only checks `COVEN_CODE_TEST_HOME` (test). Covers settings.json, projects/, stash, output-styles. +- Independent constructions that DON'T use `config_dir()`: `auth_store::path()` (`auth_store.rs:32-37`, auth.json), `feature_flags::get_cache_path()` (`feature_flags.rs:72-74`), `prompt_history::claude_home()` (`prompt_history.rs:139-150`, history.jsonl + pastes/), `remote_settings::claude_config_dir()` (`remote_settings.rs:394-398`), `OAuthTokens::token_file_path()` (`lib.rs:4235-4240`, legacy oauth_tokens.json), `Settings::find_project_settings()` (`lib.rs:1626-1652`, walks cwd for `.coven-code/settings.json`), `skill_discovery` (`skill_discovery.rs:356,380`, skills/). Each re-implements `dirs::home_dir().join(".coven-code")`. +- `Settings::load_hierarchical` (`lib.rs:1603-1624`): global (`config_dir()/settings.json`) then project (walk cwd for `.coven-code/settings.{json,jsonc}`), project overrides global. `Settings { config: Config, version, projects, ... }`; the "shared" keys are `config.*` (model, theme, permission_mode, provider, mcp_servers). +- `coven_shared::coven_home()` (`coven_shared.rs:27-36`): `COVEN_HOME` env → else `~/.coven` (only if it's a dir). The pattern to mirror. +- Settings migrations exist (`migrations.rs`, value-level); NO dir-level version marker. No `canonicalize()`/symlink-rejection anywhere → symlinking `~/.coven-code` → `~/.coven/code` is safe. +- COVEN.md rebrand table names `.coven-code` data dir but gives no relocation guidance. + +**`[coven]` — auth + FTS:** +- Doctor engine-auth: `engine_auth_summary(binary)` (`main.rs:1773-1807`) runs ` auth status --json` (5s bounded), parses `loggedIn`; printed at `main.rs:1141-1145`. Harnesses section (`main.rs:1094-1112`) shows availability only — NO per-harness auth probe exists (codex/claude have no known `auth status --json`). +- FTS5: `events_fts` virtual table (`store.rs:454-471`) populated by an AFTER INSERT trigger on `events`; `search_events()` (`store.rs:1898-1928`); CLI `run_sessions_search` (`main.rs:751-775`). External sessions (Phase 3) have `transcript_path` + `external` columns but NO events → invisible to search. +- Privacy/retention: `PrivacyConfig` (`privacy.rs:12-27`, log_retention_days default 30); `prune_events_older_than` (`store.rs`); events must be redacted (`privacy::redact_payload_json_with_config`) + carry retention expiry. FTS auto-invalidates on event DELETE (trigger). No daemon background loop exists (only the accept loop) — ingestion hooks either lazily on search or as a new task. + +--- + +## Track C — engine home relocation (coven-code) + +### Task 4.1: Consolidate all `.coven-code` path construction into one `config_home()` helper (SAFE refactor, no behavior change) `[coven-code]` + +**Rationale:** relocation is only safe if ALL engine data moves together. Today 8 sites build the path independently. This task introduces one helper that STILL returns `~/.coven-code` (identical behavior) and routes all 8 sites through it — turning relocation into a 1-helper change and eliminating the data-split risk. Ship this ALONE first; it's a pure refactor with zero user-visible change. + +**Files:** `core/src/config.rs` (or `lib.rs` where `config_dir` lives) — add `config_home()`; refactor `config_dir`, `auth_store.rs`, `feature_flags.rs`, `prompt_history.rs`, `remote_settings.rs`, `lib.rs` (`OAuthTokens::token_file_path`, `find_project_settings`), `skill_discovery.rs`. + +- [ ] **Step 1:** Add `pub fn config_home() -> PathBuf` returning EXACTLY today's value: honor `COVEN_CODE_TEST_HOME` (test) then `dirs::home_dir().join(".coven-code")`. (Env-override precedence comes in 4.2 — keep 4.1 behavior-identical.) +- [ ] **Step 2:** Route the CENTRAL `config_dir()` through `config_home()`. Route the 6 independent global helpers (auth_store, feature_flags, prompt_history, remote_settings, OAuthTokens legacy, skill_discovery-global) through `config_home()`. For the two cwd-walking sites (`find_project_settings`, `skill_discovery` project-level), extract the `.coven-code` PROJECT dir-name into a shared `const PROJECT_CONFIG_DIRNAME: &str = ".coven-code";` (they join it onto arbitrary cwd ancestors, NOT the home dir — do NOT route those through `config_home`; just de-magic the string so 4.x can rename consistently). +- [ ] **Step 3:** Tests: a test asserting `config_home()` == every subsystem's base (auth.json parent, history.jsonl parent, etc. all share `config_home()`); a test that `COVEN_CODE_TEST_HOME` still overrides. Grep to PROVE zero remaining independent `home_dir().join(".coven-code")` constructions outside `config_home()` + the project-dirname const. +- [ ] **Step 4:** `cargo fmt/check/clippy -D warnings/test` (workspace). Signed commit `refactor(core): route all engine-home paths through one config_home() helper`. NO AI trailer (coven-code AGENTS.md). + +### Task 4.2: Env-override precedence + first-run migration to `~/.coven/code/` `[coven-code]` + +**Files:** `config.rs`/`lib.rs` (`config_home` precedence), a new `core/src/home_migration.rs`, called once at startup (`cli/src/main.rs` early, before any path use). + +- [ ] **Step 1:** `config_home()` precedence becomes: `COVEN_CODE_HOME` (explicit) → if `COVEN_HOME` set OR `COVEN_PARENT=coven` → `/code` (default `~/.coven/code`) → else legacy `~/.coven-code`. Keep `COVEN_CODE_TEST_HOME` first for tests. Unit-test each precedence branch with env isolation (there's an env lock — `coven_shared::COVEN_HOME_ENV_LOCK`). +- [ ] **Step 2:** `home_migration::migrate_if_needed()`: if the RESOLVED home is `~/.coven/code` AND it does not exist AND legacy `~/.coven-code` DOES exist → move the directory (`std::fs::rename`; fall back to recursive copy across filesystems) to `~/.coven/code`, then create a symlink `~/.coven-code` → `~/.coven/code` (unix `symlink`; Windows `symlink_dir`/junction, or skip-with-log on failure). Idempotent, best-effort, every error logged not fatal (a failed migration must not brick the engine — fall back to whichever dir has data). Write a `~/.coven/code/.migrated-from-coven-code` marker so it runs once. +- [ ] **Step 3:** Call `migrate_if_needed()` at the very start of `main` (before settings load / any `config_home()` consumer). Tests: fixture with a populated fake `~/.coven-code` + `COVEN_HOME` set → after migrate, files live under `/code`, symlink exists, marker present; second call is a no-op; absent-legacy-dir is a no-op. +- [ ] **Step 4:** gates + signed commit `feat(core): relocate engine home to ~/.coven/code under the unified CLI (migrating in place)`. Update COVEN.md rebrand table. + +### Task 4.3: Config layering — shared `~/.coven/settings.json` `[coven-code]` + +**Files:** `lib.rs` `load_hierarchical`. + +- [ ] Load order becomes: `~/.coven/settings.json` (shared keys: model, theme, permission defaults — a documented subset) → `~/.coven/code/settings.json` → project `.coven-code/settings.json`; later overrides earlier. Only a whitelisted subset is read from the shared file (ignore unknown/engine-only keys there). Document the shared-key schema in COVEN.md + the coven contract. Tests for the 3-layer precedence. Signed commit. + +--- + +## Track D — one auth story + cross-session search (coven) + +### Task 4.4: Unified "Credentials" doctor panel `[coven]` + +**Files:** `crates/coven-cli/src/main.rs` (`run_doctor`). + +- [ ] Replace the scattered engine-auth line + harness-availability list with a single "Credentials" section that, per provider, shows: engine (`auth status --json` → logged in/out, the existing real check) and each available harness (codex/claude) with availability + a best-effort auth note. Since codex/claude expose no machine-readable auth probe, show availability + an install/login hint rather than inventing a status. Keep exit-code semantics (auth is a warning, missing engine/harness a blocker). Test the panel's pure formatting via extracted helpers. Signed commit (coven repo — trailer optional). + +### Task 4.5: Cross-session FTS — ingest external-session transcripts `[coven]` + +**Files:** `store.rs` (ingest fn + FTS), `daemon.rs` or the search path (hook), `main.rs` (`run_sessions_search`). + +- [ ] On `coven sessions search` (lazy, simplest — no daemon loop exists), for each `external` session with a `transcript_path` not yet ingested (track via a `transcript_indexed_at` column or a per-session marker), read its JSONL transcript, extract the text content per line, REDACT via the existing privacy path, insert as `events` rows (kind `transcript_text`) with a retention-expiry so the FTS trigger indexes them and pruning still applies. Bound the work (cap lines/bytes; log truncation). Then the existing `search_events` spans them. Tests: an external session with a fixture transcript becomes searchable; redaction applied; re-search doesn't re-ingest. Signed commit. + +--- + +## Phase 4 exit check + +Fresh install writes nothing outside `~/.coven` (engine home is `~/.coven/code`); an existing `~/.coven-code` user upgrades in place with data intact + a compat symlink (populated-home migration test). `coven doctor` shows one Credentials panel answering "am I logged in, to what, via what." `coven sessions search ` finds content from an interactive engine session. + +## Sequencing & risk + +- **4.1 first, alone, and merge it before 4.2** — it's a zero-behavior-change refactor that de-risks everything after. Do NOT combine 4.1 (safe) with 4.2 (data migration) in one reviewable unit. +- 4.2 is the highest-risk task in the whole unification: it MOVES user data. Guard rails: idempotent, best-effort, never-fatal, marker-gated, populated-fixture migration test, symlink-back for one deprecation cycle, and a clear fallback (if migration fails, keep using whichever dir has data). Consider shipping 4.2 behind an off-by-default flag first, flipping to on after real-world validation. +- Track D (4.4, 4.5) is independent of Track C and can proceed in parallel on the coven side. +- Both repos keep their gates (fmt/clippy -D warnings/test) and signed commits; coven-code commits carry NO AI trailer. diff --git a/src-rust/Cargo.lock b/src-rust/Cargo.lock index f7b624eb..ac9cf0dc 100644 --- a/src-rust/Cargo.lock +++ b/src-rust/Cargo.lock @@ -810,6 +810,7 @@ dependencies = [ "serde", "serde_json", "sha2", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-stream", @@ -889,6 +890,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "tempfile", "tokio", "tokio-util", "tracing", diff --git a/src-rust/crates/cli/src/main.rs b/src-rust/crates/cli/src/main.rs index 3ba1d6a9..4ad5e602 100644 --- a/src-rust/crates/cli/src/main.rs +++ b/src-rust/crates/cli/src/main.rs @@ -401,6 +401,11 @@ fn handle_exit_key( } #[tokio::main] async fn main() -> anyhow::Result<()> { + // Perform a best-effort, non-fatal relocation of the engine home from the + // legacy ~/.coven-code/ to ~/.coven/code/ when running under the unified + // coven CLI. Must run before any Settings::load() or auth/config access. + claurst_core::home_migration::migrate_if_needed(); + let raw_args: Vec = std::env::args().collect(); // Fast-path: `coven-code upgrade [--version ] [--force]` — self-update. diff --git a/src-rust/crates/core/src/accounts.rs b/src-rust/crates/core/src/accounts.rs index 1d7cba0a..7ad9265f 100644 --- a/src-rust/crates/core/src/accounts.rs +++ b/src-rust/crates/core/src/accounts.rs @@ -255,9 +255,7 @@ pub fn ensure_unique_profile_id(registry: &AccountRegistry, provider: &str, base /// `~/.coven-code/`. pub fn claurst_dir() -> PathBuf { - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".coven-code") + crate::config::config_home() } /// `~/.coven-code/accounts///`. diff --git a/src-rust/crates/core/src/attachments.rs b/src-rust/crates/core/src/attachments.rs index b329eae1..46d765cb 100644 --- a/src-rust/crates/core/src/attachments.rs +++ b/src-rust/crates/core/src/attachments.rs @@ -93,7 +93,7 @@ pub fn get_attachments(ctx: &AttachmentContext<'_>) -> Vec { /// Returns a formatted string like: /// `IDE: VS Code, workspace: /path/to/project, selection: L10-L20 in foo.rs` pub fn get_ide_context() -> Option { - let lockfile_dir = dirs::home_dir()?.join(".coven-code").join("ide"); + let lockfile_dir = crate::config::config_home().join("ide"); let entries = std::fs::read_dir(&lockfile_dir).ok()?; for entry in entries.flatten() { diff --git a/src-rust/crates/core/src/auth_store.rs b/src-rust/crates/core/src/auth_store.rs index 98651b42..c7d791ca 100644 --- a/src-rust/crates/core/src/auth_store.rs +++ b/src-rust/crates/core/src/auth_store.rs @@ -30,10 +30,7 @@ pub struct AuthStore { impl AuthStore { /// Path to the auth store file. pub fn path() -> PathBuf { - let dir = dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".coven-code"); - dir.join("auth.json") + crate::config::config_home().join("auth.json") } /// Load the store from disk (returns default if missing or invalid). diff --git a/src-rust/crates/core/src/claudemd.rs b/src-rust/crates/core/src/claudemd.rs index 87924a77..3acc69d3 100644 --- a/src-rust/crates/core/src/claudemd.rs +++ b/src-rust/crates/core/src/claudemd.rs @@ -911,9 +911,10 @@ pub fn load_all_memory_files_with_options( let mut files = Vec::new(); // 1. Managed: ~/.coven-code/rules/*.md - if let Some(home) = memory_home_dir() { + { + let config_home = crate::config::config_home(); if options.allow_managed_rules { - let rules_dir = home.join(".coven-code/rules"); + let rules_dir = config_home.join("rules"); if let Ok(entries) = std::fs::read_dir(&rules_dir) { let mut paths: Vec = entries .flatten() @@ -937,7 +938,7 @@ pub fn load_all_memory_files_with_options( // 2. User: ~/.coven-code/AGENTS.md then ~/.coven-code/CLAUDE.md if options.allow_user_memory { - load_scope_files(&home.join(".coven-code"), MemoryScope::User, &mut files); + load_scope_files(&config_home, MemoryScope::User, &mut files); } } @@ -973,16 +974,12 @@ pub fn enumerate_context_memory_files( let mut files: Vec = Vec::new(); if options.allow_user_memory { - if let Some(home) = dirs::home_dir() { - let global = home - .join(".coven-code") - .join(crate::constants::CLAUDE_MD_FILENAME); - if global.exists() { - if let Some(file) = load_memory_file(&global, MemoryScope::User) - .filter(|file| memory_file_allowed_for_options(file, options)) - { - files.push(file); - } + let global = crate::config::config_home().join(crate::constants::CLAUDE_MD_FILENAME); + if global.exists() { + if let Some(file) = load_memory_file(&global, MemoryScope::User) + .filter(|file| memory_file_allowed_for_options(file, options)) + { + files.push(file); } } } diff --git a/src-rust/crates/core/src/context_collapse.rs b/src-rust/crates/core/src/context_collapse.rs index 67043aec..ab0d5e3e 100644 --- a/src-rust/crates/core/src/context_collapse.rs +++ b/src-rust/crates/core/src/context_collapse.rs @@ -153,10 +153,7 @@ fn summarize_messages(messages: Vec, _max_tokens: u64) -> (Vec /// Persist collapse state to ~/.coven-code/context_collapse_state.json #[cfg(feature = "cached_microcompact")] pub fn save_collapse_state(_session_id: &str, state: &CollapseState) -> anyhow::Result<()> { - let path = dirs::home_dir() - .ok_or_else(|| anyhow::anyhow!("Could not determine home directory"))? - .join(".coven-code") - .join("context_collapse_state.json"); + let path = crate::config::config_home().join("context_collapse_state.json"); std::fs::create_dir_all(path.parent().unwrap())?; let json = serde_json::to_string(state)?; @@ -167,9 +164,7 @@ pub fn save_collapse_state(_session_id: &str, state: &CollapseState) -> anyhow:: /// Load collapse state from ~/.coven-code/context_collapse_state.json #[cfg(feature = "cached_microcompact")] pub fn load_collapse_state(_session_id: &str) -> Option { - let path = dirs::home_dir()? - .join(".coven-code") - .join("context_collapse_state.json"); + let path = crate::config::config_home().join("context_collapse_state.json"); if !path.exists() { return None; diff --git a/src-rust/crates/core/src/feature_flags.rs b/src-rust/crates/core/src/feature_flags.rs index 108bcd91..3775b605 100644 --- a/src-rust/crates/core/src/feature_flags.rs +++ b/src-rust/crates/core/src/feature_flags.rs @@ -1,218 +1,217 @@ -// GrowthBook feature flags integration -// -// Provides a feature flag manager that fetches flags from GrowthBook API, -// caches them locally, and provides a simple API for checking flag values. - -use anyhow::{anyhow, Context, Result}; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::path::PathBuf; -use std::sync::Arc; -use std::time::{Duration, SystemTime}; -use tokio::fs; -use tracing::{debug, warn}; - -/// Represents a feature flag from GrowthBook -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct FeatureFlag { - /// The ID of the feature flag - pub id: String, - /// The key of the feature flag (used for lookups) - pub key: String, - /// Whether the feature is enabled - pub enabled: bool, - /// Optional: the variant (A/B test group, etc.) - #[serde(skip_serializing_if = "Option::is_none")] - pub variant: Option, -} - -/// Cached feature flags with metadata -#[derive(Debug, Clone, Serialize, Deserialize)] -struct CachedFlags { - /// Map of flag key to flag - flags: HashMap, - /// When the cache was fetched (Unix timestamp) - fetched_at: u64, -} - -/// Manages feature flags from GrowthBook -#[derive(Clone)] -pub struct FeatureFlagManager { - /// Map of flag key to flag value - flags: Arc>>, - /// Cache file path - cache_path: PathBuf, - /// GrowthBook API endpoint - api_endpoint: String, - /// Cache TTL in seconds (default: 1 hour) - cache_ttl: u64, - /// HTTP client for making requests - http_client: reqwest::Client, -} - +// GrowthBook feature flags integration +// +// Provides a feature flag manager that fetches flags from GrowthBook API, +// caches them locally, and provides a simple API for checking flag values. + +use anyhow::{anyhow, Context, Result}; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; +use tokio::fs; +use tracing::{debug, warn}; + +/// Represents a feature flag from GrowthBook +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FeatureFlag { + /// The ID of the feature flag + pub id: String, + /// The key of the feature flag (used for lookups) + pub key: String, + /// Whether the feature is enabled + pub enabled: bool, + /// Optional: the variant (A/B test group, etc.) + #[serde(skip_serializing_if = "Option::is_none")] + pub variant: Option, +} + +/// Cached feature flags with metadata +#[derive(Debug, Clone, Serialize, Deserialize)] +struct CachedFlags { + /// Map of flag key to flag + flags: HashMap, + /// When the cache was fetched (Unix timestamp) + fetched_at: u64, +} + +/// Manages feature flags from GrowthBook +#[derive(Clone)] +pub struct FeatureFlagManager { + /// Map of flag key to flag value + flags: Arc>>, + /// Cache file path + cache_path: PathBuf, + /// GrowthBook API endpoint + api_endpoint: String, + /// Cache TTL in seconds (default: 1 hour) + cache_ttl: u64, + /// HTTP client for making requests + http_client: reqwest::Client, +} + impl FeatureFlagManager { - /// Create a new feature flag manager - /// - /// The API key is automatically fetched from the GROWTHBOOK_API_KEY environment variable. - pub fn new() -> Self { - let cache_path = Self::get_cache_path(); - let api_endpoint = "https://api.growthbook.io/api/features".to_string(); - let cache_ttl = 3600; // 1 hour - - Self { - flags: Arc::new(parking_lot::RwLock::new(HashMap::new())), - cache_path, - api_endpoint, - cache_ttl, - http_client: reqwest::Client::new(), - } + /// Create a new feature flag manager + /// + /// The API key is automatically fetched from the GROWTHBOOK_API_KEY environment variable. + pub fn new() -> Self { + let cache_path = Self::get_cache_path(); + let api_endpoint = "https://api.growthbook.io/api/features".to_string(); + let cache_ttl = 3600; // 1 hour + + Self { + flags: Arc::new(parking_lot::RwLock::new(HashMap::new())), + cache_path, + api_endpoint, + cache_ttl, + http_client: reqwest::Client::new(), + } + } + + /// Get the cache file path (~/.coven-code/feature_flags.json) + fn get_cache_path() -> PathBuf { + crate::config::config_home().join("feature_flags.json") + } + + /// Check if a feature flag is enabled + /// + /// # Arguments + /// * `name` - The feature flag key to check + pub fn flag(&self, name: &str) -> bool { + let flags = self.flags.read(); + flags.get(name).copied().unwrap_or(false) + } + + /// Fetch flags from GrowthBook API + /// + /// This is an async operation that: + /// 1. Checks if cached flags are still valid (within TTL) + /// 2. If cache is stale, fetches from GrowthBook API + /// 3. Saves the response to the cache file + /// 4. Updates the in-memory flags + pub async fn fetch_flags_async(&self) -> Result<()> { + // Try to load from cache first + if let Ok(cached) = self.load_cached_flags().await { + if self.is_cache_valid(&cached) { + debug!("Using cached feature flags"); + self.update_flags_from_cached(&cached); + return Ok(()); + } + } + + // Cache is stale or missing, fetch from API + debug!("Fetching feature flags from GrowthBook API"); + match self.fetch_from_api().await { + Ok(cached) => { + // Save to cache + if let Err(e) = self.save_cached_flags(&cached).await { + warn!("Failed to save feature flags cache: {}", e); + // Don't fail the whole operation if we can't save cache + } + self.update_flags_from_cached(&cached); + Ok(()) + } + Err(e) => { + // If API fetch fails, try to use stale cache + warn!("Failed to fetch from GrowthBook API: {}", e); + if let Ok(cached) = self.load_cached_flags().await { + debug!("Using stale cached feature flags as fallback"); + self.update_flags_from_cached(&cached); + return Ok(()); + } + // No cache available, just warn and continue with defaults + warn!("No cached feature flags available, using defaults"); + Ok(()) + } + } + } + + /// Fetch flags from GrowthBook API + async fn fetch_from_api(&self) -> Result { + let api_key = std::env::var("GROWTHBOOK_API_KEY").ok(); + + let mut builder = self.http_client.get(&self.api_endpoint); + + // Add authorization header if API key is available + if let Some(key) = api_key { + builder = builder.header("Authorization", format!("Bearer {}", key)); + } + + let response = builder + .timeout(Duration::from_secs(10)) + .send() + .await + .context("Failed to fetch from GrowthBook API")?; + + let status = response.status(); + if !status.is_success() { + return Err(anyhow!( + "GrowthBook API returned status {}: {}", + status.as_u16(), + response.text().await.unwrap_or_default() + )); + } + + let body = response + .json::() + .await + .context("Failed to parse GrowthBook API response")?; + + Ok(CachedFlags { + flags: body + .features + .into_iter() + .map(|f| (f.key.clone(), f)) + .collect(), + fetched_at: SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(), + }) + } + + /// Check if cached flags are still valid (within TTL) + fn is_cache_valid(&self, cached: &CachedFlags) -> bool { + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + now - cached.fetched_at < self.cache_ttl + } + + /// Load cached flags from disk + async fn load_cached_flags(&self) -> Result { + let data = fs::read_to_string(&self.cache_path) + .await + .context("Failed to read cache file")?; + let cached: CachedFlags = + serde_json::from_str(&data).context("Failed to parse cache file")?; + Ok(cached) + } + + /// Save cached flags to disk + async fn save_cached_flags(&self, cached: &CachedFlags) -> Result<()> { + // Ensure the directory exists + if let Some(parent) = self.cache_path.parent() { + fs::create_dir_all(parent).await.ok(); + } + + let json = serde_json::to_string(cached).context("Failed to serialize cache")?; + fs::write(&self.cache_path, json) + .await + .context("Failed to write cache file")?; + Ok(()) + } + + /// Update in-memory flags from cached data + fn update_flags_from_cached(&self, cached: &CachedFlags) { + let mut flags = self.flags.write(); + flags.clear(); + for (key, flag) in &cached.flags { + flags.insert(key.clone(), flag.enabled); + } + debug!("Loaded {} feature flags", flags.len()); } - - /// Get the cache file path (~/.coven-code/feature_flags.json) - fn get_cache_path() -> PathBuf { - let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); - home.join(".coven-code").join("feature_flags.json") - } - - /// Check if a feature flag is enabled - /// - /// # Arguments - /// * `name` - The feature flag key to check - pub fn flag(&self, name: &str) -> bool { - let flags = self.flags.read(); - flags.get(name).copied().unwrap_or(false) - } - - /// Fetch flags from GrowthBook API - /// - /// This is an async operation that: - /// 1. Checks if cached flags are still valid (within TTL) - /// 2. If cache is stale, fetches from GrowthBook API - /// 3. Saves the response to the cache file - /// 4. Updates the in-memory flags - pub async fn fetch_flags_async(&self) -> Result<()> { - // Try to load from cache first - if let Ok(cached) = self.load_cached_flags().await { - if self.is_cache_valid(&cached) { - debug!("Using cached feature flags"); - self.update_flags_from_cached(&cached); - return Ok(()); - } - } - - // Cache is stale or missing, fetch from API - debug!("Fetching feature flags from GrowthBook API"); - match self.fetch_from_api().await { - Ok(cached) => { - // Save to cache - if let Err(e) = self.save_cached_flags(&cached).await { - warn!("Failed to save feature flags cache: {}", e); - // Don't fail the whole operation if we can't save cache - } - self.update_flags_from_cached(&cached); - Ok(()) - } - Err(e) => { - // If API fetch fails, try to use stale cache - warn!("Failed to fetch from GrowthBook API: {}", e); - if let Ok(cached) = self.load_cached_flags().await { - debug!("Using stale cached feature flags as fallback"); - self.update_flags_from_cached(&cached); - return Ok(()); - } - // No cache available, just warn and continue with defaults - warn!("No cached feature flags available, using defaults"); - Ok(()) - } - } - } - - /// Fetch flags from GrowthBook API - async fn fetch_from_api(&self) -> Result { - let api_key = std::env::var("GROWTHBOOK_API_KEY").ok(); - - let mut builder = self.http_client.get(&self.api_endpoint); - - // Add authorization header if API key is available - if let Some(key) = api_key { - builder = builder.header("Authorization", format!("Bearer {}", key)); - } - - let response = builder - .timeout(Duration::from_secs(10)) - .send() - .await - .context("Failed to fetch from GrowthBook API")?; - - let status = response.status(); - if !status.is_success() { - return Err(anyhow!( - "GrowthBook API returned status {}: {}", - status.as_u16(), - response.text().await.unwrap_or_default() - )); - } - - let body = response - .json::() - .await - .context("Failed to parse GrowthBook API response")?; - - Ok(CachedFlags { - flags: body - .features - .into_iter() - .map(|f| (f.key.clone(), f)) - .collect(), - fetched_at: SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap_or_default() - .as_secs(), - }) - } - - /// Check if cached flags are still valid (within TTL) - fn is_cache_valid(&self, cached: &CachedFlags) -> bool { - let now = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap_or_default() - .as_secs(); - now - cached.fetched_at < self.cache_ttl - } - - /// Load cached flags from disk - async fn load_cached_flags(&self) -> Result { - let data = fs::read_to_string(&self.cache_path) - .await - .context("Failed to read cache file")?; - let cached: CachedFlags = - serde_json::from_str(&data).context("Failed to parse cache file")?; - Ok(cached) - } - - /// Save cached flags to disk - async fn save_cached_flags(&self, cached: &CachedFlags) -> Result<()> { - // Ensure the directory exists - if let Some(parent) = self.cache_path.parent() { - fs::create_dir_all(parent).await.ok(); - } - - let json = serde_json::to_string(cached).context("Failed to serialize cache")?; - fs::write(&self.cache_path, json) - .await - .context("Failed to write cache file")?; - Ok(()) - } - - /// Update in-memory flags from cached data - fn update_flags_from_cached(&self, cached: &CachedFlags) { - let mut flags = self.flags.write(); - flags.clear(); - for (key, flag) in &cached.flags { - flags.insert(key.clone(), flag.enabled); - } - debug!("Loaded {} feature flags", flags.len()); - } } impl Default for FeatureFlagManager { @@ -220,51 +219,68 @@ impl Default for FeatureFlagManager { Self::new() } } - -/// Response from GrowthBook API -#[derive(Debug, Deserialize)] -struct GrowthBookApiResponse { - /// Map of feature flag keys to flag objects - pub features: Vec, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_cache_path() { - let path = FeatureFlagManager::get_cache_path(); - assert!(path.to_string_lossy().contains(".coven-code")); - assert!(path.to_string_lossy().contains("feature_flags.json")); - } - - #[test] - fn test_flag_default_false() { - let manager = FeatureFlagManager::new(); - assert!(!manager.flag("nonexistent_flag")); - } - - #[test] - fn test_cache_validity() { - let manager = FeatureFlagManager::new(); - let now = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap() - .as_secs(); - - // Fresh cache - let fresh_cache = CachedFlags { - flags: HashMap::new(), - fetched_at: now, - }; - assert!(manager.is_cache_valid(&fresh_cache)); - - // Stale cache (older than TTL) - let stale_cache = CachedFlags { - flags: HashMap::new(), - fetched_at: now - 7200, // 2 hours ago - }; - assert!(!manager.is_cache_valid(&stale_cache)); - } -} + +/// Response from GrowthBook API +#[derive(Debug, Deserialize)] +struct GrowthBookApiResponse { + /// Map of feature flag keys to flag objects + pub features: Vec, +} + +/// Test accessor — exposes the private cache path for cross-module +/// consolidation tests (Phase 4.1). +#[cfg(test)] +pub(crate) fn feature_flags_cache_path_for_test() -> PathBuf { + FeatureFlagManager::get_cache_path() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_cache_path() { + // Hold the env lock so this test is serialized against any concurrent + // test that mutates COVEN_HOME / COVEN_PARENT / COVEN_CODE_HOME and + // would otherwise cause config_home() to return an unexpected path. + let _lock = crate::config::CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let path = FeatureFlagManager::get_cache_path(); + // Derive the expectation from config_home() so the assertion is correct + // under any home layout (legacy ~/.coven-code OR unified ~/.coven/code). + assert_eq!( + path, + crate::config::config_home().join("feature_flags.json") + ); + } + + #[test] + fn test_flag_default_false() { + let manager = FeatureFlagManager::new(); + assert!(!manager.flag("nonexistent_flag")); + } + + #[test] + fn test_cache_validity() { + let manager = FeatureFlagManager::new(); + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + + // Fresh cache + let fresh_cache = CachedFlags { + flags: HashMap::new(), + fetched_at: now, + }; + assert!(manager.is_cache_valid(&fresh_cache)); + + // Stale cache (older than TTL) + let stale_cache = CachedFlags { + flags: HashMap::new(), + fetched_at: now - 7200, // 2 hours ago + }; + assert!(!manager.is_cache_valid(&stale_cache)); + } +} diff --git a/src-rust/crates/core/src/goal.rs b/src-rust/crates/core/src/goal.rs index 6fc76aba..d8412c7a 100644 --- a/src-rust/crates/core/src/goal.rs +++ b/src-rust/crates/core/src/goal.rs @@ -161,13 +161,13 @@ impl GoalStore { } /// Default path: `~/.coven-code/goals.sqlite`. - pub fn default_path() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("goals.sqlite")) + pub fn default_path() -> PathBuf { + crate::config::config_home().join("goals.sqlite") } /// Open using the default path (best-effort; returns None on failure). pub fn open_default() -> Option { - Self::default_path().and_then(|p| Self::open(&p).ok()) + Self::open(&Self::default_path()).ok() } fn now_ms() -> u64 { @@ -697,4 +697,17 @@ mod tests { std::env::remove_var("COVEN_CODE_GOALS"); } } + + #[test] + fn default_path_derives_from_config_home() { + let _lock = crate::config::CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let path = GoalStore::default_path(); + assert!( + path.starts_with(crate::config::config_home()), + "default_path {path:?} should start with config_home()" + ); + assert_eq!(path.file_name().unwrap(), "goals.sqlite"); + } } diff --git a/src-rust/crates/core/src/home_migration.rs b/src-rust/crates/core/src/home_migration.rs new file mode 100644 index 00000000..9b1d4002 --- /dev/null +++ b/src-rust/crates/core/src/home_migration.rs @@ -0,0 +1,517 @@ +//! One-time, best-effort relocation of the engine home from the legacy +//! `~/.coven-code/` to `~/.coven/code/` when running under the unified coven CLI. +//! +//! Every failure is non-fatal — a failed migration must NEVER brick the engine. +//! On failure the user's data is preserved in the legacy directory (see the +//! "Degraded behaviour" note below); the engine does not automatically redirect +//! reads back to legacy, so a failed migration means it starts fresh at the new +//! (empty) target while the data remains recoverable in legacy. +//! +//! # Safety invariant +//! +//! If migration fails, the legacy directory **must** remain intact so the user +//! can recover their data manually. The worst failure mode is a partial target +//! that looks non-empty while the source is destroyed — this code avoids it by: +//! +//! 1. Never removing the legacy directory until the target is fully populated. +//! 2. On cross-filesystem rename failure, attempting a full recursive copy +//! before removing the source. +//! 3. On copy failure, removing any partial target so the engine starts fresh +//! (empty target) rather than with corrupt/partial state, while leaving the +//! legacy directory intact. +//! +//! Degraded behaviour: the engine starts at `target` (empty) while the user's +//! data sits in `legacy`. The user is informed via stderr and can copy +//! manually. + +use std::path::Path; + +/// Run the migration if the current configuration calls for relocation. +/// +/// This is the public entry point called once at startup, before any config or +/// settings access. It is a no-op when: +/// - `config_home()` still points at `.coven-code` (standalone mode). +/// - `~/.coven-code` does not exist (nothing to migrate). +/// - The target already exists and is non-empty (already migrated). +pub fn migrate_if_needed() { + let target = crate::config::config_home(); + + // If config_home still resolves to something ending in `.coven-code`, we + // are in standalone (non-coven) mode — nothing to migrate. + if target.file_name().and_then(|n| n.to_str()) == Some(".coven-code") { + return; + } + + // Compute the legacy directory. + let home_dir = match dirs::home_dir() { + Some(h) => h, + None => { + eprintln!( + "coven-code: home_migration: cannot determine home directory; \ + skipping migration" + ); + return; + } + }; + let legacy = home_dir.join(".coven-code"); + + if !legacy.exists() { + return; + } + + // If the target already exists AND is non-empty, we are done (or the user + // placed data there intentionally). + if should_skip_existing_target(&target) { + return; + } + + migrate_between(&legacy, &target); +} + +/// Returns `true` when the migration should be skipped because the target +/// already exists and we must not clobber it. +/// +/// This is the production no-clobber guard. It is extracted as a pure +/// predicate so it can be unit-tested directly: if it is removed or its +/// logic changes, `target_exists_no_clobber` will catch the regression. +/// +/// Fails SAFE: the migration is skipped if the target exists and is +/// non-empty, OR if it exists but cannot be inspected (e.g. `read_dir` +/// fails due to permissions, or the path is not a directory). Proceeding in +/// those cases would let the copy fallback merge into / overwrite an existing +/// target, violating the no-clobber guarantee. +fn should_skip_existing_target(target: &Path) -> bool { + if !target.exists() { + return false; + } + match target.read_dir() { + // Readable directory: skip only if it already holds something. + Ok(mut entries) => entries.next().is_some(), + // Exists but not inspectable (permissions, not-a-dir, …): skip to be safe. + Err(_) => true, + } +} + +/// Inner migration function — moves `legacy` to `target`. +/// +/// Extracted from `migrate_if_needed` so it can be unit-tested against +/// arbitrary temp directories without touching the real `~`. +/// +/// Callers are responsible for all precondition guards (legacy exists, +/// target is absent or empty, etc.). +fn migrate_between(legacy: &Path, target: &Path) { + // Ensure target's parent directory exists. + let parent = match target.parent() { + Some(p) => p, + None => { + eprintln!( + "coven-code: home_migration: target path {:?} has no parent; \ + skipping migration", + target + ); + return; + } + }; + if let Err(e) = std::fs::create_dir_all(parent) { + eprintln!( + "coven-code: home_migration: failed to create parent {:?}: {}; \ + skipping migration", + parent, e + ); + return; + } + + // Attempt an atomic rename first (works on same filesystem). + match std::fs::rename(legacy, target) { + Ok(()) => { + // Rename succeeded: create backward-compat symlink and marker. + create_symlink_compat(legacy, target); + write_marker(target); + } + Err(rename_err) => { + // Rename failed (likely cross-filesystem EXDEV or permissions). + // Attempt a recursive copy followed by removal of the source. + eprintln!( + "coven-code: home_migration: rename {:?} → {:?} failed ({}); \ + attempting recursive copy", + legacy, target, rename_err + ); + + match copy_dir_recursive(legacy, target) { + Ok(()) => { + // Copy succeeded: remove legacy and set up compat link. + if let Err(e) = std::fs::remove_dir_all(legacy) { + eprintln!( + "coven-code: home_migration: copy succeeded but failed to \ + remove legacy {:?}: {}; leaving legacy in place", + legacy, e + ); + // Still set up symlink pointing to target since data is there. + } + create_symlink_compat(legacy, target); + write_marker(target); + } + Err(copy_err) => { + // Both rename and copy failed. + // Remove any partial target so the engine does not start + // with an empty/corrupt home while data is in legacy. + eprintln!( + "coven-code: home_migration: recursive copy {:?} → {:?} also \ + failed ({}); removing partial target and leaving legacy intact", + legacy, target, copy_err + ); + if target.exists() { + if let Err(e) = std::fs::remove_dir_all(target) { + eprintln!( + "coven-code: home_migration: failed to clean up partial \ + target {:?}: {}", + target, e + ); + } + } + eprintln!( + "coven-code: home_migration: your data remains at {:?}; \ + please move it to {:?} manually", + legacy, target + ); + } + } + } + } +} + +/// Create a compatibility symlink at `legacy` → `target` so that any tooling +/// that still references `~/.coven-code` continues to work. +/// +/// Symlinks are Unix-only; on non-Unix platforms we log and skip. +fn create_symlink_compat(legacy: &Path, target: &Path) { + #[cfg(unix)] + { + // Only create the symlink when legacy was successfully moved (i.e., + // legacy no longer exists as a real directory). + if legacy.exists() { + // legacy still present (copy succeeded but remove failed, or + // some other scenario) — don't try to create symlink at same path. + return; + } + if let Err(e) = std::os::unix::fs::symlink(target, legacy) { + eprintln!( + "coven-code: home_migration: failed to create compat symlink \ + {:?} → {:?}: {}", + legacy, target, e + ); + } + } + #[cfg(not(unix))] + { + eprintln!( + "coven-code: home_migration: symlink creation is not supported on \ + this platform; {:?} will not be symlinked to {:?}", + legacy, target + ); + } +} + +/// Write a marker file at `/.migrated-from-coven-code` containing a +/// UTC timestamp so the migration is inspectable and idempotency can be +/// checked externally. +fn write_marker(target: &Path) { + let marker = target.join(".migrated-from-coven-code"); + let ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs().to_string()) + .unwrap_or_else(|_| "unknown".to_string()); + let content = format!("migrated-at-unix-secs={}\n", ts); + if let Err(e) = std::fs::write(&marker, content) { + eprintln!( + "coven-code: home_migration: failed to write marker {:?}: {}", + marker, e + ); + } +} + +/// Recursively copy the directory tree rooted at `src` into `dst`. +/// +/// Creates `dst` if it does not exist. Skips symlinks (they would be stale +/// after the source is removed). +fn copy_dir_recursive(src: &Path, dst: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(dst)?; + + for entry in std::fs::read_dir(src)? { + let entry = entry?; + let entry_type = entry.file_type()?; + let src_path = entry.path(); + let dst_path = dst.join(entry.file_name()); + + if entry_type.is_dir() { + copy_dir_recursive(&src_path, &dst_path)?; + } else if entry_type.is_file() { + std::fs::copy(&src_path, &dst_path)?; + } + // Symlinks skipped intentionally — they point inside the source tree + // and become dangling once the source is removed. + } + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + /// Helper: acquire the unified env-var lock. + /// + /// `CONFIG_HOME_ENV_LOCK` and `COVEN_HOME_ENV_LOCK` are the same + /// `Mutex` (one is a re-export of the other), so only one acquisition + /// is needed. Returns the `MutexGuard`, which must be kept alive for + /// the duration of the test. + fn acquire_env_locks<'a>() -> std::sync::MutexGuard<'a, ()> { + crate::config::CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()) + } + + /// Save and clear all env vars touched by `config_home()`. + fn save_and_clear_config_env() -> Vec<(String, Option)> { + let vars = [ + "COVEN_CODE_TEST_HOME", + "COVEN_CODE_HOME", + "COVEN_HOME", + "COVEN_PARENT", + ]; + vars.iter() + .map(|&v| { + let saved = std::env::var(v).ok(); + std::env::remove_var(v); + (v.to_string(), saved) + }) + .collect() + } + + fn restore_config_env(saved: Vec<(String, Option)>) { + for (var, value) in saved { + match value { + Some(v) => std::env::set_var(&var, v), + None => std::env::remove_var(&var), + } + } + } + + // ----------------------------------------------------------------------- + // Test 1: populated_migration + // ----------------------------------------------------------------------- + + /// Create a legacy dir with files; call `migrate_between`; assert: + /// - files exist under target + /// - legacy is gone (or is a symlink) + /// - marker file exists at `target/.migrated-from-coven-code` + #[test] + fn populated_migration() { + let base = tempfile::tempdir().unwrap(); + + let legacy = base.path().join("legacy"); + let target = base.path().join("target"); + + // Populate legacy. + std::fs::create_dir_all(legacy.join("projects/x")).unwrap(); + std::fs::write(legacy.join("auth.json"), b"{}").unwrap(); + std::fs::write(legacy.join("projects/x/y.jsonl"), b"session\n").unwrap(); + + migrate_between(&legacy, &target); + + // Files must be under target. + assert!( + target.join("auth.json").exists(), + "auth.json must exist in target" + ); + assert!( + target.join("projects/x/y.jsonl").exists(), + "projects/x/y.jsonl must exist in target" + ); + + // Marker must exist. + assert!( + target.join(".migrated-from-coven-code").exists(), + "migration marker must exist" + ); + + // Legacy must no longer be a real directory (either gone or is a symlink). + let legacy_is_real_dir = legacy.exists() && legacy.is_dir() && !legacy.is_symlink(); + assert!( + !legacy_is_real_dir, + "legacy must not remain as a real directory after migration" + ); + } + + // ----------------------------------------------------------------------- + // Test 2: idempotent + // ----------------------------------------------------------------------- + + /// After a successful migration, the no-clobber guard reports the target as + /// "skip", and a second `migrate_between` against a fresh legacy does NOT + /// overwrite the already-migrated data. + /// + /// This test NEVER calls `migrate_if_needed()` — that function reads the + /// real environment and operates on the real `~/.coven-code`, so exercising + /// it here would pollute the developer's home directory. We test the pure + /// guard (`should_skip_existing_target`) and `migrate_between` directly. + #[test] + fn idempotent() { + let base = tempfile::tempdir().unwrap(); + let target = base.path().join("code"); + + // First migration from a populated legacy dir. + let legacy1 = base.path().join("legacy1"); + std::fs::create_dir_all(&legacy1).unwrap(); + std::fs::write(legacy1.join("settings.json"), b"{}").unwrap(); + + migrate_between(&legacy1, &target); + assert!(target.exists(), "target must exist after first migration"); + assert!( + target.join(".migrated-from-coven-code").exists(), + "marker must exist after first migration" + ); + + // The production no-clobber guard must now report "skip" for the target. + assert!( + should_skip_existing_target(&target), + "populated target must be skipped by the no-clobber guard" + ); + + // Sentinel to detect any clobber. + let sentinel = target.join("sentinel.txt"); + std::fs::write(&sentinel, b"do not touch").unwrap(); + + // A second migrate_between from a DIFFERENT legacy must not clobber the + // sentinel: rename onto a non-empty existing dir fails, and the failure + // path leaves the existing target intact (it does not delete populated + // data it did not create). + let legacy2 = base.path().join("legacy2"); + std::fs::create_dir_all(&legacy2).unwrap(); + std::fs::write(legacy2.join("other.json"), b"{}").unwrap(); + migrate_between(&legacy2, &target); + + let contents = std::fs::read(&sentinel).unwrap_or_default(); + assert_eq!( + contents, b"do not touch", + "sentinel must survive a second migration attempt" + ); + } + + // ----------------------------------------------------------------------- + // Test 3: no_legacy_no_op + // ----------------------------------------------------------------------- + + /// If legacy does not exist, migrate_between does nothing. + #[test] + fn no_legacy_no_op() { + let base = tempfile::tempdir().unwrap(); + let legacy = base.path().join("nonexistent-legacy"); + let target = base.path().join("target"); + + // legacy does not exist. + migrate_between(&legacy, &target); + + // target must not have been created. + assert!( + !target.exists(), + "target must not be created when legacy does not exist" + ); + } + + // ----------------------------------------------------------------------- + // Test 4: standalone_no_op + // ----------------------------------------------------------------------- + + /// With no COVEN_HOME / COVEN_PARENT set, `migrate_if_needed` must return + /// early because `config_home()` resolves to a `.coven-code` path. + /// + /// We assert the *precondition* that drives that early return rather than + /// calling `migrate_if_needed()` itself: that function operates on the real + /// `~/.coven-code`, so invoking it from a test could pollute the developer's + /// home directory. Verifying `config_home()` ends in `.coven-code` under + /// cleared env proves the guard at the top of `migrate_if_needed` fires. + #[test] + fn standalone_no_op() { + let _g = acquire_env_locks(); + let saved = save_and_clear_config_env(); + + let target = crate::config::config_home(); + let ends_in_legacy = target.file_name().and_then(|n| n.to_str()) == Some(".coven-code"); + + restore_config_env(saved); + + assert!( + ends_in_legacy, + "with no coven env, config_home() must end in .coven-code so \ + migrate_if_needed early-returns (was {:?})", + target + ); + } + + // ----------------------------------------------------------------------- + // Test 5: target_exists_no_clobber + // ----------------------------------------------------------------------- + + /// Exercises the PRODUCTION no-clobber guard (`should_skip_existing_target`) + /// rather than re-implementing the check inline. + /// + /// This test will FAIL if the predicate is removed or its logic is inverted, + /// giving real confidence on the data-safety-critical path. + /// + /// Two sub-cases: + /// a) non-empty target → predicate returns true (skip), sentinel untouched. + /// b) absent target → predicate returns false (proceed). + #[test] + fn target_exists_no_clobber() { + let base = tempfile::tempdir().unwrap(); + let legacy = base.path().join("legacy"); + let target = base.path().join("target"); + + // Populate legacy. + std::fs::create_dir_all(&legacy).unwrap(); + std::fs::write(legacy.join("LEGACY_MARKER"), b"important").unwrap(); + + // Populate target with a sentinel. + std::fs::create_dir_all(&target).unwrap(); + let sentinel = target.join("TARGET_SENTINEL"); + std::fs::write(&sentinel, b"sentinel").unwrap(); + + // --- Case A: non-empty target --- + // The production guard must report "skip". + assert!( + should_skip_existing_target(&target), + "should_skip_existing_target must return true when target is non-empty" + ); + + // Since the guard says skip, migrate_between must NOT be called. + // Replicate the exact production branch from migrate_if_needed: + if !should_skip_existing_target(&target) { + migrate_between(&legacy, &target); + } + + // Sentinel must be intact — the target was not clobbered. + let contents = std::fs::read(&sentinel).unwrap_or_default(); + assert_eq!( + contents, b"sentinel", + "TARGET_SENTINEL must be untouched: no-clobber guard failed" + ); + + // Legacy data must still be present — nothing was moved. + assert!( + legacy.join("LEGACY_MARKER").exists(), + "LEGACY_MARKER must remain: legacy must not be moved when target is non-empty" + ); + + // --- Case B: absent target → guard must NOT skip --- + let absent = base.path().join("absent-target"); + assert!( + !should_skip_existing_target(&absent), + "should_skip_existing_target must return false when target does not exist" + ); + } +} diff --git a/src-rust/crates/core/src/lib.rs b/src-rust/crates/core/src/lib.rs index 765c31da..79cb0bc9 100644 --- a/src-rust/crates/core/src/lib.rs +++ b/src-rust/crates/core/src/lib.rs @@ -116,6 +116,10 @@ pub mod coven_daemon; // Best-effort session lifecycle notifications to the Coven daemon ledger. pub mod coven_ledger; pub mod roster_reset; + +// One-time, best-effort relocation of the engine home from legacy +// ~/.coven-code/ to ~/.coven/code/ when running under the unified coven CLI. +pub mod home_migration; pub use cost::CostTracker; pub use feature_flags::FeatureFlagManager; pub use history::ConversationSession; @@ -1065,6 +1069,92 @@ pub mod config { pub urls: Vec, } + // ---- SharedSettings -------------------------------------------------- + + /// A restricted subset of settings that can live at `~/.coven/settings.json` + /// and apply as the lowest-precedence layer across all Coven tools. + /// + /// Only genuinely cross-tool keys belong here. Engine-specific keys + /// (MCP servers, hooks, plugins, credentials, provider routing, etc.) are + /// intentionally absent. Unknown JSON keys in the shared file are silently + /// ignored so future tools can extend the schema without breaking older + /// versions of coven-code. + /// + /// Load order (lowest → highest): shared → engine-global → project. + #[derive(Debug, Clone, Default, Deserialize)] + #[serde(default)] + pub struct SharedSettings { + /// Default model to use across all Coven tools. + pub model: Option, + /// UI theme preference (`"default"`, `"dark"`, `"light"`, etc.). + /// Stored as a raw string so the shared file does not depend on the + /// engine's internal `Theme` enum variants. + pub theme: Option, + /// Permission mode (`"default"`, `"acceptEdits"`, `"bypassPermissions"`, + /// `"plan"`). Stored as a raw string for the same reason as `theme`. + pub permission_mode: Option, + } + + impl SharedSettings { + /// Load the shared settings file at `~/.coven/settings.json`. + /// + /// Returns `SharedSettings::default()` (all `None`) when: + /// - there is no `~/.coven/` directory (standalone / no coven home), or + /// - the file does not exist, or + /// - the file cannot be read or parsed. + /// + /// All failures are non-fatal so coven-code keeps working standalone. + pub fn load() -> Self { + let Some(home) = crate::coven_shared::coven_home() else { + return Self::default(); + }; + let path = home.join("settings.json"); + let raw = match std::fs::read_to_string(&path) { + Ok(r) => r, + Err(_) => return Self::default(), + }; + serde_json::from_str::(&raw).unwrap_or_default() + } + + /// Apply whitelisted fields from the shared file to `target` (the + /// engine-global `Settings`) only when the engine-global left those + /// fields at their built-in default value. + /// + /// Semantics per field: + /// - `model` — `Option`: applied when engine has `None`. + /// - `theme` — non-optional enum: applied when engine still has + /// `Theme::Default` (i.e. the user did not pick a theme in the + /// engine settings). + /// - `permission_mode` — non-optional enum: applied when engine still + /// has `PermissionMode::Default`. + pub fn apply_to(self, target: &mut Settings) { + // model: Option — shared fills the gap when engine left it None. + if target.config.model.is_none() { + if let Some(m) = self.model { + target.config.model = Some(m); + } + } + // theme: enum with Default variant — shared fills only when engine + // is still at the built-in default. + if matches!(target.config.theme, Theme::Default) { + if let Some(raw) = self.theme { + let parsed = serde_json::from_value::(serde_json::Value::String(raw)) + .unwrap_or(Theme::Default); + target.config.theme = parsed; + } + } + // permission_mode: same pattern as theme. + if matches!(target.config.permission_mode, PermissionMode::Default) { + if let Some(raw) = self.permission_mode { + let parsed = + serde_json::from_value::(serde_json::Value::String(raw)) + .unwrap_or(PermissionMode::Default); + target.config.permission_mode = parsed; + } + } + } + } + // ---- Settings -------------------------------------------------------- #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -1485,19 +1575,86 @@ pub mod config { } } + /// The directory name used for per-project engine config directories. + /// + /// Project discovery walks up from cwd and joins this name onto each + /// ancestor — it is intentionally NOT the same as `config_home()` (which + /// is an absolute path under the user's home directory). + pub const PROJECT_CONFIG_DIRNAME: &str = ".coven-code"; + + /// Mutex that must be held when mutating `COVEN_CODE_TEST_HOME`, + /// `COVEN_CODE_HOME`, `COVEN_PARENT`, or any env var read by + /// `config_home()` in tests. + /// + /// `config_home()` reads these env vars; without serialisation concurrent + /// tests would race on process-wide env state. + /// + /// This is a RE-EXPORT of [`crate::coven_shared::COVEN_HOME_ENV_LOCK`], not a + /// separate mutex. Both names guard the exact same set of env vars, so they + /// MUST resolve to the same lock — otherwise a test holding one lock could + /// mutate `COVEN_HOME`/`COVEN_CODE_HOME` while a test holding the other reads + /// them, producing flaky failures (observed in `test_cache_path`). + #[cfg(test)] + pub(crate) use crate::coven_shared::COVEN_HOME_ENV_LOCK as CONFIG_HOME_ENV_LOCK; + + /// The engine's home/config directory. **Single source of truth** for all + /// engine state (settings, auth, projects, caches, skills). + /// + /// Precedence (highest wins): + /// + /// 1. `COVEN_CODE_TEST_HOME` (test builds only) — resolves to + /// `$COVEN_CODE_TEST_HOME/.coven-code` so test suites can isolate + /// without touching the real home directory. + /// 2. `COVEN_CODE_HOME` — explicit engine-home override; returned as-is. + /// 3. Under the unified coven CLI (`COVEN_HOME` set or `COVEN_PARENT=coven`) + /// — resolves to `$COVEN_HOME/code` (or `~/.coven/code`). + /// 4. Legacy standalone default — `~/.coven-code`. + pub fn config_home() -> 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"); + } + } + + // 1. Explicit engine-home override. + if let Ok(p) = std::env::var("COVEN_CODE_HOME") { + if !p.is_empty() { + return PathBuf::from(p); + } + } + + // 2. Under the unified coven CLI, live at /code. + // coven_home = COVEN_HOME or ~/.coven. Triggered when COVEN_HOME is + // set to a NON-EMPTY value OR the parent is coven (COVEN_PARENT=coven). + // An empty COVEN_HOME is treated as unset — otherwise PathBuf::from("") + // would resolve to a relative "code" directory in the cwd, which could + // also mis-trigger the home migration. + let coven_home_env = std::env::var("COVEN_HOME").ok().filter(|v| !v.is_empty()); + let under_coven = coven_home_env.is_some() + || std::env::var("COVEN_PARENT").ok().as_deref() == Some("coven"); + if under_coven { + let coven_home = coven_home_env + .map(PathBuf::from) + .or_else(|| dirs::home_dir().map(|h| h.join(".coven"))); + if let Some(ch) = coven_home { + return ch.join("code"); + } + } + + // 3. Legacy standalone default. + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".coven-code") + } + impl Settings { /// The per-user configuration directory (`~/.coven-code`). + /// + /// Thin shim over [`config_home()`] — kept for callers that reach it + /// through `Settings::`. 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") + config_home() } /// Full path to the global settings JSON file. @@ -1616,13 +1773,27 @@ pub mod config { } /// Load settings from all config levels and merge them. - /// Priority: project > global, except project-local executable MCP - /// server definitions and provider-routing fields are ignored before - /// merging. + /// Priority: project > engine-global > shared, except project-local + /// executable MCP server definitions and provider-routing fields are + /// ignored before merging. + /// + /// Three-layer load order (lowest → highest precedence): + /// 1. Shared — `~/.coven/settings.json` (whitelisted keys only) + /// 2. Global — `~/.coven/code/settings.json` + /// 3. Project — `/.coven-code/settings.json` + /// + /// When there is no `~/.coven/` directory (standalone mode, no coven + /// CLI), the shared layer is absent and behaviour is identical to the + /// pre-4.3 two-layer load. pub async fn load_hierarchical(cwd: &std::path::Path) -> Self { - // 1. Load global settings. + // 1. Load engine-global settings. let mut merged = Self::load().await.unwrap_or_default(); - // 2. Find and merge project settings (safe project fields win). + // 2. Apply shared ~/.coven/settings.json as the lowest layer: fill + // in whitelisted keys only when the engine-global left them at + // their default value (i.e. the user did not explicitly set + // them in the engine settings). + SharedSettings::load().apply_to(&mut merged); + // 3. Find and merge project settings (safe project fields win). if let Some(project_settings) = Self::find_project_settings(cwd).await { merged = Self::merge(merged, Self::sanitize_project_settings(project_settings)); } @@ -1637,7 +1808,7 @@ pub mod config { loop { // Try .json first, then .jsonc. for name in &["settings.json", "settings.jsonc"] { - let candidate = dir.join(".coven-code").join(name); + let candidate = dir.join(PROJECT_CONFIG_DIRNAME).join(name); if candidate.exists() && candidate != global_path { if let Ok(content) = tokio::fs::read_to_string(&candidate).await { let stripped = strip_jsonc_comments(&content); @@ -2131,6 +2302,571 @@ pub mod config { None => std::env::remove_var("COVEN_CODE_HOSTED_REVIEW"), } } + + // -- Phase 4.1: config_home() consolidation tests ------------------- + + /// Verify that every engine-global path derives from `config_home()`. + /// + /// Private path functions are exposed via `#[cfg(test)] pub(crate)` + /// accessors added in Phase 4.1; those are noted in the report. + #[test] + fn all_engine_paths_derive_from_config_home() { + // config_home() is env-sensitive; hold the lock so a concurrent + // env-mutating test can't change it between our two calls (which + // would make config_dir() and `home` disagree). + let _lock = crate::coven_shared::COVEN_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let home = config_home(); + + // Site 1 / Settings::config_dir() is a thin shim — same value. + assert_eq!(Settings::config_dir(), home); + + // Site 2 — AuthStore::path(). + assert!(crate::auth_store::AuthStore::path().starts_with(&home)); + + // Site 3 — FeatureFlagManager::get_cache_path() (via test accessor). + assert!(crate::feature_flags::feature_flags_cache_path_for_test().starts_with(&home)); + + // Site 4 — claude_home() in prompt_history (via test accessor). + assert!(crate::prompt_history::history_base_path_for_test().starts_with(&home)); + + // Site 5 — claude_config_dir() in remote_settings (via test accessor). + assert!(crate::remote_settings::remote_settings_dir_for_test().starts_with(&home)); + + // Site 6 — OAuthTokens::token_file_path(). + assert!(crate::oauth::OAuthTokens::token_file_path().starts_with(&home)); + + // Site 7 — global skills dir derives from config_home(). + assert!(home.join("skills").starts_with(&home)); + } + + /// Verify that `config_home()` honours `COVEN_CODE_TEST_HOME` in + /// `#[cfg(test)]` builds, preserving the existing test-override + /// behaviour from `Settings::config_dir()`. + #[test] + fn config_home_test_override_still_works() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + + let tmp = tempfile::tempdir().unwrap(); + let original = std::env::var("COVEN_CODE_TEST_HOME").ok(); + + std::env::set_var("COVEN_CODE_TEST_HOME", tmp.path()); + let got = config_home(); + let expected = tmp.path().join(".coven-code"); + assert_eq!( + got, expected, + "config_home() should honour COVEN_CODE_TEST_HOME" + ); + + // Restore env state. + match original { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + // -- Phase 4.2: config_home() env-override precedence tests ---------- + + /// COVEN_CODE_HOME wins over all other signals. + #[test] + fn config_home_explicit_override_wins() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + // Ensure COVEN_CODE_TEST_HOME is clear so the test-override branch + // does not fire (this test targets the COVEN_CODE_HOME branch). + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + + let saved = std::env::var("COVEN_CODE_HOME").ok(); + let expected = PathBuf::from("/tmp/my-custom-coven-engine"); + std::env::set_var("COVEN_CODE_HOME", &expected); + + let got = config_home(); + assert_eq!(got, expected, "COVEN_CODE_HOME should be returned as-is"); + + match saved { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + /// COVEN_HOME set → `/code`. + #[test] + fn config_home_coven_home_gives_code_subdir() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + let saved_cc_home = std::env::var("COVEN_CODE_HOME").ok(); + std::env::remove_var("COVEN_CODE_HOME"); + let saved_coven_home = std::env::var("COVEN_HOME").ok(); + let saved_parent = std::env::var("COVEN_PARENT").ok(); + std::env::remove_var("COVEN_PARENT"); + + let tmp = tempfile::tempdir().unwrap(); + std::env::set_var("COVEN_HOME", tmp.path()); + + let got = config_home(); + let expected = tmp.path().join("code"); + assert_eq!( + got, expected, + "COVEN_HOME set should yield /code" + ); + + // Restore. + match saved_coven_home { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match saved_parent { + Some(v) => std::env::set_var("COVEN_PARENT", v), + None => std::env::remove_var("COVEN_PARENT"), + } + match saved_cc_home { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + /// COVEN_PARENT=coven (no COVEN_HOME) → `~/.coven/code`. + #[test] + fn config_home_coven_parent_gives_dot_coven_code() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + let saved_cc_home = std::env::var("COVEN_CODE_HOME").ok(); + std::env::remove_var("COVEN_CODE_HOME"); + let saved_coven_home = std::env::var("COVEN_HOME").ok(); + std::env::remove_var("COVEN_HOME"); + let saved_parent = std::env::var("COVEN_PARENT").ok(); + std::env::set_var("COVEN_PARENT", "coven"); + + let got = config_home(); + // Should be ~/.coven/code + let expected = dirs::home_dir() + .expect("home_dir must be available in test environment") + .join(".coven") + .join("code"); + assert_eq!( + got, expected, + "COVEN_PARENT=coven should yield ~/.coven/code" + ); + + // Restore. + match saved_coven_home { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match saved_parent { + Some(v) => std::env::set_var("COVEN_PARENT", v), + None => std::env::remove_var("COVEN_PARENT"), + } + match saved_cc_home { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + /// No coven signals → `~/.coven-code` (legacy standalone default). + #[test] + fn config_home_neither_gives_legacy_path() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + let saved_cc_home = std::env::var("COVEN_CODE_HOME").ok(); + std::env::remove_var("COVEN_CODE_HOME"); + let saved_coven_home = std::env::var("COVEN_HOME").ok(); + std::env::remove_var("COVEN_HOME"); + let saved_parent = std::env::var("COVEN_PARENT").ok(); + std::env::remove_var("COVEN_PARENT"); + + let got = config_home(); + let expected = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".coven-code"); + assert_eq!( + got, expected, + "with no coven signals, should be ~/.coven-code" + ); + + // Restore. + match saved_coven_home { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match saved_parent { + Some(v) => std::env::set_var("COVEN_PARENT", v), + None => std::env::remove_var("COVEN_PARENT"), + } + match saved_cc_home { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + /// An EMPTY `COVEN_HOME` must be treated as unset — NOT as a coven + /// signal that yields a relative `code` directory. Regression guard for + /// the empty-string edge case: with only `COVEN_HOME=""`, `config_home()` + /// must fall back to the legacy `~/.coven-code`, not `PathBuf::from("")`. + #[test] + fn config_home_empty_coven_home_is_treated_as_unset() { + let _lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + let saved_cc_home = std::env::var("COVEN_CODE_HOME").ok(); + std::env::remove_var("COVEN_CODE_HOME"); + let saved_coven_home = std::env::var("COVEN_HOME").ok(); + let saved_parent = std::env::var("COVEN_PARENT").ok(); + std::env::remove_var("COVEN_PARENT"); + + std::env::set_var("COVEN_HOME", ""); + + let got = config_home(); + let expected = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".coven-code"); + assert_eq!( + got, expected, + "empty COVEN_HOME must fall back to ~/.coven-code, not a relative path" + ); + assert!( + got.is_absolute() || got.starts_with("."), + "config_home() must not become a bare relative \"code\" dir" + ); + + // Restore. + match saved_coven_home { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match saved_parent { + Some(v) => std::env::set_var("COVEN_PARENT", v), + None => std::env::remove_var("COVEN_PARENT"), + } + match saved_cc_home { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + + // -- Phase 4.3: shared ~/.coven/settings.json layer tests ------------ + + /// Helper: hold both env locks and set COVEN_HOME + COVEN_CODE_HOME to + /// dedicated temp dirs, then run `f`. Restores env on drop. + struct SharedLayerGuard { + saved_coven_home: Option, + saved_coven_code_home: Option, + saved_test_home: Option, + _coven_tmp: tempfile::TempDir, + _code_tmp: tempfile::TempDir, + _lock: std::sync::MutexGuard<'static, ()>, + } + + impl Drop for SharedLayerGuard { + fn drop(&mut self) { + match self.saved_coven_home.take() { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match self.saved_coven_code_home.take() { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match self.saved_test_home.take() { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } + } + + /// Set up isolated temp dirs for both COVEN_HOME and COVEN_CODE_HOME and + /// return a guard that restores env on drop. + /// + /// `setup_coven(dir)` is called with the coven-home path so callers can + /// write shared files. `setup_code(dir)` is called with the engine-home + /// path so callers can write engine settings. + fn with_shared_layer(setup_coven: FC, setup_engine: FE) -> SharedLayerGuard + where + FC: FnOnce(&std::path::Path), + FE: FnOnce(&std::path::Path), + { + let lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + + let coven_tmp = tempfile::TempDir::new().unwrap(); + let code_tmp = tempfile::TempDir::new().unwrap(); + + setup_coven(coven_tmp.path()); + setup_engine(code_tmp.path()); + + let saved_coven_home = std::env::var("COVEN_HOME").ok(); + let saved_coven_code_home = std::env::var("COVEN_CODE_HOME").ok(); + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + + std::env::set_var("COVEN_HOME", coven_tmp.path()); + std::env::set_var("COVEN_CODE_HOME", code_tmp.path()); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + + SharedLayerGuard { + saved_coven_home, + saved_coven_code_home, + saved_test_home, + _coven_tmp: coven_tmp, + _code_tmp: code_tmp, + _lock: lock, + } + } + + /// 3-layer precedence: engine-global overrides shared; project overrides both. + /// + /// shared: model="shared-model" + /// engine: model="engine-model" → effective: "engine-model" + /// + /// Adding a project layer with model="proj-model" → effective: "proj-model". + #[test] + fn shared_layer_model_engine_wins_over_shared() { + let _g = with_shared_layer( + |coven_home| { + std::fs::write( + coven_home.join("settings.json"), + r#"{"model":"shared-model"}"#, + ) + .unwrap(); + }, + |code_home| { + std::fs::write( + code_home.join("settings.json"), + r#"{"config":{"model":"engine-model"}}"#, + ) + .unwrap(); + }, + ); + + let shared = SharedSettings::load(); + assert_eq!(shared.model.as_deref(), Some("shared-model")); + + let mut engine = Settings::default(); + engine.config.model = Some("engine-model".to_string()); + shared.apply_to(&mut engine); + // Engine value must not be overwritten. + assert_eq!(engine.config.model.as_deref(), Some("engine-model")); + } + + /// Shared fills a gap: shared sets theme; engine-global does NOT set theme. + /// Effective theme should come from the shared layer. + #[test] + fn shared_layer_theme_fills_engine_default() { + let _g = with_shared_layer( + |coven_home| { + std::fs::write(coven_home.join("settings.json"), r#"{"theme":"dark"}"#) + .unwrap(); + }, + |_code_home| { + // engine settings.json absent → engine stays at Theme::Default + }, + ); + + let shared = SharedSettings::load(); + assert_eq!(shared.theme.as_deref(), Some("dark")); + + let mut engine = Settings::default(); + // Engine theme is Default — shared should fill it in. + assert!(matches!(engine.config.theme, Theme::Default)); + shared.apply_to(&mut engine); + assert!(matches!(engine.config.theme, Theme::Dark)); + } + + /// Shared `permission_mode` fills the engine default and is parsed + /// through the camelCase enum representation. + #[test] + fn shared_layer_permission_mode_fills_engine_default() { + let _g = with_shared_layer( + |coven_home| { + std::fs::write( + coven_home.join("settings.json"), + r#"{"permission_mode":"acceptEdits"}"#, + ) + .unwrap(); + }, + |_code_home| {}, + ); + + let shared = SharedSettings::load(); + assert_eq!(shared.permission_mode.as_deref(), Some("acceptEdits")); + + let mut engine = Settings::default(); + assert!(matches!( + engine.config.permission_mode, + PermissionMode::Default + )); + shared.apply_to(&mut engine); + assert!(matches!( + engine.config.permission_mode, + PermissionMode::AcceptEdits + )); + } + + /// A malformed / non-JSON shared file is non-fatal: `load()` returns + /// all-`None` and leaves the engine settings untouched. + #[test] + fn shared_layer_malformed_file_is_non_fatal() { + let _g = with_shared_layer( + |coven_home| { + std::fs::write(coven_home.join("settings.json"), "this is not json {{{") + .unwrap(); + }, + |_code_home| {}, + ); + + // Must not panic; must degrade to defaults. + let shared = SharedSettings::load(); + assert!(shared.model.is_none()); + assert!(shared.theme.is_none()); + assert!(shared.permission_mode.is_none()); + + let mut engine = Settings::default(); + engine.config.model = Some("engine-model".to_string()); + shared.apply_to(&mut engine); + // Engine model preserved; malformed shared file changed nothing. + assert_eq!(engine.config.model.as_deref(), Some("engine-model")); + } + + /// Non-whitelisted keys in the shared file are silently ignored. + /// + /// We put an engine-only key (`mcp_servers` shaped as an object) in the + /// shared JSON. SharedSettings must parse successfully (unknown keys are + /// tolerated) and the engine Settings must not acquire any MCP servers. + #[test] + fn shared_layer_nonwhitelisted_key_is_ignored() { + let _g = with_shared_layer( + |coven_home| { + std::fs::write( + coven_home.join("settings.json"), + r#"{ + "model": "shared-model", + "mcp_servers": [{"name":"evil","command":"rm","args":["-rf","/"]}], + "hooks": {"PreToolUse": [{"command":"stolen"}]} + }"#, + ) + .unwrap(); + }, + |_code_home| {}, + ); + + let shared = SharedSettings::load(); + // Whitelisted key is present. + assert_eq!(shared.model.as_deref(), Some("shared-model")); + + let mut engine = Settings::default(); + shared.apply_to(&mut engine); + // Engine model was filled from shared. + assert_eq!(engine.config.model.as_deref(), Some("shared-model")); + // Non-whitelisted keys were NOT applied. + assert!( + engine.config.mcp_servers.is_empty(), + "mcp_servers must not leak from shared layer" + ); + assert!( + engine.config.hooks.is_empty(), + "hooks must not leak from shared layer" + ); + } + + /// No coven home → shared layer is absent → load_hierarchical behaves + /// as before (engine settings are used as-is; project can still override). + #[test] + fn shared_layer_absent_when_no_coven_home() { + // Use COVEN_CODE_HOME for engine isolation but keep COVEN_HOME unset + // (no coven home → SharedSettings::load() returns Default). + let lock = CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + + let saved_coven = std::env::var("COVEN_HOME").ok(); + let saved_code = std::env::var("COVEN_CODE_HOME").ok(); + let saved_test = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::remove_var("COVEN_HOME"); + std::env::remove_var("COVEN_CODE_TEST_HOME"); + + let code_tmp = tempfile::TempDir::new().unwrap(); + std::env::set_var("COVEN_CODE_HOME", code_tmp.path()); + std::fs::write( + code_tmp.path().join("settings.json"), + r#"{"config":{"model":"engine-only-model"}}"#, + ) + .unwrap(); + + // SharedSettings::load() must return all-None when there is no + // directory at the COVEN_HOME path. + let shared = SharedSettings::load(); + assert!(shared.model.is_none(), "no coven home → no shared model"); + assert!(shared.theme.is_none(), "no coven home → no shared theme"); + assert!( + shared.permission_mode.is_none(), + "no coven home → no shared permission_mode" + ); + + // apply_to on a settings with engine model must be a no-op. + let mut engine = Settings::default(); + engine.config.model = Some("engine-only-model".to_string()); + shared.apply_to(&mut engine); + assert_eq!( + engine.config.model.as_deref(), + Some("engine-only-model"), + "engine model must be unchanged when shared layer is absent" + ); + + // Restore. + drop(lock); + match saved_coven { + Some(v) => std::env::set_var("COVEN_HOME", v), + None => std::env::remove_var("COVEN_HOME"), + } + match saved_code { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + match saved_test { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + } } } @@ -4240,10 +4976,7 @@ pub mod oauth { /// Legacy token file path — kept for backward-compat reads when no /// account registry exists yet. New writes go to per-account dirs. pub fn token_file_path() -> std::path::PathBuf { - dirs::home_dir() - .unwrap_or_else(|| std::path::PathBuf::from(".")) - .join(".coven-code") - .join("oauth_tokens.json") + crate::config::config_home().join("oauth_tokens.json") } /// Save tokens for a specific account profile under diff --git a/src-rust/crates/core/src/memdir.rs b/src-rust/crates/core/src/memdir.rs index 11810f28..93720d7c 100644 --- a/src-rust/crates/core/src/memdir.rs +++ b/src-rust/crates/core/src/memdir.rs @@ -351,11 +351,7 @@ pub fn auto_memory_path(project_root: &Path) -> PathBuf { // 2. Determine the memory base directory. let memory_base = std::env::var("COVEN_CODE_REMOTE_MEMORY_DIR") .map(PathBuf::from) - .unwrap_or_else(|_| { - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".coven-code") - }); + .unwrap_or_else(|_| crate::config::config_home()); // 3. Sanitize the project root into a safe directory name. let sanitized = sanitize_path_component(&project_root.to_string_lossy()); @@ -1390,4 +1386,31 @@ mod tests { assert!(content.contains("[REDACTED: operator request]")); assert!(!content.contains("secret incident detail")); } + + #[test] + fn auto_memory_path_derives_from_config_home_when_no_env_override() { + let _lock = crate::config::CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + // Ensure the env override is not set. + let prev = std::env::var("COVEN_CODE_REMOTE_MEMORY_DIR").ok(); + std::env::remove_var("COVEN_CODE_REMOTE_MEMORY_DIR"); + // Also clear the other override so we don't short-circuit. + let prev2 = std::env::var("CLAUDE_COWORK_MEMORY_PATH_OVERRIDE").ok(); + std::env::remove_var("CLAUDE_COWORK_MEMORY_PATH_OVERRIDE"); + + let path = auto_memory_path(std::path::Path::new("/some/project")); + assert!( + path.starts_with(crate::config::config_home()), + "auto_memory_path {path:?} should start with config_home() when no env override is set" + ); + + // Restore env vars. + if let Some(v) = prev { + std::env::set_var("COVEN_CODE_REMOTE_MEMORY_DIR", v); + } + if let Some(v) = prev2 { + std::env::set_var("CLAUDE_COWORK_MEMORY_PATH_OVERRIDE", v); + } + } } diff --git a/src-rust/crates/core/src/oauth_config.rs b/src-rust/crates/core/src/oauth_config.rs index e80e3b52..50b0cc65 100644 --- a/src-rust/crates/core/src/oauth_config.rs +++ b/src-rust/crates/core/src/oauth_config.rs @@ -308,8 +308,8 @@ pub struct CodexTokens { /// Legacy single-file path: `~/.coven-code/codex_tokens.json`. Kept for /// backward-compat reads when no account registry exists. -fn codex_tokens_path() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("codex_tokens.json")) +fn codex_tokens_path() -> std::path::PathBuf { + crate::config::config_home().join("codex_tokens.json") } /// Save Codex OAuth tokens for a named profile under @@ -412,7 +412,7 @@ pub fn get_codex_tokens() -> Option { } } // Legacy fallback + migration. - let legacy = codex_tokens_path()?; + let legacy = codex_tokens_path(); if !legacy.exists() { return None; } @@ -434,7 +434,8 @@ pub fn clear_codex_tokens() -> anyhow::Result<()> { { registry.remove(crate::accounts::PROVIDER_CODEX, &active)?; } - if let Some(legacy) = codex_tokens_path() { + { + let legacy = codex_tokens_path(); if legacy.exists() { std::fs::remove_file(&legacy)?; } @@ -517,4 +518,17 @@ mod tests { assert!(url.contains("S256")); assert!(url.contains("localhost")); } + + #[test] + fn codex_tokens_path_derives_from_config_home() { + let _lock = crate::config::CONFIG_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|err| err.into_inner()); + let path = codex_tokens_path(); + assert!( + path.starts_with(crate::config::config_home()), + "codex_tokens_path {path:?} should start with config_home()" + ); + assert_eq!(path.file_name().unwrap(), "codex_tokens.json"); + } } diff --git a/src-rust/crates/core/src/prompt_history.rs b/src-rust/crates/core/src/prompt_history.rs index d08e3385..9ce07aba 100644 --- a/src-rust/crates/core/src/prompt_history.rs +++ b/src-rust/crates/core/src/prompt_history.rs @@ -137,9 +137,7 @@ static STATE: once_cell::sync::Lazy> = // --------------------------------------------------------------------------- fn claude_home() -> PathBuf { - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".coven-code") + crate::config::config_home() } fn history_path() -> PathBuf { @@ -704,6 +702,17 @@ pub fn format_image_ref(id: u32) -> String { format!("[Image #{}]", id) } +// --------------------------------------------------------------------------- +// Test accessors +// --------------------------------------------------------------------------- + +/// Expose the history base path for cross-module consolidation tests +/// (Phase 4.1). +#[cfg(test)] +pub(crate) fn history_base_path_for_test() -> std::path::PathBuf { + claude_home() +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- diff --git a/src-rust/crates/core/src/remote_settings.rs b/src-rust/crates/core/src/remote_settings.rs index b5c281e8..5f7d1ae1 100644 --- a/src-rust/crates/core/src/remote_settings.rs +++ b/src-rust/crates/core/src/remote_settings.rs @@ -392,9 +392,7 @@ pub fn merge_remote_into_local(local: &Value, remote: &Value) -> Value { /// Return the ~/.coven-code directory, falling back to the current directory. fn claude_config_dir() -> PathBuf { - dirs::home_dir() - .map(|h| h.join(".coven-code")) - .unwrap_or_else(|| PathBuf::from(".coven-code")) + crate::config::config_home() } /// Exponential backoff delay for retry attempt `n` (1-indexed). @@ -405,6 +403,17 @@ fn retry_delay(attempt: u32) -> Duration { Duration::from_secs(secs) } +// --------------------------------------------------------------------------- +// Test accessors +// --------------------------------------------------------------------------- + +/// Expose the remote settings directory for cross-module consolidation tests +/// (Phase 4.1). +#[cfg(test)] +pub(crate) fn remote_settings_dir_for_test() -> PathBuf { + claude_config_dir() +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- diff --git a/src-rust/crates/core/src/roster_reset.rs b/src-rust/crates/core/src/roster_reset.rs index 2324b2eb..642496cf 100644 --- a/src-rust/crates/core/src/roster_reset.rs +++ b/src-rust/crates/core/src/roster_reset.rs @@ -52,7 +52,10 @@ pub fn reset_familiars_and_agents( let mut agent_dirs = vec![Settings::config_dir().join("agents")]; if let Some(root) = project_root { - agent_dirs.push(root.join(".coven-code").join("agents")); + agent_dirs.push( + root.join(crate::config::PROJECT_CONFIG_DIRNAME) + .join("agents"), + ); } for dir in agent_dirs { summary.removed_agent_files += remove_agent_markdown_files(&dir)?; diff --git a/src-rust/crates/core/src/session_storage.rs b/src-rust/crates/core/src/session_storage.rs index 1ac5f121..42c473e4 100644 --- a/src-rust/crates/core/src/session_storage.rs +++ b/src-rust/crates/core/src/session_storage.rs @@ -790,9 +790,19 @@ mod tests { #[test] fn list_sessions_returns_sorted() { + // `transcript_dir` routes through `config_home()`, so the engine home + // MUST be isolated to a temp dir — otherwise this test writes + // transcripts into the developer's real `~/.coven-code/projects/`. + // `COVEN_CODE_TEST_HOME` is honored by `config_home()` in test builds + // and resolves to `/.coven-code`. Hold the env lock while it is set. let _lock = crate::coven_shared::COVEN_HOME_ENV_LOCK .lock() .unwrap_or_else(|err| err.into_inner()); + + let home_tmp = tempdir().unwrap(); + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::set_var("COVEN_CODE_TEST_HOME", home_tmp.path()); + let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -802,7 +812,14 @@ mod tests { let project_root = tmp.path().join("myproject"); tokio::fs::create_dir_all(&project_root).await.unwrap(); + // Assert isolation: the resolved transcript dir lives under the + // temp home, never under the real `~/.coven-code`. let tdir = transcript_dir(&project_root); + assert!( + tdir.starts_with(home_tmp.path()), + "transcript_dir must be isolated to the temp home; got {:?}", + tdir + ); tokio::fs::create_dir_all(&tdir).await.unwrap(); for id in ["aaaa", "bbbb"] { @@ -821,6 +838,11 @@ mod tests { assert_eq!(sessions[0].session_id, "bbbb"); assert_eq!(sessions[1].session_id, "aaaa"); }); + + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } } #[test] diff --git a/src-rust/crates/core/src/settings_sync.rs b/src-rust/crates/core/src/settings_sync.rs index 9fe65e46..6095c33a 100644 --- a/src-rust/crates/core/src/settings_sync.rs +++ b/src-rust/crates/core/src/settings_sync.rs @@ -510,9 +510,7 @@ async fn write_file_for_sync(path: &PathBuf, content: &str) -> Result<()> { /// Return the ~/.coven-code directory. fn claude_config_dir() -> PathBuf { - dirs::home_dir() - .map(|h| h.join(".coven-code")) - .unwrap_or_else(|| PathBuf::from(".coven-code")) + crate::config::config_home() } /// Exponential backoff delay for retry attempt `n` (1-indexed), capped at 30 s. diff --git a/src-rust/crates/core/src/skill_discovery.rs b/src-rust/crates/core/src/skill_discovery.rs index c2bc791b..07fd2086 100644 --- a/src-rust/crates/core/src/skill_discovery.rs +++ b/src-rust/crates/core/src/skill_discovery.rs @@ -353,7 +353,8 @@ pub fn discover_skills( break; } add(scan_skill_root( - &dir.join(".coven-code").join("skills"), + &dir.join(crate::config::PROJECT_CONFIG_DIRNAME) + .join("skills"), SkillScope::Project, "coven", )); @@ -377,7 +378,7 @@ pub fn discover_skills( // ---- 2. User skills: home directory ------------------------------------- if let Some(home) = home { add(scan_skill_root( - &home.join(".coven-code").join("skills"), + &crate::config::config_home().join("skills"), SkillScope::User, "coven", )); diff --git a/src-rust/crates/core/src/tips.rs b/src-rust/crates/core/src/tips.rs index 0ed43f8a..14459d7e 100644 --- a/src-rust/crates/core/src/tips.rs +++ b/src-rust/crates/core/src/tips.rs @@ -9,7 +9,8 @@ // a `cooldown_sessions` field — the tip won't be shown again until that many // sessions have passed since the last display. // -// History is persisted to `~/.coven-code/tip_history.json`. +// History is persisted to `config_home()/tip_history.json` (legacy +// `~/.coven-code/`, or `~/.coven/code/` under the unified coven CLI). use std::collections::HashMap; @@ -189,19 +190,17 @@ pub struct TipHistory { } impl TipHistory { - /// Path to the persisted history file: `~/.coven-code/tip_history.json`. - fn history_path() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("tip_history.json")) + /// Path to the persisted history file: `config_home()/tip_history.json` + /// (legacy `~/.coven-code/`, or `~/.coven/code/` under the unified coven CLI). + fn history_path() -> std::path::PathBuf { + crate::config::config_home().join("tip_history.json") } - /// Load history from `~/.coven-code/tip_history.json`. + /// Load history from `config_home()/tip_history.json`. /// Returns an empty `TipHistory` if the file does not exist or cannot be /// parsed. pub fn load() -> Self { - let path = match Self::history_path() { - Some(p) => p, - None => return Self::default(), - }; + let path = Self::history_path(); match std::fs::read_to_string(&path) { Ok(contents) => serde_json::from_str(&contents).unwrap_or_default(), Err(_) => Self::default(), @@ -211,10 +210,7 @@ impl TipHistory { /// Persist history to `~/.coven-code/tip_history.json`. /// Silently ignores I/O errors (tips are non-critical). pub fn save(&self) { - let path = match Self::history_path() { - Some(p) => p, - None => return, - }; + let path = Self::history_path(); // Ensure the parent directory exists. if let Some(parent) = path.parent() { let _ = std::fs::create_dir_all(parent); @@ -380,6 +376,17 @@ mod tests { #[test] fn select_tip_respects_cooldown() { + // `TipHistory::save()` writes to `config_home()/tip_history.json`, so the + // engine home MUST be isolated to a temp dir — otherwise this test writes + // into the developer's real `~/.coven-code/`. `COVEN_CODE_TEST_HOME` is + // honored by `config_home()` in test builds. Hold the env lock while set. + let _lock = crate::coven_shared::COVEN_HOME_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let home_tmp = tempfile::tempdir().unwrap(); + let saved_test_home = std::env::var("COVEN_CODE_TEST_HOME").ok(); + std::env::set_var("COVEN_CODE_TEST_HOME", home_tmp.path()); + // Record all tips as shown in session 1000, then ask for session 1001. // Tips with cooldown > 1 should not be returned. let mut history = TipHistory::default(); @@ -388,6 +395,11 @@ mod tests { } history.save(); + match saved_test_home { + Some(v) => std::env::set_var("COVEN_CODE_TEST_HOME", v), + None => std::env::remove_var("COVEN_CODE_TEST_HOME"), + } + // Session 1001 — only tips with cooldown ≤ 1 are eligible. // Since all our built-in tips have cooldown ≥ 3, nothing should be // returned right away. diff --git a/src-rust/crates/mcp/Cargo.toml b/src-rust/crates/mcp/Cargo.toml index 2ad6df4a..6e88e3fc 100644 --- a/src-rust/crates/mcp/Cargo.toml +++ b/src-rust/crates/mcp/Cargo.toml @@ -26,3 +26,6 @@ chrono = { workspace = true } open = { workspace = true } urlencoding = { workspace = true } rmcp = { version = "1.4.0", default-features = false, features = ["client", "auth", "transport-child-process", "transport-streamable-http-client-reqwest", "reqwest-native-tls"] } + +[dev-dependencies] +tempfile = { workspace = true } diff --git a/src-rust/crates/mcp/src/lib.rs b/src-rust/crates/mcp/src/lib.rs index da847b71..15df02cc 100644 --- a/src-rust/crates/mcp/src/lib.rs +++ b/src-rust/crates/mcp/src/lib.rs @@ -1636,6 +1636,9 @@ mod tests { #[test] fn test_auth_state_uses_token_expiry_datetime() { + // Isolate the token store to a temp home so this never reads or writes + // the developer's real `~/.coven-code/mcp-tokens/`. + let _home = crate::oauth::test_support::TokenHomeGuard::new(); let mut mgr = McpManager::new(); mgr.server_configs.insert( "remote".to_string(), diff --git a/src-rust/crates/mcp/src/oauth.rs b/src-rust/crates/mcp/src/oauth.rs index 19323ebc..8b6e938a 100644 --- a/src-rust/crates/mcp/src/oauth.rs +++ b/src-rust/crates/mcp/src/oauth.rs @@ -50,10 +50,14 @@ impl McpToken { } /// Path to the token store for a given MCP server. +/// +/// Routed through `config_home()` so MCP tokens live in the same engine home +/// as all other state and migrate together under the unified coven CLI. (This +/// previously constructed `~/.coven-code/mcp-tokens` directly, which would have +/// stranded tokens in the legacy dir after relocation.) fn token_path(server_name: &str) -> PathBuf { - dirs::home_dir() - .unwrap_or_default() - .join(".coven-code/mcp-tokens") + claurst_core::config::config_home() + .join("mcp-tokens") .join(format!("{}.json", server_name)) } @@ -592,8 +596,53 @@ pub async fn refresh_mcp_token( Ok(new_token) } +/// Test-only helpers shared across this crate's test modules. +#[cfg(test)] +pub(crate) mod test_support { + /// Serializes tests that write token files (which land under + /// `config_home()`), pointing `config_home()` at a temp dir so they never + /// touch the developer's real `~/.coven-code/mcp-tokens/`. + /// + /// Uses `COVEN_CODE_HOME` (honored in all builds), not the + /// `#[cfg(test)]`-gated `COVEN_CODE_TEST_HOME`, which is compiled out when + /// `claurst_core` is a plain dependency of this crate's test binary. + static TOKEN_HOME_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// RAII guard that redirects `config_home()` at a fresh temp dir for the + /// duration of a test and restores the previous `COVEN_CODE_HOME` on drop. + pub(crate) struct TokenHomeGuard { + _tmp: tempfile::TempDir, + prev: Option, + _lock: std::sync::MutexGuard<'static, ()>, + } + + impl TokenHomeGuard { + pub(crate) fn new() -> Self { + let lock = TOKEN_HOME_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let tmp = tempfile::tempdir().expect("tempdir"); + let prev = std::env::var("COVEN_CODE_HOME").ok(); + std::env::set_var("COVEN_CODE_HOME", tmp.path()); + Self { + _tmp: tmp, + prev, + _lock: lock, + } + } + } + + impl Drop for TokenHomeGuard { + fn drop(&mut self) { + match self.prev.take() { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + } + } +} + #[cfg(test)] mod tests { + use super::test_support::TokenHomeGuard; use super::*; #[test] @@ -647,6 +696,7 @@ mod tests { #[tokio::test] async fn refresh_reports_unbound_legacy_token_binding() { + let _home = TokenHomeGuard::new(); let server_name = format!("legacy-unbound-{}", std::process::id()); let _ = remove_mcp_token(&server_name); let token = McpToken { diff --git a/src-rust/crates/plugins/src/lib.rs b/src-rust/crates/plugins/src/lib.rs index 1068f236..5f6dae81 100644 --- a/src-rust/crates/plugins/src/lib.rs +++ b/src-rust/crates/plugins/src/lib.rs @@ -205,9 +205,7 @@ pub async fn load_plugins( let mut search_dirs: Vec = Vec::new(); // 1. User-global plugins directory. - if let Some(user_dir) = default_user_plugins_dir() { - search_dirs.push(user_dir); - } + search_dirs.push(default_user_plugins_dir()); // 2. Project-local plugins directory. search_dirs.push(project_plugins_dir(project_dir)); @@ -216,7 +214,8 @@ pub async fn load_plugins( search_dirs.extend_from_slice(extra_paths); // User plugins. - if let Some(user_dir) = default_user_plugins_dir() { + { + let user_dir = default_user_plugins_dir(); let (plugins, errors) = discover_plugins(&[user_dir], PluginSource::User).await; registry.extend(plugins, errors); } @@ -499,15 +498,7 @@ pub fn install_plugin_from_path(source_path: &Path) -> Result d.join(&plugin_name), - None => { - return Err(PluginError::Io { - path: String::new(), - message: "Cannot determine home directory for plugin installation".to_string(), - }) - } - }; + let dest = default_user_plugins_dir().join(&plugin_name); // Copy source to dest. copy_dir_all(source_path, &dest).map_err(|e| PluginError::Io { diff --git a/src-rust/crates/plugins/src/loader.rs b/src-rust/crates/plugins/src/loader.rs index 4a892b94..6c1c30d5 100644 --- a/src-rust/crates/plugins/src/loader.rs +++ b/src-rust/crates/plugins/src/loader.rs @@ -16,9 +16,11 @@ use std::path::{Path, PathBuf}; // Public helpers // --------------------------------------------------------------------------- -/// Return the default user-level plugins directory: `~/.coven-code/plugins`. -pub fn default_user_plugins_dir() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("plugins")) +/// Return the default user-level plugins directory: `config_home()/plugins` +/// (legacy `~/.coven-code/plugins`, or `~/.coven/code/plugins` under the +/// unified coven CLI). +pub fn default_user_plugins_dir() -> PathBuf { + claurst_core::config::config_home().join("plugins") } /// Return the project-level plugins directory: `/.coven-code/plugins`. @@ -345,3 +347,18 @@ fn extract_description_from_markdown_file(path: &Path) -> Option { } None } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_user_plugins_dir_derives_from_config_home() { + let dir = default_user_plugins_dir(); + assert!( + dir.starts_with(claurst_core::config::config_home()), + "default_user_plugins_dir {dir:?} should start with config_home()" + ); + assert_eq!(dir.file_name().unwrap(), "plugins"); + } +} diff --git a/src-rust/crates/plugins/src/marketplace.rs b/src-rust/crates/plugins/src/marketplace.rs index 09c13139..d23ba167 100644 --- a/src-rust/crates/plugins/src/marketplace.rs +++ b/src-rust/crates/plugins/src/marketplace.rs @@ -208,9 +208,7 @@ pub async fn marketplace_update(name: &str) -> Result, String> { /// List all installed plugins. pub fn list_installed() -> Vec { - let plugins_dir = dirs::home_dir() - .map(|h| h.join(".coven-code").join("plugins")) - .unwrap_or_default(); + let plugins_dir = claurst_core::config::config_home().join("plugins"); let Ok(entries) = std::fs::read_dir(&plugins_dir) else { return Vec::new(); @@ -268,9 +266,7 @@ pub fn marketplace_uninstall(name: &str) -> Result<(), String> { } fn plugin_install_dir(name: &str) -> std::path::PathBuf { - dirs::home_dir() - .unwrap_or_default() - .join(".coven-code") + claurst_core::config::config_home() .join("plugins") .join(name) } diff --git a/src-rust/crates/query/src/lib.rs b/src-rust/crates/query/src/lib.rs index 466901ca..e7895c53 100644 --- a/src-rust/crates/query/src/lib.rs +++ b/src-rust/crates/query/src/lib.rs @@ -2155,11 +2155,12 @@ pub async fn run_query_loop( // the spawn doesn't call run_query_loop recursively from within // its own future (which would make the future !Send). { - let memory_dir = dirs::home_dir().map(|h| h.join(".coven-code").join("memory")); + let memory_dir = claurst_core::config::config_home().join("memory"); let conversations_dir = - dirs::home_dir().map(|h| h.join(".coven-code").join("conversations")); - if let (Some(mem), Some(conv)) = (memory_dir, conversations_dir) { - let dreamer = crate::auto_dream::AutoDream::new(mem, conv); + claurst_core::config::config_home().join("conversations"); + { + let dreamer = + crate::auto_dream::AutoDream::new(memory_dir, conversations_dir); if let Ok(Some(task)) = dreamer.maybe_trigger().await { // Run the consolidation subagent in a background Tokio // task. We use the AgentTool execute path (via diff --git a/src-rust/crates/query/src/skill_prefetch.rs b/src-rust/crates/query/src/skill_prefetch.rs index f35dfb65..ba109090 100644 --- a/src-rust/crates/query/src/skill_prefetch.rs +++ b/src-rust/crates/query/src/skill_prefetch.rs @@ -80,14 +80,10 @@ pub async fn prefetch_skills(project_root: &Path, index: SharedSkillIndex) { .unwrap_or_default(); // 1. User-defined skills: ~/.coven-code/skills/*.md + {project_root}/.coven-code/skills/*.md - let search_dirs: Vec = { - let mut dirs = Vec::new(); - if let Some(home) = dirs::home_dir() { - dirs.push(home.join(".coven-code").join("skills")); - } - dirs.push(project_root.join(".coven-code").join("skills")); - dirs - }; + let search_dirs: Vec = vec![ + claurst_core::config::config_home().join("skills"), + project_root.join(".coven-code").join("skills"), + ]; for dir in &search_dirs { if let Ok(entries) = std::fs::read_dir(dir) { diff --git a/src-rust/crates/tools/Cargo.toml b/src-rust/crates/tools/Cargo.toml index 9d1a43db..93f468e5 100644 --- a/src-rust/crates/tools/Cargo.toml +++ b/src-rust/crates/tools/Cargo.toml @@ -35,5 +35,8 @@ portable-pty = "0.9" [target.'cfg(unix)'.dependencies] nix = { workspace = true } +[dev-dependencies] +tempfile = { workspace = true } + [features] computer-use = ["dep:enigo", "dep:xcap", "dep:image"] diff --git a/src-rust/crates/tools/src/cron.rs b/src-rust/crates/tools/src/cron.rs index b8e1e4b1..1dd65a4f 100644 --- a/src-rust/crates/tools/src/cron.rs +++ b/src-rust/crates/tools/src/cron.rs @@ -56,9 +56,10 @@ static CRON_STORE: Lazy>>> = // Disk path helpers // --------------------------------------------------------------------------- -/// Path to `~/.coven-code/scheduled_tasks.json`. -fn scheduled_tasks_path() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("scheduled_tasks.json")) +/// Path to `config_home()/scheduled_tasks.json` (legacy `~/.coven-code/`, or +/// `~/.coven/code/` under the unified coven CLI). +fn scheduled_tasks_path() -> PathBuf { + claurst_core::config::config_home().join("scheduled_tasks.json") } /// Ensure the store has been loaded from disk (once per process). @@ -70,10 +71,7 @@ async fn ensure_store_loaded() { *init = true; // Load from ~/.coven-code/scheduled_tasks.json if it exists. - let path = match scheduled_tasks_path() { - Some(p) => p, - None => return, - }; + let path = scheduled_tasks_path(); let data = match tokio::fs::read_to_string(&path).await { Ok(d) => d, @@ -510,8 +508,7 @@ async fn persist_tasks_to_disk(store: &HashMap) -> Result<(), let durable: Vec<&CronTask> = store.values().filter(|t| t.durable).collect(); let json = serde_json::to_string_pretty(&durable).map_err(|e| e.to_string())?; - let path = - scheduled_tasks_path().ok_or_else(|| "Cannot determine home directory".to_string())?; + let path = scheduled_tasks_path(); let dir = path.parent().ok_or("No parent directory")?; tokio::fs::create_dir_all(dir) diff --git a/src-rust/crates/tools/src/skill_tool.rs b/src-rust/crates/tools/src/skill_tool.rs index 5582f216..0729baf2 100644 --- a/src-rust/crates/tools/src/skill_tool.rs +++ b/src-rust/crates/tools/src/skill_tool.rs @@ -147,9 +147,7 @@ impl Tool for SkillTool { fn skill_search_dirs(ctx: &ToolContext) -> Vec { let mut dirs = vec![ctx.working_dir.join(".coven-code").join("commands")]; - if let Some(home) = dirs::home_dir() { - dirs.push(home.join(".coven-code").join("commands")); - } + dirs.push(claurst_core::config::config_home().join("commands")); dirs } diff --git a/src-rust/crates/tools/src/team_tool.rs b/src-rust/crates/tools/src/team_tool.rs index dd4ae16a..f49ad1c1 100644 --- a/src-rust/crates/tools/src/team_tool.rs +++ b/src-rust/crates/tools/src/team_tool.rs @@ -105,12 +105,12 @@ static ACTIVE_TEAMS: Lazy>> = Lazy::new(D // Shared helpers // --------------------------------------------------------------------------- -fn teams_base_dir() -> Option { - dirs::home_dir().map(|h| h.join(".coven-code").join("teams")) +fn teams_base_dir() -> std::path::PathBuf { + claurst_core::config::config_home().join("teams") } -fn team_dir(team_name: &str) -> Option { - teams_base_dir().map(|b| b.join(sanitize_name(team_name))) +fn team_dir(team_name: &str) -> std::path::PathBuf { + teams_base_dir().join(sanitize_name(team_name)) } /// Sanitize a team name to a safe directory component. @@ -282,18 +282,12 @@ impl Tool for TeamCreateTool { let lead_agent_id = format!("team-lead@{}", safe_name); // Resolve team directory, disambiguating if name already exists. - let dir = match team_dir(¶ms.team_name) { - Some(d) => d, - None => return ToolResult::error("Could not determine home directory".to_string()), - }; + let dir = team_dir(¶ms.team_name); let (final_name, final_dir) = if dir.exists() { let suffix = &Uuid::new_v4().to_string()[..6]; let new_name = format!("{}-{}", safe_name, suffix); - let new_dir = match team_dir(&new_name) { - Some(d) => d, - None => return ToolResult::error("Could not determine home directory".to_string()), - }; + let new_dir = team_dir(&new_name); (new_name, new_dir) } else { (safe_name.clone(), dir) @@ -537,10 +531,7 @@ impl Tool for TeamDeleteTool { }; // Remove the team directory from disk. - let dir = match team_dir(¶ms.team_name) { - Some(d) => d, - None => return ToolResult::error("Could not determine home directory".to_string()), - }; + let dir = team_dir(¶ms.team_name); if !dir.exists() { // Directory already gone — treat as success if we cancelled agents, diff --git a/src-rust/crates/tools/src/todo_write.rs b/src-rust/crates/tools/src/todo_write.rs index 55a285aa..c907382a 100644 --- a/src-rust/crates/tools/src/todo_write.rs +++ b/src-rust/crates/tools/src/todo_write.rs @@ -13,9 +13,7 @@ use tracing::debug; /// Returns the path to the persisted todo list for `session_id`. pub fn todos_path(session_id: &str) -> PathBuf { - dirs::home_dir() - .unwrap_or_default() - .join(".coven-code") + claurst_core::config::config_home() .join("todos") .join(format!("{}.json", session_id)) } @@ -30,7 +28,8 @@ pub fn load_todos(session_id: &str) -> Vec { .unwrap_or_default() } -/// Persist `todos` to `~/.coven-code/todos/.json`. +/// Persist `todos` to `config_home()/todos/.json` (legacy +/// `~/.coven-code/`, or `~/.coven/code/` under the unified coven CLI). pub fn save_todos(session_id: &str, todos: &[Value]) { let path = todos_path(session_id); if let Some(parent) = path.parent() { @@ -339,17 +338,61 @@ impl Tool for TodoWriteTool { mod tests { use super::*; + /// Serializes tests that mutate `COVEN_CODE_HOME` (which steers + /// `config_home()`), so the todo-persistence tests never write into the + /// developer's real `~/.coven-code/` and never race each other's env. + /// + /// NOTE: we use `COVEN_CODE_HOME`, not `COVEN_CODE_TEST_HOME`. The latter is + /// `#[cfg(test)]`-gated inside `claurst_core`, so it is compiled OUT when + /// `claurst_core` is built as a normal dependency of this crate's test + /// binary. `COVEN_CODE_HOME` is honored in all builds. + static TEST_HOME_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// RAII guard: point `config_home()` at a fresh temp dir for the duration + /// of a test, restoring the previous `COVEN_CODE_HOME` on drop. + struct TodoHomeGuard { + _tmp: tempfile::TempDir, + prev: Option, + _lock: std::sync::MutexGuard<'static, ()>, + } + + impl TodoHomeGuard { + fn new() -> Self { + let lock = TEST_HOME_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let tmp = tempfile::tempdir().expect("tempdir"); + let prev = std::env::var("COVEN_CODE_HOME").ok(); + std::env::set_var("COVEN_CODE_HOME", tmp.path()); + Self { + _tmp: tmp, + prev, + _lock: lock, + } + } + } + + impl Drop for TodoHomeGuard { + fn drop(&mut self) { + match self.prev.take() { + Some(v) => std::env::set_var("COVEN_CODE_HOME", v), + None => std::env::remove_var("COVEN_CODE_HOME"), + } + } + } + #[test] fn test_todos_path_contains_session_id() { + let _g = TodoHomeGuard::new(); let path = todos_path("my-session-123"); let path_str = path.to_string_lossy(); assert!( path_str.contains("my-session-123"), "todos_path should embed the session id" ); + // Derive the base from config_home() so the assertion holds under any + // home layout (legacy .coven-code OR unified ~/.coven/code). assert!( - path_str.contains(".coven-code"), - "todos_path should be under ~/.coven-code" + path.starts_with(claurst_core::config::config_home().join("todos")), + "todos_path should live under config_home()/todos; got {path_str}" ); assert!( path_str.ends_with(".json"), @@ -359,12 +402,14 @@ mod tests { #[test] fn test_load_todos_missing_file_returns_empty() { + let _g = TodoHomeGuard::new(); let todos = load_todos("nonexistent-session-zzzzzz-99999"); assert!(todos.is_empty(), "Missing file should yield empty vec"); } #[test] fn test_save_and_load_roundtrip() { + let _g = TodoHomeGuard::new(); let session_id = format!( "test-session-{}", std::time::SystemTime::now() @@ -381,8 +426,7 @@ mod tests { assert_eq!(loaded.len(), 2); assert_eq!(loaded[0]["id"].as_str(), Some("1")); assert_eq!(loaded[1]["status"].as_str(), Some("completed")); - // Clean up. - let _ = std::fs::remove_file(todos_path(&session_id)); + // Temp home is cleaned up when the guard drops. } // --- Status parsing ------------------------------------------------------ diff --git a/src-rust/crates/tui/src/agents_view.rs b/src-rust/crates/tui/src/agents_view.rs index e84b7db8..94ec1c94 100644 --- a/src-rust/crates/tui/src/agents_view.rs +++ b/src-rust/crates/tui/src/agents_view.rs @@ -533,8 +533,12 @@ impl Default for AgentsMenuState { pub fn load_agent_definitions(project_root: &std::path::Path) -> Vec { let mut defs = Vec::new(); let dirs = [ - dirs::home_dir().map(|h| h.join(".coven-code").join("agents")), - Some(project_root.join(".coven-code").join("agents")), + Some(claurst_core::config::config_home().join("agents")), + Some( + project_root + .join(claurst_core::config::PROJECT_CONFIG_DIRNAME) + .join("agents"), + ), ]; for dir_opt in &dirs { diff --git a/src-rust/crates/tui/src/familiar_image.rs b/src-rust/crates/tui/src/familiar_image.rs index 05443a70..3a8f8bbb 100644 --- a/src-rust/crates/tui/src/familiar_image.rs +++ b/src-rust/crates/tui/src/familiar_image.rs @@ -32,7 +32,11 @@ pub fn familiar_image_path(familiar_id: &str) -> Option { let extensions = ["png", "jpg", "jpeg", "webp"]; let search_dirs: Vec = [ dirs::home_dir().map(|h| h.join(".coven").join("assets").join("familiars")), - dirs::home_dir().map(|h| h.join(".coven-code").join("assets").join("familiars")), + Some( + claurst_core::config::config_home() + .join("assets") + .join("familiars"), + ), ] .into_iter() .flatten() diff --git a/src-rust/crates/tui/src/overlays.rs b/src-rust/crates/tui/src/overlays.rs index 984d7b73..55beb8f4 100644 --- a/src-rust/crates/tui/src/overlays.rs +++ b/src-rust/crates/tui/src/overlays.rs @@ -632,17 +632,15 @@ impl HistoryEntry { } // --------------------------------------------------------------------------- -// Pinned-entry persistence (~/.coven-code/history_pins.json) +// Pinned-entry persistence (config_home()/history_pins.json) // --------------------------------------------------------------------------- fn pins_path() -> std::path::PathBuf { - dirs::home_dir() - .unwrap_or_else(|| std::path::PathBuf::from(".")) - .join(".coven-code") - .join("history_pins.json") + claurst_core::config::config_home().join("history_pins.json") } -/// Load the set of pinned entry texts from `~/.coven-code/history_pins.json`. +/// Load the set of pinned entry texts from `config_home()/history_pins.json` +/// (legacy `~/.coven-code/`, or `~/.coven/code/` under the unified coven CLI). /// Returns an empty set if the file does not exist or cannot be parsed. pub fn load_pinned_texts() -> std::collections::HashSet { let path = pins_path(); diff --git a/src-rust/crates/tui/src/render.rs b/src-rust/crates/tui/src/render.rs index 934829ff..a629bf43 100644 --- a/src-rust/crates/tui/src/render.rs +++ b/src-rust/crates/tui/src/render.rs @@ -3515,6 +3515,25 @@ mod welcome_tests { app } + /// Build a test app whose familiar roster is isolated to an EMPTY temp + /// `COVEN_HOME`, so `streaming_status_label` and friends never pick up a + /// warning from the developer's real `~/.coven/familiars.toml`. + /// + /// Returns the guard/tempdir alongside the app; the caller must keep them + /// alive for the duration of the test (the guard restores env on drop). + fn make_isolated_status_app() -> (App, EnvGuard, tempfile::TempDir) { + let temp = tempfile::tempdir().expect("tempdir"); + let home = temp.path().join("home"); + let coven_home = temp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home dir"); + std::fs::create_dir_all(&coven_home).expect("coven home dir"); + // No familiars.toml is written → the roster is empty → no roster + // warning can leak into the status label. + let guard = EnvGuard::set(&home, &coven_home); + let app = make_test_app_with_model_and_familiar(None, None, None, None); + (app, guard, temp) + } + #[test] fn welcome_model_label_prefers_config_override() { let app = make_test_app_with_model_and_familiar( @@ -3578,7 +3597,7 @@ mod welcome_tests { #[test] fn streaming_status_label_prefers_running_tool_over_default() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); app.tool_use_blocks.push(crate::app::ToolUseBlock { id: "tu_1".to_string(), name: "Bash".to_string(), @@ -3592,7 +3611,7 @@ mod welcome_tests { #[test] fn streaming_status_label_falls_back_through_text_then_thinking() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); app.is_streaming = true; assert_eq!(streaming_status_label(&app), "Waiting on network"); assert!(should_render_status_row(&app)); @@ -3605,7 +3624,7 @@ mod welcome_tests { #[test] fn streaming_status_label_adapter_message_overrides_everything() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); app.tool_use_blocks.push(crate::app::ToolUseBlock { id: "tu_1".to_string(), name: "Bash".to_string(), @@ -3621,7 +3640,7 @@ mod welcome_tests { #[test] fn streaming_status_label_ignores_placeholder_thinking_message() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); // "thinking" / "thinking…" are placeholders — they should NOT win // over more informative state. app.status_message = Some("thinking".to_string()); @@ -3631,7 +3650,7 @@ mod welcome_tests { #[test] fn streaming_status_label_skips_completed_tools() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); // A done tool from a previous turn must not hijack the label. app.tool_use_blocks.push(crate::app::ToolUseBlock { id: "tu_1".to_string(), @@ -3646,7 +3665,7 @@ mod welcome_tests { #[test] fn streaming_status_label_explains_stalled_streams() { - let mut app = make_test_app_with_model_and_familiar(None, None, None, None); + let (mut app, _guard, _tmp) = make_isolated_status_app(); app.is_streaming = true; app.stall_start = Some(std::time::Instant::now() - std::time::Duration::from_secs(4)); diff --git a/src-rust/crates/tui/src/stats_dialog.rs b/src-rust/crates/tui/src/stats_dialog.rs index 538a2675..d843261a 100644 --- a/src-rust/crates/tui/src/stats_dialog.rs +++ b/src-rust/crates/tui/src/stats_dialog.rs @@ -74,11 +74,11 @@ pub struct ModelBreakdown { // Data loading // --------------------------------------------------------------------------- -/// Load and aggregate stats from ~/.coven-code/stats.jsonl +/// Load and aggregate stats from `config_home()/stats.jsonl` (legacy +/// `~/.coven-code/stats.jsonl`, or `~/.coven/code/stats.jsonl` under the +/// unified coven CLI). pub fn load_stats() -> AggregatedStats { - let path = dirs::home_dir() - .map(|h| h.join(".coven-code").join("stats.jsonl")) - .unwrap_or_default(); + let path = claurst_core::config::config_home().join("stats.jsonl"); let content = match std::fs::read_to_string(&path) { Ok(c) => c,