Skip to content

fix(queue): complete #5021's cron-path fix -- the fan-out dispatcher itself was still isRegistered-gated - #5694

Merged
JSONbored merged 1 commit into
mainfrom
fix/cron-backfill-dispatch-isinstalled
Jul 14, 2026
Merged

fix(queue): complete #5021's cron-path fix -- the fan-out dispatcher itself was still isRegistered-gated#5694
JSONbored merged 1 commit into
mainfrom
fix/cron-backfill-dispatch-isinstalled

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Test plan

  • Regression test: cron fan-out (no repoFullName) includes an installed-but-not-registered repo and excludes a registered-but-not-installed one
  • npm run test:ci — full local gate, clean (run twice for stability; one unrelated selfhost-ai.test.ts subprocess-timing flake on the first run, confirmed passing 165/165 in isolation and on the clean second run)

Part of #5016

…itself was still isRegistered-gated

#5021 retargeted the two downstream entry points
(backfillRegisteredRepositories, enqueueRepositoryOpenDataBackfill) from
isRegistered to isInstalled, but never touched the actual candidate-
selection step for the periodic (30-min) cron sweep: processJob's
"backfill-registered-repos" no-repoFullName branch in job-dispatch.ts
still filtered listRepositories() on isRegistered before ever dispatching
a per-repo job.

An installed-but-not-subnet-registered repo therefore never got a
per-repo backfill job enqueued for it in the first place -- #5021's fix
never actually took effect on the real cron path, only on direct/API-
triggered single-repo calls. Found via a full-codebase audit of
remaining isRegistered call sites after #5021 merged.

Part of #5016
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (21d13b2) to head (32cd75d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5694   +/-   ##
=======================================
  Coverage   95.02%   95.02%           
=======================================
  Files         577      577           
  Lines       45978    45978           
  Branches    14724    14724           
=======================================
  Hits        43689    43689           
  Misses       1530     1530           
  Partials      759      759           
Flag Coverage Δ
shard-1 43.63% <100.00%> (-0.43%) ⬇️
shard-2 35.84% <0.00%> (+0.06%) ⬆️
shard-3 32.33% <0.00%> (-0.09%) ⬇️
shard-4 33.04% <0.00%> (-0.02%) ⬇️
shard-5 31.51% <0.00%> (-0.26%) ⬇️
shard-6 44.62% <0.00%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/queue/job-dispatch.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 added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-14 05:34:23 UTC

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

⏸️ Suggested Action - Manual Review

Review summary
This is a targeted one-line fix: the cron fan-out branch in processJob's "backfill-registered-repos" case was still filtering listRepositories() on isRegistered instead of isInstalled, meaning #5021's downstream isInstalled retargeting never actually took effect for the periodic 30-minute cron sweep since candidates were filtered out before dispatch. The fix correctly changes the filter predicate to match the downstream entry points, and the new regression test in queue.test.ts directly exercises the real cron path (no repoFullName) with an installed-but-unregistered repo and a registered-but-uninstalled repo, asserting only the former gets dispatched. The change is narrow, the trace from symptom to root cause is sound, and the added test targets the actual code path rather than a fabricated scenario.

Nits — 3 non-blocking
  • The inline comment block in job-dispatch.ts (lines 78-82) is fairly long for a one-line fix; consider trimming to the essential why once merged.
  • test/unit/queue-2.test.ts adds an access_tokens fetch stub without clear explanation in the diff context of why it's now needed — worth a one-line comment for future readers.
  • Consider adding a brief code comment near isInstalled filter usages elsewhere (if any) noting the convention, to prevent a third missed call-site in a future PR.

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.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. 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: 45 registered-repo PR(s), 37 merged, 320 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 320 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
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, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 45 PR(s), 320 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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 14, 2026
@JSONbored
JSONbored merged commit 54b0f18 into main Jul 14, 2026
17 checks passed
@JSONbored
JSONbored deleted the fix/cron-backfill-dispatch-isinstalled branch July 14, 2026 05:35
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant