Repository navigation
cargo-target-janitor(queue_claims): offset pagination over the live claimed set can skip a still-claimed task — it loses never-delete protection #6790
Description
cargo-target-janitor(queue_claims): offset pagination over the live claimed set can skip a still-claimed task — it loses never-delete protection
Summary
queue_claims in infra/cargo-target-janitor/bin/cargo_target_janitor.py (lines 194–221 at origin/main e48256bb8f) reads the pilot queue's claimed task IDs — the janitor's never-delete set — by following limit=1000&offset=N pages until a short page arrives. The query carries no order= parameter, and offset pagination is evaluated by PostgREST against the current filtered set on every request. The claimed set is live: claimed tasks complete, fail, and are released constantly while a janitor run is paging.
When a row leaves the claimed set mid-pagination, every later row shifts one position toward the front, and the next offset page skips exactly one still-claimed row. That task's cargo target is then treated as unclaimed by both the plan-time read and the apply-loop fresh re-check (both call this same function), so a target directory belonging to a task a worker is actively building in can be selected and deleted.
This is the residual axis #6618 did not close: #6618 made the read complete-or-fail-closed for a static set (page past 1000, fail closed past 100 pages). It does not make the read stable for a changing set, and no ordering is requested.
Evidence (verified this run)
Probe on the laptop, ~/git/fleet/probe-janitor-offset/ — the real cargo_target_janitor.py copied verbatim from origin/main e48256bb8f (diff-verified identical to current origin/main), driven through a stub queue client that simulates PostgREST semantics (each request re-evaluates filter + limit/offset against the current claimed set):
- Control (static set): 1,500 claimed tasks →
queue_claimsreturns all 1,500. Pagination itself works. - Live churn: 1,500 claimed tasks; after page 1 is served, task 1 completes and leaves the claimed set (ordinary queue churn — claimed tasks finish throughout every janitor run). Page 2 is then requested at
offset=1000against the shifted set. Result:queue_claimsreturns 1,499 IDs, and task 1001 — still claimed — is missing from the returned set. - Queries issued (both cases):
tasks?select=id,queue,status,payload&queue=eq.repo-health-code-pilot&status=eq.claimed&limit=1000&offset=0, then...&offset=1000— noorder=parameter in either.
A missed claimed ID has a concrete consequence in this tool, unlike an ordinary listing: in discover_targets, task_id in claimed is the live-claimed-task protection, and in the --apply loop target.task_id in fresh_claimed is the stop check. A task skipped by both reads has neither.
Why it matters
The janitor is a deleter whose every other safety input fails closed (stat failures protect the target, unreadable process tables abort the run, page-overflow aborts the run). This read silently returns a plausible, almost-complete set — no error, no short page, nothing in the log — while dropping the one protection that keeps a live build's target directory from being removed under it. The window needs >1,000 simultaneously claimed tasks, so it is latent today; it opens exactly when the queue is busiest, which is also when a wrongful deletion costs the most in-flight work.
Acceptance criteria
queue_claimspages in a way that a row leaving (or entering) the claimed set mid-read cannot cause a still-claimed row to be skipped — e.g. keyset pagination (order=id.ascplusid=gt.<last-seen-id>; task IDs are issued in increasing order, so newly claimed rows sort after the cursor), or another scheme whose stability argument is written down at the read site.- The request explicitly orders the result (
order=id.ascor equivalent); offset-only paging over the unordered live set is gone. - A regression test in
infra/cargo-target-janitor/tests/test_cargo_target_janitor.pymodels a live set: a stub client that removes an already-served row between pages, asserting every still-claimed ID is returned (the probe above is a ready-made shape); the existing cargo-target-janitor: both deletion-safety liveness reads are single-page — queue_claims caps at 1000 rows and open_pr_branches at 500 PRs, so a claimed task or open-PR worktree past the page silently loses its never-delete protection #6618 tests (pages past 1000, cap boundary, never-ending pages fail closed) still pass. - The apply-loop fresh re-check inherits the fix automatically because it calls the same function — no second, weaker copy is introduced.
Dedup
Searched open and closed issues for cargo-target-janitor, queue_claims, offset pagination, and keyset: #6618 (closed) fixed the single-page truncation and is the static-set axis only; #6622 is the repo-wide pagination-contract design (cursor type / volume / completeness) and does not name ordering stability or mid-read churn for this read. No issue tracks the mutating-set skip.
Activity
developing-today commentedon Nov 7, 2026
CODE-2090
dezren39 commentedon Nov 7, 2026
🤖 Agent note · filed
queue_claimsininfra/cargo-target-janitor/bin/cargo_target_janitor.pypage stably over the live claimed set (explicit ordering + keyset-style paging, or an equivalent scheme with its stability argument written at the read site), so a still-claimed task can never be skipped from the never-delete set by mid-read churn.~/git/fleet/probe-janitor-offset(laptop; real cargo_target_janitor.py verbatim, diff-verified identical to origin/main e48256b)dezren39 commentedon Nov 7, 2026
Picked up by repo-worker-code-p (lane code-p, repo-queue worker shift, FALLBACK MODE — queue backend down: fleet queue health/claim failing HTTP 503 PGRST002, code#6619). Working issue #6790 under the worker runbook §0a fallback protocol.
dezren39 commentedon Nov 7, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 7, 2026
Heartbeat: targeted_tests_passed — queue_claims now pages by keyset (order=id.asc, id=gt.) instead of offset, so a task leaving the claimed set mid-read cannot shift a still-claimed task out of the never-delete set; the #6618 fail-closed page cap is unchanged. Laptop: python3 -m unittest tests.test_cargo_target_janitor — 21/21, including a new churn regression test (task 1 completes after page 1 of 1,500 claimed; the read still returns every claimed id, task 1001 included — the row the filing's probe showed offset paging skipping). Next: commit, PR, Codex check, merge if clean.
fix(cargo-target-janitor): page queue_claims by keyset, not offset
Merge pull request #6793 from developing-today/queue/6790-janitor-keyset
dezren39 commentedon Nov 7, 2026
Released by repo-worker-code-p (fallback mode): issue #6790 fixed and merged via PR #6793 (#6793, merge c560db7) — queue_claims now pages the claimed set by keyset (order=id.asc, id=gt.) instead of offset, so churn in the live claimed set behind the cursor can no longer skip a still-claimed task out of the janitor's never-delete set; the #6618 fail-closed page cap is unchanged. Verified: python3 -m unittest tests.test_cargo_target_janitor 21/21 on the laptop, including a churn regression test reproducing the filing's probe (task 1001 survives), Codex review completed with no findings. Issue auto-closed by the PR's Fixes line.
dezren39 commentedon Nov 7, 2026
🤖 Agent note · done