From 741abc35c301846ff4e8a09369189988e1c42544 Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 02:06:40 +0000 Subject: [PATCH 1/3] fix(disk): relocate cargo target off 28G `/` volume + anti-thrash cleanup gate (#4803) Fixes #4803. The root filesystem `/` (28G, holds /home and ~/.simard) saturated to 0 bytes free, driving a ~25-min emergency-cleanup crash-loop: each pass deleted target/debug + target/llvm-cov-target only for cargo to instantly rebuild and refill `/` within one cycle. That 0-bytes-free condition cascaded into cognitive-lock refusals, typed `database is locked`, and memory-IPC write failures across goal-session engineers. Root cause: cargo build artifacts defaulted onto `/` (under $HOME) while a 196G /tmp volume sat with free space, and emergency cleanup freed space that instantly rebuilt. Additive, non-breaking fix confined to the cited surface: - tmux.rs: drop the `$HOME/.cargo-targets` default branch so the per-worktree CARGO_TARGET_DIR defaults onto the large-volume `/tmp/simard-cargo-targets` fallback even when HOME is set; `SIMARD_CARGO_TARGETS_ROOT` override still wins. Resolver made pub(crate) as the single source of truth. - lifecycle/spawn.rs: direct-exec path delegates to that unified resolver instead of the divergent hardcoded `/tmp/simard-engineer-target`. - disk_health.rs: emergency_cleanup gains hysteresis (high=95/low=85) plus a persistent time-backoff marker under `/disk-health/` (SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS, default 900, clamped [0,86400]) so it cannot thrash within one build window; symlink + containment guards before every remove_dir_all; fail-open marker I/O. - advance_goal/spawn.rs: build-heavy dispatch preflight probes `/` via the existing disk_pressure gate; Refuse -> benign retry-next-cycle skip with loud warn (no silent fallback); probe error fails open. Structured tracing only (no new print!/println!), no silent fallbacks. Docs added for concept/howto/reference. cargo check + clippy clean; disk_health, tmux-env, and preflight unit tests green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../root-disk-saturation-relocation.md | 118 +++++++ ...elocate-build-artifacts-off-root-volume.md | 153 ++++++++ ...tifact-relocation-and-disk-thrash-guard.md | 208 +++++++++++ mkdocs.yml | 3 + src/agent_supervisor/lifecycle/spawn.rs | 18 +- src/agent_supervisor/tests_tmux.rs | 40 ++- src/agent_supervisor/tmux.rs | 56 +-- src/disk_health.rs | 329 +++++++++++++++++- src/ooda_actions/advance_goal/spawn.rs | 120 ++++++- 9 files changed, 1005 insertions(+), 40 deletions(-) create mode 100644 docs/concepts/root-disk-saturation-relocation.md create mode 100644 docs/howto/relocate-build-artifacts-off-root-volume.md create mode 100644 docs/reference/build-artifact-relocation-and-disk-thrash-guard.md diff --git a/docs/concepts/root-disk-saturation-relocation.md b/docs/concepts/root-disk-saturation-relocation.md new file mode 100644 index 000000000..f492585c0 --- /dev/null +++ b/docs/concepts/root-disk-saturation-relocation.md @@ -0,0 +1,118 @@ +--- +title: Root-disk saturation relocation and thrash guard +description: Why Simard relocates cargo build artifacts off the small root volume, adds hysteresis to emergency disk cleanup, and refuses build-heavy dispatch under disk pressure — the fix for the #4803 crash-loop. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: explanation +related: + - ./automated-disk-health.md + - ./agentic-disk-reclamation.md + - ../howto/relocate-build-artifacts-off-root-volume.md + - ../reference/build-artifact-relocation-and-disk-thrash-guard.md + - ../reference/resource-admission-api.md +--- + +# Root-disk saturation relocation and thrash guard + +Fixes [#4803](https://github.com/rysweet/Simard/issues/4803). + +## The failure this prevents + +On hosts where `/home` and `~/.simard` live on a small root volume (a 28 GiB +`/` in the reference incident), cargo build artifacts written under +`~/.simard` — chiefly `target/debug` and `target/llvm-cov-target` — filled the +root filesystem to **0 bytes free**. That single condition cascaded across the +whole daemon: + +- `cognitive-open-lock` refusals and typed `database is locked` errors, +- `memory-ipc` write failures in goal-session engineers, +- stalled goal advancement across every workstream. + +The daemon's Tier-1 **emergency disk cleanup** re-fired roughly every 25 +minutes (observed: 21:58 → 99%, 23:00 → 94%, 23:35 → 94%, 00:10 → 96%, 00:42 +→ 97%). Each run deleted `target/debug` + `target/llvm-cov-target` (0.7–2.6 GB) +and pruned backups, but `/` refilled within one cycle because cargo +immediately rebuilt into the same location. Cleanup was an **ineffective +band-aid that thrashed** — it never removed the *cause*, only the symptom, and +each pass burned I/O and starved progress. + +Meanwhile a 196 GiB `/tmp` volume sat with ~26 GiB free, unused for build +artifacts. + +## Root cause + +Two independent defaults pointed the fastest-refilling artifacts at the small +root volume: + +1. **`default_cargo_target_for_worktree`** (in `src/agent_supervisor/tmux.rs`) + defaulted `CARGO_TARGET_DIR` to `$HOME/.cargo-targets/` — i.e. + under `~` on the 28 GiB `/`. +2. A divergent hardcoded fallback in + `src/agent_supervisor/lifecycle/spawn.rs` set + `CARGO_TARGET_DIR=/tmp/simard-engineer-target` for the single-process spawn + path, so the two spawn paths disagreed about where artifacts lived. + +Emergency cleanup could not win against a build target that lived on the volume +it was trying to protect. + +## The fix, in three moves + +The fix is **additive and non-breaking** — it changes defaults and adds guard +rails; it does not change any public CLI surface. + +### 1. Relocate build artifacts onto the large volume (primary) + +The default cargo target root moves off `/`. The existing +`SIMARD_CARGO_TARGETS_ROOT` override still wins; when it is unset the default +now resolves to the large-volume fallback (`/tmp/simard-cargo-targets`) instead +of `$HOME/.cargo-targets`. Both spawn paths delegate to **one** resolver, so +they can no longer diverge. This alone stops `/` from re-saturating. + +See [`default_cargo_target_for_worktree`](../reference/build-artifact-relocation-and-disk-thrash-guard.md#c1-cargo-target-root-relocation). + +### 2. Give emergency cleanup hysteresis (stops the thrash) + +Emergency cleanup gains a **high/low watermark** (trigger at ≥ 95% used, do +not re-fire until usage falls back below a distinct low watermark) plus a +**persistent backoff marker** under `/disk-health/`. Cleanup can no +longer re-fire on every timer tick inside a single build window. After the +relocation in move 1, `/` is no longer the fill target, so cleanup becomes +rare rather than perpetual. + +See [emergency-cleanup hysteresis](../reference/build-artifact-relocation-and-disk-thrash-guard.md#c3-emergency-cleanup-hysteresis--backoff). + +### 3. Refuse build-heavy dispatch under disk pressure (belt) + +Before dispatching a build-heavy goal session, a **preflight** probes `/` +through the existing [`disk_pressure`](../reference/resource-admission-api.md) +gate. That gate classifies against a single min-free threshold `T` +(`SIMARD_DISK_PRESSURE_MIN_FREE_GB`, default **20 GiB**) in two bands: +`Warn` when free space is below `T`, and `Refuse` when it drops below `T/2`. +So at defaults the preflight **warns below 20 GiB free and refuses below +10 GiB free**. On a `Refuse`, dispatch is **loudly skipped for this cycle** +(retried next cycle) rather than writing the daemon into ENOSPC again. This +reuses the existing min-free threshold and 90% admission ceiling; it introduces +no parallel constants. + +See [dispatch preflight](../reference/build-artifact-relocation-and-disk-thrash-guard.md#c4-build-heavy-dispatch-preflight). + +## Design principles honoured + +- **No silent fallbacks.** The preflight refuses loudly (`warn!`) and the + worktree allocator returns a hard `Err`; a degraded spawn is never preferred + over a visible refusal. +- **Structured tracing + OTel only.** No `print!`/`println!`/`eprintln!` is + added; every signal flows through `tracing`. +- **Containment before deletion.** Every `remove_dir_all` site is guarded by a + `symlink_metadata` check and a `starts_with(repo_root | state_root)` + containment assertion, so a hostile `target -> /` symlink cannot make cleanup + escape the tree. +- **Fail-open on the marker, never fail-shut.** A corrupted or unreadable + backoff marker allows cleanup to proceed (logged), so the guard can never + suppress cleanup forever. + +## Where to go next + +- Operators: [Relocate build artifacts off the root volume](../howto/relocate-build-artifacts-off-root-volume.md). +- Engineers: [Build-artifact relocation and disk-thrash guard — reference](../reference/build-artifact-relocation-and-disk-thrash-guard.md). diff --git a/docs/howto/relocate-build-artifacts-off-root-volume.md b/docs/howto/relocate-build-artifacts-off-root-volume.md new file mode 100644 index 000000000..284840e36 --- /dev/null +++ b/docs/howto/relocate-build-artifacts-off-root-volume.md @@ -0,0 +1,153 @@ +--- +title: "How to relocate build artifacts off the root volume" +description: Move cargo build artifacts off a small root volume, tune the emergency-cleanup backoff, and verify the daemon no longer crash-loops on a full disk. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: howto +related: + - ../concepts/root-disk-saturation-relocation.md + - ../reference/build-artifact-relocation-and-disk-thrash-guard.md + - ./configure-disk-health-check.md + - ./configure-disk-reclamation.md + - ./reclaim-disk-space-and-run-low-space-rust-builds.md +--- + +# How to relocate build artifacts off the root volume + +By default Simard writes cargo build artifacts to +`/tmp/simard-cargo-targets/` so they land on a large volume instead +of the small root filesystem that hosts `~/.simard`. This guide shows how to +confirm the relocation, point it at a different volume, tune the emergency +cleanup, and verify the [#4803](https://github.com/rysweet/Simard/issues/4803) +crash-loop is gone. + +## When to use this + +Use this guide when: + +- `/home` and `~/.simard` share a small root volume that keeps filling up. +- `~/.simard/ooda.log` shows emergency disk cleanup re-firing every ~25 minutes. +- You see `database is locked`, `cognitive-open-lock` refusals, or + `memory-ipc` write failures that correlate with `/` at 0 bytes free. +- You want build artifacts on a specific data volume rather than `/tmp`. + +## Step 1: Confirm which volume is filling up + +```bash +df -h / /tmp +``` + +If `/` is at or near 100% while `/tmp` (or another data volume) has room, the +default relocation applies. On the reference host this looked like a 28 GiB `/` +at 100% next to a 196 GiB `/tmp` with ~26 GiB free. + +## Step 2: Confirm where cargo artifacts are being written + +With no override set, the default target root is `/tmp/simard-cargo-targets`. +Verify the daemon is using it: + +```bash +ls -d /tmp/simard-cargo-targets/*/ 2>/dev/null +``` + +You should see one directory per active engineer worktree, each containing a +`debug/` (and, during coverage runs, `llvm-cov-target/`) subtree. If instead +you find large artifacts under `~/.cargo-targets` or `~/.simard/**/target`, the +daemon is running an old build — deploy the #4803 fix and restart. + +## Step 3: (Optional) point relocation at a specific volume + +To use a dedicated data volume instead of `/tmp`, set +`SIMARD_CARGO_TARGETS_ROOT` before launching the daemon: + +```bash +export SIMARD_CARGO_TARGETS_ROOT=/data/simard-cargo-targets +``` + +Rules: + +- The override **wins** over the default. +- An **empty** value is ignored (treated as unset) so it can never resolve to + `/` and re-saturate `/`. +- A per-worktree basename is appended automatically; point this at the volume + root, not at a single worktree. + +To pin an exact directory for a one-off local build instead, set +`CARGO_TARGET_DIR` directly — it is honoured verbatim by both spawn paths. + +## Step 4: Tune the emergency-cleanup backoff (optional) + +Emergency cleanup now uses hysteresis (trigger at ≥ 95% used, do not re-fire +until usage drops below 85%) plus a minimum re-fire interval. Adjust the +interval with: + +```bash +# Minimum seconds between two emergency cleanups (default 900, clamped 0–86400). +export SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS=1800 +``` + +Leave this at the default unless cleanup still fires too often after +relocation — post-relocation, `/` is no longer the fill target, so cleanup +should become rare on its own. + +The backoff marker lives under `/disk-health/`. If it is deleted or +corrupted, cleanup **fails open** (runs anyway) and logs a warning — it can +never be stuck suppressed. + +## Step 5: Confirm the dispatch preflight threshold + +Build-heavy goal dispatch is gated by the `disk_pressure` classifier, which +compares `/` free space against a single min-free threshold `T` set by +`SIMARD_DISK_PRESSURE_MIN_FREE_GB` (default **20 GiB**): + +- **`Warn`** — free space below `T` (below **20 GiB** at defaults): dispatch + still proceeds, but logs a `warn!`. +- **`Refuse`** — free space below `T/2` (below **10 GiB** at defaults): + dispatch is skipped this cycle. + +Note the `Refuse` line is **half** the `min-free` knob, not equal to it. Tune +the knob if you want a different floor: + +```bash +# T = 20 GiB → Warn below 20 GiB, Refuse below 10 GiB. +export SIMARD_DISK_PRESSURE_MIN_FREE_GB=20 +``` + +When the preflight refuses, the daemon logs a `warn!` and **retries the goal +next cycle** — it does not block or fail the goal. + +## Step 6: Verify the crash-loop is gone + +After deploying and restarting the daemon: + +```bash +# Emergency cleanup should NOT re-fire every ~25 minutes anymore. +grep -i "emergency" ~/.simard/ooda.log | tail -10 + +# Root volume should hold steady well below 95%. +watch -n 60 'df -h /' + +# Preflight refusals (if any) are visible and benign: +grep -i "disk pressure" ~/.simard/ooda.log | tail -10 +``` + +Success looks like: `/` stays below the high watermark, emergency cleanup +entries become sparse instead of appearing every cycle, and the +`database is locked` / `cognitive-open-lock` / `memory-ipc` errors clear. + +## Troubleshooting + +| Symptom | Likely cause | Fix | +|---------|-------------|-----| +| Artifacts still under `~/.cargo-targets` | Old binary before the #4803 fix | Redeploy and restart the daemon. | +| `SIMARD_CARGO_TARGETS_ROOT` set but ignored | Value is empty | Set a non-empty absolute path (empty is intentionally ignored). | +| Cleanup still thrashes | `/tmp` itself is small / is `/` | Point `SIMARD_CARGO_TARGETS_ROOT` at a genuinely large volume. | +| Coverage jobs can't find artifacts | Tooling hardcodes `./target` | Point llvm-cov at the relocated `CARGO_TARGET_DIR` (env-consistent). | +| Goals never dispatch | `/` chronically below the refuse line | Reclaim space (see [reclaim disk](./reclaim-disk-space-and-run-low-space-rust-builds.md)); the preflight is protecting you from ENOSPC. | + +## See also + +- Concept: [Root-disk saturation relocation and thrash guard](../concepts/root-disk-saturation-relocation.md) +- Reference: [Build-artifact relocation and disk-thrash guard](../reference/build-artifact-relocation-and-disk-thrash-guard.md) +- [Configure and monitor the disk health check](./configure-disk-health-check.md) diff --git a/docs/reference/build-artifact-relocation-and-disk-thrash-guard.md b/docs/reference/build-artifact-relocation-and-disk-thrash-guard.md new file mode 100644 index 000000000..c85ef835e --- /dev/null +++ b/docs/reference/build-artifact-relocation-and-disk-thrash-guard.md @@ -0,0 +1,208 @@ +--- +title: Build-artifact relocation and disk-thrash guard +description: Reference for the #4803 fix — cargo target relocation resolver, emergency-cleanup hysteresis + backoff marker, build-heavy dispatch preflight, and all associated environment variables and invariants. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: reference +related: + - ../concepts/root-disk-saturation-relocation.md + - ../howto/relocate-build-artifacts-off-root-volume.md + - ./disk-health-api.md + - ./resource-admission-api.md + - ./engineer-worktree-isolation.md +--- + +# Build-artifact relocation and disk-thrash guard + +Fixes [#4803](https://github.com/rysweet/Simard/issues/4803). + +This page is the reference for the four components that together stop the root +volume (`/`) from saturating and crash-looping the daemon. For the *why*, see +[Root-disk saturation relocation](../concepts/root-disk-saturation-relocation.md). + +> **Prescriptive identifiers.** Code identifiers coined by this spec — +> notably the C1 constant `DEFAULT_CARGO_TARGETS_ROOT_FALLBACK` and the C4 +> function `dispatch_spawn_engineer` — are the intended names for the +> implementation. If the implementation lands under different identifiers, +> update this reference in the same change so the two do not drift. Names that +> already exist in the tree (`default_cargo_target_for_worktree`, the +> `disk_pressure` surface in C5) are quoted from current source. + +## Components at a glance + +| ID | File | Kind | Responsibility | +|----|------|------|----------------| +| C1 | `src/agent_supervisor/tmux.rs` | modified | Cargo target root defaults onto the large volume; single source of truth. | +| C2 | `src/agent_supervisor/lifecycle/spawn.rs` | modified | Single-process spawn delegates to the C1 resolver — no divergent hardcoded path. | +| C3 | `src/disk_health.rs` | modified | Emergency cleanup gains high/low-watermark hysteresis + persistent backoff marker + containment guards. | +| C4 | `src/ooda_actions/advance_goal/spawn.rs` | modified | Build-heavy dispatch preflight probes `/` and refuses under disk pressure. | +| C5 | `src/disk_pressure/` | reused | Existing `PressureLevel` gate; no new constants. | +| C6 | coverage tooling | verified | `target/llvm-cov-target` still resolves under the relocated root. | + +## Environment variables + +| Variable | Default | Clamp / notes | +|----------|---------|---------------| +| `SIMARD_CARGO_TARGETS_ROOT` | *(unset)* → `/tmp/simard-cargo-targets` | **Override wins.** Empty string is ignored (treated as unset) so it can never yield `/`. Per-worktree basename is appended. | +| `SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS` | `900` | Parsed with fallback; clamped to `[0, 86400]`. Minimum seconds between two emergency-cleanup runs. | +| `SIMARD_DISK_PRESSURE_MIN_FREE_GB` | `20` | Reused from `disk_pressure`; min-free threshold for the dispatch preflight. | +| `CARGO_TARGET_DIR` | *(operator override)* | If set in the parent environment it is honoured verbatim by both spawn paths; the C1 default is only used when it is unset. | + +> **Backward compatibility.** The only default that *changed* is the cargo +> target root: it moved from `$HOME/.cargo-targets` to +> `/tmp/simard-cargo-targets`. Operators who already set +> `SIMARD_CARGO_TARGETS_ROOT` or `CARGO_TARGET_DIR` see no change. + +## C1: cargo target root relocation + +**`default_cargo_target_for_worktree(worktree_path, parent_pairs) -> String`** +(`src/agent_supervisor/tmux.rs`) + +Resolution order for the cargo target root: + +1. `SIMARD_CARGO_TARGETS_ROOT` (from `parent_pairs`), if set **and non-empty**. +2. `DEFAULT_CARGO_TARGETS_ROOT_FALLBACK` = `/tmp/simard-cargo-targets`. + +The pre-#4803 step 2 — `$HOME/.cargo-targets/` — is **removed** from +the default chain. `HOME` is no longer consulted to build the default target +root, so artifacts never default onto the volume that hosts `~/.simard`. + +The per-worktree basename comes from `worktree_path.file_name()`. If the path +has no terminal component the literal `"engineer-worktree"` is substituted so +the result is always well-formed. + +Final path: `/`. + +### Invariants + +- **IV-1 (empty-string guard).** An empty `SIMARD_CARGO_TARGETS_ROOT` is + filtered out, so the resolver can never produce `/` (which would + re-saturate `/`). +- **Single source of truth.** This resolver is `pub(crate)` and is the only + place that computes a default cargo target root. Both the tmux path + (`compute_tmux_env`) and the single-process path (C2) call it. + +## C2: single-process spawn delegation + +**`spawn_subordinate`** (`src/agent_supervisor/lifecycle/spawn.rs`) + +The previous hardcoded fallback — + +```rust +if std::env::var_os("CARGO_TARGET_DIR").is_none() { + cmd.env("CARGO_TARGET_DIR", "/tmp/simard-engineer-target"); // divergent +} +``` + +— is replaced by a call to the C1 resolver. The operator `CARGO_TARGET_DIR` +override guard is preserved (an explicit `CARGO_TARGET_DIR` still wins). This +eliminates the divergence where the tmux path and the single-process path +disagreed about the artifact location. + +## C3: emergency-cleanup hysteresis + backoff + +**`emergency_cleanup(...)`** (`src/disk_health.rs`) — signature preserved. + +Tier-1 emergency cleanup (pure Rust, no recipe, no LLM) now runs behind a +hysteresis band and a persistent backoff marker. + +### Watermarks + +| Watermark | Value | Meaning | +|-----------|-------|---------| +| High | 95% used | Cleanup **may** trigger at or above this. | +| Low | 85% used | Cleanup will not re-fire until usage has fallen back **below** this. | + +Between the two watermarks the system is in the hysteresis dead-band: a prior +cleanup does not re-fire even though usage is still elevated. This is what stops +the "delete → cargo rebuilds → delete again 25 min later" thrash. + +### Backoff marker + +- **Location:** `/disk-health/` (a small marker file recording the + last successful cleanup timestamp). +- **Gate:** a new cleanup is suppressed if fewer than + `SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS` (default 900, clamped `[0, 86400]`) + seconds have elapsed since the last run. +- **Fail-open (DP-3).** If the marker cannot be read or is corrupted, cleanup + is **allowed** to proceed and the condition is logged via `tracing::warn!`. + The guard can never suppress cleanup forever. + +### Deletion safety guards + +Before every `remove_dir_all`: + +- **IV-4 (symlink check).** `symlink_metadata` is inspected first; a symlinked + `target` cannot cause deletion to follow the link out of the tree. +- **DP-1 (containment).** The path must `starts_with(repo_root)` **or** + `starts_with(state_root)`. Deletion roots remain a static code allow-list + (IV-5); they are never composed from environment input. + +## C4: build-heavy dispatch preflight + +**`dispatch_spawn_engineer` / build-heavy dispatch** +(`src/ooda_actions/advance_goal/spawn.rs`) + +Before dispatching a build-heavy goal session, the preflight probes the **root +filesystem `/`** through the reused `disk_pressure` gate (default threshold — +see C5) and maps the result: + +| `PressureLevel` | Preflight action | +|-----------------|------------------| +| `Ok` | Proceed. | +| `Warn` | Proceed, emit `tracing::warn!`. | +| `Refuse` | **Skip this cycle** — benign retry-next-cycle, loud `warn!`. No spawn, no silent fallback. | +| probe error | Proceed (Warn-equivalent), log the probe failure. | + +The `Refuse` skip is deliberately **benign**: the goal is not blocked or +failed, it is simply not dispatched until free space recovers, so a transient +disk-pressure spike cannot permanently starve progress. Its visibility comes +from the `warn!` line, never a swallowed error. + +## C5: `disk_pressure` reuse + +`src/disk_pressure/` is **reused unchanged**. Relevant surface: + +- `PressureLevel { Ok, Warn, Refuse }` +- `check_disk_pressure` / `check_disk_pressure_with` +- `DEFAULT_MIN_FREE_GB` = 20 +- `exceeds_admission_ceiling` (90% ceiling), `used_pct` + +Decision bands (from the module): + +- `free >= T` → `Ok` +- `T/2 <= free < T` → `Warn` +- `free < T/2` → `Refuse` + +where `T` is `SIMARD_DISK_PRESSURE_MIN_FREE_GB` in bytes. **No constants are +added or changed** by the #4803 fix. + +> **Concrete defaults.** With `T = DEFAULT_MIN_FREE_GB = 20 GiB`, `Warn` fires +> when free space drops below **20 GiB** and `Refuse` fires only below +> **10 GiB** (`T/2`). The `Refuse` line is half the `min-free` knob, not equal +> to it — an operator tuning `SIMARD_DISK_PRESSURE_MIN_FREE_GB` moves both +> bands together (`Warn` at the knob, `Refuse` at half the knob). + +## C6: coverage-artifact resolution (invariant) + +`target/llvm-cov-target` is produced under the relocated `CARGO_TARGET_DIR`, +not `./target`. Coverage runs point llvm-cov at the same env-consistent target +root, so relocation does not break coverage. This is a **verification gate**, +not a code change — CI coverage jobs must resolve artifacts under the relocated +root. + +## Tests + +| Area | Location | +|------|----------| +| C1 default-root + override precedence + IV-1 empty-string | `src/agent_supervisor/tests_tmux.rs` | +| C3 hysteresis, backoff, symlink/containment guards | inline `#[cfg(test)] mod tests` in `src/disk_health.rs` | +| C4 preflight `Refuse` → skip, probe-error → proceed | inline `#[cfg(test)] mod tests` in `src/ooda_actions/advance_goal/spawn.rs` | + +## Verification gates + +- `cargo build` green. +- Grep-gate: zero new `print!`/`println!`/`eprintln!`; zero `Bridge` naming. +- `/` stops saturating — emergency cleanup no longer re-fires each cycle. +- llvm-cov coverage artifacts resolve under the relocated target root. diff --git a/mkdocs.yml b/mkdocs.yml index caa691262..8e9531abf 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -162,6 +162,7 @@ nav: - Goal Stewardship Mode: concepts/stewardship-mode.md - The Simard Whisperer: concepts/simard-whisperer.md - Automated Disk-Health Management: concepts/automated-disk-health.md + - Root-Disk Saturation Relocation: concepts/root-disk-saturation-relocation.md - Agentic Disk Reclamation: concepts/agentic-disk-reclamation.md - Update-Check Design: concepts/update-check-design.md - The amplihack Freshness Gate: concepts/amplihack-freshness-gate.md @@ -259,6 +260,7 @@ nav: - Configure the Simard Whisperer: howto/configure-the-simard-whisperer.md - Configure the Monthly Self-Quality-Audit: howto/configure-self-quality-audit.md - Configure the Disk-Health Check: howto/configure-disk-health-check.md + - Relocate Build Artifacts Off the Root Volume: howto/relocate-build-artifacts-off-root-volume.md - Configure Disk Reclamation: howto/configure-disk-reclamation.md - Reclaim Disk Space (Low-Space Rust Builds): howto/reclaim-disk-space-and-run-low-space-rust-builds.md - Fix CI Linker OOM: howto/fix-ci-linker-oom.md @@ -466,6 +468,7 @@ nav: - Brain Introspection API: reference/brain-introspection-api.md - Self-Quality-Audit API: reference/self-quality-audit-api.md - Disk-Health API: reference/disk-health-api.md + - Build-Artifact Relocation & Disk-Thrash Guard: reference/build-artifact-relocation-and-disk-thrash-guard.md - Disk Reclaim API: reference/disk-reclaim-api.md - Disk Reclaim Telemetry: reference/disk-reclaim-telemetry.md - Dashboard E2E Tests: reference/dashboard-e2e-tests.md diff --git a/src/agent_supervisor/lifecycle/spawn.rs b/src/agent_supervisor/lifecycle/spawn.rs index d2ea0f14d..b5d4cf26f 100644 --- a/src/agent_supervisor/lifecycle/spawn.rs +++ b/src/agent_supervisor/lifecycle/spawn.rs @@ -59,12 +59,20 @@ pub fn spawn_subordinate(config: &SubordinateConfig) -> SimardResult = std::env::vars().collect(); + let target = crate::agent_supervisor::tmux::default_cargo_target_for_worktree( + &config.worktree_path, + &parent_pairs, + ); + cmd.env("CARGO_TARGET_DIR", target); } if let Some((out, err)) = open_agent_log(&config.agent_name) { diff --git a/src/agent_supervisor/tests_tmux.rs b/src/agent_supervisor/tests_tmux.rs index 77c63d0f3..3892cf773 100644 --- a/src/agent_supervisor/tests_tmux.rs +++ b/src/agent_supervisor/tests_tmux.rs @@ -240,16 +240,46 @@ fn compute_tmux_env_uses_per_worktree_default_when_parent_unset() { } #[test] -fn compute_tmux_env_default_uses_home_when_present() { - // Production case: the OODA daemon inherits HOME from the operator - // shell. Default must be /.cargo-targets/. +fn compute_tmux_env_default_relocates_off_home_when_present() { + // Issue #4803 (root-disk saturation): the OODA daemon inherits HOME from + // the operator shell, and HOME lives on the 28G `/` volume that saturates. + // The OLD contract routed the per-worktree cargo target dir to + // `/.cargo-targets/`, which piled `target/debug` + + // `target/llvm-cov-target` (0.7–2.6 GB each) onto `/` and drove the + // ~25-min emergency-cleanup crash-loop. The FIX drops the `$HOME` + // default branch so, absent an explicit `SIMARD_CARGO_TARGETS_ROOT` + // override, the default relocates onto the large-volume fallback + // `/tmp/simard-cargo-targets/` EVEN WHEN HOME IS SET. let config = make_test_config("e1", 0); let parent = vec![("HOME".to_string(), "/home/azureuser".to_string())]; let env = compute_tmux_env(&config, parent); assert_eq!( env_value(&env, "CARGO_TARGET_DIR"), - Some("/home/azureuser/.cargo-targets/worktree"), - "default must be /.cargo-targets/" + Some("/tmp/simard-cargo-targets/worktree"), + "with HOME set (no override), default must relocate to \ + /tmp/simard-cargo-targets/, NOT /.cargo-targets/" + ); +} + +#[test] +fn compute_tmux_env_default_never_lands_under_home() { + // Hard regression guard for issue #4803: the resolved CARGO_TARGET_DIR + // default must NEVER be placed under the HOME subtree (which is on the + // saturating `/` volume). This pins the ROOT-CAUSE fix: build artifacts + // must not refill `/`. + let config = make_test_config("e1", 0); + let home = "/home/azureuser"; + let parent = vec![("HOME".to_string(), home.to_string())]; + let env = compute_tmux_env(&config, parent); + let target = env_value(&env, "CARGO_TARGET_DIR").expect("CARGO_TARGET_DIR must be set"); + assert!( + !target.starts_with(home), + "CARGO_TARGET_DIR default must not live under HOME ({home}) — that is the \ + 28G `/`-volume path that saturates (#4803); got {target}" + ); + assert!( + !target.contains("/.cargo-targets/"), + "the $HOME/.cargo-targets default branch must be removed (#4803); got {target}" ); } diff --git a/src/agent_supervisor/tmux.rs b/src/agent_supervisor/tmux.rs index 96c4315c6..9b3c903ac 100644 --- a/src/agent_supervisor/tmux.rs +++ b/src/agent_supervisor/tmux.rs @@ -71,27 +71,33 @@ pub fn build_tmux_wrapped_command( argv } -/// Default fallback root for per-worktree cargo target dirs when neither -/// `CARGO_TARGET_DIR` nor `SIMARD_CARGO_TARGETS_ROOT` is set in the parent -/// env, AND `HOME` is also absent. Kept under `/tmp` so a missing-HOME -/// edge case can never escalate into writing target artifacts somewhere -/// the operator did not anticipate. +/// Default root for per-worktree cargo target dirs when +/// `SIMARD_CARGO_TARGETS_ROOT` is unset (issue #4803). This is the +/// large-volume relocation target: the 28G `/` volume that holds `$HOME` +/// and `~/.simard` saturates under accumulated `target/debug` + +/// `target/llvm-cov-target` artifacts, so the default deliberately routes +/// build artifacts off `/` onto the roomier `/tmp` volume. Operators who +/// have a dedicated data volume can point `SIMARD_CARGO_TARGETS_ROOT` at it. pub const DEFAULT_CARGO_TARGETS_ROOT_FALLBACK: &str = "/tmp/simard-cargo-targets"; -/// Default subdirectory of `$HOME` used as the per-worktree cargo targets -/// root when `SIMARD_CARGO_TARGETS_ROOT` is unset. +/// Default subdirectory of `$HOME` that HISTORICALLY held the per-worktree +/// cargo targets root (pre-issue #4803). The resolver no longer routes here +/// — `$HOME` lives on the saturating `/` volume — but the name is retained +/// so the legacy `cap_home_cargo_targets` cleanup (`cmd_cleanup/disk.rs`) +/// can still LRU-rotate any artifacts left behind by older daemons. pub const DEFAULT_CARGO_TARGETS_HOME_SUBDIR: &str = ".cargo-targets"; /// Compute the default `CARGO_TARGET_DIR` for an engineer worktree at -/// `worktree_path`. Pure — pulls `HOME` and `SIMARD_CARGO_TARGETS_ROOT` -/// from `parent_pairs` only (never `std::env`). +/// `worktree_path`. Pure — pulls `SIMARD_CARGO_TARGETS_ROOT` from +/// `parent_pairs` only (never `std::env`). /// -/// Resolution order: +/// Resolution order (issue #4803 — the `$HOME` branch was removed): /// 1. `/` if the env var is set. -/// 2. `/.cargo-targets/` if `HOME` is set in -/// parent_pairs (the production case — the OODA daemon always inherits -/// `HOME` from the operator shell). -/// 3. `/tmp/simard-cargo-targets/` as a last-resort fallback. +/// 2. `/tmp/simard-cargo-targets/` otherwise — the +/// large-volume default. The previous `/.cargo-targets/...` default +/// piled build artifacts onto the 28G `/` volume and drove the ~25-min +/// emergency-cleanup crash-loop, so it is no longer used even when `HOME` +/// is set. /// /// The basename is taken from `worktree_path.file_name()`. If the path has /// no terminal component (extremely unlikely — would require `/`), the @@ -99,7 +105,10 @@ pub const DEFAULT_CARGO_TARGETS_HOME_SUBDIR: &str = ".cargo-targets"; /// path is still well-formed and per-engineer (the worktree path's full /// hash gets folded in by callers via the directory layout, but for this /// purely defensive branch we accept a shared fallback dir). -fn default_cargo_target_for_worktree( +/// +/// `pub(crate)` so the direct-exec spawn path (`lifecycle/spawn.rs`) shares +/// this single source of truth instead of hardcoding a divergent root. +pub(crate) fn default_cargo_target_for_worktree( worktree_path: &Path, parent_pairs: &[(String, String)], ) -> String { @@ -117,8 +126,14 @@ fn default_cargo_target_for_worktree( .filter(|v| !v.is_empty()) }; + // Issue #4803: the `$HOME` default branch is REMOVED. `HOME` lives on the + // 28G `/` volume that saturates; routing per-worktree cargo target dirs + // there piled `target/debug` + `target/llvm-cov-target` (0.7–2.6 GB each) + // onto `/` and drove the ~25-min emergency-cleanup crash-loop. Absent an + // explicit `SIMARD_CARGO_TARGETS_ROOT` override, the default now relocates + // onto the large-volume fallback `/tmp/simard-cargo-targets` EVEN WHEN + // HOME IS SET, so build artifacts stop refilling `/`. let root = lookup("SIMARD_CARGO_TARGETS_ROOT") - .or_else(|| lookup("HOME").map(|h| format!("{h}/{DEFAULT_CARGO_TARGETS_HOME_SUBDIR}"))) .unwrap_or_else(|| DEFAULT_CARGO_TARGETS_ROOT_FALLBACK.to_string()); format!("{root}/{basename}") @@ -143,11 +158,12 @@ fn default_cargo_target_for_worktree( /// cargo target dir (which would deadlock cargo's file lock or corrupt /// incremental output). The default is /// `/`, where `` resolves -/// in this order: +/// in this order (issue #4803 relocated the default off the `/` volume): /// 1. `SIMARD_CARGO_TARGETS_ROOT` env (operator override), -/// 2. `/.cargo-targets` (production default), -/// 3. `/tmp/simard-cargo-targets` (last-resort fallback when HOME -/// is absent — should never happen under the OODA daemon). +/// 2. `/tmp/simard-cargo-targets` (large-volume default). The former +/// `/.cargo-targets` default was removed: `$HOME` is on the 28G +/// `/` volume that saturates, so per-worktree target dirs there refilled +/// `/` within one build cycle and thrashed the emergency cleanup. /// /// This intentionally REPLACES the previous shared /// `/tmp/simard-engineer-target` default: that path caused 7-12 GB diff --git a/src/disk_health.rs b/src/disk_health.rs index 908fc20f4..1f6296e0c 100644 --- a/src/disk_health.rs +++ b/src/disk_health.rs @@ -63,31 +63,187 @@ fn resolve_recipe_path(repo_root: &Path, home_override: Option<&Path>) -> Option None } +/// High-watermark: emergency cleanup TRIGGERS at or above this used-% (#4803). +pub(crate) const EMERGENCY_HIGH_WATERMARK_PCT: u8 = 95; + +/// Low-watermark: the re-arm line of the hysteresis band (#4803). Documented +/// as the used-% the disk must fall back below before a fresh trigger is +/// semantically "re-armed". Anti-thrash is enforced concretely by the +/// persistent time-backoff below; this constant pins the band so `high > low` +/// is a real two-edge gate, never a single edge that re-fires every timer tick. +pub(crate) const EMERGENCY_LOW_WATERMARK_PCT: u8 = 85; + +/// Default minimum seconds between two emergency cleanups (#4803). 15 min is +/// longer than the observed ~25-min refill cadence (one fire per timer tick), +/// so a single build window can never trigger a second delete-then-rebuild +/// storm. Override via `SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS`. +pub(crate) const DEFAULT_EMERGENCY_MIN_REFIRE_SECS: u64 = 900; + +/// Upper clamp for the min-refire backoff (24h) so a fat-fingered override +/// can't wedge cleanup off for days. +const EMERGENCY_MIN_REFIRE_CEILING_SECS: u64 = 86_400; + +/// Parse `SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS`, clamping to `[0, 86400]`. +/// Unset / empty / unparseable / negative → the 900s default. An explicit `0` +/// disables the backoff gate (documented escape hatch). +pub(crate) fn emergency_refire_min_secs_from(raw: Option<&str>) -> u64 { + match raw.map(str::trim) { + Some(s) if !s.is_empty() => match s.parse::() { + Ok(n) => n.min(EMERGENCY_MIN_REFIRE_CEILING_SECS), + Err(_) => DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + }, + _ => DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + } +} + +/// Pure hysteresis + time-backoff decision for the emergency cleanup (#4803). +/// +/// Returns `true` iff cleanup should fire now: +/// - `pct` must be at or above [`EMERGENCY_HIGH_WATERMARK_PCT`] (95%); below +/// that (including at the 85% low watermark) it never fires. +/// - If `min_refire_secs == 0` the backoff is disabled and any at/above-high +/// tick fires. +/// - Otherwise a prior run within `min_refire_secs` SUPPRESSES the re-fire +/// (the anti-thrash contract); once the window elapses — or there is no +/// prior run — a genuinely-full disk may fire again. +pub(crate) fn should_fire_emergency_cleanup( + pct: u8, + last_run: Option, + now: std::time::SystemTime, + min_refire_secs: u64, +) -> bool { + if pct < EMERGENCY_HIGH_WATERMARK_PCT { + return false; + } + if min_refire_secs == 0 { + return true; + } + match last_run { + None => true, + Some(prev) => match now.duration_since(prev) { + Ok(elapsed) => elapsed.as_secs() >= min_refire_secs, + // Clock skew (prev in the future): treat as "just ran" and + // suppress, so a backwards clock can't reopen the thrash loop. + Err(_) => false, + }, + } +} + +/// Path of the persistent last-emergency-cleanup marker under +/// `/disk-health/`. +fn emergency_marker_path(state_root: &Path) -> PathBuf { + state_root + .join("disk-health") + .join("last-emergency-cleanup") +} + +/// Read the last-run timestamp (unix secs) from the marker. Fail-open: any +/// I/O or parse error is treated as "no prior run" (returns `None`) so a +/// missing/corrupt marker can never wedge a genuinely-needed cleanup. +fn read_emergency_marker(path: &Path) -> Option { + let raw = std::fs::read_to_string(path).ok()?; + let secs: u64 = raw.trim().parse().ok()?; + Some(std::time::UNIX_EPOCH + std::time::Duration::from_secs(secs)) +} + +/// Persist `now` as the last-run timestamp. Fail-open with a warn log: if the +/// marker cannot be written the next tick simply lacks backoff (it stays gated +/// by the high watermark), which is strictly safer than aborting the cleanup +/// that just freed space. +fn write_emergency_marker(path: &Path, now: std::time::SystemTime) { + let secs = now + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + if let Some(parent) = path.parent() + && let Err(e) = std::fs::create_dir_all(parent) + { + warn!( + target: "simard::disk_health", + path = %parent.display(), + error = %e, + "could not create disk-health marker dir; emergency backoff not persisted this cycle", + ); + return; + } + if let Err(e) = std::fs::write(path, secs.to_string()) { + warn!( + target: "simard::disk_health", + path = %path.display(), + error = %e, + "could not write emergency-cleanup marker; backoff not persisted this cycle", + ); + } +} + +/// Guard every destructive `remove_dir_all` (#4803): refuse to delete a path +/// that is a symlink (following it could redirect the delete outside the tree) +/// or that is not contained within one of the `allowed_roots` +/// (`repo_root` / `state_root`). Uses `symlink_metadata` so a symlink is +/// detected WITHOUT being followed. +fn safe_to_remove(path: &Path, allowed_roots: &[&Path]) -> bool { + match std::fs::symlink_metadata(path) { + Ok(md) if md.file_type().is_symlink() => { + warn!( + target: "simard::disk_health", + path = %path.display(), + "refusing to remove symlinked build-artifact path (containment guard)", + ); + false + } + Ok(_) => allowed_roots.iter().any(|root| path.starts_with(root)), + Err(_) => false, + } +} + /// Deterministic emergency disk cleanup — no LLM, no recipe, just rm. /// -/// Runs when disk usage is critically high (≥95%). Deletes known-safe build -/// artifacts that can always be regenerated by `cargo build`: +/// Runs when disk usage is critically high (≥95%) AND the anti-thrash backoff +/// window is clear (#4803). Deletes known-safe build artifacts that can always +/// be regenerated by `cargo build`: /// - `repo_root/target/debug/` (main build cache) /// - `repo_root/worktrees/*/target/` (engineer worktree build caches) /// - `repo_root/target/llvm-cov-target/` (coverage artifacts) /// - `state_root/cargo-target/` and `state_root/shared-target/` /// - stale backups beyond the 2 most recent /// -/// Returns a report of what was done, or None if disk is below threshold. +/// Returns a report of what was done, or None if disk is below threshold or the +/// min-refire backoff suppressed this tick. pub fn emergency_cleanup(repo_root: &Path, state_root: &Path) -> Option { let pct = get_disk_usage_pct(repo_root)?; - if pct < 95 { + // Issue #4803: hysteresis high watermark + persistent time-backoff. The + // old `pct < 95` edge re-fired every timer tick, deleting target/debug + + // target/llvm-cov-target only for cargo to instantly rebuild and refill + // `/` within one cycle (~25-min thrash). The backoff marker makes a second + // fire within one build window impossible. + let min_refire = emergency_refire_min_secs_from( + std::env::var("SIMARD_DISK_EMERGENCY_MIN_REFIRE_SECS") + .ok() + .as_deref(), + ); + let marker = emergency_marker_path(state_root); + let last_run = read_emergency_marker(&marker); + let now = std::time::SystemTime::now(); + + if !should_fire_emergency_cleanup(pct, last_run, now, min_refire) { return None; } - warn!(disk_pct = pct, "Emergency disk cleanup triggered (≥95%)"); + warn!( + disk_pct = pct, + min_refire_secs = min_refire, + high_watermark = EMERGENCY_HIGH_WATERMARK_PCT, + low_watermark = EMERGENCY_LOW_WATERMARK_PCT, + "Emergency disk cleanup triggered (≥95% high watermark; backoff window clear)" + ); + let allowed_roots: [&Path; 2] = [repo_root, state_root]; let mut freed: u64 = 0; let mut actions: Vec = Vec::new(); // 1. Main target/debug/ — the single biggest consumer let debug_dir = repo_root.join("target/debug"); - if debug_dir.is_dir() { + if debug_dir.is_dir() && safe_to_remove(&debug_dir, &allowed_roots) { let size = dir_size_bytes(&debug_dir); if std::fs::remove_dir_all(&debug_dir).is_ok() { freed += size; @@ -97,7 +253,7 @@ pub fn emergency_cleanup(repo_root: &Path, state_root: &Path) -> Option Option Option Option low, + "high watermark ({high}) must exceed low watermark ({low})" + ); + assert_eq!( + EMERGENCY_HIGH_WATERMARK_PCT, 95, + "high watermark must remain the documented ≥95% trigger" + ); + assert_eq!( + EMERGENCY_LOW_WATERMARK_PCT, 85, + "low watermark must remain the documented 85% re-arm line" + ); + } + + #[test] + fn below_high_watermark_never_fires() { + let now = SystemTime::now(); + // No prior run and 94% is still below the 95% trigger. + assert!( + !should_fire_emergency_cleanup(94, None, now, DEFAULT_EMERGENCY_MIN_REFIRE_SECS), + "94% (below high watermark) must not trigger cleanup even on first look" + ); + assert!( + !should_fire_emergency_cleanup( + EMERGENCY_LOW_WATERMARK_PCT, + None, + now, + DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + ), + "at the low watermark cleanup must not fire" + ); + } + + #[test] + fn at_or_above_high_watermark_fires_on_first_run() { + let now = SystemTime::now(); + assert!( + should_fire_emergency_cleanup(95, None, now, DEFAULT_EMERGENCY_MIN_REFIRE_SECS), + "≥95% with no prior run must fire (no backoff marker yet)" + ); + assert!( + should_fire_emergency_cleanup(100, None, now, DEFAULT_EMERGENCY_MIN_REFIRE_SECS), + "100% with no prior run must fire" + ); + } + + #[test] + fn backoff_suppresses_refire_within_window() { + // This is the core anti-thrash contract: even at 99% full, if the + // last cleanup ran more recently than the min-refire window, the + // next tick must be SUPPRESSED — otherwise we recreate the observed + // delete-then-rebuild-then-delete crash-loop. + let now = SystemTime::now(); + let last_run = now - Duration::from_secs(60); // 1 min ago + assert!( + !should_fire_emergency_cleanup(99, Some(last_run), now, 900), + "a cleanup 60s ago with a 900s window must be suppressed (anti-thrash)" + ); + } + + #[test] + fn backoff_allows_refire_after_window() { + let now = SystemTime::now(); + let last_run = now - Duration::from_secs(1_000); // > 900s window + assert!( + should_fire_emergency_cleanup(99, Some(last_run), now, 900), + "once the min-refire window has elapsed, a genuinely-full disk may fire again" + ); + } + + #[test] + fn zero_min_refire_disables_backoff() { + // An explicit 0 disables the backoff gate: every tick at/above the + // high watermark fires. This is the documented escape hatch. + let now = SystemTime::now(); + let last_run = now - Duration::from_secs(1); + assert!( + should_fire_emergency_cleanup(96, Some(last_run), now, 0), + "min_refire_secs == 0 must disable the backoff gate" + ); + } + + #[test] + fn refire_secs_default_when_unset_or_invalid() { + assert_eq!( + emergency_refire_min_secs_from(None), + DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + "unset env must yield the 900s default" + ); + assert_eq!( + emergency_refire_min_secs_from(Some("")), + DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + "empty string must fall back to default" + ); + assert_eq!( + emergency_refire_min_secs_from(Some("not-a-number")), + DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + "unparseable value must fall back to default" + ); + assert_eq!( + emergency_refire_min_secs_from(Some("-5")), + DEFAULT_EMERGENCY_MIN_REFIRE_SECS, + "negative value must fall back to default (u64 parse fails)" + ); + } + + #[test] + fn refire_secs_parses_and_clamps() { + assert_eq!( + emergency_refire_min_secs_from(Some("300")), + 300, + "a valid in-range value must be honored" + ); + assert_eq!( + emergency_refire_min_secs_from(Some("0")), + 0, + "an explicit 0 (disable backoff) is in-range and honored" + ); + assert_eq!( + emergency_refire_min_secs_from(Some("99999999")), + 86_400, + "values above the 86400s (24h) ceiling must clamp down" + ); + } } diff --git a/src/ooda_actions/advance_goal/spawn.rs b/src/ooda_actions/advance_goal/spawn.rs index fb66352f9..af1ae4a7b 100644 --- a/src/ooda_actions/advance_goal/spawn.rs +++ b/src/ooda_actions/advance_goal/spawn.rs @@ -193,6 +193,48 @@ fn posture_observe_only_refusal( /// /// Takes the shared state behind a `Mutex` and holds it only for short /// critical sections (assignment re-check, goal lookup, status writeback). +/// Verdict of the build-heavy dispatch preflight (issue #4803). +#[derive(Debug)] +pub(crate) enum BuildDispatchPreflight { + /// Root `/` has adequate free space (or the probe failed and we fail + /// open) — the engineer spawn may proceed. + Proceed, + /// Root `/` is critically low — SKIP this spawn this cycle. Carries the + /// operator-facing reason for the benign retry-next-cycle outcome. + Skip { detail: String }, +} + +/// Pure decision seam mapping a `/`-filesystem disk-pressure probe to a +/// Proceed / Skip verdict for the build-heavy engineer dispatch (issue #4803). +/// +/// - `Ok(Ok)` / `Ok(Warn)` → Proceed (Warn is logged upstream by +/// `check_with_default_threshold`; it is not fatal to a single spawn). +/// - `Ok(Refuse)` → Skip (NO silent fallback — the caller must not +/// spawn; it returns a benign retry-next-cycle outcome). +/// - `Err(probe)` → Proceed (fail-open: a `statvfs` error must not +/// wedge all goal advancement; it is logged, not fatal). +pub(crate) fn build_dispatch_preflight( + probe: Result, +) -> BuildDispatchPreflight { + use crate::disk_pressure::PressureLevel; + match probe { + Ok(PressureLevel::Ok) | Ok(PressureLevel::Warn) => BuildDispatchPreflight::Proceed, + Ok(PressureLevel::Refuse) => BuildDispatchPreflight::Skip { + detail: "spawn_engineer skipped: root filesystem '/' is critically low on free \ + space; deferring build-heavy dispatch to next cycle (#4803)" + .to_string(), + }, + Err(e) => { + tracing::warn!( + target: "simard::ooda_brain", + error = %e, + "build-dispatch preflight: '/' disk probe failed; failing open (proceeding) (#4803)", + ); + BuildDispatchPreflight::Proceed + } + } +} + /// The slow work — target-repo resolution, git worktree allocation, and the /// detached subprocess spawn — runs WITHOUT the lock held, so multiple /// engineers start concurrently within one OODA round (bounded by the AIMD @@ -549,6 +591,34 @@ pub fn dispatch_spawn_engineer( }; let task = engineer_task_base.as_str(); + // Issue #4803: build-heavy dispatch preflight. Spawning an engineer + // allocates a worktree and launches parallel `cargo` builds that write GBs + // of artifacts. BEFORE that, probe the ROOT filesystem `/` (which holds + // `~/.simard` and, historically, the cargo target dirs) against the shared + // disk-pressure threshold. A `Refuse` verdict means `/` is critically low; + // SKIP this spawn (benign retry-next-cycle, loud warn) rather than pile + // another build onto a saturating volume and cascade into the observed + // cognitive-lock / typed 'database is locked' / memory-IPC write failures. + // A probe error fails OPEN (proceed but log) so a `statvfs` hiccup can + // never wedge all goal advancement. Reuses `disk_pressure` — no new + // thresholds — and probes `/` specifically (distinct from the + // resource-admission gate below, which weighs the worktrees-root volume). + { + let probe = + crate::disk_pressure::check_with_default_threshold(Path::new("/")).map(|r| r.level); + match build_dispatch_preflight(probe) { + BuildDispatchPreflight::Proceed => {} + BuildDispatchPreflight::Skip { detail } => { + tracing::warn!( + target: "simard::ooda_brain", + goal = %goal_id, + "{detail}", + ); + return make_outcome(action, true, detail); + } + } + } + // Issue #2706: resource-aware engineer ADMISSION gate. AFTER the overlap gate // and BEFORE worktree allocation. Spawning another engineer allocates a git // worktree and runs parallel `cargo` builds; this gate weighs the HOST @@ -1071,8 +1141,56 @@ mod tests { .expect("epoch suffix must parse even with empty goal id"); } - // ── truncate_for_log ─────────────────────────────────────────────────── + // ── build-heavy dispatch preflight (issue #4803, TDD) ─────────────────── + // + // Before allocating a worktree and launching parallel `cargo` builds, the + // dispatcher probes the root filesystem `/` via + // `disk_pressure::check_with_default_threshold`. These pin the pure + // decision seam that maps that probe to a Proceed / Skip verdict: + // - Ok / Warn → Proceed (build may run; Warn is logged upstream). + // - Refuse → Skip (benign retry-next-cycle skip, loud warn!, + // NEVER a silent fallback that spawns anyway). + // - Err(probe) → Proceed (fail-open: a statvfs error must not wedge all + // goal advancement; it is logged, not fatal). + + #[test] + fn preflight_ok_pressure_proceeds() { + let decision = build_dispatch_preflight(Ok(crate::disk_pressure::PressureLevel::Ok)); + assert!( + matches!(decision, BuildDispatchPreflight::Proceed), + "PressureLevel::Ok must permit the build-heavy dispatch to proceed, got {decision:?}" + ); + } + + #[test] + fn preflight_warn_pressure_proceeds() { + let decision = build_dispatch_preflight(Ok(crate::disk_pressure::PressureLevel::Warn)); + assert!( + matches!(decision, BuildDispatchPreflight::Proceed), + "PressureLevel::Warn proceeds (warn is emitted upstream), got {decision:?}" + ); + } + #[test] + fn preflight_refuse_pressure_skips() { + let decision = build_dispatch_preflight(Ok(crate::disk_pressure::PressureLevel::Refuse)); + assert!( + matches!(decision, BuildDispatchPreflight::Skip { .. }), + "PressureLevel::Refuse must SKIP the spawn (no silent fallback), got {decision:?}" + ); + } + + #[test] + fn preflight_probe_error_fails_open_and_proceeds() { + let err = std::io::Error::other("statvfs failed"); + let decision = build_dispatch_preflight(Err(err)); + assert!( + matches!(decision, BuildDispatchPreflight::Proceed), + "a probe error must fail OPEN (proceed but log), never wedge advancement, got {decision:?}" + ); + } + + // ── truncate_for_log ─────────────────────────────────────────────────── #[test] fn truncate_for_log_passes_short_strings_through_unchanged() { let s = "a short, safe log line"; From b175c316c8e108a44675ae82a1e3c8d8f7c9703b Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 03:47:43 +0000 Subject: [PATCH 2/3] refactor(ooda): reunite dispatch_spawn_engineer doc block split by #4803 preflight Step 9 (refactor/simplify) for #4803. The build-heavy dispatch preflight (BuildDispatchPreflight enum + build_dispatch_preflight fn) was inserted into the MIDDLE of dispatch_spawn_engineer's doc-comment block, so its first paragraph wrongly documented the new enum while dispatch_spawn_engineer kept only the second half. Move the new items ahead of that doc block so each item is documented correctly. Pure reordering, no behavior change; targeted tests (disk_health, preflight, tmux, tests_tmux) green, spawn.rs fmt-clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/ooda_actions/advance_goal/spawn.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ooda_actions/advance_goal/spawn.rs b/src/ooda_actions/advance_goal/spawn.rs index af1ae4a7b..419088e65 100644 --- a/src/ooda_actions/advance_goal/spawn.rs +++ b/src/ooda_actions/advance_goal/spawn.rs @@ -187,12 +187,6 @@ fn posture_observe_only_refusal( )) } -/// Spawn a subordinate engineer for a goal that the LLM picked -/// `spawn_engineer` for, then mutate the active board to record the -/// assignment. -/// -/// Takes the shared state behind a `Mutex` and holds it only for short -/// critical sections (assignment re-check, goal lookup, status writeback). /// Verdict of the build-heavy dispatch preflight (issue #4803). #[derive(Debug)] pub(crate) enum BuildDispatchPreflight { @@ -235,6 +229,13 @@ pub(crate) fn build_dispatch_preflight( } } +/// Spawn a subordinate engineer for a goal that the LLM picked +/// `spawn_engineer` for, then mutate the active board to record the +/// assignment. +/// +/// Takes the shared state behind a `Mutex` and holds it only for short +/// critical sections (assignment re-check, goal lookup, status writeback). +/// /// The slow work — target-repo resolution, git worktree allocation, and the /// detached subprocess spawn — runs WITHOUT the lock held, so multiple /// engineers start concurrently within one OODA round (bounded by the AIMD From 9e6b6d8b630674b9742d01f9b8a606c49bf33ca2 Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 03:56:53 +0000 Subject: [PATCH 3/3] wip: checkpoint after review feedback (steps 10-11) Automatic checkpoint to preserve review-addressed changes. Saved before running pre-commit hooks and tests. --- src/disk_reclaim/executor.rs | 406 ++++++++++++++++++++- src/disk_reclaim/mod.rs | 5 +- src/operator_commands_ooda/daemon/mod.rs | 16 + tests/disk_reclaim_build_artifact_prune.rs | 86 +++++ 4 files changed, 510 insertions(+), 3 deletions(-) create mode 100644 tests/disk_reclaim_build_artifact_prune.rs diff --git a/src/disk_reclaim/executor.rs b/src/disk_reclaim/executor.rs index 1b8dd2c8f..93e037edf 100644 --- a/src/disk_reclaim/executor.rs +++ b/src/disk_reclaim/executor.rs @@ -12,11 +12,14 @@ use std::process::Command; use serde::Serialize; use crate::disk_pressure::check::{DiskStatProvider, used_pct}; +use crate::worktree_gc::liveness::LiveProcessProbe; use crate::worktree_gc::under_any_root; use super::ReclaimMode; use super::candidate::{CandidateKind, ReclaimCandidate}; -use super::guard::{GuardContext, ReclaimPrimitive, RejectReason, Verdict, vet_candidate}; +use super::guard::{ + GuardContext, ReclaimPrimitive, RejectReason, SizeMeasurer, Verdict, vet_candidate, +}; /// A path the executor removed (or would remove, in dry-run). #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -245,6 +248,233 @@ pub fn exec_reclaim( report } +/// The regenerable Cargo build-artifact subpaths (relative to a build tree) that +/// routine reclaim proactively prunes **below** the emergency threshold. +/// +/// Root cause of issue #4825: these directories live under the daemon's +/// protected working dir (`worktrees/main`), so the worktree rails +/// ([`vet_candidate`]) reject them as [`RejectReason::ProtectedPath`] and only +/// the ≥95% Tier-1 emergency net (`disk_health::emergency_cleanup`) ever removed +/// them — after `/home` had already oscillated up to 99%. Each entry is fully +/// reconstructable by `cargo build`, so removing it frees GiB without the crash +/// risk of removing the working directory itself. +/// +/// Deliberately a **closed, non-operator-widenable** allow-list: the carve-out +/// from the protected deny-set can never grow to cover non-regenerable data. +pub const REGENERABLE_BUILD_ARTIFACTS: [&str; 2] = ["target/debug", "target/llvm-cov-target"]; + +/// The structured outcome of one proactive build-artifact prune pass. Mirrors +/// the shape of [`ReclaimReport`] so telemetry and logs stay consistent. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct BuildArtifactPruneReport { + pub mode: ReclaimMode, + /// Artifacts actually removed (apply mode only). + pub removed: Vec, + /// Artifacts that would be removed (dry-run mode only). + pub would_remove: Vec, + /// Artifacts skipped because a live process referenced them (fail-closed). + pub skipped_live: Vec, + /// Individual removals that failed (does not abort the pass). + pub failures: Vec, + pub bytes_freed: u64, +} + +impl BuildArtifactPruneReport { + fn new(mode: ReclaimMode) -> Self { + Self { + mode, + removed: Vec::new(), + would_remove: Vec::new(), + skipped_live: Vec::new(), + failures: Vec::new(), + bytes_freed: 0, + } + } + + /// Whether this pass removed anything. + pub fn pruned_any(&self) -> bool { + self.bytes_freed > 0 || !self.removed.is_empty() + } + + /// Daemon one-liner summary. + pub fn summary(&self) -> String { + format!( + "build-artifact prune: freed {} bytes, {} removed, {} would-remove, {} live-skipped, {} failed", + self.bytes_freed, + self.removed.len(), + self.would_remove.len(), + self.skipped_live.len(), + self.failures.len(), + ) + } +} + +/// Proactively prune the [`REGENERABLE_BUILD_ARTIFACTS`] under `build_tree` +/// through the injectable `remover` seam (tests record; production `rm -rf`s). +/// +/// This is intentionally **separate** from [`vet_candidate`]: those artifacts +/// deliberately sit under the protected working dir, so routing them through the +/// worktree deny-set would (correctly, for every other path) reject them. This +/// pass applies its own tight, fail-closed policy for the closed artifact +/// allow-list only, leaving protected-path vetting fully intact for all +/// non-build paths: +/// - the path must be a **real directory** — never a symlink (a symlinked +/// `target/debug` could redirect the removal outside the build tree); +/// - a **live-PID veto** (fail-closed) skips any artifact a running process +/// still references; +/// - dry-run performs zero destructive ops (records `would_remove`). +pub fn prune_regenerable_build_artifacts( + build_tree: &Path, + mode: ReclaimMode, + live_probe: &dyn LiveProcessProbe, + measurer: &dyn SizeMeasurer, + remover: &dyn PathRemover, +) -> BuildArtifactPruneReport { + let mut report = BuildArtifactPruneReport::new(mode); + + for rel in REGENERABLE_BUILD_ARTIFACTS { + let path = build_tree.join(rel); + + // Fail-closed existence/type check via `symlink_metadata` (does NOT + // follow the final symlink): must be a real directory. Absent → nothing + // to prune; a symlink or non-dir → refuse (never chase a redirection). + let meta = match std::fs::symlink_metadata(&path) { + Ok(meta) => meta, + Err(_) => continue, + }; + if meta.file_type().is_symlink() || !meta.is_dir() { + tracing::warn!( + target: "simard::disk_reclaim", + path = %path.display(), + "skipping build-artifact prune: not a real directory (symlink or non-dir)", + ); + continue; + } + + // Live-PID veto (fail-closed): never yank a build dir a live process + // still holds; the ≥95% emergency net remains the backstop. + if live_probe.worktree_has_live_process(&path) { + tracing::warn!( + target: "simard::disk_reclaim", + path = %path.display(), + "skipping build-artifact prune: live process references the path", + ); + report.skipped_live.push(path); + continue; + } + + let bytes = measurer.measure(&path); + let entry = RemovedPath { + path: path.clone(), + kind: CandidateKind::StaleBuildCache, + bytes, + primitive: ReclaimPrimitive::RemoveDir, + }; + + match mode { + ReclaimMode::DryRun => { + tracing::info!( + target: "simard::disk_reclaim", + path = %path.display(), + bytes, + "build-artifact prune (dry-run): would remove regenerable Cargo artifact", + ); + report.would_remove.push(entry); + } + ReclaimMode::Apply => match remover.remove(ReclaimPrimitive::RemoveDir, &path) { + Ok(()) => { + tracing::info!( + target: "simard::disk_reclaim", + path = %path.display(), + bytes, + "build-artifact prune: removed regenerable Cargo artifact", + ); + report.bytes_freed = report.bytes_freed.saturating_add(bytes); + report.removed.push(entry); + } + Err(error) => { + tracing::warn!( + target: "simard::disk_reclaim", + path = %path.display(), + error = %error, + "build-artifact prune failed", + ); + report.failures.push(ReclaimFailure { path, error }); + } + }, + } + } + + report +} + +/// Production entry for the daemon's Tier-3 coordination: prune the regenerable +/// build artifacts under `build_tree`, wiring the live probe, size measurer, and +/// a hardened [`RealPathRemover`] whose allow-roots are scoped to **exactly** +/// those artifact directories (so the TOCTOU containment re-assert passes for +/// the carve-out without widening any other allow-root). Refuses to delete as +/// root (defense in depth, mirroring [`super::reclaim_candidates`]). +pub fn prune_build_tree_artifacts( + build_tree: &Path, + mode: ReclaimMode, +) -> BuildArtifactPruneReport { + let effective_mode = if mode == ReclaimMode::Apply && super::is_root() { + tracing::warn!( + target: "simard::disk_reclaim", + "refusing build-artifact prune apply as root (euid 0); downgraded to dry-run \ + — deletion would nullify the path-ownership policy", + ); + ReclaimMode::DryRun + } else { + mode + }; + + let allow_roots: Vec = REGENERABLE_BUILD_ARTIFACTS + .iter() + .map(|rel| build_tree.join(rel)) + .collect(); + let remover = RealPathRemover { + // `RemoveDir` never consults `parent_repo`. + parent_repo: PathBuf::new(), + allow_roots, + }; + let live = crate::worktree_gc::ProcfsLiveProcessProbe::new(); + let du = super::guard::DuSizeMeasurer; + let measurer = super::guard::CachingSizeMeasurer::new(&du); + + let report = + prune_regenerable_build_artifacts(build_tree, effective_mode, &live, &measurer, &remover); + emit_build_artifact_prune_telemetry(&report); + report +} + +/// Emit the `simard.disk.reclaim.*` counters for one build-artifact prune, +/// reusing the existing reclaim series (source = `daemon`, kind = +/// `stale_build_cache`) so dashboards need no new instruments. +fn emit_build_artifact_prune_telemetry(report: &BuildArtifactPruneReport) { + use crate::telemetry::{names, registry}; + + const SRC: &str = "daemon"; + + if report.bytes_freed > 0 { + registry::counter_add( + names::DISK_RECLAIM_BYTES_FREED, + report.bytes_freed, + &[(names::ATTR_SOURCE, SRC)], + ); + } + for _removed in &report.removed { + registry::counter_add( + names::DISK_RECLAIM_PATHS_REMOVED, + 1, + &[ + (names::ATTR_SOURCE, SRC), + (names::ATTR_KIND, "stale_build_cache"), + ], + ); + } +} + #[cfg(test)] mod tests { use super::*; @@ -597,4 +827,178 @@ mod tests { "disk reclaim: 88% -> 84% used, freed 12026531840 bytes, 0 paths removed, 0 skipped for review", ); } + + // ---- issue #4825: regenerable build-artifact prune ---------------- + + /// Create `build_tree/target/debug` and `build_tree/target/llvm-cov-target` + /// as real dirs; return the tempdir + the two artifact paths. + fn build_tree_with_artifacts() -> (TempDir, PathBuf, PathBuf) { + let root = TempDir::new().unwrap(); + let debug = root.path().join("target/debug"); + let cov = root.path().join("target/llvm-cov-target"); + std::fs::create_dir_all(&debug).unwrap(); + std::fs::create_dir_all(&cov).unwrap(); + (root, debug, cov) + } + + #[test] + fn prune_removes_both_regenerable_artifacts_in_apply() { + let (root, debug, cov) = build_tree_with_artifacts(); + let live = FakeLiveProcessProbe::default(); + let measurer = MapMeasurer::default(); + measurer.set(&debug, 2_000_000_000); + measurer.set(&cov, 600_000_000); + let remover = RecordingRemover::default(); + + let report = prune_regenerable_build_artifacts( + root.path(), + ReclaimMode::Apply, + &live, + &measurer, + &remover, + ); + + assert_eq!(report.removed.len(), 2, "both artifacts must be removed"); + assert_eq!(report.bytes_freed, 2_600_000_000); + assert!(report.pruned_any()); + assert!(report.skipped_live.is_empty()); + assert!(report.failures.is_empty()); + let calls = remover.calls.borrow(); + assert!(calls.iter().all(|(p, _)| *p == ReclaimPrimitive::RemoveDir)); + let removed_paths: Vec<_> = calls.iter().map(|(_, p)| p.clone()).collect(); + assert!(removed_paths.contains(&debug)); + assert!(removed_paths.contains(&cov)); + } + + #[test] + fn prune_dry_run_deletes_nothing() { + let (root, debug, _cov) = build_tree_with_artifacts(); + let live = FakeLiveProcessProbe::default(); + let measurer = MapMeasurer::default(); + measurer.set(&debug, 1_000); + let remover = RecordingRemover::default(); + + let report = prune_regenerable_build_artifacts( + root.path(), + ReclaimMode::DryRun, + &live, + &measurer, + &remover, + ); + + assert!(report.removed.is_empty(), "dry-run must not remove"); + assert_eq!(report.bytes_freed, 0); + assert_eq!(report.would_remove.len(), 2); + assert!(!report.pruned_any()); + assert!( + remover.calls.borrow().is_empty(), + "dry-run must never call the destructive remover", + ); + } + + #[test] + fn prune_live_process_veto_skips_that_artifact_only() { + let (root, debug, cov) = build_tree_with_artifacts(); + let live = FakeLiveProcessProbe::default(); + live.mark_live(debug.clone()); + let measurer = MapMeasurer::default(); + measurer.set(&cov, 500); + let remover = RecordingRemover::default(); + + let report = prune_regenerable_build_artifacts( + root.path(), + ReclaimMode::Apply, + &live, + &measurer, + &remover, + ); + + // The live-held debug dir is vetoed (fail-closed); cov is still reclaimed. + assert_eq!(report.skipped_live, vec![debug.clone()]); + assert_eq!(report.removed.len(), 1); + assert_eq!(report.removed[0].path, cov); + let calls = remover.calls.borrow(); + assert_eq!( + calls.len(), + 1, + "the vetoed path must never reach the remover" + ); + assert_eq!(calls[0].1, cov); + } + + #[test] + fn prune_refuses_a_symlinked_artifact() { + let root = TempDir::new().unwrap(); + std::fs::create_dir_all(root.path().join("target")).unwrap(); + let elsewhere = TempDir::new().unwrap(); + // target/debug is a SYMLINK to an unrelated dir — must never be chased. + std::os::unix::fs::symlink(elsewhere.path(), root.path().join("target/debug")).unwrap(); + let live = FakeLiveProcessProbe::default(); + let measurer = MapMeasurer::default(); + let remover = RecordingRemover::default(); + + let report = prune_regenerable_build_artifacts( + root.path(), + ReclaimMode::Apply, + &live, + &measurer, + &remover, + ); + + assert!( + report.removed.is_empty(), + "a symlinked artifact must be refused" + ); + assert!(report.skipped_live.is_empty()); + assert!( + remover.calls.borrow().is_empty(), + "a symlinked artifact must never reach the remover", + ); + assert!( + elsewhere.path().exists(), + "the symlink target must be untouched", + ); + } + + #[test] + fn prune_missing_artifacts_is_a_noop() { + let root = TempDir::new().unwrap(); // no target/ at all + let live = FakeLiveProcessProbe::default(); + let measurer = MapMeasurer::default(); + let remover = RecordingRemover::default(); + + let report = prune_regenerable_build_artifacts( + root.path(), + ReclaimMode::Apply, + &live, + &measurer, + &remover, + ); + + assert!(!report.pruned_any()); + assert!(report.removed.is_empty()); + assert!(report.would_remove.is_empty()); + assert!(remover.calls.borrow().is_empty()); + } + + #[test] + fn prune_summary_is_stable() { + let report = BuildArtifactPruneReport { + mode: ReclaimMode::Apply, + removed: vec![RemovedPath { + path: PathBuf::from("/b/target/debug"), + kind: CandidateKind::StaleBuildCache, + bytes: 2_600_000_000, + primitive: ReclaimPrimitive::RemoveDir, + }], + would_remove: vec![], + skipped_live: vec![], + failures: vec![], + bytes_freed: 2_600_000_000, + }; + assert_eq!( + report.summary(), + "build-artifact prune: freed 2600000000 bytes, 1 removed, 0 would-remove, 0 live-skipped, 0 failed", + ); + } } diff --git a/src/disk_reclaim/mod.rs b/src/disk_reclaim/mod.rs index 623d835f0..d2ef47263 100644 --- a/src/disk_reclaim/mod.rs +++ b/src/disk_reclaim/mod.rs @@ -32,8 +32,9 @@ pub mod recipe; pub use candidate::{CandidateKind, MAX_CANDIDATES, ReclaimCandidate, parse_candidates}; pub use daemon_dir::resolve_daemon_working_dirs; pub use executor::{ - PathRemover, RealPathRemover, ReclaimFailure, ReclaimReport, RemovedPath, SkippedPath, - exec_reclaim, + BuildArtifactPruneReport, PathRemover, REGENERABLE_BUILD_ARTIFACTS, RealPathRemover, + ReclaimFailure, ReclaimReport, RemovedPath, SkippedPath, exec_reclaim, + prune_build_tree_artifacts, prune_regenerable_build_artifacts, }; pub use guard::{ CachingSizeMeasurer, DuSizeMeasurer, GuardContext, ProtectedDenySet, ReclaimPrimitive, diff --git a/src/operator_commands_ooda/daemon/mod.rs b/src/operator_commands_ooda/daemon/mod.rs index 48ff6efd1..8394d2eaa 100644 --- a/src/operator_commands_ooda/daemon/mod.rs +++ b/src/operator_commands_ooda/daemon/mod.rs @@ -1197,6 +1197,22 @@ pub fn run_ooda_daemon( match used_now { Some(used) if crate::disk_reclaim::daemon_should_trigger(used, reclaim_pct) => { let mode = crate::disk_reclaim::daemon_apply_from_env(); + // Issue #4825: deterministically prune the regenerable + // Cargo build artifacts (target/debug, target/llvm-cov-target) + // under the daemon's build tree HERE, at the routine + // threshold — BELOW the ≥95% Tier-1 emergency threshold. + // These sit under the protected working dir, so the + // worktree rails reject them and only the emergency net + // ever removed them, after `/home` had oscillated to 99%. + // Pruning them proactively stops the oscillation and the + // per-cycle emergency-branch firing (the emergency net + // above remains the backstop). Same apply-gating as the + // agentic reclaim below. + let prune = crate::disk_reclaim::prune_build_tree_artifacts( + &memories.repo_root, + mode, + ); + daemon_log(&state_root, &format!("[simard] {}", prune.summary())); match crate::disk_reclaim::run_disk_reclaim( &memories.repo_root, &state_root, diff --git a/tests/disk_reclaim_build_artifact_prune.rs b/tests/disk_reclaim_build_artifact_prune.rs new file mode 100644 index 000000000..4bb363235 --- /dev/null +++ b/tests/disk_reclaim_build_artifact_prune.rs @@ -0,0 +1,86 @@ +//! Outside-in integration tests for the issue #4825 disk-reclaim churn fix. +//! +//! These drive the **real production entry** the OODA daemon's Tier-3 +//! coordination calls — [`simard::disk_reclaim::prune_build_tree_artifacts`] — +//! against a real on-disk build tree, exercising the actual hardened +//! `RealPathRemover` (real `rm`), `du -sb` measurer, and `/proc` live-PID probe. +//! No test doubles: this is the consumer boundary a running daemon hits. +//! +//! Root cause being validated: the regenerable Cargo build artifacts +//! (`target/debug`, `target/llvm-cov-target`) sit under the protected daemon +//! working dir, so routine reclaim used to Reject them as "skipped for review" +//! and only the ≥95% emergency net removed them — after `/home` had oscillated +//! to 99%. This proactive prune reclaims them at the routine threshold instead. + +use std::fs; +use std::path::Path; + +use simard::disk_reclaim::{ReclaimMode, prune_build_tree_artifacts}; + +/// Create `/` with one non-empty file so `du` reports > 0 bytes. +fn make_artifact(build_tree: &Path, rel: &str) { + let dir = build_tree.join(rel); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join("blob.bin"), vec![0u8; 4096]).unwrap(); +} + +/// Scenario 1 (simple): the most basic user-visible behavior — an apply-mode +/// prune actually removes the regenerable `target/debug` from the build tree and +/// reports the freed bytes. This is what stops `/home` from re-climbing to 95%. +#[test] +fn prune_apply_actually_removes_target_debug() { + let build_tree = tempfile::tempdir().unwrap(); + make_artifact(build_tree.path(), "target/debug"); + let debug = build_tree.path().join("target/debug"); + assert!(debug.is_dir(), "precondition: target/debug exists"); + + let report = prune_build_tree_artifacts(build_tree.path(), ReclaimMode::Apply); + + assert!( + !debug.exists(), + "target/debug must be gone after apply prune" + ); + assert_eq!(report.removed.len(), 1, "exactly the one present artifact"); + assert_eq!(report.removed[0].path, debug); + assert!(report.bytes_freed > 0, "freed bytes must be measured (> 0)"); + assert!(report.pruned_any()); + assert!(report.failures.is_empty()); +} + +/// Scenario 2 (complex / edge + integration): a full build tree. +/// - dry-run must delete NOTHING (safe default), +/// - a subsequent apply removes BOTH regenerable artifacts, +/// - a non-artifact sibling (`target/release`) is NEVER touched — the +/// carve-out is a closed allow-list, not "everything under target/". +#[test] +fn prune_dry_run_is_safe_then_apply_removes_only_regenerable_artifacts() { + let build_tree = tempfile::tempdir().unwrap(); + make_artifact(build_tree.path(), "target/debug"); + make_artifact(build_tree.path(), "target/llvm-cov-target"); + // A sibling that must survive: release artifacts are NOT on the allow-list. + make_artifact(build_tree.path(), "target/release"); + + let debug = build_tree.path().join("target/debug"); + let cov = build_tree.path().join("target/llvm-cov-target"); + let release = build_tree.path().join("target/release"); + + // --- dry-run: zero destructive ops ----------------------------------- + let dry = prune_build_tree_artifacts(build_tree.path(), ReclaimMode::DryRun); + assert!(debug.is_dir() && cov.is_dir(), "dry-run must not delete"); + assert_eq!(dry.would_remove.len(), 2, "dry-run reports both intents"); + assert!(dry.removed.is_empty()); + assert_eq!(dry.bytes_freed, 0); + assert!(!dry.pruned_any()); + + // --- apply: remove exactly the two regenerable artifacts ------------- + let apply = prune_build_tree_artifacts(build_tree.path(), ReclaimMode::Apply); + assert!(!debug.exists(), "target/debug removed"); + assert!(!cov.exists(), "target/llvm-cov-target removed"); + assert!( + release.is_dir(), + "target/release must be preserved (not a regenerable-artifact allow-list entry)", + ); + assert_eq!(apply.removed.len(), 2); + assert!(apply.bytes_freed > 0); + assert!(apply.failures.is_empty()); +}