Repository navigation
perf(llc): the disposal sweep rereads its whole approval history every hour, and acts on decisions that have aged out #17738
Description
Problem
The workspace disposal sweep (#16818, PR #17725) reads its approved proposals like this:
select(Approval).where(
Approval.approval_type == ApprovalType.DESTRUCTIVE_ACTION.value,
Approval.status == ApprovalStatus.APPROVED.value,
Approval.requested_by_agent == SWEEP_REQUESTER,
).order_by(Approval.decided_at)Nothing ever leaves that set. An executed proposal keeps status = APPROVED — execution is
recorded in context["executed_at"], because the approval vocabulary is six values shared by every
gate and inventing a seventh to suit one sweep would change all of them. So every proposal this
sweep has ever raised and had approved stays matched, for ever, each carrying its workspace list and
its execution_outcomes.
The sweep runs hourly and deserialises that entire history every tick to find the few unexecuted
rows. At roughly 24 rows a day that is tens of megabytes per tick after a year, growing monotonically.
It will not fail. It is the same shape as the defect PR #17725 just fixed — a query whose cost is
governed by history rather than by work — on a gentler slope.
Why a time window is not the bound that was just removed
The LIMIT removed in #17725 bounded which rows the filter could see, so executed rows could
crowd out the unexecuted one and silently stop all disposal. A window bounds which rows are
eligible at all: an approval outside it is not hidden from the filter, it is genuinely not a
candidate. Those are different operations and only the first can starve.
The reason this is a correctness fix, not only a cost one
PR #17725 re-proves landedness at execution rather than trusting the proposal, on the argument that
an approval authorises the removal; it does not vouch for the evidence. That handles the
evidence going stale. It does nothing about the decision going stale.
Someone who approved a workspace removal six weeks ago approved it on the state they were shown
then. Re-proving landedness today does not make that a current authorisation — it makes it a current
fact attached to an old decision. A window makes the PR's own claim literal: an approval has a
lifetime, and past it the sweep asks again rather than acting on consent that has aged out.
Acceptance criteria
-
_approved_proposalsfilters onApproval.decided_at >= now - EXECUTION_WINDOW_DAYS. -
EXECUTION_WINDOW_DAYSis an env-var-backed module constant, measured in days — a window
in hours could drop an approval at the edge during a sweep outage, which is a silently unactioned
human decision. - A decision whether an aged-out approval is skipped or marked
ApprovalStatus.EXPIRED.
Marking is preferable if the queue surfaces it, because a skipped approval is invisible to the
person who granted it; skipping is acceptable if nothing reads that state. Record which and why. - A test asserts an approval older than the window is not executed, and one inside it is.
- The test is mutation-checked: remove the window predicate and it must fail. A test that
passes either way is what feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725's first attempt at this class produced.
Related
Follow-up to #16818, found by review on PR #17725. Does not block that PR.
Activity
mrveiss commentedon Sep 28, 2026
Premise check — not verifiable against
main: this reviews code that has not landed. Read-only atorigin/mainab05ba3aef.The file this issue is about does not exist on
main.git ls-tree -r origin/mainfinds noworkspace_lease_sweep*and noworkspace_disposal*anywhere in the tree. Both this issue and its sibling are reviews of #16818's implementation in PR #17725, which is reviewed and in the CI queue rather than merged.That has three consequences worth recording before anyone schedules it:
maincannot confirm or refute it. The evidence for it lives at feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725's head. Any verdict I posted againstmainwould be a statement about a tree that does not contain the subject.mainthe moment it lands — not new work discovered later, but a known finding arriving with the code. Folding it into feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725 before merge costs one review round; landing it and fixing it afterwards costs a second PR and leavesmainbriefly carrying a defect that was known in advance.Nothing here disputes the finding. It is a statement about where it can be verified and when it is cheapest to act on.
fix(llc): bound the approval window, and stop the fake session lying …
fix(llc): bound the aged-out read, and make the release guard able to…
feat(llc): make the workspace lease real — acquire, release, reclaim,…