Fix #144: hermetic user_prompt_submit tests via TERRAPHIM_DEFAULT_DATA_PATH

[AlexMikhalev]

Fixes #144: user_prompt_submit_tests capture failures via
terraphim_agent::learnings, which historically writes correction
artefacts under dirs::data_dir(). On macOS that path resolves to
$HOME/Library/Application Support and ignores XDG_DATA_HOME,
so the tests could not redirect storage into a hermetic temp root.

Two coordinated changes:

1. Production: LearningCaptureConfig::default() honours the
   TERRAPHIM_DEFAULT_DATA_PATH environment variable when computing
   its global_dir. This brings the hook path in line with the
   existing terraphim_settings::DeviceSettings, which already reads
   the same env var. storage_location() short-circuits to the
   global_dir when that variable is set, so the test never lands in
   the per-project directory by accident.

2. Tests: each user_prompt_submit_tests case gets a unique hermetic
   root, derives its learnings_dir = root.join("data").join("terraphim").join("learnings"),
   spawns the agent with TERRAPHIM_DEFAULT_DATA_PATH pointing at
   the root's data dir, and reads back the correction files from
   that derived path. cli_test_env::create_hermetic_root() and
   set_hermetic_env() are new helpers in tests/support/cli_test_env.rs.

Local verification: cargo test -p terraphim_agent --test user_prompt_submit_tests reports 4 passed; 0 failed after this
change.

[AlexMikhalev]

Summary

user_prompt_submit_tests in terraphim_agent had been failing on every non-Linux runner since the 2026-07-31 --lib-only gate regression. The root cause is platform-specific: LearningCaptureConfig::default() resolves global_dir via dirs::data_dir(), which on macOS and Windows returns $HOME/Library/Application Support and ignores XDG_DATA_HOME. The tests set HOME and XDG_DATA_HOME, but the production hook still wrote to the macOS data dir, so the assertions never found the expected files.

This PR makes two coordinated changes. In production:

• LearningCaptureConfig::default() now honours TERRAPHIM_DEFAULT_DATA_PATH when computing global_dir, joining <root>/terraphim/learnings. This brings the hook path into line with the existing terraphim_settings::DeviceSettings, which already reads the same env var.
• storage_location() short-circuits to global_dir whenever TERRAPHIM_DEFAULT_DATA_PATH is set, so the per-project .terraphim/learnings/ no longer wins in that case. When the env var is unset, behaviour is identical to before.

In tests:

• support::cli_test_env::create_hermetic_root() and set_hermetic_env() are introduced. create_hermetic_root() returns the path of a fresh temp root so callers can read files written by the spawned subprocess; set_hermetic_env() points TERRAPHIM_DEFAULT_DATA_PATH at the root's data/ subdir.
• All four tests (user_prompt_submit_use_instead_of_creates_tool_preference, user_prompt_submit_use_not_creates_tool_preference, user_prompt_submit_prefer_over_creates_tool_preference, user_prompt_submit_personal_preference_does_not_capture) now steer the spawned subprocess through set_hermetic_env and derive learnings_dir from the same root via a new hermetic_learnings_dir() helper. The pre-existing clear_correction_files() helper is removed (no longer needed -- each test gets a unique root).

What was done well: the production change is strictly additive (env var unset == previous behaviour), the rationale is laid out in the commit message with a clear before/after, no mocks or #[ignore] are introduced, and the test pattern matches what neighbouring test suites in this crate already use. All four tests pass locally; the personal-preference test, which was previously vacuous on macOS, now genuinely asserts that no file is created.

What remains problematic: the new tests/user_prompt_submit_tests.rs is missing the trailing newline at end of file (git shows \ No newline at end of file), which cargo fmt would otherwise add. There is also a small consistency gap: create_hermetic_root() creates root/data/ but the production code reads TERRAPHIM_DEFAULT_DATA_PATH and joins terraphim/learnings, so the storage location is root/data/terraphim/learnings while the hermetic root itself sits at root -- this is fine and well-commented, but the relationship between create_hermetic_root and the env-var contract could be tightened with one extra doc line.

Confidence Score: 4/5

• Safe to merge with awareness of the missing trailing newline (P2) and the documentation-tightening suggestion (P2).
• The production-side behaviour change is gated on an environment variable that is unlikely to be set by anyone except the test runner, so the risk to existing users is low. All four tests pass on macOS after the change.
• No files require special attention beyond tests/user_prompt_submit_tests.rs.

Important Files Changed

Filename	Overview
crates/terraphim_agent/src/learnings/mod.rs	Default::default() honours TERRAPHIM_DEFAULT_DATA_PATH; storage_location() short-circuits to global_dir when the env var is set. Both paths documented inline. No issues found.
crates/terraphim_agent/tests/support/cli_test_env.rs	Adds create_hermetic_root() and set_hermetic_env(); existing apply_hermetic_env() retained and refactored to call the new helpers. No issues found.
crates/terraphim_agent/tests/user_prompt_submit_tests.rs	All four test cases rewritten to use the hermetic helpers. clear_correction_files() removed. Missing trailing newline at end of file.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[user_prompt_submit test] --> B[create_hermetic_root]
    B --> C["root: PathBuf"]
    A --> D["set_hermetic_env(cmd, &root)"]
    D --> E["cmd.env(TERRAPHIM_DEFAULT_DATA_PATH, root/data)"]
    A --> F[spawn terraphim-agent learn hook]
    F --> G["LearningCaptureConfig::default()"]
    G --> H{"TERRAPHIM_DEFAULT_DATA_PATH set?"}
    H -->|Yes| I["global_dir = root/data/terraphim/learnings"]
    H -->|No| J["global_dir = dirs::data_dir() ... (legacy)"]
    I --> K[storage_location returns global_dir]
    J --> L{"project_dir exists?"}
    L -->|Yes| M[storage_location returns project_dir]
    L -->|No| K
    K --> N[hook writes correction file]
    M --> N
    N --> O[test reads file via hermetic_learnings_dir root]
    O --> P[assert one file or zero files]

    style B fill:#d4edda,stroke:#28a745
    style D fill:#d4edda,stroke:#28a745
    style H fill:#fff3cd,stroke:#ffc107
    style I fill:#d4edda,stroke:#28a745
    style K fill:#d4edda,stroke:#28a745

Loading

Inline Findings

P2 crates/terraphim_agent/tests/user_prompt_submit_tests.rs, end of file: Missing trailing newline

git diff reports \ No newline at end of file for the new file. The rest of the workspace's Rust files are uniformly newline-terminated, and cargo fmt would re-add the newline on the next run. Trivial fix; just append one \n after the final }.

P2 crates/terraphim_agent/tests/support/cli_test_env.rs, lines 48-57 (create_hermetic_root): Contract between helper and env var is implicit

The helper creates root/data/ and set_hermetic_env points TERRAPHIM_DEFAULT_DATA_PATH at root/data/, while the production code joins <TERRAPHIM_DEFAULT_DATA_PATH>/terraphim/learnings. The two halves of the contract are correct but the link between create_hermetic_root's return value and the env-var value is implicit. A one-line doc comment on create_hermetic_root clarifying that callers should derive their expected-write directory as root.join("data").join("terraphim").join("learnings") (which is exactly what hermetic_learnings_dir does) would prevent the next reader from re-deriving the relationship. The same wording belongs on set_hermetic_env.

_{Last reviewed commit: e00c958 | Reviews (1)}