refactor(queue): extract live CI-aggregate/merge-state resolution into its own module - #4960
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…o its own module Part of #4013's module-split sequence (step 6, after transient-locks.ts, signal-snapshot.ts, duplicate-detection.ts, slop-detection.ts, and review-evasion.ts): the live CI-aggregate/merge-state caching and resolution functions move to src/queue/ci-resolution.ts. Pure mechanical move, no behavior change. LiveGithubFacts (the request-scoped memo these functions read/write) stays in processors.ts -- it is a foundational type used across dozens of unrelated call sites there -- so this file imports it back type-only, which creates no runtime circular dependency even though processors.ts also imports several functions from this file for its own remaining call sites. liveFactKey/liveFactTokenPart and cachedRequiredStatusContexts are exported for the same reason: each still has one remaining caller left in processors.ts. Isolating this code surfaced a genuine, previously-invisible coverage gap: cachedLiveCiAggregate's own request-scoped cache-hit branch (a second call within the same webhook pass reusing the first call's in-flight/settled promise) had no direct test. Added one, importing the function directly rather than threading a full webhook simulation through two call sites that happen to share a cache key.
4ddcce0 to
f76a6d6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4960 +/- ##
=======================================
Coverage 94.26% 94.26%
=======================================
Files 448 449 +1
Lines 38900 38900
Branches 14172 14172
=======================================
+ Hits 36669 36670 +1
Misses 1574 1574
+ Partials 657 656 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-11 04:49:31 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 3 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Summary
src/queue/processors.ts. Step 6, aftertransient-locks.ts(refactor(queue): extract transient-lock primitives into their own module #4157),signal-snapshot.ts(refactor(queue): extract signal-snapshot generation into its own module #4820),duplicate-detection.ts(refactor(queue): extract duplicate-cluster adjudication into its own module #4823),slop-detection.ts(refactor(queue): extract AI-slop-advisory gating/orchestration into its own module #4946), andreview-evasion.ts(refactor(queue): extract review-evasion close-enforcement guards into their own module #4957).src/queue/ci-resolution.ts:cachedFetchLiveCiAggregate,fetchLiveCiAggregateWithRequiredContexts,cachedLiveCiAggregate,refreshLiveCiAggregate,reuseOrRefreshLiveCiAggregate,cachedLiveMergeState,refreshLiveMergeState,reuseOrRefreshLiveMergeState,cachedRequiredStatusContexts,evictLiveFactOnReject,liveFactKey,liveFactTokenPart,expectedCiContextsKeyPart,resolvedRequiredContextsKeyPart. Pure mechanical move, no behavior change.LiveGithubFacts(the request-scoped memo these functions read/write) andRequiredStatusContextsLookupstay inprocessors.ts— both are foundational types used across dozens of unrelated call sites there, not specific to this concern — soci-resolution.tsimports them back type-only, which creates no runtime circular dependency even thoughprocessors.tsalso imports several functions from this file for its own remaining call sites (primeLiveMergeState'sliveFactKey/liveFactTokenPartuse, one remainingcachedRequiredStatusContextscaller, and 6 disposition/readiness call sites for the other exported functions).processors.tsbefore, so there's no existing externalimport { ... } from "../../src/queue/processors"to preserve.cachedLiveCiAggregate's own request-scoped cache-hit branch (a second call within the same webhook pass reusing the first call's in-flight/settled promise, the whole point of the#4498memo) had no direct test. Added a small dedicatedtest/unit/ci-resolution.test.tsthat imports the function directly and asserts a second call with the samefacts+ args reuses the first result without a second live fetch, rather than trying to thread a full webhook simulation through two call sites that happen to share a cache key.Part of #4013 (more extraction steps remain in the sequence — not closing the tracking issue).
Test plan
npx tsc --noEmit -p .— zero errorsnpx vitest run test/unit test/integration— 697/698 files passed (1 skipped, pre-existing)npm run test:coverage(unsharded) — 94.44% statements / 93.42% branches / 93.61% functions / 95.03% lines, no threshold failures;ci-resolution.tsitself is 100% line + branch + function coverednpm run docs:drift-check,npm run manifest:drift-check,npm run engine-parity:drift-check— all oknpm audit --audit-level=moderate— 0 vulnerabilities