fix(queue): isolate per-repo failures in backfill-registered-repos fan-out (#8355)#8473
Conversation
…n-out (JSONbored#8355) The cron-scheduled full-fleet sweep fanned out one per-repo job via a bare Promise.all -- a single transient env.JOBS.send rejection aborted the whole fan-out, and the queue's retry then re-ran the entire repositories map from scratch, duplicate-dispatching every repo whose send had already succeeded before the failure. Switches to Promise.allSettled so every repo's send is attempted exactly once regardless of an earlier one's outcome, logs each individual failure (repoFullName + reason, matching this file's existing structured-logging convention), and still throws after the settle so the cron invocation is marked failed for observability -- but only once every send has been attempted.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8473 +/- ##
==========================================
+ Coverage 80.86% 89.65% +8.78%
==========================================
Files 791 98 -693
Lines 79320 22864 -56456
Branches 23954 3918 -20036
==========================================
- Hits 64142 20498 -43644
+ Misses 12156 2187 -9969
+ Partials 3022 179 -2843
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 14:59:41 UTC
Review summary Nits — 5 non-blocking
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.
|
Summary
processJob's"backfill-registered-repos"case fans out one per-repo job via a barePromise.allwhen the cron-scheduled full-fleet sweep runs with norepoFullName.Promise.allrejects as soon as any singleenv.JOBS.sendcall rejects, throwing out of the wholeprocessJobinvocation.repositories.mapfan-out again — including for every repo whoseenv.JOBS.sendhad already succeeded, causing duplicate per-repo backfill dispatch across the whole fleet from a single transient send failure.Promise.allSettledso every repo's send is attempted exactly once regardless of an earlier one's outcome. Each individual failure is logged (repoFullName+ reason) matching this file's existing structured-logging convention (the"sync-brokered-installed-repos"case a few lines above). A genuine failure still surfaces via a thrown error after the settle, so the cron invocation itself is marked failed for observability — but only once every repo's send has been attempted.Closes #8355
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/queue/job-dispatch.ts(pure backend logic, no UI/MCP/workers/OpenAPI surface) plus its test file. Verified vianpx vitest run test/unit/job-dispatch.test.ts: all 4 tests pass, including the 2 new ones — one asserting every OTHER repo's send is still attempted when one repo's send rejects (and that the job throws with the exact failed repo named, after every send was attempted), and one asserting no throw/log when every send succeeds. Coverage on the changed lines (verified directly againstcoverage/lcov.info): the rejection-detection branch inside thesettled.forEachshows both arms hit (BRDA:135,10,0,1/BRDA:135,10,1,4), and the finalfailedRepoFullNames.length > 0throw-condition shows both arms hit (BRDA:141,11,0,1/BRDA:141,11,1,1).Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A — no visible UI changes.)Notes
Promise.allSettleddirectly at the call site rather thanmapWithConcurrency(also acceptable per the issue):mapWithConcurrency's own worker pool is itself built onPromise.allinternally, so it would not provide per-item isolation without the mapper also catching its own errors —Promise.allSettledgives that isolation directly with no extra wrapping.