[Mathih13]

The disk guard can suspend bots during a successful cleanup. systemd clears the cleanup process's exit timestamp while its next run starts. On Argus, that overlap latched capacity off despite about 50 GiB of free space.

Retain the last observed successful cleanup time while cleanup is running. Bind that time to the current boot and preserve its 30-minute deadline. Failed cleanup, missing history, stale history and a different boot still revoke capacity. Existing suspension still requires explicit resume.

Validation on Argus as lyracore: the regression failed on the preceding guard, then all 15 disk-policy tests passed with the fix. Tests cover the startup overlap, deadline expiry, failures and untrusted history. No live Realm state changed during these checks.

Authored by GPT-6 Astra through the Codex harness in T3 Code.

Summary by CodeRabbit

• Bug Fixes
  • Disk capacity is now evaluated using the last successful cleanup from the current boot. Starting a cleanup no longer resets the 30-minute deadline; failed, overdue, or missing cleanup history revokes capacity even while cleanup is running.
• Documentation
  • Updated the disk policy guide to explain how cleanup history affects capacity availability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3add3c14-2eff-4a66-8e0b-125d5c57804b

📥 Commits

Reviewing files that changed from the base of the PR and between c4042fc and 5d9ebe1.

📒 Files selected for processing (3)

• deploy/lyracore-disk-guard.py
• deploy/test_disk_policy.py
• docs/operations/disk-policy.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The disk guard uses the last successful prune completion from the current boot to calculate prune age while cleanup is activating. Tests cover lease renewal and revocation, and the operations policy documents the deadline behavior.

Changes

Disk prune freshness

Layer / File(s)	Summary
Track prune history deploy/lyracore-disk-guard.py	The guard reads systemd properties and computes prune age from a valid successful completion time. During activation, it reuses the saved completion time only when the boot ID matches.
Validate prune lease behavior deploy/test_disk_policy.py, docs/operations/disk-policy.md	Tests cover renewal during activation and lease revocation for failed, missing, or invalid history. The policy describes the same-boot completion deadline.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5d9eb

Cleanup starting again can retain bot capacity until the last successful cleanup reaches its existing deadline. The change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preserving bot capacity during a successful cleanup run.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Mathih13]

Code review: main...5d9ebe1 (1 commit)

Spec source: no linked issue, so I used the PR description as the spec. I ran both axes in a single pass because the diff is small (98 lines across 3 files). python3 -m unittest deploy/test_disk_policy.py passes all 15 tests locally. No workflow in .github/ runs this suite, so CI does not cover it.

Standards

Medium: the end-to-end test fakes an internal collaborator (deploy/test_disk_policy.py, test_cleanup_start_does_not_revoke_a_recently_verified_lease)
CODING_STANDARDS.md, Tests: "Do not fake internal collaborators." The test patches guard.renew with a lambda that re-injects run. It has to, because renew(..., run=subprocess.run) binds the default when the function is defined, so patching guard.subprocess.run never reaches it. A cleaner fix is to make renew look up subprocess.run at call time (run=None → run = run or subprocess.run). Then the test only fakes systemctl and the SpacetimeDB CLI at the external edge.

Low: the test depends on the host (same test)
main() reads /proc/sys/kernel/random/boot_id without a fake. On a non-Linux dev machine the test fails with FileNotFoundError instead of exercising the behaviour. Either patch the read, or pass the boot id path in through the same seam as systemctl.

Low, judgement call: mixed names (possible Mysterious Name)
The code, units and status keys say prune (check_prune, prune_success_monotonic, spacetimedb-prune.service). The new tests and docs/operations/disk-policy.md say cleanup. CONTEXT.md defines neither term. Pick one for the new prose and test names. prune matches the unit an operator will look up.

Info: check_prune mutates status and also returns health. This follows the existing require_recent_prune, so it is fine as is.

Spec

All requirements from the description are implemented:

• Retain last success while cleanup runs: ActiveState == 'activating' falls back to previous['prune_success_monotonic']. ✔
• Bound to the current boot: boot_id comparison. ✔
• Preserve the 30-minute deadline: the age is measured from the original success, and test_running_cleanup_cannot_extend_the_last_success_deadline covers the 1900/1901 edge. ✔
• Failed, missing, stale or other-boot history revokes: covered by the subTest table. ✔
• Suspension still needs explicit resume: sample/decide are unchanged. ✔

Low: the "failed cleanup" guarantee depends on timer cadence, not on the code path the test covers.
systemd resets Result to success when a oneshot starts. So Result=exit-code together with ActiveState=activating (the first subTest row) never occurs in practice. A failed run is caught because the guard samples every 60 s (lyracore-disk-guard.timer) and prune runs every 15 min, so some sample sees the failure and stores prune_success_monotonic = 0 before the next run starts. That holds today. A one-line comment near the fallback would keep a future timer change from silently opening the gap.

Info: rollout edge. The first guard sample after deploy has no boot_id in status.json. If prune is activating at that moment, capacity is revoked and the suspension latches until someone runs a manual --resume. The window is short, but it is worth knowing when deploying to Argus.

Scope: no creep. The new status.json keys (boot_id, prune_success_monotonic) and the returncode != 0 → {} change are both needed for the fix.

---

Summary: Standards has 3 findings; the worst is that the end-to-end test fakes the internal renew. Spec has 2 findings, both low or info; the worst is that "failed cleanup revokes" relies on the timer cadence and has no comment saying so.

---

Generated by Claude Code