fix(cache): set temp spillover root in cache_inspect test to survive nix sandbox#2877
fix(cache): set temp spillover root in cache_inspect test to survive nix sandbox#2877LeoAlex0 wants to merge 1 commit into
Conversation
…nix sandbox
The test cache_inspect_displays_tool_result_budget_metadata relied on a
writable $HOME/.codewhale/tool_outputs/ for tool-result wire-dedup
persistence. nix build sandboxes have a read-only home tree, so the
first tool-result SHA spillover write failed, the dedup hash table was
never populated, and the second identical tool result was not marked
deduplicated — causing the expect("repeat tool-result sighting should
report dedup metadata") assertion to fail.
Set TEST_SPILLOVER_ROOT to a tempdir inside the test (matching the
with_tool_result_sha_spillover_root pattern in chat.rs), so the
wire-dedup path works in any environment without depending on $HOME.
|
Thanks @LeoAlex0 for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
There was a problem hiding this comment.
Code Review
This pull request updates a test in crates/tui/src/commands/debug.rs to set a temporary spillover root using a temporary directory. This ensures that SHA-addressed tool-result files can be persisted during testing without relying on a writable $HOME directory, which prevents failures in environments like Nix sandboxes. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
Fix the flaky
cache_inspect_displays_tool_result_budget_metadatatest that fails in nix build sandboxes.Root cause
nix sandboxes have a read-only home tree. The test relied on a writable
$HOME/.codewhale/tool_outputs/for tool-result wire-dedup persistence. The first tool-result SHA spillover write failed silently, the dedup hash table was never populated, and the second identical tool result was not marked deduplicated.Fix
Set
TEST_SPILLOVER_ROOTto a tempdir inside the test, matching the existingwith_tool_result_sha_spillover_rootpattern inchat.rs.Scope
Only test code (
#[cfg(test)]). No production code paths affected.Validation
Greptile Summary
This PR fixes a flaky test (
cache_inspect_displays_tool_result_budget_metadata) that silently failed in nix build sandboxes because the test was trying to write SHA-addressed spillover files to$HOME/.codewhale/tool_outputs/, which is read-only in nix sandbox environments.tempfile::tempdir()spillover root override inside the test, using the sameset_test_spillover_root+ RAIIRestoreguard pattern that already exists incrates/tui/src/tui/ui/tests.rs:1477–1487.#[cfg(test)]); no production code paths are affected.Confidence Score: 5/5
Safe to merge — the change is confined to test code and uses an established, well-tested RAII pattern already present in the codebase.
The fix is a straightforward test-only addition that mirrors the identical set_test_spillover_root + Restore guard idiom already used in ui/tests.rs. Drop order is correct (_restore before tmp), the mutex guard prevents parallel tests from racing on the global root, and no production paths are touched.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Test as cache_inspect_displays_tool_result_budget_metadata participant Guard as TEST_SPILLOVER_GUARD participant Root as TEST_SPILLOVER_ROOT participant TmpDir as tempfile::tempdir() participant Spillover as spillover_root() Test->>Guard: lock() — serialize against other root-overriding tests Test->>TmpDir: tempdir() — create writable scratch directory Test->>Root: set_test_spillover_root(tmp/.deepseek/tool_outputs) → returns prior Note over Test,Root: Restore(prior) held as _restore Test->>Spillover: called by wire-dedup on first tool result Spillover-->>Root: reads TEST_SPILLOVER_ROOT → tmp path Spillover-->>Test: returns writable tmp path (not $HOME) Test->>Spillover: called by wire-dedup on second tool result Spillover-->>Root: reads TEST_SPILLOVER_ROOT → same tmp path Note over Test: dedup finds SHA file → deduplicated=true ✓ Note over Test: test assertions pass Test->>Root: _restore drops → set_test_spillover_root(prior) Test->>TmpDir: tmp drops → tempdir deleted Test->>Guard: _spill_guard drops → mutex releasedReviews (1): Last reviewed commit: "fix(cache): set temp spillover root in c..." | Re-trigger Greptile