diff --git a/docs/concepts/agentic-disk-reclamation.md b/docs/concepts/agentic-disk-reclamation.md index ec627721f..4e903399e 100644 --- a/docs/concepts/agentic-disk-reclamation.md +++ b/docs/concepts/agentic-disk-reclamation.md @@ -1,7 +1,7 @@ --- title: Agentic disk reclamation description: Design rationale for Simard's fully agentic disk-reclamation capability — why the reclaim agent proposes candidates while a deterministic Rust executor disposes of them, the non-bypassable protected-path rails, and how the capability self-heals disk pressure without per-cycle hand-crafted cleanup heuristics. -last_updated: 2026-07-07 +last_updated: 2026-07-27 review_schedule: as-needed owner: simard doc_type: concept @@ -122,7 +122,47 @@ Only three candidate classes can *ever* be reclaimed, and only after passing 1. **Tracked worktrees** whose PR is MERGED or CLOSED and which are idle, 2. **Orphaned, de-registered** (untracked) worktree directories, -3. **Stale build caches** (`target/` and shared cargo target dirs). +3. **Stale build caches** — per-managed-repo `/target/` (its `debug/`, + `release/`, `llvm-cov-target/`, and incremental caches) plus the shared cargo + target dirs under the state root. + +### Routine reclaim frees `target/` between emergency passes + +The reclamation **allow-root set** includes `/target` for every managed +repo (rooted at the `target/` *parent* so the guard's strict-inside containment +still confines removal to descendants of `target/`, never `/src` or +`/.git`). This is what lets **routine** (non-emergency) reclaim actually +free rebuildable build artifacts. + +Without that root, routine reclaim had nothing in scope: every `target/debug` +candidate the agent proposed was rejected as *outside allow-root* and pushed to +the human-review list, producing the "freed 0 bytes, 0 paths removed, N skipped +for review" no-op. Disk then climbed unchecked until the deterministic +`emergency_cleanup` (Tier 1) fired at ~95% and freed space in a burst — a +~30-minute saw-tooth that rode the partition at 94–99% and risked `ENOSPC` +between passes. With `/target` in scope, routine reclaim removes the same +rebuildable artifacts *proactively and incrementally*, so the partition no longer +depends on the emergency backstop to avoid filling. `emergency_cleanup` remains +the deterministic hard stop, but it should now fire rarely rather than every +cycle. + +Widening to `target/` stays **additive and non-breaking**: the artifacts are the +same rebuildable class emergency cleanup already treats as safe, removal is still +gated by every hard rail below (the live-PID rail refuses any candidate that a +running process is sitting *inside* via `/proc//cwd`; the protected deny-set +still overrides the allow-root), and no new candidate *category* is introduced — only the containment +scope of the existing `StaleBuildCache` kind is corrected. + +### Observability: per-candidate skip reasons + +Every rejected candidate is logged with a structured `tracing` event carrying its +`path`, the closed `RejectReason` enum, and its `CandidateKind` — turning the +opaque "N skipped for review" summary into a per-path audit trail. An operator +can now tell a `target/debug` skipped for **Live process** (a build is running; +expected) apart from one skipped for **Outside allow-root** (a scope bug worth +investigating). The agent's free-text rationale is never logged as a field (anti +log-forging); only the enum reason is. See +[disk-reclaim telemetry](../reference/disk-reclaim-telemetry.md). ### Fail-closed, everywhere diff --git a/docs/concepts/goal-reblock-backoff-dedup.md b/docs/concepts/goal-reblock-backoff-dedup.md new file mode 100644 index 000000000..c11862928 --- /dev/null +++ b/docs/concepts/goal-reblock-backoff-dedup.md @@ -0,0 +1,161 @@ +--- +title: Goal-reblock backoff & stewardship dedup — the Overseer stops relaunching blocked goals every cycle +description: > + Why the Overseer no longer re-observes and re-enqueues an already-blocked goal + on every ~15-minute cycle, and no longer files a fresh stewardship issue for + the same block (issues #4817, #4828). Explains the observed 8h+ churn + (identical "GoalHygiene ... blocked (0 no-action cycle(s))" from + 2026-07-26T18:54Z through 2026-07-27T02:19Z, repeatedly hitting "held: + per-cycle launch cap reached"), why GoalHygiene briefs escaped the existing + gap-scan backoff, how a dedicated `goal_reblock_backoff` BackoffGate keyed on + goal:blocked:{goal_id} suppresses the relaunch, and how stabilising the + failure signature folds the duplicate stewardship issues into one. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: concept +status: implemented +related: + - ./gap-scan-backoff-dedup.md + - ./blocked-goal-escalation-backoff.md + - ./ooda-reinvestigate-blocked-goals.md + - ./no-progress-breaker-storm-suppression.md + - ../reference/goal-reblock-backoff-api.md + - ../reference/overseer-backoff-gate-api.md + - ../reference/stewardship-api.md + - ../howto/diagnose-recurring-goal-reblock-churn.md +--- + +# Goal-reblock backoff & stewardship dedup + +> **Status: implemented (issues +> [#4817](https://github.com/rysweet/Simard/issues/4817), +> [#4828](https://github.com/rysweet/Simard/issues/4828)).** The Overseer's +> goal-hygiene path now runs through a dedicated `goal_reblock_backoff` +> `BackoffGate` and files exactly one stewardship issue per still-blocked goal. +> Primary sources: +> [`src/overseer/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/mod.rs) +> (`goal_reblock_backoff` field + `gate()`/`act()` wiring, and the `GoalBlocked` +> signature text), +> [`src/overseer/guardrails.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/guardrails.rs) +> (the shared `BackoffGate`), and +> [`src/stewardship/dedup.rs`](https://github.com/rysweet/Simard/blob/main/src/stewardship/dedup.rs) +> (`normalize_for_signature` counter redaction). API surface: +> [goal-reblock backoff reference](../reference/goal-reblock-backoff-api.md). + +## The defect this fixes + +The Overseer's goal-hygiene step observes blocked goals and, for each, decides +whether to relaunch a covering recipe workstream and/or file a stewardship +issue. Its `dedup_key` and human-facing text are built here: + +```rust +// ProblemKind::GoalHygiene +format!("goal:blocked:{goal_id}"), // dedup_key +format!("goal {goal_id} blocked ({consecutive_no_action} no-action cycle(s))"), // signature text +``` + +Two things went wrong at once: + +1. **No relaunch suppression for GoalHygiene.** The + [gap-scan backoff](./gap-scan-backoff-dedup.md) and the in-flight guard were + scoped to `WORKSTREAM_COVERAGE_GROUP`; `GoalHygiene` briefs have + `sequence_group = None` and slipped straight past them. So every ~15-minute + cycle the overseer **re-observed the same two already-in-flight/blocked + goals and relaunched their Simard recipe workstreams**, repeatedly hitting + `held: per-cycle launch cap reached` without ever clearing the block. This ran + for **8h+** — identical `GoalHygiene … blocked (0 no-action cycle(s))` lines + from `2026-07-26T18:54Z` through `2026-07-27T02:19Z`. + +2. **The signature fluctuated, so dedup never matched.** The + `({consecutive_no_action} no-action cycle(s))` counter is part of the + **signature-bearing error text**, and `failure_signature()` hashes that text + (via `normalize_for_signature`). Each time the counter ticked (`0`, then `1`, + …) the signature changed, so `find_existing()` never matched the prior issue + and the overseer **self-filed duplicate stewardship issues** — the + `recurring_goal_reblock` clusters reported as #4817 and #4828. + +The net effect: a goal that was *already blocked and already being worked* got +re-observed, re-enqueued, and re-issued forever, hammering the launch cap and +spamming duplicate stewardship issues. + +## The fix, part 1: relaunch backoff keyed per goal + +A dedicated gate suppresses per-cycle relaunch of a still-blocked goal: + +```rust +goal_reblock_backoff: BackoffGate, // in the Overseer struct +``` + +keyed on a stable per-goal key: + +```rust +let key = format!("overseer-obs:goal:blocked:{goal_id}"); +``` + +It reuses the same bounded-exponential-backoff semantics as the +[gap-scan `BackoffGate`](../reference/overseer-backoff-gate-api.md): + +- The **first** observation of a blocked goal admits (arms the base window). +- A re-observation **inside** the window is **suppressed** — the goal is not + relaunched — honoring the in-flight/held state and the + `consecutive_no_action` counter. +- The window grows `× multiplier` per re-hit, capped, so a persistently blocked + goal is retried on a *slowing* cadence rather than every cycle. +- The gate **re-admits immediately when the block clears** (a long silence, ≥ 2× + the window, resets to the base window), so a goal that becomes actionable again + resurfaces promptly. + +`gate()` **peeks** (decide without recording) and `act()` **commits** only after +a successful relaunch, so a launch that is itself held (cap reached) does not +consume the dedup slot. + +> **Suppression is relaunch-only.** A blocked goal that genuinely needs human +> attention still surfaces **exactly one** stewardship issue for visibility — the +> gate only silences the wasteful *relaunch churn*, never the human signal. + +## The fix, part 2: a stable signature folds duplicate issues into one + +The fluctuating counter is moved **out of the signature-bearing error text** and +kept only in the issue **body/title annotation** (where the human still sees the +full context). The hashed signature input therefore no longer changes as the +counter ticks, so `find_existing()` matches the prior issue and dedup fires +across cycles → **one** stewardship issue per blocked goal. + +As defense-in-depth, `normalize_for_signature` (`src/stewardship/dedup.rs`) +additionally **redacts** residual counter patterns — `( no-action cycle(s))` +and `no progress for cycles` — so any counter that leaks into a signature +input still folds to a single signature. This mirrors the existing UUID-redaction +contract that already scrubs volatile session/run ids. + +```mermaid +flowchart LR + obs["Observe blocked goal_id"] --> key["key = overseer-obs:goal:blocked:{goal_id}"] + key --> peek{"goal_reblock_backoff.peek()"} + peek -->|Suppress| hold["skip relaunch this cycle (no launch-cap churn)"] + peek -->|Admit| relaunch["relaunch workstream → commit() on success"] + obs --> sig["failure_signature(GoalBlocked, text w/o counter)"] + sig --> dedup{"find_existing()?"} + dedup -->|match| one["reuse the single open stewardship issue"] + dedup -->|none| file["file exactly one issue"] +``` + +## Safety & scope + +- **Additive / non-breaking.** New gate field + signature-text change + + redaction rule; no API or PRD change, no `print!`/`println!` — structured + `tracing` + OTel only. +- **Untrusted `goal_id`.** The goal id is normalized (bounded length, + `[A-Za-z0-9._:-]`) before it is embedded in a dedup key or an issue + title/body, preventing signature collisions and issue-body injection. +- **Bounded state.** Backoff state is saturating and evicts on block-clear/TTL, + so per-goal keys cannot grow unbounded on a long-running daemon. +- **Exactly-once escalation.** Relaunch suppression never suppresses the single + stewardship issue — information availability is preserved for the human. + +## See also + +- [Gap-scan dedup & backoff](./gap-scan-backoff-dedup.md) — the sibling rail for coverage gaps. +- [Blocked-goal escalation backoff](./blocked-goal-escalation-backoff.md) — the WhisperGate backoff for escalations. +- [Goal-reblock backoff reference](../reference/goal-reblock-backoff-api.md) — the typed API. +- [Diagnose recurring goal-reblock churn](../howto/diagnose-recurring-goal-reblock-churn.md) — the operator runbook. diff --git a/docs/concepts/reclaim-effectiveness-backoff.md b/docs/concepts/reclaim-effectiveness-backoff.md new file mode 100644 index 000000000..73bcbb9ac --- /dev/null +++ b/docs/concepts/reclaim-effectiveness-backoff.md @@ -0,0 +1,157 @@ +--- +title: Reclaim effectiveness backoff — disk-reclaim stops churning when it frees nothing +description: > + Why the OODA daemon no longer re-runs agentic disk-reclamation every cycle + when reclamation keeps freeing 0 bytes (issues #4809, #4825, #4810). Explains + the observed churn on the ~94%-full /tmp/state host, why the plain + %-used trigger re-fired a proven-ineffective run every ~15 minutes, how the + new `ReclaimEffectivenessGate` applies bounded exponential cooldown after a + streak of no-op reclaims, how a genuinely filling disk still bypasses the + cooldown at a hard %-used ceiling, and why the gate is suppress-only (it can + never turn a dry-run into an apply). +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: concept +status: implemented +related: + - ./agentic-disk-reclamation.md + - ./automated-disk-health.md + - ../reference/reclaim-effectiveness-gate-api.md + - ../reference/disk-reclaim-telemetry.md + - ../reference/disk-reclaim-api.md + - ../operations/reclaim-effectiveness-kill-switch.md + - ../howto/configure-reclaim-effectiveness.md + - ../howto/configure-disk-reclamation.md +--- + +# Reclaim effectiveness backoff + +> **Status: implemented (issues +> [#4809](https://github.com/rysweet/Simard/issues/4809), +> [#4825](https://github.com/rysweet/Simard/issues/4825), +> [#4810](https://github.com/rysweet/Simard/issues/4810)).** The OODA daemon's +> disk-reclaim trigger now consults a `ReclaimEffectivenessGate` before firing, +> so a reclamation run that keeps freeing nothing is not re-attempted on every +> cycle. Primary sources: +> [`src/disk_reclaim/effectiveness.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/effectiveness.rs) +> (the gate), +> [`src/operator_commands_ooda/daemon/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/operator_commands_ooda/daemon/mod.rs) +> (the daemon trigger wiring), and +> [`src/disk_reclaim/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/mod.rs) +> (`emit_reclaim_telemetry`). API surface: +> [reclaim effectiveness gate reference](../reference/reclaim-effectiveness-gate-api.md). + +## The defect this fixes + +The daemon's Tier-3 self-heal step (issue #2704, see +[agentic disk reclamation](./agentic-disk-reclamation.md)) fires whenever a +cheap `df` probe reports `%-used ≥ SIMARD_DISK_RECLAIM_PCT`. On the production +OODA host the working partition sat at **~94–99% used with ~12 GiB free of +196 GiB**, permanently above the trigger threshold, while every reclamation run +freed **0 bytes** — the candidates the analysis agent proposed were all +undeletable (protected paths, live processes, uncommitted/unpushed worktrees) +and every rail correctly refused them. + +The trigger had **no memory that the previous run accomplished nothing**. So +each ~15-minute daemon cycle: + +1. observed `used ≥ threshold`, +2. re-invoked the full agentic reclaim capability (a brain call + a + recipe-runner scratch dir + per-cycle run artifacts), +3. freed 0 bytes, +4. and — because the run itself consumed scratch space — sometimes left the + partition *fuller* than before. + +The result is the churn reported in #4809 (*"routine disk-reclaim is +ineffective"*), #4825 (*"systemic disk-reclaim churn in the OODA daemon"*), and +#4810 (*"OODA daemon rides disk at 94–99%"*): reclamation re-scanned the same +undeletable paths forever without ever reclaiming space, and the churn itself +added disk pressure. + +## The fix: effectiveness-aware exponential cooldown + +Reclamation is now gated on **whether it recently worked**, not only on +**how full the disk is**. The daemon records the outcome of each run and, after +a streak of ineffective runs, backs off exponentially before trying again. + +```mermaid +flowchart TD + tick["Daemon cycle: used_pct ≥ SIMARD_DISK_RECLAIM_PCT?"] -->|no| noop["log 'under threshold', done"] + tick -->|yes| ceiling{"used_pct ≥ hard ceiling?"} + ceiling -->|yes| run["RUN reclaim (bypass cooldown)"] + ceiling -->|no| gate{"ReclaimEffectivenessGate.peek()"} + gate -->|Suppress| skip["SKIP this cycle; WARN + suppressed_cycles metric"] + gate -->|Admit| run + run --> eff{"effective? (bytes_freed > 0 or used_pct dropped)"} + eff -->|yes| reset["record success → reset noop_streak & cooldown"] + eff -->|no| grow["record no-op → grow noop_streak & cooldown window"] +``` + +The gate reuses the same bounded-exponential-backoff semantics as the +Overseer's [`BackoffGate`](../reference/overseer-backoff-gate-api.md): + +- **First no-op** arms a base cooldown window (default 15 min). +- **Each further no-op** grows the window `× multiplier` (default ×2), hard-capped + (default 4 h). While inside the window, the daemon **skips** the reclaim run + entirely — no brain call, no scratch dir, no artifacts. +- **A run that frees space** (positive `bytes_freed`, or a measured drop in + `used_pct`) is *effective*: it resets the streak and the cooldown so genuine + reclamation stays responsive. +- **A long silence** since the last attempt (≥ 2× the current window) also resets + to the base window, so a disk that fills again after a quiet period resurfaces + promptly. + +### Genuine fill-ups are never masked + +Suppression is bypassed whenever locally-observed `%-used` crosses a **hard +ceiling** (`SIMARD_DISK_RECLAIM_HARD_CEILING_PCT`, default `97`). The bypass +authority is derived from a *fresh local `df` sample* — never from re-ingested +telemetry — so a real, accelerating fill-up always triggers reclamation +regardless of the cooldown. The cooldown only silences the pathological case: +*already above the trigger, but reclamation demonstrably cannot help.* + +Every suppressed cycle is visible: it emits a `WARN` daemon log line and +increments a `simard.disk.reclaim.suppressed_cycles` counter, and the +per-run telemetry gains `noop_streak` / `suppressed_cycles` / `effective` +attributes (see [disk-reclaim telemetry](../reference/disk-reclaim-telemetry.md)). +Operators can therefore see *"reclaim is being deliberately held back because it +keeps freeing nothing"* rather than silence. + +## Cross-cycle skip memory (don't re-propose the same undeletable path) + +Independently of the cooldown, the executor now remembers the **canonicalized** +paths a rail rejected and refuses to re-propose them on the next cycle, so a +single undeletable worktree is not re-vetted every run. Paths are +canonicalized *before* both the guard check and the skip-memory lookup, which +prevents a symlink or `..` alias from smuggling a protected path past the guard. +See the [reclaim effectiveness gate reference](../reference/reclaim-effectiveness-gate-api.md#cross-cycle-skip-memory). + +## Safety posture + +- **Suppress-only.** The gate can only *skip* a run. It never changes the + destructive posture: the daemon stays **dry-run by default** and + `SIMARD_DISK_RECLAIM_DAEMON_APPLY=1` remains the sole apply opt-in. A unit + test asserts the gate can never transition dry-run → apply. +- **Fail safe, not open.** Any parse/validation/canonicalization error treats a + candidate as *not authorized to delete* and does **not** suppress a cycle + where suppression could hide real pressure. +- **Bounded arithmetic.** The no-op streak, cooldown exponent, and window are + saturating so a long-running daemon cannot overflow into an absurd cooldown. +- **Additive.** No existing metric, env var, or behavior changes; the PRD is + preserved and there is no `print!`/`println!` — the gate emits structured + `tracing` + OTel only. + +## Turning it off + +The whole effectiveness gate is a single kill switch, +`SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE=off`, which reverts to the previous +"fire on every over-threshold cycle" behavior. See the +[reclaim-effectiveness kill switch](../operations/reclaim-effectiveness-kill-switch.md). + +## See also + +- [Configure reclaim effectiveness](../howto/configure-reclaim-effectiveness.md) — the operator knobs. +- [Agentic disk reclamation](./agentic-disk-reclamation.md) — the propose/dispose design this gates. +- [Automated disk health](./automated-disk-health.md) — the daemon step the gate lives in. +- [Reclaim effectiveness gate reference](../reference/reclaim-effectiveness-gate-api.md) — the typed API. diff --git a/docs/howto/configure-disk-reclamation.md b/docs/howto/configure-disk-reclamation.md index cf119b56a..60f645596 100644 --- a/docs/howto/configure-disk-reclamation.md +++ b/docs/howto/configure-disk-reclamation.md @@ -1,7 +1,7 @@ --- title: Configure and run disk reclamation description: Operator guide for Simard's agentic disk-reclamation capability — running dry-run and live reclamation from the CLI, reading the report and human-review list, tuning the SIMARD_DISK_RECLAIM_PCT threshold, and understanding the self-healing daemon trigger. -last_updated: 2026-07-07 +last_updated: 2026-07-27 review_schedule: as-needed owner: simard doc_type: howto @@ -51,6 +51,7 @@ Typical output: ```text disk-reclaim (dry-run) — home partition 88% used, target 85% WOULD REMOVE tracked_worktree ~/.simard/engineer-worktrees/goal-1841-... 3.9G pr #1841 merged, idle +WOULD REMOVE stale_build_cache /home/azureuser/src/Simard/target 8.4G rebuildable target/ (routine) WOULD REMOVE stale_build_cache /home/azureuser/src/Simard/worktrees/feat-x/target 6.1G stale target/ WOULD REMOVE orphan_dir ~/.simard/engineer-worktrees/leftover-9f3 1.2G de-registered, no gitdir SKIP (review) tracked_worktree ~/src/amplihack-rs/worktrees/wip-parser 2.0G unpushed commits not in a merged/closed PR @@ -62,6 +63,15 @@ Nothing is deleted in dry-run. `WOULD REMOVE` shows what a live run would reclaim (largest-first); `SKIP (review)` shows candidates a rail refused — these go to the human-review list, never auto-deleted. +> **Routine reclaim covers each managed repo's own `target/`.** The reclamation +> scope includes `/target` (`debug/`, `release/`, `llvm-cov-target/`, and +> incremental caches) for every managed repo — the same rebuildable artifacts the +> deterministic `emergency_cleanup` removes at severe pressure. This lets routine +> reclaim free build artifacts *proactively between* emergency passes instead of +> no-op'ing (`freed 0 bytes … N skipped for review`) until disk hits the +> emergency threshold. The protected `worktrees/main` checkout (and its +> `target/`) is never reclaimed — the deny-set overrides the allow-root. + To actually reclaim, pass `--apply`: ```bash @@ -210,7 +220,10 @@ For the full per-candidate detail, read the machine-readable report Candidates a rail refused are **never deleted** and are surfaced for a human. They appear as `SKIP (review)` in the CLI, in `skipped[]` of `--report-json`, -and as `WARN` tracing from the daemon. Each carries a `reject_reason`: +and as structured `tracing` from the daemon. Each skipped candidate is logged +with its `path`, its `reject_reason` (the closed enum below), and its `kind`, so +you can see *which* path was refused and *why* without reconstructing it from the +summary counters. Each carries a `reject_reason`: | `reject_reason` | Meaning | What to do | | --------------- | ------- | ---------- | diff --git a/docs/howto/configure-reclaim-effectiveness.md b/docs/howto/configure-reclaim-effectiveness.md new file mode 100644 index 000000000..dafe3a7f8 --- /dev/null +++ b/docs/howto/configure-reclaim-effectiveness.md @@ -0,0 +1,116 @@ +--- +title: Configure reclaim effectiveness (stop disk-reclaim churn) +description: > + Operator guide for the disk-reclaim effectiveness gate — tuning the + exponential cooldown that stops the OODA daemon from re-running reclamation + every cycle when it keeps freeing nothing (#4809 / #4825 / #4810). Covers the + cooldown base/multiplier/cap, the hard %-used ceiling that always bypasses the + cooldown, how to read the new telemetry, how to diagnose held cycles, and how + to disable the gate. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: howto +related: + - ../concepts/reclaim-effectiveness-backoff.md + - ../reference/reclaim-effectiveness-gate-api.md + - ../reference/disk-reclaim-telemetry.md + - ./configure-disk-reclamation.md + - ../operations/reclaim-effectiveness-kill-switch.md +--- + +# Configure reclaim effectiveness + +The OODA daemon reclaims disk when `%-used` crosses a threshold (see +[configure and run disk reclamation](./configure-disk-reclamation.md)). On a +host whose partition is *stuck* above the threshold with nothing safely +deletable, that trigger used to re-run reclamation every cycle and free 0 bytes +— wasteful churn that itself added disk pressure (#4809 / #4825 / #4810). + +The **effectiveness gate** fixes this: after a streak of no-op runs it backs off +exponentially and skips over-threshold cycles, while still running immediately if +the disk crosses a hard ceiling. This guide shows how to tune and observe it. +For *why* it works this way, see +[reclaim effectiveness backoff](../concepts/reclaim-effectiveness-backoff.md). + +## When to use this + +- The daemon logs `disk reclaim held — N consecutive no-op run(s), cooling down` + and you want to change how aggressively it backs off. +- You want genuine fill-ups to force reclamation sooner (lower the ceiling). +- Reclamation on your host *can* recover space and you think the cooldown is too + long (shorten the base/cap) — or you want to disable the gate entirely. + +## The knobs + +All are environment variables read at daemon boot; all fail safe to the default +on invalid input. Set them the same way as the other daemon vars (systemd +drop-in, then `systemctl daemon-reload && systemctl restart simard-daemon`). + +| Env var | Default | Effect | +| ------- | ------- | ------ | +| `SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE` | `on` | Master switch. `off` disables the cooldown (fire every over-threshold cycle). | +| `SIMARD_DISK_RECLAIM_COOLDOWN_BASE_SECS` | `900` | Cooldown after the **first** no-op run (15 min). | +| `SIMARD_DISK_RECLAIM_COOLDOWN_MULTIPLIER` | `2` | Growth per additional no-op run. Values `< 2` clamp to `2`. | +| `SIMARD_DISK_RECLAIM_COOLDOWN_MAX_SECS` | `14400` | Cooldown cap (4 h). | +| `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT` | `97` | Locally-observed `%-used` at/above which the cooldown is **bypassed** and reclamation always runs. | + +The pre-existing threshold `SIMARD_DISK_RECLAIM_PCT` (default `85`) still decides +*whether reclaim is even considered*; the effectiveness gate only decides +*whether to run given it is over threshold and hasn't been working*. + +### Example: back off faster, bypass sooner + +```ini +# /etc/systemd/system/simard-daemon.service.d/reclaim-effectiveness.conf +[Service] +Environment=SIMARD_DISK_RECLAIM_COOLDOWN_BASE_SECS=300 +Environment=SIMARD_DISK_RECLAIM_COOLDOWN_MAX_SECS=3600 +Environment=SIMARD_DISK_RECLAIM_HARD_CEILING_PCT=95 +``` + +This arms a 5-minute base cooldown capped at 1 h, and bypasses the cooldown once +the disk hits 95% so an accelerating fill-up recovers promptly. + +## Read the telemetry + +The gate is fully observable through the unified telemetry facade (see +[disk-reclaim telemetry](../reference/disk-reclaim-telemetry.md)): + +- `simard.disk.reclaim.suppressed_cycles` — counter, incremented every cycle the + gate holds a run back. +- `noop_streak` / `suppressed_cycles` / `effective` — additive attributes on the + existing `simard.disk.reclaim.*` series describing the current streak and + whether the last run freed space. + +```bash +simard status | grep 'disk.reclaim' +``` + +A rising `suppressed_cycles` with a flat `bytes_freed` is the expected healthy +signature of *"nothing to reclaim, so we're correctly holding back."* A rising +`bytes_freed` means reclamation is working and the streak keeps resetting. + +## Diagnose a held cycle + +1. Confirm the disk really has nothing safely reclaimable: run + `simard disk-reclaim` (dry-run) and read the human-review list — if every + candidate is skipped by a rail, the hold is correct. +2. If space *is* actually recoverable, the analysis agent isn't proposing it — + that is a reclaim-analysis problem, not a gate problem; investigate the + recipe, not the cooldown. +3. To force attempts while you investigate, either lower + `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT` or set the + [kill switch](../operations/reclaim-effectiveness-kill-switch.md) `off`. + +## Disable the gate + +See the [reclaim-effectiveness kill switch](../operations/reclaim-effectiveness-kill-switch.md): + +```ini +[Service] +Environment=SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE=off +``` + +This reverts to fire-every-over-threshold-cycle behavior. It does **not** change +the dry-run default or any deletion rail. diff --git a/docs/howto/diagnose-recurring-goal-reblock-churn.md b/docs/howto/diagnose-recurring-goal-reblock-churn.md new file mode 100644 index 000000000..d8b77ffaa --- /dev/null +++ b/docs/howto/diagnose-recurring-goal-reblock-churn.md @@ -0,0 +1,123 @@ +--- +title: Diagnose recurring goal-reblock churn +description: > + Operator runbook for the goal-reblock backoff & stewardship-dedup rail + (#4817 / #4828): how to recognise the "GoalHygiene ... blocked (N no-action + cycle(s))" relaunch storm and the duplicate stewardship issues, confirm the + rail is suppressing relaunches and folding issues into one, read the state, + tune the shared backoff window, and clear a genuinely stuck block. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: howto +related: + - ../concepts/goal-reblock-backoff-dedup.md + - ../reference/goal-reblock-backoff-api.md + - ../reference/overseer-backoff-gate-api.md + - ../concepts/gap-scan-backoff-dedup.md + - ./unblock-stuck-ooda-goals.md + - ./configure-overseer-gap-scan-backoff.md +--- + +# Diagnose recurring goal-reblock churn + +The Overseer suppresses per-cycle **relaunch** of an already-blocked goal and +files **one** stewardship issue per block (see +[goal-reblock backoff & stewardship dedup](../concepts/goal-reblock-backoff-dedup.md)). +This runbook helps you confirm the rail is doing its job and act when a block is +genuinely stuck. + +## Recognise the symptom + +Before this rail (#4817 / #4828) the daemon log showed the **same** blocked goal +relaunched every ~15 minutes for hours: + +``` +GoalHygiene goal blocked (0 no-action cycle(s)) +... held: per-cycle launch cap reached +GoalHygiene goal blocked (0 no-action cycle(s)) # next cycle, identical +... held: per-cycle launch cap reached +``` + +…together with a stream of near-identical `recurring_goal_reblock` stewardship +issues (the pattern that produced #4817 and #4828). + +With the rail in place you should instead see the relaunch **suppressed** after +the first observation, and a **single** open stewardship issue for the goal. + +## Confirm the rail is working + +1. **One issue, not many.** Check for duplicate stewardship issues for the same + goal: + + ```bash + gh issue list --repo rysweet/Simard --state open \ + --search 'recurring_goal_reblock in:title,body' --limit 50 + ``` + + You should find **one** open issue per blocked goal. Multiple open issues for + the *same* goal means the signature is not stable — see + [signature debugging](#signature-not-folding) below. + +2. **Relaunch suppressed.** In the daemon log, after the first + `GoalHygiene goal blocked` you should see the relaunch held by the + backoff (not by the launch cap) on subsequent cycles, e.g. + `goal-reblock backoff: suppressed (window s)`. The `held: + per-cycle launch cap reached` line should no longer recur for that goal every + cycle. + +3. **Re-admit on clear.** When the underlying block clears, the goal should + re-admit within one base window and the workstream relaunch/close normally. + +## Read the backoff / dedup state + +The suppression key is `overseer-obs:goal:blocked:{goal_id}` and it uses the +shared Overseer backoff window (see +[BackoffGate reference](../reference/overseer-backoff-gate-api.md#configuration-accessors)). +The stewardship signature is stable per goal — the +`consecutive_no_action` counter is kept in the issue body/title only, not in the +hashed signature. + +## Tune the backoff window + +The goal-reblock gate reuses the shared `SIMARD_OVERSEER_BACKOFF_*` window +configuration (same knobs as the gap-scan rail — see +[configure Overseer gap-scan backoff](./configure-overseer-gap-scan-backoff.md)): + +| Env var | Default | Effect | +| ------- | ------- | ------ | +| `SIMARD_OVERSEER_BACKOFF_BASE_SECS` | (shared default) | base suppression window after the first observation | +| `SIMARD_OVERSEER_BACKOFF_MULTIPLIER` | (shared default) | growth per re-hit (`≥ 2`) | +| `SIMARD_OVERSEER_BACKOFF_MAX_SECS` | (shared default) | cap on the window | + +Apply via a systemd drop-in and restart the daemon. A longer base window quiets +a persistently-blocked goal further between retries; a shorter one retries a +recoverable goal sooner. + +## Duplicate issues still appearing + +If you see more than one open stewardship issue for the **same** goal: + +1. Open two of the duplicates and compare their `stewardship-signature: ` + footer. If the signatures **differ**, some volatile text is still leaking into + the signature input — most likely a counter or id the redaction does not yet + cover. This is a `normalize_for_signature` gap + (`src/stewardship/dedup.rs`), not an operator misconfiguration; file it with + the two issue bodies attached. +2. If the signatures **match** but both issues are open, the dedup **read** may + be stale (both were filed before the first became visible) — close the older + duplicate; the rail will reuse the survivor going forward. + +## Clear a genuinely stuck block + +Suppressing the *relaunch churn* does not fix the *underlying block*. If the +single stewardship issue shows a goal that truly needs intervention, resolve the +block itself — see [unblock stuck OODA goals](./unblock-stuck-ooda-goals.md). +Once the block clears, the goal-reblock gate re-admits automatically and the +stewardship issue can be closed. + +## See also + +- [Goal-reblock backoff & stewardship dedup](../concepts/goal-reblock-backoff-dedup.md) — the rationale. +- [Goal-reblock backoff reference](../reference/goal-reblock-backoff-api.md) — the typed API. +- [Unblock stuck OODA goals](./unblock-stuck-ooda-goals.md) — resolving the underlying block. diff --git a/docs/operations/reclaim-effectiveness-kill-switch.md b/docs/operations/reclaim-effectiveness-kill-switch.md new file mode 100644 index 000000000..4a947a307 --- /dev/null +++ b/docs/operations/reclaim-effectiveness-kill-switch.md @@ -0,0 +1,98 @@ +--- +title: "Operations: reclaim-effectiveness kill switch (SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE)" +description: > + The environment variable that disables the disk-reclaim effectiveness gate at + daemon boot — what it does (and, critically, what it does NOT disable: the + %-used trigger, every deletion safety rail, and the dry-run default all keep + running), when to use it, how to set it via systemd, how to verify which mode + the daemon is in, and how to remove it. Secure default is the gate ON. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: howto +status: implemented +related: + - ../concepts/reclaim-effectiveness-backoff.md + - ../reference/reclaim-effectiveness-gate-api.md + - ../reference/disk-reclaim-telemetry.md + - ../howto/configure-reclaim-effectiveness.md + - resource-admission-kill-switch.md + - index.md +--- + +# Reclaim-effectiveness kill switch (`SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE`) + +> **Status: implemented.** This page describes the shipped kill switch in +> present tense. The gate it toggles lives in +> [`src/disk_reclaim/effectiveness.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/effectiveness.rs) +> and is wired into the daemon trigger in +> [`src/operator_commands_ooda/daemon/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/operator_commands_ooda/daemon/mod.rs). +> See [reclaim effectiveness backoff](../concepts/reclaim-effectiveness-backoff.md) +> and the [reclaim effectiveness gate reference](../reference/reclaim-effectiveness-gate-api.md). + +This variable disables the **effectiveness cooldown** that stops the OODA daemon +from re-running disk-reclamation every cycle when reclamation keeps freeing +nothing (issues #4809 / #4825 / #4810). + +> **The kill switch disables the cooldown REASONING, NOT any safety.** Turning +> the gate off reverts to the previous behavior — reclaim fires on **every** +> cycle where `%-used ≥ SIMARD_DISK_RECLAIM_PCT` — but it does **not** re-open +> any deletion path. The dry-run default, `SIMARD_DISK_RECLAIM_DAEMON_APPLY` +> apply-opt-in, and every candidate rail (protected paths, live processes, +> uncommitted/unpushed, active worktree, allow-root) all keep running unchanged. +> Disabling the gate only makes the daemon *churn again*; it never makes it +> *delete more*. + +--- + +## What this variable does + +| Value | Behavior | +|---|---| +| Unset, or any value other than `off` (case-insensitive) | **Gate ON (default).** Before firing reclaim, the daemon consults the `ReclaimEffectivenessGate`. After a streak of no-op runs it applies bounded exponential cooldown and **skips** over-threshold cycles, emitting a `WARN` line and incrementing `simard.disk.reclaim.suppressed_cycles`. Suppression is bypassed above `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT`. | +| `off` (case-insensitive) | **Gate OFF.** The cooldown is skipped entirely. Reclaim fires on every cycle where `%-used ≥ SIMARD_DISK_RECLAIM_PCT`, exactly as before this feature. No `suppressed_cycles` metric is emitted and the `noop_streak` / `effective` attributes are absent. | + +## When to use it + +Set `off` only to **temporarily** diagnose or work around the gate — for +example, if you suspect the cooldown is holding back reclamation on a +legitimately recoverable disk and you want to force every-cycle attempts while +you investigate. In steady state, leave it ON: the whole point of the gate is to +stop the wasteful churn that #4809 / #4810 reported. + +## Set it via systemd + +```ini +# /etc/systemd/system/simard-daemon.service.d/reclaim-effectiveness.conf +[Service] +Environment=SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE=off +``` + +```bash +sudo systemctl daemon-reload +sudo systemctl restart simard-daemon +``` + +Remove the drop-in (or set any non-`off` value) and restart to return to the +secure default. + +## Verify which mode the daemon is in + +- **Logs:** with the gate ON, a held cycle logs + `WARN: disk reclaim held — N consecutive no-op run(s), cooling down`. + With the gate OFF you never see that line; you instead see the reclaim run (or + the `under threshold` line) on every cycle. +- **Telemetry:** with the gate ON, `simard.disk.reclaim.suppressed_cycles` + appears in `simard status` / the OTLP export and increments on held cycles. + With the gate OFF the counter is absent. See + [disk-reclaim telemetry](../reference/disk-reclaim-telemetry.md). + +## Related knobs + +Prefer **tuning** the gate over disabling it — see +[configure reclaim effectiveness](../howto/configure-reclaim-effectiveness.md): + +- `SIMARD_DISK_RECLAIM_COOLDOWN_BASE_SECS` — shorten the initial cooldown. +- `SIMARD_DISK_RECLAIM_COOLDOWN_MAX_SECS` — cap the maximum cooldown. +- `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT` — lower the ceiling so genuine + fill-ups bypass the cooldown sooner. diff --git a/docs/reference/disk-reclaim-api.md b/docs/reference/disk-reclaim-api.md index 32141e7a9..932e91e8a 100644 --- a/docs/reference/disk-reclaim-api.md +++ b/docs/reference/disk-reclaim-api.md @@ -1,7 +1,7 @@ --- title: Disk reclaim API -description: Reference for the src/disk_reclaim module — the ReclaimCandidate serde contract, the non-bypassable guard::vet_candidate rail and its RejectReason set, resolve_daemon_working_dirs, the exec_reclaim executor and ReclaimReport, the disk-reclaim.yaml analysis-only recipe contract, and the simard disk-reclaim CLI surface. -last_updated: 2026-07-07 +description: Reference for the src/disk_reclaim module — the ReclaimCandidate serde contract, the non-bypassable guard::vet_candidate rail and its RejectReason set, the allow_roots reclamation scope (including per-repo target/ for routine build-cache reclaim), resolve_daemon_working_dirs, the exec_reclaim executor with per-candidate structured skip-reason tracing and ReclaimReport, the disk-reclaim.yaml analysis-only recipe contract, and the simard disk-reclaim CLI surface. +last_updated: 2026-07-27 review_schedule: as-needed owner: simard doc_type: reference @@ -195,17 +195,61 @@ once per run from the same managed-repo set the recipe inspects: ``` allow_roots = { /engineer-worktrees } // ~/.simard engineer worktrees - ∪ { /worktrees for repo in MANAGED_REPOS } // Simard, amplihack-rs, amplihack-memory-lib - ∪ { shared cargo target dirs under } // stale build caches + ∪ { /worktrees for repo in MANAGED_REPOS } // per-repo worktree dirs + ∪ { /target for repo in MANAGED_REPOS } // per-repo build artifacts + ∪ { /cargo-target, /shared-target } // shared build caches ``` `MANAGED_REPOS` is the same hardcoded managed-repo list the recipe enumerates (§`recipe.rs`); it is **not** operator-configurable free-form (operators widen the *deny*-set via `SIMARD_GIT_PROTECTED_REPOS`, never the allow-set — widening the delete scope from the environment would be a footgun). A candidate outside -every allow-root — anywhere in `$HOME` not under a managed worktree/cache root, -or any absolute path the agent hallucinates — is refused before any other rail -is even consulted. +every allow-root — anywhere in `$HOME` not under a managed worktree/target/cache +root, or any absolute path the agent hallucinates — is refused before any other +rail is even consulted. + +#### The per-repo `target/` root (routine-reclaim scope) + +`allow_roots` includes **`/target`** (the `target/` **parent** directory) +for every managed repo. This is the root that lets **routine** reclaim free +rebuildable Cargo artifacts — `target/debug/`, `target/release/`, +`target/llvm-cov-target/`, and the incremental-compilation caches — the same +build artifacts the deterministic `emergency_cleanup` (Tier 1) removes at severe +pressure. + +The root is deliberately the **parent** `target/`, not an exact-match +`target/debug` entry, because `is_safe_to_delete` +(`maintenance::is_safe_to_delete`) requires a candidate to be **strictly inside** +an allow-root (component-wise `Path::starts_with`, never string-prefix). Rooting +at `target/`'s parent is the smallest change that lets a `StaleBuildCache` +candidate for `target/debug` clear the containment check without altering the +guard's strict-inside semantics. Widening to `target/` is **additive**: every +direct child of `target/` is a rebuildable artifact already inside the +build-cache category the guard permits, and removal is still gated by + +- the agent proposing a `CandidateKind::StaleBuildCache` candidate, +- the `ProtectedDenySet` (which still wins over any allow-root, so + `worktrees/main` and daemon working dirs remain unreclaimable), +- the live-PID rail (any process whose `/proc//cwd` resolves *inside* the + candidate is refused with `LiveProcess`; note this keys on cwd, so a + `cargo build` invoked with its cwd at the repo root is not caught by this rail + — such artifacts are instead safe because they are the rebuildable + `StaleBuildCache` class), and +- the TOCTOU re-assert + symlink refusal at the syscall boundary. + +Rooting at `target/`'s parent **cannot** reach `/src`, `/.git`, or +sibling files: strict-inside containment confines removal to descendants of +`target/`. `MANAGED_REPOS` never includes bare `$HOME` +(`managed_repos_do_not_include_bare_home` guards this), so the widened root +cannot escalate to a home-directory sweep. + +**Why this matters (regression fixed):** before the `target/` root existed, +routine reclaim had no allow-root covering `/target`, so every proposed +`target/debug` candidate was rejected with `OutsideAllowRoot` and pushed to the +human-review list — the "freed 0 bytes, 0 paths removed, N skipped for review" +no-op that let disk climb until the ~30-minute `emergency_cleanup` pass. With +`target/` in scope, routine reclaim frees those artifacts **proactively between** +emergency passes instead of skipping them. ### `ProtectedDenySet` @@ -259,6 +303,40 @@ Sorts candidates **largest-first** (by fresh measurement), then loops: Failures during an individual removal are captured in `failures[]` and do not abort the run. +### Structured skip-reason tracing + +Every rejected candidate is both pushed to `skipped[]` **and** logged with a +structured `tracing` event at the reject site (immediately after `vet_candidate` +returns `Verdict::Reject`), so an operator can see *why* a path was not reclaimed +without reconstructing it from counters: + +```rust +tracing::info!( + target: "simard::disk_reclaim", + path = %candidate.path.display(), + reason = ?reason, // the closed RejectReason enum + kind = ?candidate.kind, // CandidateKind + "reclaim candidate skipped by guard", +); +``` + +- **Fields only** — `path`, the `RejectReason` enum, and `CandidateKind`. The + agent's free-text `reason` field is **never** logged as a field (anti + log-forging; see [telemetry](./disk-reclaim-telemetry.md)), + and no file contents or env values are emitted. +- **`info` level**, one event per skipped candidate, OTLP-compatible (rides the + same `tracing` → OTel pipeline as the rest of the module). No `print!` / + `println!` — library paths use `tracing` exclusively. +- Complements, and does not replace, the existing `summary()` one-liner + (`"… N skipped for review"`) and the + `simard.disk.reclaim.candidates_skipped` counter. The per-candidate events + answer *which* path and *why*; the summary and counter answer *how many*. + +This is the observability that turns a silent "N skipped for review" line into +an actionable, per-path audit trail — e.g. distinguishing a `target/debug` +skipped for `LiveProcess` (a build is running; expected) from one skipped for +`OutsideAllowRoot` (a scope bug worth investigating). + **Subprocess hardening:** git is invoked with `env_clear` (only `PATH`/`HOME`), argument vectors only (no shell), `--` separators, and leading-dash paths rejected — blocking `GIT_*` / `LD_PRELOAD` hijacking and option injection. `gh` @@ -434,12 +512,13 @@ tempdirs, a fabricated `/proc` root, and `FakeLiveProcessProbe` / fake | Test file | Proves | | --------- | ------ | -| `tests_guard.rs` | Refusal of: `worktrees/main`, daemon cwd, live-PID path, uncommitted/unpushed, active worktree, outside-root, unknown-PR, symlink, option-injection — each asserted **skipped even when instructed to delete**. Plus the **OPEN-PR merge-base-is-ancestor regression** (a fresh worktree whose merge-base is an ancestor of `main` is **not** reclaimed). | +| `tests_guard.rs` | Refusal of: `worktrees/main`, daemon cwd, live-PID path, uncommitted/unpushed, active worktree, outside-root, unknown-PR, symlink, option-injection — each asserted **skipped even when instructed to delete**. Plus the **OPEN-PR merge-base-is-ancestor regression** (a fresh worktree whose merge-base is an ancestor of `main` is **not** reclaimed). With the widened `/target` allow-root: `/src` and `/.git` are still refused (`OutsideAllowRoot`), a symlinked `target/*` is refused, a live-PID `target/debug` is refused (`LiveProcess`), and `worktrees/main` in the deny-set still overrides the allow-root. | | `tests_candidate.rs` | `deny_unknown_fields`; bad-element-skip vs bad-array-hard-error; bounds. | -| `tests_executor.rs` | Largest-first ordering; threshold stop; dry-run zero-ops; TOCTOU re-assert; fake `DiskStatProvider`. | +| `tests_executor.rs` | Largest-first ordering; threshold stop; dry-run zero-ops; TOCTOU re-assert; fake `DiskStatProvider`. **Routine-reclaim regression:** a `target/debug` candidate outside the old scope reproduces `bytes_freed == 0` with the path in `skipped[]` (`reject_reason = OutsideAllowRoot`); with the `/target` root it moves to `removed[]` with `bytes_freed > 0`. The reject arm emits the per-candidate `tracing::info!(path, reason, kind)` event; the test asserts the recorded `SkippedPath.reject_reason`. | | `tests_daemon_dir.rs` | Union resolution with injected `proc_root`; hardcoded `main` always present. | | `tests_recipe.rs` | Marker parse; no-fallback `AdapterInvocationFailed` on recipe/parse failure. | | `tests_env.rs` | `reclaim_pct_from_env` clamping; `daemon_apply_from_env` returns `DryRun` unless `SIMARD_DISK_RECLAIM_DAEMON_APPLY=1`. | +| `mod.rs` (unit) | `allow_roots_cover_engineer_and_managed_worktrees` (extended to cover per-repo `target/`); `managed_repos_do_not_include_bare_home` (widened root never resolves to bare `$HOME`). | ## Related diff --git a/docs/reference/goal-reblock-backoff-api.md b/docs/reference/goal-reblock-backoff-api.md new file mode 100644 index 000000000..7b4ce1dd5 --- /dev/null +++ b/docs/reference/goal-reblock-backoff-api.md @@ -0,0 +1,167 @@ +--- +title: Goal-reblock backoff & stewardship dedup — API reference +description: > + The typed surface of the Overseer's goal-reblock suppression rail (#4817 / + #4828): the `goal_reblock_backoff` `BackoffGate` field and its + `overseer-obs:goal:blocked:{goal_id}` key, the peek/commit wiring in the + Overseer `gate()` / `act()` goal-hygiene path, the stabilised `GoalBlocked` + failure-signature text, the `normalize_for_signature` counter-redaction + contract in `src/stewardship/dedup.rs`, and the goal_id normalization applied + before keys and issue text. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: reference +status: implemented +related: + - ../concepts/goal-reblock-backoff-dedup.md + - ./overseer-backoff-gate-api.md + - ./stewardship-api.md + - ./overseer-recipe-launch-idempotency.md + - ../concepts/gap-scan-backoff-dedup.md + - ../howto/diagnose-recurring-goal-reblock-churn.md +--- + +# Goal-reblock backoff & stewardship dedup — API reference + +> **Status: implemented (#4817 / #4828).** The `goal_reblock_backoff` gate field +> and its `gate()`/`act()` wiring, plus the stabilised `GoalBlocked` signature +> text, live in +> [`src/overseer/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/mod.rs); +> the shared `BackoffGate` primitive in +> [`src/overseer/guardrails.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/guardrails.rs); +> and the counter-redaction rule in +> [`src/stewardship/dedup.rs`](https://github.com/rysweet/Simard/blob/main/src/stewardship/dedup.rs). +> For the rationale see +> [goal-reblock backoff & stewardship dedup](../concepts/goal-reblock-backoff-dedup.md). + +## The gate field + +```rust +// in the Overseer struct (src/overseer/mod.rs) +/// Per-goal exponential-backoff suppression for still-blocked GoalHygiene +/// briefs, which have `sequence_group = None` and so escape the gap-scan / +/// coverage in-flight guards. Keyed on `overseer-obs:goal:blocked:{goal_id}`. +goal_reblock_backoff: BackoffGate, +``` + +It is the same [`BackoffGate`](./overseer-backoff-gate-api.md) primitive used by +the gap-scan rail (`peek` / `commit` / `admit`, bounded exponential window, +clock-regression-safe, reset-on-long-silence). It is constructed from the shared +`SIMARD_OVERSEER_BACKOFF_*` window configuration (see +[BackoffGate reference](./overseer-backoff-gate-api.md#configuration-accessors)). + +### The dedup key + +```rust +let key = format!("overseer-obs:goal:blocked:{}", normalize_goal_id(goal_id)); +``` + +The `overseer-obs:` prefix namespaces it away from the gap-scan keys; the +per-goal suffix means each blocked goal backs off independently. + +## Wiring: `gate()` peeks, `act()` commits + +Mirroring the gap-scan idempotency pattern +([recipe-launch idempotency](./overseer-recipe-launch-idempotency.md)): + +- **`gate()`** — before admitting a GoalHygiene relaunch, it + `goal_reblock_backoff.peek(key, now_secs)`. On `BackoffDecision::Suppress` the + relaunch is dropped for this cycle (the goal is still blocked / in-flight); + on `Admit` it proceeds. +- **`act()`** — after a **successful** relaunch it + `goal_reblock_backoff.commit(key, now_secs)`, arming/growing the window. A + launch that is itself held (`held: per-cycle launch cap reached`) or fails does + **not** commit, so it does not consume the dedup slot. + +```rust +match self.goal_reblock_backoff.peek(&key, now_secs) { + BackoffDecision::Suppress => { /* skip relaunch; goal still blocked */ } + BackoffDecision::Admit => { + // ... relaunch the covering workstream ... + // on success only: + self.goal_reblock_backoff.commit(&key, now_secs); + } +} +``` + +Suppression applies **only to the relaunch**. The single stewardship issue for +the blocked goal is still filed (see below) so the human always has visibility. + +## Stabilised `GoalBlocked` signature + +The `GoalHygiene` problem's `dedup_key` is unchanged +(`goal:blocked:{goal_id}`), but the **signature-bearing error text** no longer +embeds the fluctuating counter: + +```rust +// BEFORE (#4817/#4828): counter in the signature text → signature changed every tick +format!("goal {goal_id} blocked ({consecutive_no_action} no-action cycle(s))") + +// AFTER: counter removed from the signature input; kept only in the issue body/title +format!("goal {goal_id} blocked{}", + if needs_review { " — needs human review" } else { "" }) +``` + +`failure_signature(ProblemKind::GoalHygiene, text)` therefore produces a +**stable** signature across cycles, so +[`find_existing()`](./stewardship-api.md) matches the already-open issue and the +overseer reuses it instead of filing a duplicate. + +The `consecutive_no_action` count is still surfaced to the human — it is written +into the issue **body / title annotation**, just not into the hashed signature +input. + +## `normalize_for_signature` counter redaction + +Defense-in-depth in `src/stewardship/dedup.rs`: `normalize_for_signature` now +redacts residual counter patterns so any counter that leaks into a signature +input still collapses to a single signature. This sits alongside the existing +UUID/timestamp redaction (`uuid_redaction_tests`). + +Redacted patterns (case-insensitive, whitespace-tolerant): + +| Pattern | Normalized to | +| ------- | ------------- | +| `( no-action cycle(s))` | `( no-action cycle(s))` | +| `no progress for cycles` | `no progress for cycles` | + +Two `GoalBlocked` failures for the same goal that differ **only** by the counter +now share one signature → `MatchedExisting` → one issue. + +## `goal_id` normalization (untrusted input) + +Before a `goal_id` is embedded in a dedup key or an issue title/body it is +normalized: + +- **length-bounded** (truncated to a fixed max), +- **charset-restricted** to `[A-Za-z0-9._:-]` (other characters dropped/replaced). + +This prevents a crafted goal id from colliding signatures or injecting content +into a stewardship issue body/title. + +## Invariants (asserted by unit tests) + +Tests live in +[`src/overseer/tests_goal_health.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/tests_goal_health.rs) +and the `dedup.rs` sibling of `uuid_redaction_tests`: + +- **Relaunch suppressed:** a still-blocked goal is not relaunched on the next + cycle while inside the window. +- **Re-admit on clear:** once the block clears (long silence), the goal + re-admits promptly. +- **Never starves:** exactly **one** stewardship issue is still filed for a + suppressed-relaunch goal. +- **Stable signature:** two `GoalBlocked` failures differing only by the + no-action counter share one signature (`MatchedExisting`). +- **Hostile goal_id:** an oversized / non-charset goal id is normalized before + it reaches any key or issue text. +- **Failed launch doesn't commit:** a held/failed relaunch does not consume the + dedup slot (peek-then-commit-on-success). + +## See also + +- [Overseer BackoffGate reference](./overseer-backoff-gate-api.md) — the shared primitive + config. +- [Stewardship API](./stewardship-api.md) — `failure_signature` / `find_existing`. +- [Goal-reblock backoff & stewardship dedup](../concepts/goal-reblock-backoff-dedup.md) — the rationale. +- [Diagnose recurring goal-reblock churn](../howto/diagnose-recurring-goal-reblock-churn.md) — the runbook. diff --git a/docs/reference/reclaim-effectiveness-gate-api.md b/docs/reference/reclaim-effectiveness-gate-api.md new file mode 100644 index 000000000..abbd02730 --- /dev/null +++ b/docs/reference/reclaim-effectiveness-gate-api.md @@ -0,0 +1,207 @@ +--- +title: Reclaim effectiveness gate — API reference +description: > + The typed surface of the disk-reclaim effectiveness gate (#4809 / #4825 / + #4810): the `ReclaimEffectivenessGate` suppress-only cooldown primitive and + its `EffectivenessDecision` enum in `src/disk_reclaim/effectiveness.rs`, the + cross-cycle canonicalized skip-memory in `src/disk_reclaim/executor.rs`, the + `SIMARD_DISK_RECLAIM_COOLDOWN_*` / `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT` + configuration accessors, the new additive `emit_reclaim_telemetry` attributes, + and how the gate is wired into the OODA daemon disk-reclaim trigger. +last_updated: 2026-07-27 +review_schedule: as-needed +owner: simard +doc_type: reference +status: implemented +related: + - ../concepts/reclaim-effectiveness-backoff.md + - ./disk-reclaim-api.md + - ./disk-reclaim-telemetry.md + - ./overseer-backoff-gate-api.md + - ../operations/reclaim-effectiveness-kill-switch.md + - ../howto/configure-reclaim-effectiveness.md +--- + +# Reclaim effectiveness gate — API reference + +> **Status: implemented (#4809 / #4825 / #4810).** The +> `ReclaimEffectivenessGate` and `EffectivenessDecision` types live in +> [`src/disk_reclaim/effectiveness.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/effectiveness.rs); +> the cross-cycle skip-memory in +> [`src/disk_reclaim/executor.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/executor.rs); +> the config accessors alongside the existing reclaim knobs in +> [`src/disk_reclaim/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/mod.rs); +> the telemetry attributes in +> [`src/disk_reclaim/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/disk_reclaim/mod.rs) +> (`emit_reclaim_telemetry`) with names in +> [`src/telemetry/names.rs`](https://github.com/rysweet/Simard/blob/main/src/telemetry/names.rs); +> and the daemon wiring in +> [`src/operator_commands_ooda/daemon/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/operator_commands_ooda/daemon/mod.rs). +> For the rationale see [reclaim effectiveness backoff](../concepts/reclaim-effectiveness-backoff.md). + +## `EffectivenessDecision` + +```rust +/// The gate's verdict for the current daemon cycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EffectivenessDecision { + /// Run reclamation this cycle (unseen key, cooldown elapsed, effective last + /// time, or hard-ceiling bypass). + Run, + /// Skip reclamation this cycle — a streak of no-op runs is in cooldown. + Suppress, +} +``` + +## `ReclaimEffectivenessGate` + +A suppress-only wrapper around the same bounded-exponential-backoff semantics as +[`BackoffGate`](./overseer-backoff-gate-api.md). It tracks a per-key no-op streak +and cooldown window and, crucially, **cannot alter the reclaim run's destructive +posture** — it only decides *whether* a run happens, never *how* it runs. + +```rust +pub struct ReclaimEffectivenessGate { /* private */ } + +impl ReclaimEffectivenessGate { + /// base cooldown / growth multiplier / cap, plus the hard %-used ceiling + /// above which suppression is always bypassed. + pub fn new( + base_window_secs: i64, + multiplier: i64, + max_window_secs: i64, + hard_ceiling_pct: u8, + ) -> Self; + + /// Decide WITHOUT recording. `used_pct` is a FRESH local df sample (never + /// telemetry): at/above `hard_ceiling_pct` this always returns `Run` + /// (bypassing any cooldown). Otherwise an unseen key, an elapsed cooldown, + /// or a backwards clock jump returns `Run`; a re-hit strictly inside the + /// current cooldown returns `Suppress`. + pub fn peek(&self, key: &str, used_pct: u8, now_secs: i64) -> EffectivenessDecision; + + /// Record the OUTCOME of a run that actually happened. `effective == true` + /// (bytes were freed or used_pct dropped) RESETS the streak and cooldown; + /// `effective == false` grows the no-op streak and the cooldown window + /// (× multiplier, saturating, capped). A silence ≥ 2× the current window + /// since the last record also resets to the base window. + pub fn record(&mut self, key: &str, effective: bool, now_secs: i64); +} +``` + +### Semantics + +| Situation | `peek` result | Effect | +| --------- | ------------- | ------ | +| Key never seen | `Run` | first attempt always runs | +| `used_pct ≥ hard_ceiling_pct` | `Run` | **bypass** — genuine fill-up always reclaims | +| Cooldown window elapsed | `Run` | retry after backoff | +| Backwards clock jump | `Run` | fail toward surfacing; never suppress on an untrusted clock | +| Re-hit inside cooldown, below ceiling | `Suppress` | skip cycle | + +| `record` input | Effect on streak / window | +| -------------- | ------------------------- | +| `effective = true` | reset streak → 0, cooldown → base | +| `effective = false` (first) | arm base cooldown | +| `effective = false` (subsequent) | window `× multiplier`, saturating, capped | +| silence ≥ 2× window | reset to base window | + +The daemon **peeks then records** — it records the outcome only *after* a run +completes, so a run that is skipped (suppressed) never advances the streak, and +a run that errors is not counted as an effective reclaim. + +### The dedup key + +The daemon keys the gate on the reclamation target partition: + +```rust +let key = format!("disk-reclaim:{}", state_root_partition_id); +``` + +so distinct partitions back off independently. + +## Cross-cycle skip memory + +`exec_reclaim` (`src/disk_reclaim/executor.rs`) now carries a set of +**canonicalized** candidate paths that a rail rejected on a previous cycle and +declines to re-propose them: + +- Every candidate is `canonicalize`d **before** both the guard check and the + skip-memory lookup, so a symlink or `..` alias cannot smuggle a + previously-rejected (or protected) path past the guard. +- A candidate whose canonical path is in skip-memory is dropped without + re-vetting; the corresponding `candidates_skipped` telemetry is still emitted + so the human-review list stays complete. +- Any canonicalization failure treats the path as **not authorized to delete** + (skip), never as authorized. + +Skip-memory is bounded and evicts on TTL / effective-run reset so it cannot grow +unbounded on a long-running daemon. + +## Configuration + +All accessors fail safe (invalid input → the documented default) and are pure +functions of an injected `lookup: impl Fn(&str) -> Option` for testing, +mirroring the existing reclaim/overseer config style. + +| Env var | Default | Meaning | +| ------- | ------- | ------- | +| `SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE` | `on` | Master kill switch. `off` (case-insensitive) reverts to fire-every-over-threshold-cycle. See the [kill switch](../operations/reclaim-effectiveness-kill-switch.md). | +| `SIMARD_DISK_RECLAIM_COOLDOWN_BASE_SECS` | `900` | Base cooldown window after the first no-op run. | +| `SIMARD_DISK_RECLAIM_COOLDOWN_MULTIPLIER` | `2` | Growth factor per additional no-op run (`≥ 2`; lower values clamp to `2`). | +| `SIMARD_DISK_RECLAIM_COOLDOWN_MAX_SECS` | `14400` | Hard cap on the cooldown window (4 h). | +| `SIMARD_DISK_RECLAIM_HARD_CEILING_PCT` | `97` | Locally-observed `%-used` at/above which suppression is bypassed and reclamation always runs. | + +The pre-existing reclaim knobs are unchanged: +`SIMARD_DISK_RECLAIM_PCT` (trigger threshold, default `85`) and +`SIMARD_DISK_RECLAIM_DAEMON_APPLY` (the sole apply opt-in). + +## Telemetry (additive) + +`emit_reclaim_telemetry` gains three additive attributes on the existing +`simard.disk.reclaim.*` series plus one new counter. Existing metrics keep their +names and shapes — see [disk-reclaim telemetry](./disk-reclaim-telemetry.md) for +the full catalog. + +| Name | Type | Attributes | Meaning | +| ---- | ---- | ---------- | ------- | +| `simard.disk.reclaim.suppressed_cycles` | counter | `source` | daemon cycles the effectiveness gate skipped (would-have-run-but-held). | +| *(existing series)* | — | `+ noop_streak`, `+ suppressed_cycles`, `+ effective` | current no-op streak, cumulative suppressed count, and whether the just-completed run freed space. Low-cardinality; **no raw paths, env, or secrets** are ever emitted as attributes. | + +## Daemon wiring + +In the Tier-3 disk-reclaim block of +`src/operator_commands_ooda/daemon/mod.rs`, the trigger now: + +1. samples fresh `used_pct` via the existing `df` probe; +2. if `used_pct ≥ SIMARD_DISK_RECLAIM_PCT`, calls + `gate.peek(key, used_pct, now)`; +3. on `Suppress`, logs an `info` line, increments + `simard.disk.reclaim.suppressed_cycles`, and skips the run; +4. on `Run`, invokes `run_disk_reclaim(..)` as before, then calls + `gate.record(key, report.was_effective(), now)` where `was_effective()` is + `bytes_freed > 0 || used_pct_after < used_pct_before`. + +The gate is a suppress-only pre-filter in front of the *unchanged* +propose/dispose path; every existing safety rail (protected paths, live +processes, uncommitted/unpushed, active worktree, allow-root, dry-run default) +runs exactly as before. + +## Invariants (asserted by unit tests) + +- **Suppress-only:** the gate can never turn a dry-run into an apply. +- **Ceiling bypass:** `used_pct ≥ hard_ceiling_pct` always returns `Run`, even + deep inside a cooldown. +- **Effective reset:** an effective run immediately re-admits the next cycle. +- **Saturating counters:** streak/exponent never overflow. +- **Canonicalized skip-memory:** a symlink/`..` alias of a rejected path is + still rejected. +- **Least-data telemetry:** new attributes carry only counts/booleans — no paths + or env values. + +## See also + +- [Reclaim effectiveness backoff](../concepts/reclaim-effectiveness-backoff.md) — the rationale. +- [Disk reclaim API](./disk-reclaim-api.md) — the propose/dispose contract this pre-filters. +- [Overseer BackoffGate reference](./overseer-backoff-gate-api.md) — the shared backoff primitive. +- [Reclaim-effectiveness kill switch](../operations/reclaim-effectiveness-kill-switch.md) — how to disable it. diff --git a/mkdocs.yml b/mkdocs.yml index caa691262..545967a59 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -98,6 +98,8 @@ nav: - Concepts: - Operational Autonomy Model: concepts/operational-autonomy-model.md - Gap-Scan Dedup & Backoff: concepts/gap-scan-backoff-dedup.md + - Reclaim Effectiveness Backoff (disk-reclaim stops churning): concepts/reclaim-effectiveness-backoff.md + - Goal-Reblock Backoff & Stewardship Dedup: concepts/goal-reblock-backoff-dedup.md - Autonomous Self-Merge Sensor (ready_prs wire): concepts/autonomous-self-merge-sensor.md - Agentic Merge-Queue + Issue Reasoning (observe/orient): concepts/agentic-merge-queue-reasoning.md - Overseer Agentic Health-Review (self-heal crash-loops): concepts/overseer-agentic-health-review.md @@ -260,6 +262,7 @@ nav: - Configure the Monthly Self-Quality-Audit: howto/configure-self-quality-audit.md - Configure the Disk-Health Check: howto/configure-disk-health-check.md - Configure Disk Reclamation: howto/configure-disk-reclamation.md + - Configure Reclaim Effectiveness (stop disk-reclaim churn): howto/configure-reclaim-effectiveness.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 - Triage Stale Pull Requests: howto/triage-stale-pull-requests.md @@ -284,6 +287,7 @@ nav: - Review Overseer Workstream Gaps: howto/review-overseer-workstream-gaps.md - Diagnose Recurring Cognitive-Memory Signature: howto/diagnose-recurring-cognitive-memory-signature.md - Configure Overseer Gap-Scan Backoff: howto/configure-overseer-gap-scan-backoff.md + - Diagnose Recurring Goal-Reblock Churn: howto/diagnose-recurring-goal-reblock-churn.md - Configure Overseer Root-Cause Principle: howto/configure-overseer-root-cause-why.md - Configure Cognitive-Thread Scheduling: howto/configure-cognitive-thread-scheduling.md - Add a New Cognitive Thread: howto/add-a-new-cognitive-thread.md @@ -324,6 +328,7 @@ nav: - Overseer Signal JSON-RPC Transport: reference/overseer-signal-jsonrpc-transport.md - Overseer Workstream Gap-Scan: reference/overseer-workstream-gap-scan.md - Overseer BackoffGate & Gap-Scan Dedup: reference/overseer-backoff-gate-api.md + - Goal-Reblock Backoff & Stewardship Dedup API: reference/goal-reblock-backoff-api.md - Overseer Root-Cause (WHY) API: reference/overseer-root-cause-why-api.md - Overseer Self-Observation Stability: reference/overseer-self-observation-stability.md - simard-engineer-step CLI: reference/simard-engineer-step.md @@ -468,6 +473,7 @@ nav: - Disk-Health API: reference/disk-health-api.md - Disk Reclaim API: reference/disk-reclaim-api.md - Disk Reclaim Telemetry: reference/disk-reclaim-telemetry.md + - Reclaim Effectiveness Gate API: reference/reclaim-effectiveness-gate-api.md - Dashboard E2E Tests: reference/dashboard-e2e-tests.md - Dashboard Action-Detail Humanization: reference/dashboard-action-detail-humanization.md - Dashboard Goal Lifecycle-Status Badges: reference/dashboard-goal-lifecycle-status.md @@ -510,6 +516,7 @@ nav: - Engineer-Admission Kill Switch: operations/engineer-admission-kill-switch.md - Claim-Reaper Kill Switch & Tuning: operations/claim-reaper-kill-switch.md - Resource-Admission Kill Switch: operations/resource-admission-kill-switch.md + - Reclaim-Effectiveness Kill Switch: operations/reclaim-effectiveness-kill-switch.md - Creative-Ideas Semantic-Dedup Kill Switch: operations/creative-ideas-semantic-dedup-kill-switch.md - Safe Self-Update: safe-self-update.md - Pre-Commit Setup: operations/pre-commit-setup.md diff --git a/prompt_assets/simard/recipes/disk-reclaim.yaml b/prompt_assets/simard/recipes/disk-reclaim.yaml index 85411eed1..eaf819455 100644 --- a/prompt_assets/simard/recipes/disk-reclaim.yaml +++ b/prompt_assets/simard/recipes/disk-reclaim.yaml @@ -83,11 +83,24 @@ steps: ``` Skip any candidate whose path is at/under a live cwd. - 5. **Find stale build caches.** Locate `target/` directories that are - obviously stale (belong to a worktree you are already proposing, or the - shared cargo caches `{{state_root}}/cargo-target/` and - `{{state_root}}/shared-target/`). Measure sizes with - `du -sb `. + 5. **Find stale build caches.** Propose rebuildable Cargo artifact + directories, measuring each with `du -sb `. Always propose the + **children** of a `target/` (e.g. `target/debug`, `target/release`, + `target/llvm-cov-target`), never a bare `target/` parent — the + executor's strict-inside containment refuses a path that equals an + allow-root. Cover: + - each managed repo's top-level `target/` children — + `/home/azureuser/src/Simard/target/*`, + `/home/azureuser/src/amplihack-rs/target/*`, and + `/home/azureuser/src/amplihack-memory-lib/target/*` — when no live + process (step 4) holds them. These are the routine build-cache wins + that keep the partition off the emergency backstop between passes, + - `target/` children belonging to a worktree you are already proposing, + - the shared cargo caches under `{{state_root}}/cargo-target/` and + `{{state_root}}/shared-target/`. + A running `cargo build` simply rebuilds a deleted artifact directory, so + this is only a cost, never a correctness risk — but still skip any path + with a live process; the executor refuses it anyway. 6. **Reason largest-first.** Sort your proposed candidates by measured size descending. Propose the biggest safe wins first. Propose enough to get diff --git a/src/disk_reclaim/effectiveness.rs b/src/disk_reclaim/effectiveness.rs new file mode 100644 index 000000000..dafd27528 --- /dev/null +++ b/src/disk_reclaim/effectiveness.rs @@ -0,0 +1,552 @@ +//! Reclaim effectiveness gate — the suppress-only cooldown that stops the OODA +//! daemon re-running agentic disk-reclamation every cycle when reclamation keeps +//! freeing nothing (issues #4809 / #4825 / #4810). +//! +//! # Why this exists +//! The plain `%-used` trigger re-fired a *proven-ineffective* reclaim run every +//! ~15 minutes on the ~94%-full host — churn that burned CPU/IO and log volume +//! without freeing a byte. This gate wraps the same bounded-exponential-backoff +//! semantics as [`crate::overseer::guardrails::BackoffGate`] into a **suppress +//! -only** pre-filter: it decides *whether* a run happens, and can never change +//! *how* it runs (a dry-run can never become an apply). +//! +//! # Contract (mirrors `BackoffGate`'s peek/record split) +//! - [`ReclaimEffectivenessGate::new`]`(base, multiplier, max, hard_ceiling_pct)` +//! - [`ReclaimEffectivenessGate::peek`] decides WITHOUT recording. A fresh local +//! `used_pct` at/above `hard_ceiling_pct` always returns [`Run`] (a genuinely +//! filling disk always reclaims). Otherwise an unseen key, an elapsed cooldown, +//! or a backwards clock jump returns [`Run`]; a re-hit strictly inside the +//! current cooldown (below ceiling) returns [`Suppress`]. +//! - [`ReclaimEffectivenessGate::record`] records the OUTCOME of a run that +//! actually happened: `effective == true` re-admits the next cycle (streak → 0, +//! no cooldown armed); `effective == false` arms/grows the cooldown window +//! `× multiplier` (saturating, capped). A silence `>= 2× window` resets to base. +//! +//! [`Run`]: EffectivenessDecision::Run +//! [`Suppress`]: EffectivenessDecision::Suppress +//! +//! See `docs/reference/reclaim-effectiveness-gate-api.md` for the full contract +//! and `docs/concepts/reclaim-effectiveness-backoff.md` for the rationale. + +use std::collections::HashMap; + +/// Master kill switch (see the operations doc). `off`/`false`/`0`/`no` (case +/// -insensitive) reverts to the fire-every-over-threshold-cycle behaviour. +pub const EFFECTIVENESS_GATE_ENV: &str = "SIMARD_DISK_RECLAIM_EFFECTIVENESS_GATE"; +/// Base cooldown window (seconds) after the first no-op run. +pub const COOLDOWN_BASE_SECS_ENV: &str = "SIMARD_DISK_RECLAIM_COOLDOWN_BASE_SECS"; +/// Growth factor per additional no-op run (`>= 2`; lower values clamp to `2`). +pub const COOLDOWN_MULTIPLIER_ENV: &str = "SIMARD_DISK_RECLAIM_COOLDOWN_MULTIPLIER"; +/// Hard cap (seconds) on the cooldown window. +pub const COOLDOWN_MAX_SECS_ENV: &str = "SIMARD_DISK_RECLAIM_COOLDOWN_MAX_SECS"; +/// Locally-observed `%-used` at/above which suppression is bypassed. +pub const HARD_CEILING_PCT_ENV: &str = "SIMARD_DISK_RECLAIM_HARD_CEILING_PCT"; + +/// Default base cooldown window: 15 minutes. +pub const DEFAULT_COOLDOWN_BASE_SECS: i64 = 900; +/// Default cooldown growth multiplier. +pub const DEFAULT_COOLDOWN_MULTIPLIER: i64 = 2; +/// Default cooldown cap: 4 hours. +pub const DEFAULT_COOLDOWN_MAX_SECS: i64 = 14_400; +/// Default hard `%-used` ceiling above which suppression is always bypassed. +pub const DEFAULT_HARD_CEILING_PCT: u8 = 97; + +/// The gate's verdict for the current daemon cycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EffectivenessDecision { + /// Run reclamation this cycle (unseen key, cooldown elapsed, effective last + /// time, or hard-ceiling bypass). + Run, + /// Skip reclamation this cycle — a streak of no-op runs is in cooldown. + Suppress, +} + +/// Per-key cooldown bookkeeping: when the last run was recorded, the current +/// suppression window, and the consecutive no-op streak (for telemetry). +#[derive(Debug, Clone, Copy)] +struct GateState { + last_record_secs: i64, + window_secs: i64, + noop_streak: u32, +} + +/// A suppress-only, per-key exponential-backoff cooldown on an INJECTED +/// `now_secs` clock. It **cannot alter a run's destructive posture** — it only +/// decides whether a run happens, never how. Every key backs off independently, +/// so two partitions never starve each other. On any ambiguity (clock +/// regression, overflow) it fails toward [`EffectivenessDecision::Run`] — it +/// never permanently silences a genuinely filling disk. +#[derive(Debug, Clone)] +pub struct ReclaimEffectivenessGate { + base_window_secs: i64, + multiplier: i64, + max_window_secs: i64, + hard_ceiling_pct: u8, + state: HashMap, +} + +impl ReclaimEffectivenessGate { + /// A gate whose cooldown starts at `base_window_secs` and grows + /// `× multiplier` per consecutive no-op run, hard-capped at + /// `max_window_secs`. `hard_ceiling_pct` is the locally-observed `%-used` + /// at/above which suppression is always bypassed. The multiplier is floored + /// at `2` so the window always actually grows. + pub fn new( + base_window_secs: i64, + multiplier: i64, + max_window_secs: i64, + hard_ceiling_pct: u8, + ) -> Self { + Self { + base_window_secs: base_window_secs.max(1), + multiplier: multiplier.max(2), + max_window_secs: max_window_secs.max(base_window_secs.max(1)), + hard_ceiling_pct, + state: HashMap::new(), + } + } + + /// Construct from the process environment (production entry point). + pub fn from_env() -> Self { + // Non-capturing ⇒ `Copy`, so it can be passed by value to each accessor. + let lookup = |k: &str| std::env::var(k).ok(); + Self::new( + cooldown_base_secs_from(lookup), + cooldown_multiplier_from(lookup), + cooldown_max_secs_from(lookup), + hard_ceiling_pct_from(lookup), + ) + } + + /// Decide WITHOUT recording — the daemon peeks, and only records the outcome + /// *after* a run actually completes, so a suppressed cycle never advances the + /// streak. `used_pct` is a FRESH local `df` sample (never telemetry): + /// + /// - `used_pct >= hard_ceiling_pct` ⇒ [`Run`] (bypass; genuine fill always + /// reclaims, even deep inside a cooldown), + /// - unseen key, elapsed cooldown, or backwards clock ⇒ [`Run`], + /// - a re-hit strictly inside the current cooldown, below ceiling ⇒ + /// [`Suppress`]. + /// + /// [`Run`]: EffectivenessDecision::Run + /// [`Suppress`]: EffectivenessDecision::Suppress + pub fn peek(&self, key: &str, used_pct: u8, now_secs: i64) -> EffectivenessDecision { + // Ceiling bypass: a genuinely filling disk is never suppressed. + if used_pct >= self.hard_ceiling_pct { + return EffectivenessDecision::Run; + } + match self.state.get(key) { + None => EffectivenessDecision::Run, + Some(s) => { + let elapsed = now_secs - s.last_record_secs; + // Clock regression OR elapsed window ⇒ fail toward running. + if elapsed < 0 || elapsed >= s.window_secs { + EffectivenessDecision::Run + } else { + EffectivenessDecision::Suppress + } + } + } + } + + /// Record the OUTCOME of a run that actually happened. + /// + /// - `effective == true` (bytes freed / `used_pct` dropped) clears the key so + /// the **next cycle re-admits immediately** — an effective run is never + /// penalised with a cooldown. + /// - `effective == false` (first) arms the base cooldown and streak `1`. + /// - `effective == false` (subsequent, within `2× window`) grows the window + /// `× multiplier` (saturating, capped) and increments the streak. + /// - A silence `>= 2× window` (or a backwards clock) resets to the base + /// window and streak `1`, so a genuinely recurring gap is never + /// permanently silenced. + pub fn record(&mut self, key: &str, effective: bool, now_secs: i64) { + if effective { + // Effective reset: drop the key entirely so the next `peek` sees an + // unseen key and re-admits immediately (streak → 0, no cooldown). + self.state.remove(key); + return; + } + let next = match self.state.get(key) { + // First no-op ⇒ arm the base window. + None => GateState { + last_record_secs: now_secs, + window_secs: self.base_window_secs, + noop_streak: 1, + }, + Some(s) => { + let elapsed = now_secs - s.last_record_secs; + if elapsed < 0 || elapsed >= s.window_secs.saturating_mul(2) { + // Long silence / clock regression ⇒ reset to base. + GateState { + last_record_secs: now_secs, + window_secs: self.base_window_secs, + noop_streak: 1, + } + } else { + // Consecutive no-op ⇒ grow the window (saturating, capped). + GateState { + last_record_secs: now_secs, + window_secs: s + .window_secs + .saturating_mul(self.multiplier) + .min(self.max_window_secs), + noop_streak: s.noop_streak.saturating_add(1), + } + } + } + }; + self.state.insert(key.to_string(), next); + } + + /// The current consecutive no-op streak for `key` (0 if unseen / just reset). + /// Low-cardinality telemetry only — never a raw path. + pub fn noop_streak(&self, key: &str) -> u32 { + self.state.get(key).map(|s| s.noop_streak).unwrap_or(0) + } +} + +// ── Configuration (injectable `lookup`, mirroring the reclaim/overseer style) ── + +fn is_falsey(v: &str) -> bool { + matches!( + v.trim().to_ascii_lowercase().as_str(), + "0" | "false" | "no" | "off" + ) +} + +/// Whether the effectiveness gate is enabled. Default **on**; only an explicit +/// falsey value (`0`/`false`/`no`/`off`, case-insensitive) disables it — +/// unset/empty/garbage leaves it enabled. +pub fn effectiveness_gate_enabled_from(lookup: impl Fn(&str) -> Option) -> bool { + !matches!(lookup(EFFECTIVENESS_GATE_ENV).as_deref(), Some(v) if is_falsey(v)) +} + +/// Production entry point: read the real process environment. +pub fn effectiveness_gate_enabled() -> bool { + effectiveness_gate_enabled_from(|k| std::env::var(k).ok()) +} + +/// Base cooldown seconds; invalid/non-positive → [`DEFAULT_COOLDOWN_BASE_SECS`]. +pub fn cooldown_base_secs_from(lookup: impl Fn(&str) -> Option) -> i64 { + lookup(COOLDOWN_BASE_SECS_ENV) + .and_then(|s| s.trim().parse::().ok()) + .filter(|&v| v > 0) + .unwrap_or(DEFAULT_COOLDOWN_BASE_SECS) +} + +/// Growth multiplier; clamped to `>= 2`; invalid → [`DEFAULT_COOLDOWN_MULTIPLIER`]. +pub fn cooldown_multiplier_from(lookup: impl Fn(&str) -> Option) -> i64 { + lookup(COOLDOWN_MULTIPLIER_ENV) + .and_then(|s| s.trim().parse::().ok()) + .map(|v| v.max(2)) + .unwrap_or(DEFAULT_COOLDOWN_MULTIPLIER) +} + +/// Cooldown cap seconds; invalid/non-positive → [`DEFAULT_COOLDOWN_MAX_SECS`]. +pub fn cooldown_max_secs_from(lookup: impl Fn(&str) -> Option) -> i64 { + lookup(COOLDOWN_MAX_SECS_ENV) + .and_then(|s| s.trim().parse::().ok()) + .filter(|&v| v > 0) + .unwrap_or(DEFAULT_COOLDOWN_MAX_SECS) +} + +/// Hard `%-used` ceiling; clamped to `[1, 100]`; invalid → [`DEFAULT_HARD_CEILING_PCT`]. +pub fn hard_ceiling_pct_from(lookup: impl Fn(&str) -> Option) -> u8 { + lookup(HARD_CEILING_PCT_ENV) + .and_then(|s| s.trim().parse::().ok()) + .map(|v| v.clamp(1, 100)) + .unwrap_or(DEFAULT_HARD_CEILING_PCT) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + + use EffectivenessDecision::{Run, Suppress}; + + /// Below-ceiling `%-used`, so the ceiling bypass never masks cooldown logic. + const LOW: u8 = 90; + const CEILING: u8 = 97; + const BASE: i64 = 900; + + fn gate() -> ReclaimEffectivenessGate { + ReclaimEffectivenessGate::new(BASE, 2, 14_400, CEILING) + } + + fn env(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let map: HashMap = pairs + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(); + move |k: &str| map.get(k).cloned() + } + + // ── peek: admission paths ──────────────────────────────────────────────── + + #[test] + fn unseen_key_runs() { + assert_eq!(gate().peek("p", LOW, 0), Run); + } + + #[test] + fn ceiling_bypasses_cooldown() { + // Arm a cooldown, then a re-hit AT the hard ceiling must still Run — + // a genuinely filling disk always reclaims, even deep inside a cooldown. + let mut g = gate(); + g.record("p", false, 0); + assert_eq!( + g.peek("p", LOW, 1), + Suppress, + "below ceiling, inside window" + ); + assert_eq!(g.peek("p", CEILING, 1), Run, "at ceiling ⇒ bypass"); + assert_eq!(g.peek("p", 100, 1), Run, "above ceiling ⇒ bypass"); + } + + #[test] + fn rehit_inside_cooldown_suppresses() { + let mut g = gate(); + g.record("p", false, 0); + assert_eq!(g.peek("p", LOW, BASE - 1), Suppress); + } + + #[test] + fn cooldown_elapsed_runs() { + let mut g = gate(); + g.record("p", false, 0); + assert_eq!(g.peek("p", LOW, BASE), Run, "elapsed == window admits"); + assert_eq!(g.peek("p", LOW, BASE + 10), Run); + } + + #[test] + fn backwards_clock_runs() { + // Never suppress on a clock we cannot trust. + let mut g = gate(); + g.record("p", false, 1_000); + assert_eq!(g.peek("p", LOW, 500), Run); + } + + // ── record: no-op backoff growth ───────────────────────────────────────── + + #[test] + fn first_noop_arms_base_window() { + let mut g = gate(); + g.record("p", false, 0); + assert_eq!(g.noop_streak("p"), 1); + assert_eq!(g.peek("p", LOW, BASE - 1), Suppress); + assert_eq!(g.peek("p", LOW, BASE), Run); + } + + #[test] + fn consecutive_noops_grow_window_exponentially() { + let mut g = gate(); + // t0: arm base (900). Re-run just as it elapses; each no-op doubles. + g.record("p", false, 0); + g.record("p", false, BASE); // window → 1800 + assert_eq!(g.noop_streak("p"), 2); + assert_eq!(g.peek("p", LOW, BASE + 1799), Suppress); + assert_eq!(g.peek("p", LOW, BASE + 1800), Run); + } + + #[test] + fn window_is_capped() { + let mut g = ReclaimEffectivenessGate::new(1_000, 10, 5_000, CEILING); + // 1000 → 5000 (capped, not 10_000). Re-record at the window edge so the + // "silence >= 2× window" reset never trips. + let mut now = 0; + g.record("p", false, now); // 1000 + now += 1_000; + g.record("p", false, now); // 10_000 → capped 5_000 + now += 5_000; + g.record("p", false, now); // stays capped at 5_000 + assert_eq!(g.peek("p", LOW, now + 4_999), Suppress); + assert_eq!(g.peek("p", LOW, now + 5_000), Run); + } + + #[test] + fn long_silence_resets_to_base() { + let mut g = gate(); + g.record("p", false, 0); + g.record("p", false, BASE); // window → 1800, streak 2 + // Silence >= 2× current window (2*1800=3600) ⇒ reset to base + streak 1. + g.record("p", false, BASE + 3_600); + assert_eq!(g.noop_streak("p"), 1); + assert_eq!(g.peek("p", LOW, BASE + 3_600 + BASE - 1), Suppress); + assert_eq!(g.peek("p", LOW, BASE + 3_600 + BASE), Run); + } + + // ── record: effective reset ────────────────────────────────────────────── + + #[test] + fn effective_run_readmits_next_cycle_immediately() { + // The invariant: an effective run must NOT arm a cooldown — the very next + // peek (same instant) re-admits. + let mut g = gate(); + g.record("p", false, 0); // arm cooldown + assert_eq!(g.peek("p", LOW, 1), Suppress); + g.record("p", true, 1); // freed space + assert_eq!(g.peek("p", LOW, 1), Run, "effective ⇒ immediate re-admit"); + assert_eq!(g.noop_streak("p"), 0); + } + + #[test] + fn effective_run_after_streak_clears_streak() { + let mut g = gate(); + g.record("p", false, 0); + g.record("p", false, BASE); + assert_eq!(g.noop_streak("p"), 2); + g.record("p", true, BASE + 10); + assert_eq!(g.noop_streak("p"), 0); + } + + // ── independence & robustness ──────────────────────────────────────────── + + #[test] + fn keys_back_off_independently() { + let mut g = gate(); + g.record("a", false, 0); + // "b" is unseen ⇒ runs; "a" is in cooldown ⇒ suppressed. + assert_eq!(g.peek("a", LOW, 1), Suppress); + assert_eq!(g.peek("b", LOW, 1), Run); + } + + #[test] + fn growth_saturates_without_panic() { + // Enormous multiplier and window near i64::MAX must not overflow-panic; + // saturating_mul + cap keep it bounded. + let mut g = ReclaimEffectivenessGate::new(i64::MAX - 1, i64::MAX, i64::MAX, CEILING); + g.record("p", false, 0); + g.record("p", false, 1); + // No panic; still a valid (suppressing) window. + assert_eq!(g.peek("p", LOW, 2), Suppress); + } + + #[test] + fn new_floors_multiplier_and_windows() { + // A multiplier < 2 would never grow the window; `new` floors it to 2. + let mut g = ReclaimEffectivenessGate::new(0, 1, 0, CEILING); + g.record("p", false, 0); + // base floored to 1, so it suppresses within [0,1) and grows thereafter. + assert_eq!(g.peek("p", LOW, 0), Suppress); + assert_eq!(g.peek("p", LOW, 1), Run); + } + + // ── suppress-only invariant ────────────────────────────────────────────── + + #[test] + fn decision_is_binary_run_or_suppress() { + // The gate can only ever decide Run/Suppress — it has no surface to turn a + // dry-run into an apply. Exhaustive match documents that closed set. + for d in [Run, Suppress] { + match d { + Run | Suppress => {} + } + } + } + + // ── configuration ──────────────────────────────────────────────────────── + + #[test] + fn gate_enabled_defaults_on() { + assert!(effectiveness_gate_enabled_from(env(&[]))); + assert!(effectiveness_gate_enabled_from(env(&[( + EFFECTIVENESS_GATE_ENV, + "on" + )]))); + assert!(effectiveness_gate_enabled_from(env(&[( + EFFECTIVENESS_GATE_ENV, + "garbage" + )]))); + } + + #[test] + fn gate_disabled_only_by_explicit_falsey() { + for v in ["off", "OFF", "0", "false", "no", " No "] { + assert!( + !effectiveness_gate_enabled_from(env(&[(EFFECTIVENESS_GATE_ENV, v)])), + "value {v:?} should disable the gate" + ); + } + } + + #[test] + fn cooldown_base_defaults_and_validates() { + assert_eq!( + cooldown_base_secs_from(env(&[])), + DEFAULT_COOLDOWN_BASE_SECS + ); + assert_eq!( + cooldown_base_secs_from(env(&[(COOLDOWN_BASE_SECS_ENV, "120")])), + 120 + ); + assert_eq!( + cooldown_base_secs_from(env(&[(COOLDOWN_BASE_SECS_ENV, "0")])), + DEFAULT_COOLDOWN_BASE_SECS, + "non-positive falls back to default" + ); + assert_eq!( + cooldown_base_secs_from(env(&[(COOLDOWN_BASE_SECS_ENV, "nope")])), + DEFAULT_COOLDOWN_BASE_SECS + ); + } + + #[test] + fn cooldown_multiplier_clamps_to_two() { + assert_eq!( + cooldown_multiplier_from(env(&[])), + DEFAULT_COOLDOWN_MULTIPLIER + ); + assert_eq!( + cooldown_multiplier_from(env(&[(COOLDOWN_MULTIPLIER_ENV, "5")])), + 5 + ); + assert_eq!( + cooldown_multiplier_from(env(&[(COOLDOWN_MULTIPLIER_ENV, "1")])), + 2, + "sub-2 clamps to 2" + ); + assert_eq!( + cooldown_multiplier_from(env(&[(COOLDOWN_MULTIPLIER_ENV, "-9")])), + 2 + ); + } + + #[test] + fn cooldown_max_defaults_and_validates() { + assert_eq!(cooldown_max_secs_from(env(&[])), DEFAULT_COOLDOWN_MAX_SECS); + assert_eq!( + cooldown_max_secs_from(env(&[(COOLDOWN_MAX_SECS_ENV, "60")])), + 60 + ); + assert_eq!( + cooldown_max_secs_from(env(&[(COOLDOWN_MAX_SECS_ENV, "0")])), + DEFAULT_COOLDOWN_MAX_SECS + ); + } + + #[test] + fn hard_ceiling_defaults_and_clamps() { + assert_eq!(hard_ceiling_pct_from(env(&[])), DEFAULT_HARD_CEILING_PCT); + assert_eq!( + hard_ceiling_pct_from(env(&[(HARD_CEILING_PCT_ENV, "95")])), + 95 + ); + assert_eq!( + hard_ceiling_pct_from(env(&[(HARD_CEILING_PCT_ENV, "0")])), + 1, + "clamps up to 1" + ); + assert_eq!( + hard_ceiling_pct_from(env(&[(HARD_CEILING_PCT_ENV, "250")])), + 100, + "valid u8 above 100 clamps down to 100" + ); + assert_eq!( + hard_ceiling_pct_from(env(&[(HARD_CEILING_PCT_ENV, "300")])), + DEFAULT_HARD_CEILING_PCT, + "u8 overflow ⇒ parse fails ⇒ default" + ); + } +} diff --git a/src/disk_reclaim/executor.rs b/src/disk_reclaim/executor.rs index 1b8dd2c8f..11e6ed030 100644 --- a/src/disk_reclaim/executor.rs +++ b/src/disk_reclaim/executor.rs @@ -63,6 +63,16 @@ impl ReclaimReport { self.bytes_freed > 0 || !self.removed.is_empty() } + /// Whether this run **freed space** — the signal the reclaim-effectiveness + /// gate keys its per-partition backoff on (issue #4809/#4825/#4810). True + /// when bytes were freed or the measured `%-used` dropped across the run; + /// a dry-run (removes nothing, `%-used` unchanged) is therefore *not* + /// effective, so a daemon stuck dry-running proven-no-op candidates backs + /// off instead of re-firing every cycle. + pub fn was_effective(&self) -> bool { + self.bytes_freed > 0 || self.used_pct_after < self.used_pct_before + } + /// Daemon one-liner summary. pub fn summary(&self) -> String { format!( @@ -212,11 +222,26 @@ pub fn exec_reclaim( } match vet_candidate(&candidate, ctx) { - Verdict::Reject { reason } => report.skipped.push(SkippedPath { - path: candidate.path.clone(), - kind: candidate.kind, - reject_reason: reason, - }), + Verdict::Reject { reason } => { + // Per-candidate structured skip-reason trace feeding the + // human-review audit trail. Fields-only (no `{}` interpolation + // of the agent's free-text `reason`), so a hostile candidate + // string can never forge or inject log lines. `info` level: + // routine, expected, and useful for observing which rail refused + // each path between reclaim passes. No `print!`/`println!`. + tracing::info!( + target: "simard::disk_reclaim", + path = %candidate.path.display(), + reason = ?reason, + kind = ?candidate.kind, + "reclaim candidate skipped by guard", + ); + report.skipped.push(SkippedPath { + path: candidate.path.clone(), + kind: candidate.kind, + reject_reason: reason, + }); + } Verdict::Allow { primitive, bytes } => { let entry = RemovedPath { path: candidate.path.clone(), @@ -597,4 +622,121 @@ mod tests { "disk reclaim: 88% -> 84% used, freed 12026531840 bytes, 0 paths removed, 0 skipped for review", ); } + + #[test] + fn was_effective_tracks_freed_space_for_the_backoff_gate() { + // Base: a dry-run that removed nothing and left %-used unchanged is NOT + // effective — the daemon must back off instead of re-firing (#4809). + let mut report = ReclaimReport { + mode: ReclaimMode::DryRun, + used_pct_before: 94, + used_pct_after: 94, + target_pct: 85, + bytes_freed: 0, + removed: vec![], + would_remove: vec![], + skipped: vec![], + failures: vec![], + }; + assert!(!report.was_effective(), "no-op dry-run is not effective"); + + // Bytes freed ⇒ effective even if %-used has not yet ticked down. + report.bytes_freed = 4096; + assert!(report.was_effective(), "freed bytes ⇒ effective"); + + // A %-used drop alone (e.g. large file unlinked by our removal) ⇒ + // effective even without a byte count. + report.bytes_freed = 0; + report.used_pct_after = 90; + assert!(report.was_effective(), "%-used dropped ⇒ effective"); + } + + fn stale_cache(path: &Path) -> ReclaimCandidate { + ReclaimCandidate { + path: path.to_path_buf(), + kind: CandidateKind::StaleBuildCache, + parent_repo: None, + reason: None, + est_bytes: None, + } + } + + /// Routine-reclaim regression (issue #4809): a `/target/debug` + /// candidate frees nothing while `/target` is outside the allow-roots + /// (the pre-fix no-op between emergency passes), and is reclaimed once the + /// per-repo `target/` root is in scope. + #[test] + fn routine_reclaim_frees_target_only_when_target_root_is_in_scope() { + let repo = TempDir::new().unwrap(); + let target = repo.path().join("target"); + let debug = target.join("debug"); + std::fs::create_dir_all(&debug).unwrap(); + + let protected = ProtectedDenySet::from_paths(vec![]); + let live = FakeLiveProcessProbe::default(); + let wt = AllowAllWtProbe; + let measurer = MapMeasurer::default(); + measurer.set(&debug, 4096); + + // Old scope: the only allow-root is `/worktrees`, which does NOT + // contain `/target/debug` → rejected `OutsideAllowRoot`, freeing + // nothing. This reproduces the routine no-op. + let old_roots = vec![repo.path().join("worktrees")]; + let old_guard = GuardContext { + allow_roots: &old_roots, + protected: &protected, + live_probe: &live, + wt_probe: &wt, + measurer: &measurer, + }; + let remover_old = RecordingRemover::default(); + let before = exec_reclaim( + vec![stale_cache(&debug)], + &old_guard, + ReclaimMode::Apply, + 85, + &ScriptedDisk::new(vec![90]), + Path::new("/home"), + &remover_old, + ); + assert_eq!(before.bytes_freed, 0, "no target root → routine no-op"); + assert!(before.removed.is_empty()); + assert_eq!(before.skipped.len(), 1); + assert_eq!(before.skipped[0].path, debug); + assert_eq!( + before.skipped[0].reject_reason, + RejectReason::OutsideAllowRoot, + ); + assert!(remover_old.calls.borrow().is_empty()); + + // New scope: the `/target` parent is an allow-root → `target/debug` + // is strictly inside, clears the guard, and is reclaimed. + let new_roots = vec![target.clone()]; + let new_guard = GuardContext { + allow_roots: &new_roots, + protected: &protected, + live_probe: &live, + wt_probe: &wt, + measurer: &measurer, + }; + let remover_new = RecordingRemover::default(); + let after = exec_reclaim( + vec![stale_cache(&debug)], + &new_guard, + ReclaimMode::Apply, + 85, + &ScriptedDisk::new(vec![90]), + Path::new("/home"), + &remover_new, + ); + assert!( + after.bytes_freed > 0, + "target root in scope → artifact freed" + ); + assert_eq!(after.removed.len(), 1); + assert_eq!(after.removed[0].path, debug); + assert_eq!(after.removed[0].kind, CandidateKind::StaleBuildCache); + assert!(after.skipped.is_empty()); + assert_eq!(remover_new.calls.borrow().len(), 1); + } } diff --git a/src/disk_reclaim/guard.rs b/src/disk_reclaim/guard.rs index da13950f7..2f2cb3104 100644 --- a/src/disk_reclaim/guard.rs +++ b/src/disk_reclaim/guard.rs @@ -619,6 +619,50 @@ mod tests { ); } + /// Safety boundary the issue-#4809 fix depends on: making `/target` + /// an allow-root must NOT make the bare `target/` directory itself + /// deletable. `is_safe_to_delete` requires a candidate to be *strictly + /// inside* an allow-root (`real != root`), so a candidate that **equals** an + /// allow-root is refused `OutsideAllowRoot`, while its rebuildable child + /// (`target/debug`) clears containment and is reclaimed. This is exactly why + /// the recipe proposes `target/` children and never the bare parent — if + /// this equality guard ever regressed, routine reclaim could `rm -rf` a + /// whole `target/` allow-root. + #[test] + fn rail_refuses_candidate_equal_to_allow_root_but_allows_its_child() { + // The allow-root stands in for `/target`; `debug` is its child. + let target = TempDir::new().expect("target allow-root"); + let debug = target.path().join("debug"); + std::fs::create_dir_all(&debug).expect("target/debug"); + let allow_roots = vec![target.path().to_path_buf()]; + let protected = ProtectedDenySet::from_paths(vec![]); + let live = FakeLiveProcessProbe::default(); + let wt = FixedWtProbe(WorktreeVerdict::Reclaimable); + let measurer = MapMeasurer::default(); + measurer.set(&debug, 4096); + + // A candidate that IS the allow-root (bare `target/`) must be refused. + let bare = cand(target.path(), CandidateKind::StaleBuildCache); + assert_eq!( + vet(&bare, &allow_roots, &protected, &live, &wt, &measurer), + Verdict::Reject { + reason: RejectReason::OutsideAllowRoot + }, + "a path equal to an allow-root (bare target/) must never be deletable", + ); + + // Its child (`target/debug`) is strictly inside → reclaimed. + let child = cand(&debug, CandidateKind::StaleBuildCache); + assert_eq!( + vet(&child, &allow_roots, &protected, &live, &wt, &measurer), + Verdict::Allow { + primitive: ReclaimPrimitive::RemoveDir, + bytes: 4096, + }, + "a rebuildable child of the target allow-root must be reclaimable", + ); + } + #[test] fn rail_refuses_hallucinated_nonexistent_path() { let allow = TempDir::new().expect("allow root"); diff --git a/src/disk_reclaim/mod.rs b/src/disk_reclaim/mod.rs index 623d835f0..a0aa10fac 100644 --- a/src/disk_reclaim/mod.rs +++ b/src/disk_reclaim/mod.rs @@ -24,6 +24,7 @@ use crate::error::SimardResult; pub mod candidate; pub mod daemon_dir; +pub mod effectiveness; pub mod executor; pub mod guard; pub mod prod; @@ -31,6 +32,11 @@ pub mod recipe; pub use candidate::{CandidateKind, MAX_CANDIDATES, ReclaimCandidate, parse_candidates}; pub use daemon_dir::resolve_daemon_working_dirs; +pub use effectiveness::{ + EffectivenessDecision, ReclaimEffectivenessGate, cooldown_base_secs_from, + cooldown_max_secs_from, cooldown_multiplier_from, effectiveness_gate_enabled, + effectiveness_gate_enabled_from, hard_ceiling_pct_from, +}; pub use executor::{ PathRemover, RealPathRemover, ReclaimFailure, ReclaimReport, RemovedPath, SkippedPath, exec_reclaim, @@ -82,11 +88,23 @@ pub fn managed_repos() -> Vec { /// given Simard state root: /// - `/engineer-worktrees` (the `~/.simard` engineer worktrees), /// - `/worktrees` for each managed repo, +/// - `/target` for each managed repo (routine build-cache reclaim), /// - the shared cargo target dirs under the state root. +/// +/// The per-repo `/target` root is the `target/` **parent** directory, not +/// an exact `target/debug` entry: `is_safe_to_delete` requires a candidate to be +/// *strictly inside* an allow-root, so rooting at `target/` is the smallest +/// scope that lets a `StaleBuildCache` candidate for `target/debug` (and the +/// other rebuildable children) clear containment. Widening is **additive** — +/// every direct child of `target/` is a rebuildable Cargo artifact, and the +/// live-PID, symlink, and `.git`-worktree rails still gate each deletion. This +/// is what lets **routine** reclaim free `target/` between emergency passes +/// instead of rejecting every `target/debug` candidate with `OutsideAllowRoot`. pub fn allow_roots(state_root: &Path) -> Vec { let mut roots = vec![state_root.join("engineer-worktrees")]; for repo in managed_repos() { roots.push(repo.join("worktrees")); + roots.push(repo.join("target")); } roots.push(state_root.join("cargo-target")); roots.push(state_root.join("shared-target")); @@ -423,6 +441,14 @@ mod tests { assert!(roots.contains(&PathBuf::from( "/home/azureuser/src/amplihack-memory-lib/worktrees" ))); + // The per-repo `target/` roots that make routine build-cache reclaim + // possible (issue #4809): every managed repo's `target/` parent is in + // scope so a `target/debug` candidate clears containment. + assert!(roots.contains(&PathBuf::from("/home/azureuser/src/Simard/target"))); + assert!(roots.contains(&PathBuf::from("/home/azureuser/src/amplihack-rs/target"))); + assert!(roots.contains(&PathBuf::from( + "/home/azureuser/src/amplihack-memory-lib/target" + ))); } #[test] diff --git a/src/operator_commands_ooda/daemon/mod.rs b/src/operator_commands_ooda/daemon/mod.rs index 48ff6efd1..5931beeec 100644 --- a/src/operator_commands_ooda/daemon/mod.rs +++ b/src/operator_commands_ooda/daemon/mod.rs @@ -822,6 +822,24 @@ pub fn run_ooda_daemon( ); // ------------------------------------------------------------------- + // --- disk-reclaim effectiveness gate state (issue #4809/#4825/#4810) - + // A suppress-only per-partition exponential-backoff cooldown that stops the + // Tier-3 reclaim below from re-firing every ~15-min cycle once a streak of + // runs has freed nothing — the churn that burned CPU/IO/log volume on the + // ~94%-full host without reclaiming a byte. A genuinely filling disk (used + // %-used at/above the hard ceiling) always bypasses the cooldown, and a run + // that actually frees space immediately re-admits the next cycle. + let reclaim_gate_enabled = crate::disk_reclaim::effectiveness_gate_enabled(); + let mut reclaim_effectiveness_gate = crate::disk_reclaim::ReclaimEffectivenessGate::from_env(); + daemon_log( + &state_root, + &format!( + "[simard] OODA daemon: disk-reclaim effectiveness gate = {}", + if reclaim_gate_enabled { "on" } else { "off" } + ), + ); + // ------------------------------------------------------------------- + // --- periodic engineer worktree sweep state (issue #2167) ----------- let worktree_sweep_interval_secs: u64 = std::env::var("SIMARD_WORKTREE_SWEEP_INTERVAL_SECS") .ok() @@ -1196,22 +1214,76 @@ pub fn run_ooda_daemon( .map(|p| p.round().clamp(0.0, 100.0) as u8); match used_now { Some(used) if crate::disk_reclaim::daemon_should_trigger(used, reclaim_pct) => { - let mode = crate::disk_reclaim::daemon_apply_from_env(); - match crate::disk_reclaim::run_disk_reclaim( - &memories.repo_root, - &state_root, - None, - mode, - reclaim_pct, - crate::disk_reclaim::ReclaimSource::Daemon, - ) { - Ok(report) => { - daemon_log(&state_root, &format!("[simard] {}", report.summary())); - } - Err(e) => daemon_log( + // Effectiveness gate (issue #4809/#4825/#4810): a fresh + // local `used` sample keys a per-partition backoff. A + // streak of runs that freed nothing arms a growing + // cooldown so this branch stops re-firing every cycle; + // a disk at/above the hard ceiling always bypasses it. + let reclaim_key = format!("disk-reclaim:{}", state_root.display()); + let now_secs = SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + if reclaim_gate_enabled + && reclaim_effectiveness_gate.peek(&reclaim_key, used, now_secs) + == crate::disk_reclaim::EffectivenessDecision::Suppress + { + let streak = reclaim_effectiveness_gate.noop_streak(&reclaim_key); + crate::telemetry::registry::counter_add( + crate::telemetry::names::DISK_RECLAIM_SUPPRESSED_CYCLES, + 1, + &[( + crate::telemetry::names::ATTR_SOURCE, + crate::disk_reclaim::ReclaimSource::Daemon.as_str(), + )], + ); + tracing::info!( + target: "simard::disk_reclaim", + used_pct = used, + noop_streak = streak, + "disk reclaim suppressed: prior runs freed nothing; in \ + effectiveness cooldown", + ); + daemon_log( &state_root, - &format!("[simard] WARN: disk reclaim failed: {e}"), - ), + &format!( + "[simard] disk reclaim: {used}% used, suppressed \ + (ineffective-run cooldown, streak {streak})" + ), + ); + } else { + let mode = crate::disk_reclaim::daemon_apply_from_env(); + match crate::disk_reclaim::run_disk_reclaim( + &memories.repo_root, + &state_root, + None, + mode, + reclaim_pct, + crate::disk_reclaim::ReclaimSource::Daemon, + ) { + Ok(report) => { + // Record the outcome so a no-op run arms/grows + // the cooldown and a run that actually freed + // space re-admits the next cycle. A failed run + // (Err below) is left unrecorded so a transient + // error never arms a cooldown. + if reclaim_gate_enabled { + reclaim_effectiveness_gate.record( + &reclaim_key, + report.was_effective(), + now_secs, + ); + } + daemon_log( + &state_root, + &format!("[simard] {}", report.summary()), + ); + } + Err(e) => daemon_log( + &state_root, + &format!("[simard] WARN: disk reclaim failed: {e}"), + ), + } } } Some(used) => daemon_log( diff --git a/src/telemetry/names.rs b/src/telemetry/names.rs index e96729583..33ce68e8e 100644 --- a/src/telemetry/names.rs +++ b/src/telemetry/names.rs @@ -121,6 +121,10 @@ pub const DISK_RECLAIM_CANDIDATES_SKIPPED: &str = "simard.disk.reclaim.candidate pub const DISK_RECLAIM_USED_PCT_BEFORE: &str = "simard.disk.reclaim.used_pct_before"; /// Home-partition `%-used` after the run (gauge, 0–100). pub const DISK_RECLAIM_USED_PCT_AFTER: &str = "simard.disk.reclaim.used_pct_after"; +/// Daemon cycles the effectiveness gate skipped (counter, tagged by +/// [`ATTR_SOURCE`]) — a would-have-run reclaim held because a streak of no-op +/// runs is in cooldown (#4809 / #4825 / #4810). +pub const DISK_RECLAIM_SUPPRESSED_CYCLES: &str = "simard.disk.reclaim.suppressed_cycles"; // ── Attribute keys ──────────────────────────────────────────────────────────