Skip to content

refactor(queue): extract transient-lock primitives into their own module - #4157

Merged
JSONbored merged 1 commit into
mainfrom
refactor/processors-actuation-locks-4013
Jul 8, 2026
Merged

refactor(queue): extract transient-lock primitives into their own module#4157
JSONbored merged 1 commit into
mainfrom
refactor/processors-actuation-locks-4013

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Step 1 of a ~10-step sequence to split src/queue/processors.ts (14,000+ lines, growing fast under this repo's high commit velocity — expect the issue's own cited line numbers to be stale) into cohesive modules. Only step 1 is attempted here, per the issue's own recommended sequencing and this repo's small-PR house style — the remaining steps are separate, later PRs.
  • Investigated first (per the issue's own "step 0"): whether this repo's Codecov setup recognizes a pure move as a rename. No flag_management/carryforward/rename-specific setting exists in codecov.yml — patch coverage is computed straight off the diff. Rather than gamble the PR's mergeability on unconfirmed rename detection, this extraction is structured as a literal move (identical code, export added only where a name crosses the new module boundary) AND ships with genuinely complete, direct test coverage regardless of how Codecov scores the diff.
  • Moves claimTransientLock/releaseTransientLockIfOwner/TransientLockClaim (the generic best-effort exclusive-lock primitive) and the per-PR actuation-lock domain wrapper built on it (claimPrActuationLock/releasePrActuationLock/PrActuationLockContendedError) into a new src/queue/transient-locks.ts. Zero logic changes — this is a pure structural move.
  • A boundary discovery worth flagging: the actuation-lock cluster isn't quite as self-contained as the issue's own sequencing assumed — it shares the generic claimTransientLock/releaseTransientLockIfOwner/TransientLockClaim primitive with a SECOND consumer, the per-(repo, PR, head SHA, mode) AI-review lock (claimAiReviewLock/releaseAiReviewLock), which stays in processors.ts. Extracting just the actuation-lock wrapper alone would have required either code duplication or a circular import (transient-locks.tsprocessors.tstransient-locks.ts). Moving the shared primitive alongside its first consumer, with the second consumer importing it back, avoids both — and conveniently anticipates the issue's own later ai-review-orchestration.ts step, which is presumably where claimAiReviewLock/releaseAiReviewLock will eventually land.
  • A temporary re-export (export { claimPrActuationLock, releasePrActuationLock } from "./transient-locks";) keeps test/unit/queue.test.ts's existing import { ... } from "../../src/queue/processors" working unchanged. Those specific tests are individual it() blocks deeply interspersed with ~20 unrelated tests in the same file, not inside a cleanly extractable describe block — relocating them is deliberately deferred rather than forced into this PR (splitting test/unit/queue.test.ts itself is also outside refactor(queue): split processors.ts into cohesive modules #4013's literal scope, which is about src/queue/processors.ts).

Scope

Validation

  • git diff --check
  • npm run typecheck
  • npm run db:migrations:check / npm run db:schema-drift:check — no-ops confirmed (no DB schema touched).
  • npx vitest run test/unit/queue.test.ts test/unit/transient-locks.test.ts — 707/707 passing.
  • Scoped coverage on src/queue/transient-locks.ts: 100% statements/branches/functions/lines. Two new dedicated tests in test/unit/transient-locks.test.ts close a real, pre-existing gap the extraction exposed (not introduced): every existing "cache.claim() throws" test in queue.test.ts uses a mock cache that omits releaseIfValue, so it hits claimTransientLock's earlier !cache.releaseIfValue fail-open guard and never actually reaches the try/catch around cache.claim() itself — invisible before because it was diluted inside processors.ts's aggregate coverage; now that this is a standalone file, patch coverage requires it closed. Same story for releaseTransientLockIfOwner's !cache?.releaseIfValue early-return branch. Both gaps closed with minimal, targeted tests; the existing ~20 indirect tests in queue.test.ts (unmoved) provide the rest of the module's real coverage for free, confirming the extraction didn't change behavior.
  • npm run test:workers, build:mcp, test:mcp-pack, ui:openapi:check, ui:lint, ui:typecheck, ui:build, npm audit — not re-run locally (no worker/MCP/OpenAPI/UI-component code touched); CI runs them authoritatively.

Safety

  • No secrets, wallet/hotkey/trust-score/reward data anywhere.
  • No behavior change of any kind — pure structural move, verified by the full existing queue.test.ts suite passing unchanged.
  • No UI changes — no UI Evidence section needed.
  • No docs/changelog changes needed.

