feat(cleanup): make idle-destroy disk-pressure-aware, remove inert runtime-gc - #48
Merged
Conversation
…ntime-gc Resolves #22. Two documented features did nothing: disk.pressure-detected was declared and subscribed to but never emitted, and runtime-gc always returned no proposals. A documented no-op is worse than an absent feature because it reads as working, so disk pressure would never get investigated. Disk pressure: implemented. idle-destroy now reads view.diskFreeBytes directly and, once free space drops under diskPressure.freeBytesThreshold, destroys shutdown devices past T1 (idle.shutdownAfterMs) instead of waiting for T2 (idle.deleteAfterMs) -- the rule stays a pure function of the view, never depending on the event having fired. The reaper now emits disk.pressure-detected itself, edge-triggered on the not-under-pressure -> under-pressure crossing, so a sustained low-disk condition emits once instead of once per tick; the reaper keeps its existing subscription to the event for any future external emitter. Runtime GC: removed. Deleted runtime-gc.ts and its registration, the gc-runtime action, and the runtime.deleted event declaration -- nothing implemented or emitted either. This left manualCleanupRules with a single, now-removed member and CleanupReaper#rulesFor filtering --rule against it, so `pitlane cleanup --rule <name>` matched nothing for any rule name (including today's already-nonfunctional --rule idle-destroy). --rule now selects by name across the automatic rules that actually exist, and the now-pointless manualCleanupRules export is gone. Docs updated to match: EVENTS.md flips disk.pressure-detected to implemented (emitter CleanupReaper) and drops the runtime.deleted row; ARCHITECTURE.md and CLI.md describe only the rules and --rule behavior that exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The crossing check sat in #view(), which every run shares -- so `pitlane cleanup --dry-run` under pressure would emit disk.pressure-detected, wake this reaper through its own subscription, and turn a preview into a real cleanup run. A dry run must stay a preview (safety rule 5). The fact is now noted only on runs that actually execute. The test pins it by counting rule evaluations: without the guard the preview evaluates twice, once for itself and once for the run it woke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GbpHvzYe8vEs71QHbpAhyT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #22. The issue said either resolution was acceptable per feature, so: disk pressure is implemented, runtime GC is removed. Reasoning for each choice below, since they went opposite ways.
Disk pressure — implemented
Removing it would have meant dropping
diskPressure.freeBytesThreshold, a config key that is already validated and documented, so the honest cheap direction was to make it do what the docs imply.idle-destroynow readsdiskFreeBytesoff the view — which the reaper was already computing and no rule was reading — and shortens its own threshold from T2 to T1 when free space is under the configured threshold. No new config keys. The reason string names pressure as the cause, per safety rule 6:disk pressure (free 2GiB < 10GiB): idle 12m > T1=10m.The behaviour deliberately does not flow through the event bus. The reaper emits
disk.pressure-detectedas a post-commit observer fact, edge-triggered so a sustained condition emits once per crossing rather than once per tick, but the rule stays a pure function of the view and never depends on an event having fired (safety rule 3). The existing subscription is kept so an external emitter can still wake the reaper.One defect found in review and fixed here: the crossing check initially sat in
#view(), which every run shares — sopitlane cleanup --dry-rununder pressure would emit the fact, wake this reaper through its own subscription, and turn a preview into a real cleanup run. A dry run must stay a preview (safety rule 5). The fact is now noted only on runs that execute, and the test pins it by counting rule evaluations: without the guard, the preview evaluates twice — once for itself, once for the run it woke.Runtime GC — removed
Implementing it needs an Android system-image inventory the driver doesn't expose, so it would be a feature, not a fix.
runtime-gc.ts, its registration, thegc-runtimeCleanupAction, and theruntime.deletedevent declaration are gone, along with thedocs/CLI.mdanddocs/ARCHITECTURE.mdclaims that it works. iOS runtimes stay Xcode-managed regardless.Consequence that had to be handled:
runtimeGcRulewas the only entry inmanualCleanupRules, and#rulesFor(name)filtered--ruleagainst manual rules only — so removing it would have leftcleanup --rulematching nothing at all. In fact--rule idle-destroyalready matched nothing today, which is a live bug this surfaces.--rule <name>now selects across the rules that actually exist, so bothidle-shutdownandidle-destroywork,manualCleanupRulesis gone rather than left as an empty exported list, andlist --rulesreports the real registry.Tests
idle-destroy: destroys at T1 under pressure with pressure named in the reason; still waits for T2 when not under pressure; never proposes a leased device under pressure (safety rule 2).reaper: emits once per crossing and not per sustained tick, and again after recovering and re-crossing;--ruleselects by name across the registered rules; the dry-run test above.Verification
pnpm run checkgreen (505 unit, 28 e2e) andpnpm fallow --ciclean — re-run independently after the review fix, not only as reported.Generated by Claude Code