fix(orb): boot-time orphaned-lock flush, resume sweep catch-up, contributor-cap-lock TTL#9109
Merged
Merged
Conversation
…sume, and widen the contributor-cap-lock TTL Three related lock/liveness fixes: #9021 -- every Redis-backed lock (pr-actuation-lock, ai-review-lock, contributor-cap-wake/-lock) survives a container restart with its TTL intact. On this single-instance deployment, any lock present at boot is provably orphaned -- the process that claimed it is gone. Left alone, each class strands real work for its own TTL (30 min for ai-review-lock, #8998). Adds flushOrphanedLocksAtBoot (selfhost/redis-cache.ts), a best-effort SCAN-delete over the four exclusivity-lock prefixes, wired in at server boot before the queue starts. Deliberately leaves delivery:*, pr-panel-retrigger-pending:*, ci-pending-first-seen:*, and fresh-rebase-forced:* untouched -- none are exclusivity locks, and each has its own restart-survival contract. #9018 -- a paused repo's PRs can go green DURING the pause window (CI- completion passes plan-and-suppress the whole time), and resuming performed no catch-up: if a PR was ALSO regated once before the pause, agent-sweep.ts's #never-endless-reregate rule permanently excludes it from future sweep candidacy, stranding it silently. Adds clearPullRequestsRegatedAtForOpenPrs and calls it on the paused->live transition, both from the single-repo MCP pause/resume tool and the installation-wide bulk-settings route -- restoring one-shot sweep candidacy for the repo's open PRs exactly once. #9024 -- claimContributorCapLock's 30s TTL was sized only for the executor's brief pre-merge recheck, but maybeCloseForContributorCapOnOpen holds the SAME lock across a much longer body (token mint, live GitHub calls, label writes, the nested pr-actuation-lock, a full executeAgentMaintenanceActions pass) that can exceed 30s under GitHub rate-limit backoff -- reopening the exact #7284 TOCTOU this lock exists to close. Widened to 600s, matching claimPrActuationLock's own TTL for a comparably long mutating body. Closes #9018, #9021, #9024 Tests: 3 new flushOrphanedLocksAtBoot tests (deletes matching keys, returns 0 with nothing to do, fails open per-pattern on a scan error), a paused->live MCP-tool test (clears open PRs' markers, never closed ones, never on pause or a repeat resume), two bulk-settings-route tests (clears across all repos in an installation; a non-agentPaused bulk change never touches the marker), and a TTL-parity regression pinning claimContributorCapLock's TTL to claimPrActuationLock's. 100% line+branch coverage on every changed line (src/server.ts is Codecov's own documented ignore-listed entrypoint file).
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…check The miner-UI typecheck reaches into packages/loopover-miner/lib/** (declared as an input), and those files import @loopover/engine -- whose types resolve to packages/loopover-engine/dist/index.d.ts. But unlike @loopover/ui#typecheck (which declares the explicit @loopover/engine#build dependsOn edge for exactly this reason), the miner-UI task only had ^build, and miner-ui has no package.json dependency on engine to create that edge implicitly. Turbo therefore ran engine build CONCURRENTLY with this typecheck; whenever the engine cache missed AND the scheduler interleaved the two the wrong way, the typecheck raced ahead of the dist emit and failed with dozens of phantom "Cannot find module '@loopover/engine'" errors -- an intermittent, whole-job validate-code failure with no real defect behind it (observed live on #9107's first run and on main run 30214733191, while sibling runs of the identical commit passed).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9109 +/- ##
==========================================
- Coverage 93.89% 92.66% -1.23%
==========================================
Files 813 813
Lines 80677 80693 +16
Branches 24479 24482 +3
==========================================
- Hits 75748 74774 -974
- Misses 3562 4842 +1280
+ Partials 1367 1077 -290
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Summary
Three related lock/liveness fixes bundled per the audit backlog:
#9021 — boot leaves every Redis lock orphaned. No boot-time flush exists anywhere, so every exclusivity lock (
pr-actuation-lock600s,ai-review-lock1800s,contributor-cap-wake1800s,contributor-cap-lock) survives a container restart with its TTL intact. On a single-instance deployment, any lock present at boot is provably orphaned — the process that claimed it is gone — and each class strands real work for its full TTL (theai-review-lockcase is #8998's 30-minute review starvation). AddsflushOrphanedLocksAtBoot(redis-cache.ts): a best-effort SCAN-delete over the four exclusivity-lock prefixes, wired into server boot before the queue starts. Deliberately leavesdelivery:*,pr-panel-retrigger-pending:*,ci-pending-first-seen:*, andfresh-rebase-forced:*untouched — none are exclusivity locks, and each has its own restart-survival contract.#9018 — a PR that goes green during a pause is never re-evaluated after unpause. CI-completion passes plan-and-suppress for the whole pause window, and resume performs no catch-up. If the PR was also regated once before the pause,
#never-endless-reregatepermanently excludes it from sweep candidacy — it sits green, approved, and silently stranded forever. AddsclearPullRequestsRegatedAtForOpenPrsand calls it on the paused→live transition from BOTH write paths (the single-repoloopover_set_agent_pausedMCP tool and the installation-widebulk-settingsroute), restoring one-shot sweep candidacy for the repo's open PRs exactly once.#9024 — contributor-cap-lock TTL (30s) is far shorter than the work it guards. Sized for the executor's brief pre-merge recheck, but
maybeCloseForContributorCapOnOpenholds the same lock across a much longer body (token mint, live GitHub calls, label writes, the nestedpr-actuation-lock, a fullexecuteAgentMaintenanceActionspass) that can exceed 30s under GitHub rate-limit backoff — Redis then expires the lock mid-hold and a concurrent sibling's cap check races the in-flight close, reopening the exact #7284 TOCTOU this lock exists to close. Widened to 600s, matchingclaimPrActuationLock's TTL for a comparably long mutating body.Closes #9018
Closes #9021
Closes #9024
Test plan
flushOrphanedLocksAtBoottests: deletes exactly the matching lock keys (protected keys survive), returns 0 with nothing to do, fails open per-pattern when one SCAN throwslastRegatedAt(closed PRs untouched); pausing and repeat-resume never clearagentPausedbulk change never touches the markerclaimContributorCapLock's TTL toclaimPrActuationLock's (600s)src/server.tsis codecov's own documented ignore-listed boot entrypoint)npx tsc --noEmit,check-migrations.ts,check-schema-drift.ts,check-engine-parity.tsall green