Notes

  • Next steps in the sequence (separate PRs, per the issue): signal-snapshot.ts next (the other self-contained "prove the pattern" candidate the issue names), then duplicate-detection.ts, slop-detection.ts, review-evasion.ts, ci-resolution.ts, retention.ts, then the interdependent trio gate-checks.ts/ai-review-orchestration.ts/job-dispatch.ts last (the AI-review-lock wrapper likely moves as part of that last group).
  • Given this repo's very high commit velocity (dozens of PRs/day touching processors.ts), each subsequent step in this sequence should re-derive its own boundary at the time it's picked up rather than trusting this PR's snapshot of the file.

Step 1 of a ~10-step sequence to split src/queue/processors.ts (14000+
lines) into cohesive modules. Moves the generic transient-lock primitive
(claimTransientLock/releaseTransientLockIfOwner/TransientLockClaim) and
the per-PR actuation-lock domain wrapper built on it into
src/queue/transient-locks.ts -- a pure structural move, no logic changes.

The AI-review-lock domain wrapper (a second consumer of the same
primitive) stays in processors.ts for now and imports the primitive back
from the new module; its own extraction is a later step in the sequence.
A temporary re-export shim keeps test/unit/queue.test.ts's existing
import path working -- those tests are deeply interspersed with unrelated
ones, not in a cleanly extractable block, so relocating them is deferred.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 8, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.73%. Comparing base (3a3f67c) to head (03f7bc1).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4157   +/-   ##
=======================================
  Coverage   93.73%   93.73%           
=======================================
  Files         386      387    +1     
  Lines       36304    36304           
  Branches    13298    13298           
=======================================
+ Hits        34029    34031    +2     
+ Misses       1618     1617    -1     
+ Partials      657      656    -1     
Files with missing lines Coverage Δ
src/queue/processors.ts 95.26% <ø> (+0.03%) ⬆️
src/queue/transient-locks.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 08:32:40 UTC

3 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This is a clean, pure structural move of the transient-lock primitives (claimTransientLock, releaseTransientLockIfOwner, TransientLockClaim, claimPrActuationLock, releasePrActuationLock, PrActuationLockContendedError) from processors.ts into a new transient-locks.ts, with no logic changes — verified by diffing the moved blocks line-for-line against the new file. processors.ts re-imports what it still needs and adds a re-export shim for claimPrActuationLock/releasePrActuationLock so the existing queue.test.ts import path keeps working, which is called out transparently as deliberate debt. The new test/unit/transient-locks.test.ts targets two real, previously-diluted branches (cache.claim() throwing after passing the releaseIfValue guard, and releaseTransientLockIfOwner no-op when the cache isn't configured) rather than fabricating scenarios, and CI is green across build/typecheck/tests.

Nits — 5 non-blocking
  • Two blank lines are left behind at src/queue/processors.ts around the removed PrActuationLockContendedError and claimTransientLock blocks — worth a quick tidy pass.
  • The re-export shim for claimPrActuationLock/releasePrActuationLock in processors.ts is acknowledged temporary debt; consider filing/linking a tracking note so it doesn't get forgotten once the later split steps land.
  • PrActuationLockContendedError goes from an unexported class to `export class` — fine given the new module boundary, but worth a one-line confirmation in the PR body that no other consumer outside processors.ts now expects it exported.
  • Since this PR is explicitly step 1 of a ~10-step sequence, consider adding a short checklist or link to the tracking issue (refactor(queue): split processors.ts into cohesive modules #4013) in the PR description itself (not just code comments) so reviewers of later steps can see progress at a glance.
  • The AI-review lock (claimAiReviewLock/releaseAiReviewLock) staying in processors.ts while importing the primitive back from transient-locks.ts is a reasonable call given the circular-import constraint — worth keeping as a one-line rationale in a follow-up PR description for step 2's reviewer.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 52 registered-repo PR(s), 43 merged, 486 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 52 PR(s), 486 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 52 PR(s), 486 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 8, 2026
@JSONbored
JSONbored merged commit 08b4495 into main Jul 8, 2026
10 checks passed
@JSONbored
JSONbored deleted the refactor/processors-actuation-locks-4013 branch July 8, 2026 08:34
JSONbored added a commit that referenced this pull request Jul 11, 2026
…le (#4820)

Part of #4013's module-split sequence (step 2, after transient-locks.ts
in #4157): generateSignalSnapshots and its loadOpenQueueCounts helper
move to src/queue/signal-snapshot.ts. Pure mechanical move, no behavior
change -- a re-export shim keeps processors.ts's existing external
callers (src/api/routes.ts, test/unit/queue-trends.test.ts) working
unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant