feat(core): unify engine home under ~/.coven with in-place migration (Phase 4, Track C)#153
Conversation
… helper
Introduces `config::config_home()` as the single source of truth for
the engine's `~/.coven-code` directory. Routes the six independent
`dirs::home_dir().join(".coven-code")` constructions in auth_store,
feature_flags, prompt_history, remote_settings, skill_discovery, and
OAuthTokens::token_file_path through it. Adds
`PROJECT_CONFIG_DIRNAME` const for the project-walking sites so the
string literal is centralized there too. `Settings::config_dir()`
becomes a thin shim. Behaviour is byte-identical (still ~/.coven-code).
Phase 4.1.
…LI (in-place migration) config_home() now has a four-level precedence: 1. COVEN_CODE_TEST_HOME (test builds only) 2. COVEN_CODE_HOME (explicit override) 3. ~/.coven/code (when COVEN_HOME is set or COVEN_PARENT=coven) 4. ~/.coven-code (legacy standalone default) home_migration::migrate_if_needed() runs at the very top of main() and moves ~/.coven-code → ~/.coven/code on first launch under the unified coven CLI. All failures are non-fatal: rename is tried first (atomic on same fs), recursive-copy is the fallback for cross-fs moves, and a failed copy leaves legacy intact with a clear stderr message. A backward-compat symlink and a .migrated-from-coven-code marker are written on success. Five unit tests cover populated migration, idempotency, no-legacy no-op, standalone no-op, and no-clobber of an existing non-empty target. closes #22
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR is Phase 4 of the CLI unification work: it centralizes all engine-home path resolution behind claurst_core::config::config_home() and relocates the engine home under the unified coven CLI from ~/.coven-code/ to ~/.coven/code/, including a one-time best-effort migration on first launch.
Changes:
- Add
config_home()with documented env-var precedence and update engine-global path construction across crates to derive from it. - Introduce
home_migrationto move legacy~/.coven-codestate to~/.coven/codewith guardrails (no-clobber, best-effort, never fatal), invoked at CLI startup. - Update project-local config directory joins to use
PROJECT_CONFIG_DIRNAMEwhere applicable and add tests asserting key paths derive fromconfig_home().
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src-rust/crates/tui/src/stats_dialog.rs | Stats file path now derives from config_home() instead of hardcoding ~/.coven-code. |
| src-rust/crates/tui/src/overlays.rs | History pins persistence path now derives from config_home(). |
| src-rust/crates/tui/src/familiar_image.rs | Familiar asset search now includes config_home()/assets/familiars. |
| src-rust/crates/tui/src/agents_view.rs | User agents dir now derives from config_home(), project dir uses PROJECT_CONFIG_DIRNAME. |
| src-rust/crates/tools/src/todo_write.rs | Todos persistence moved under config_home(). |
| src-rust/crates/tools/src/team_tool.rs | Team storage base dir now derives from config_home() (no longer optional). |
| src-rust/crates/tools/src/skill_tool.rs | Global commands/skills search now uses config_home()/commands. |
| src-rust/crates/tools/src/cron.rs | Scheduled task persistence moved under config_home(). |
| src-rust/crates/query/src/skill_prefetch.rs | Skill prefetch now scans config_home()/skills + project-local skills. |
| src-rust/crates/query/src/lib.rs | Auto-dream directories now derive from config_home() (memory + conversations). |
| src-rust/crates/plugins/src/marketplace.rs | Plugin install/list paths now derive from config_home(). |
| src-rust/crates/plugins/src/loader.rs | Default user plugins directory now derives from config_home(); adds test. |
| src-rust/crates/plugins/src/lib.rs | Plugin discovery/install now assumes user dir always resolves via config_home(). |
| src-rust/crates/core/src/tips.rs | Tip history persistence now derives from config_home(). |
| src-rust/crates/core/src/skill_discovery.rs | User skills now derive from config_home(), project skills via PROJECT_CONFIG_DIRNAME. |
| src-rust/crates/core/src/settings_sync.rs | Sync config dir now derives from config_home(). |
| src-rust/crates/core/src/roster_reset.rs | Project agents dir now uses PROJECT_CONFIG_DIRNAME. |
| src-rust/crates/core/src/remote_settings.rs | Remote settings dir now derives from config_home(); adds test accessor. |
| src-rust/crates/core/src/prompt_history.rs | History base dir now derives from config_home(); adds test accessor. |
| src-rust/crates/core/src/oauth_config.rs | Legacy Codex tokens path now derives from config_home(); adds test. |
| src-rust/crates/core/src/memdir.rs | Auto memory base dir default now derives from config_home(); adds test. |
| src-rust/crates/core/src/lib.rs | Introduces config_home(), PROJECT_CONFIG_DIRNAME, env locks, and consolidation/precedence tests. |
| src-rust/crates/core/src/home_migration.rs | New module implementing one-time best-effort migration from ~/.coven-code to ~/.coven/code. |
| src-rust/crates/core/src/goal.rs | Default goals DB path now derives from config_home(); adds test. |
| src-rust/crates/core/src/feature_flags.rs | Feature flag cache path now derives from config_home(); adds test accessor. |
| src-rust/crates/core/src/context_collapse.rs | Collapse-state cache now derives from config_home(). |
| src-rust/crates/core/src/claudemd.rs | Managed/user memory/rules locations now derive from config_home(). |
| src-rust/crates/core/src/auth_store.rs | Auth store path now derives from config_home(). |
| src-rust/crates/core/src/attachments.rs | IDE lockfile dir now derives from config_home(). |
| src-rust/crates/core/src/accounts.rs | claurst_dir() now derives from config_home(). |
| src-rust/crates/cli/src/main.rs | Runs home_migration::migrate_if_needed() before settings/auth/config access. |
| COVEN.md | Documents standalone vs unified-CLI engine home locations and migration behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let under_coven = std::env::var_os("COVEN_HOME").is_some() | ||
| || std::env::var("COVEN_PARENT").ok().as_deref() == Some("coven"); | ||
| if under_coven { | ||
| let coven_home = std::env::var_os("COVEN_HOME") | ||
| .map(PathBuf::from) | ||
| .or_else(|| dirs::home_dir().map(|h| h.join(".coven"))); | ||
| if let Some(ch) = coven_home { | ||
| return ch.join("code"); | ||
| } | ||
| } |
| fn should_skip_existing_target(target: &Path) -> bool { | ||
| target.exists() && target.read_dir().ok().and_then(|mut d| d.next()).is_some() | ||
| } |
| @@ -76,9 +76,7 @@ pub struct ModelBreakdown { | |||
|
|
|||
| /// Load and aggregate stats from ~/.coven-code/stats.jsonl | |||
| 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`. |
| /// Path to the persisted history file: `~/.coven-code/tip_history.json`. | ||
| fn history_path() -> Option<std::path::PathBuf> { | ||
| dirs::home_dir().map(|h| h.join(".coven-code").join("tip_history.json")) | ||
| fn history_path() -> std::path::PathBuf { | ||
| crate::config::config_home().join("tip_history.json") | ||
| } | ||
|
|
| /// Path to `~/.coven-code/scheduled_tasks.json`. | ||
| fn scheduled_tasks_path() -> Option<PathBuf> { | ||
| dirs::home_dir().map(|h| h.join(".coven-code").join("scheduled_tasks.json")) | ||
| fn scheduled_tasks_path() -> PathBuf { | ||
| claurst_core::config::config_home().join("scheduled_tasks.json") | ||
| } | ||
|
|
| /// Return the default user-level plugins directory: `~/.coven-code/plugins`. | ||
| pub fn default_user_plugins_dir() -> Option<PathBuf> { | ||
| dirs::home_dir().map(|h| h.join(".coven-code").join("plugins")) | ||
| pub fn default_user_plugins_dir() -> PathBuf { | ||
| claurst_core::config::config_home().join("plugins") | ||
| } |
| //! Every failure is non-fatal — a failed migration must NEVER brick the engine; | ||
| //! it falls back to whichever directory has the data. | ||
| //! |
…ings Add SharedSettings — a whitelisted (model, theme, permission_mode) struct that reads ~/.coven/settings.json and applies as the lowest-precedence layer in load_hierarchical (shared → engine-global → project). When no ~/.coven/ directory exists (standalone mode) the layer is absent and behaviour is unchanged. Six tests cover the 3-layer precedence, gap-filling, malformed- file tolerance, non-whitelisted key isolation, and no-coven-home fallback. Document the shared key schema and layer order in COVEN.md.
Hold CONFIG_HOME_ENV_LOCK in test_cache_path and assert against config_home()-derived expectation, making it safe to run concurrently with tests that mutate COVEN_HOME/COVEN_PARENT/COVEN_CODE_HOME.
…oster The streaming_status_label_* welcome tests built an App without an EnvGuard, so App construction read the developer's real ~/.coven/familiars.toml. A roster warning (e.g. an unknown access tier) then leaked into the status label and overrode the expected value. Route these tests through an isolated, empty COVEN_HOME via a make_isolated_status_app helper so the roster is always empty.
…do tests token_path() built ~/.coven-code/mcp-tokens directly via dirs::home_dir(), bypassing config_home(). Under the unified coven CLI that would have stranded MCP OAuth tokens in the legacy dir after the engine home relocated to ~/.coven/code. Route it through config_home() so tokens migrate with the rest of the engine state (this was the last path site not yet consolidated by 4.1). Also isolate the token-store and todo-persistence tests to a temp COVEN_CODE_HOME so they never read or write the developer's real ~/.coven-code. These tests set COVEN_CODE_HOME (honored in all builds) rather than COVEN_CODE_TEST_HOME, which is #[cfg(test)]-gated inside claurst_core and compiled out when core is a plain dependency of another crate's test binary. Add tempfile dev-deps for mcp/tools.
…nify test env lock
Correctness fixes from the PR review:
- config_home(): treat an empty COVEN_HOME as unset. Previously the mere
presence of the var (even "") triggered the coven-cli branch, and
PathBuf::from("") resolved to a relative "code" dir in the cwd, which could
also mis-trigger the home migration. Now only a non-empty value counts.
- should_skip_existing_target(): fail SAFE when the target exists but cannot be
inspected (read_dir error: permissions, not-a-directory). Previously it
returned false and proceeded, letting the copy fallback merge into an existing
target — violating the no-clobber guarantee.
- Unify the test env lock: CONFIG_HOME_ENV_LOCK is now a re-export of
COVEN_HOME_ENV_LOCK. They guarded the same env vars with two different
mutexes, so tests holding one could race tests holding the other (observed
flake in feature_flags::test_cache_path). One lock now serializes them all.
Also refresh stale doc comments that still hardcoded ~/.coven-code for paths
now derived from config_home() (stats, history pins, tips, todos, scheduled
tasks, plugins), and correct a misleading sentence in the home_migration module
docs about fallback behaviour.
|
Addressed the review feedback in f074fab: Correctness
Test-infra flake
Stale docs
Separately, this branch also fixed several tests that were writing into the real |
Summary
Phase 4, Track C: unify the engine's home under
~/.coven. When running under the unifiedcovenCLI, the engine's home relocates from~/.coven-code/to~/.coven/code/(migrating existing data in place), and a shared~/.coven/settings.jsonlayer provides cross-tool defaults. Pure coven-code-side; companion Track D (Credentials panel + cross-session search) is a separate PR on OpenCoven/coven.What's in this PR (8 commits)
config_home()helper (aa3047b,fbeb9d3). The audit found path construction scattered across ~23 independent sites (codex tokens, goals, memory, conversations, plugins, agents, teams, cron, …); every one now derives from the single helper. A grep proves zero independenthome_dir().join(".coven-code")constructions remain — so relocation is a one-function change, not a data-split risk.25f651b):config_home()precedence isCOVEN_CODE_HOME→ under-coven (COVEN_HOME/COVEN_PARENT=coven) →<coven_home>/code→ legacy~/.coven-code. A first-runhome_migrationmoves an existing~/.coven-code/to~/.coven/code/and leaves a compatibility symlink. Data-safety-reviewed: no failure path can lose or destructively overwrite user data — copy success is confirmed before legacy removal, an existing target is never clobbered, and every error is non-fatal (falls back to whichever dir has the data). Guarded by a pure, temp-dir-testedmigrate_between(no test touches the real~).88164f3): a shared~/.coven/settings.jsonsits beneath engine and project settings (precedence: shared < engine < project). Only a tight whitelist —model,theme,permission_mode— is read from it; engine-only/security-sensitive keys (mcp_servers, hooks, plugins, credentials) are ignored. Malformed/absent file and no-coven-home both degrade gracefully.ce191de,1703d60,7a0f769): becauseconfig_home()is now env-sensitive, tests that read/write it are serialized under the env lock and isolated to temp homes (preventing both parallel env-pollution and writes into the developer's real home).1892504): the full Phase 4 plan doc (Tracks C and D).Verification
cargo test --workspace --locked— all green, 0 failed (core 667 + all crates); confirmed stable across repeated parallel runs.cargo clippy --workspace --all-targets --locked -- -D warnings— clean.cargo fmt --check— clean.Review
Each task passed review (4.1 grep-verified complete; 4.2 got a dedicated data-safety review; 4.3 + the test-isolation fixes got a whole-branch integration review confirming correct precedence and genuine isolation). A test-pollution regression (a hardcoded
.coven-codeassertion that broke onceconfig_home()became env-sensitive) was caught and fixed before landing.Known follow-ups (non-blocking)
load_hierarchicalbut notload_sync, so the settings screen shows the engine-global value while the live session uses the shared value (arguably correct — avoids persisting shared values into the engine file).~/.coven/codewithout a symlink; a later standalone run would create a separate~/.coven-code. No data loss — a proactive compat symlink is a reasonable follow-up.Update — additional consolidation fix (
c2c1d28)A final path site was found after the initial grep: MCP
token_path()built~/.coven-code/mcp-tokensvia a multi-linedirs::home_dir().join(".coven-code/mcp-tokens")construction that evaded the single-line completeness grep. It's now routed throughconfig_home()(so MCP OAuth tokens migrate with the rest of the engine home), plus the mcp/tools token/todo tests are isolated to a temp home. Full workspace suite remains green; 9 signed commits total, no AI trailers.