fix(miner): register worktree-allocator in purge-cli's right-to-be-forgotten sweep (#8320) - #8563
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
❌ 31 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
…rgotten sweep (JSONbored#8320) worktree_slots carries repo_full_name like every other repo-scoped store, but was absent from purge-cli's REAL_PURGE_TARGETS. Adds WorktreeAllocator.purgeByRepo, a hand-written UPDATE (not the generic purgeStoreByRepo DELETE, which would shrink the fixed slot pool) that only ever clears a free slot's stale repo_full_name -- an active slot's live worktree checkout is never touched. Registers the store in REAL_PURGE_TARGETS with its own dry-run counting query matching the same free-only match condition.
593e42e to
2d3c71b
Compare
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-24 21:16:24 UTC
Review summary Nits — 3 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
packages/loopover-miner/lib/worktree-allocator.ts'sworktree_slotstable has arepo_full_namecolumn, just like every other repo-scoped local store, but it was absent frompurge-cli.ts's right-to-be-forgotten sweep (REAL_PURGE_TARGETS) — the same recurring gap class already fixed for other stores (#7091, #6599, #8009).worktree_slotsis a fixed pool ofmaxConcurrencypre-allocated rows (slot_indexis the primary key and every slot index must always exist), not an append-only ledger, so the genericpurgeStoreByRepohelper (a hardDELETE) is the wrong shape here — it would shrink the pool belowmaxConcurrencyand breakensureSlots'/selectFreeSlot's invariant. This PR followsgovernor-state.ts's own precedent for a non-uniform store: a hand-writtenpurgeByRepomethod directly on theWorktreeAllocatorobject.WorktreeAllocator.purgeByRepo(repoFullName): number— anUPDATE, never aDELETE, that only clears a row wherestatus = 'free' AND repo_full_name = ?. Anactiveslot's row (a live, currently-running attempt's real worktree checkout) is never touched or counted — force-clearing it would desync the allocator from the live checkout on disk.worktree-allocatorinpurge-cli.ts'sREAL_PURGE_TARGETSwith nospec/specsfield (its purge logic lives on the store object itself), mirroring howgovernor-state's entry carriesspecsinstead of a singlespecfor its own non-uniform case.countDryRunextension point toPurgeTarget+runPurgeDryRunfor stores whose dry-run preview can't be expressed as a genericLedgerPurgeSpec; worktree-allocator's own dry-run query matches the real purge'sstatus = 'free' AND repo_full_name = ?condition exactly, so--dry-runnever reports an active slot as purgeable.miner-purge-cli.test.tsfixture referencing the "twelve real stores" /toHaveLength(12)/ a fullREAL_PURGE_TARGETS-shaped options object is updated to the new thirteenth store.Scope
packages/loopover-miner/**only (plus its test files) — not Codecov-gated per this issue's own text.Validation
npm --workspace @loopover/miner run build:tsc— cleannpm run typecheck— cleannpx vitest run test/unit/miner-worktree-allocator.test.ts test/unit/miner-purge-cli.test.ts test/unit/miner-worktree-allocator-collisions.test.ts test/unit/miner-worktree-allocator-lease-expiry.test.ts test/unit/miner-attempt-cli.test.ts— all green except 4 pre-existing, unrelated Windows-local-only failures (path-separator string assertions expecting/, achmod-permission-bit assertion Windows doesn't enforce, and twoEBUSY: resource busy or lockedtemp-dir cleanup races) — reproduced identically on agit stash'd clean baseline before this change, so confirmed environment noise, not a regression.git diff --check upstream/main HEAD— clean, no trailing whitespace.ui:*checks — skipped (backend/CLI-only change, noapps/gittensory-uifiles touched).New coverage
purgeByRepoclears a free slot carrying a stalerepo_full_nameand counts it (seeded directly, bypassing the normal release path since normal operation never leaves one stale).purgeByReponever touches anactiveslot for the target repo — 0 purged, row unchanged.purgeByReporeturns 0 when no slot matches the repo.purge-cli's dry-run + real purge both exercise the new store end-to-end (free-stale-row counted, active-row excluded, file-doesn't-exist-yet, corrupted-file error path, real on-disk default-path open/close).Safety
Notes
Closes #8320