Skip to content

fix(review): add missing GitHub-budget rate-limit admission for 5 maintenance job types - #4570

Merged
JSONbored merged 1 commit into
mainfrom
claude/github-budget-admission-audit-4505
Jul 10, 2026
Merged

fix(review): add missing GitHub-budget rate-limit admission for 5 maintenance job types#4570
JSONbored merged 1 commit into
mainfrom
claude/github-budget-admission-audit-4505

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Closes fix(queue): reconcile-open-prs has no GitHub rate-limit admission or job dedup #4505. reconcile-open-prs was missing from GITHUB_BUDGET_BACKGROUND_TYPES and had no jobCoalesceKey case, so once GITTENSORY_PR_RECONCILIATION is enabled its real paginated per-repo GitHub REST calls would never yield to an exhausted shared budget, and every 10-min tick would insert a duplicate row instead of coalescing into one still-pending scan.
  • Fixing this one prompted a systematic audit of every MAINTENANCE_JOB_TYPES member against GITHUB_BUDGET_BACKGROUND_TYPES (the module's own header comment claims universal coverage — it wasn't true), which found four more of the same gap:
    • refresh-installation-healthgetAppInstallation (a direct, unprotected GET /app/installations/{id} REST call) per installation, plus resolveRepositorySettings per installed repo. Runs every 30 min, UNCONDITIONALLY — the most severe of the five, since it's exercised in every deployment today, not just after an operator opts into a flag.
    • backlog-convergence-sweepresolveRepositorySettings per repo in both the fan-out and per-repo handler. Runs every 30 min, unconditional for self-hosted runtimes.
    • selftuneresolveRepositorySettings per registered repo. Hourly, flag-gated.
    • generate-review-recaploadRepoFocusManifest directly. Not yet cron-enqueued (manual/API trigger only today), and had no jobCoalesceKey case at all — every trigger inserted a fresh duplicate row.
    • Every OTHER MAINTENANCE_JOB_TYPES member not already in the set was traced and confirmed to make zero GitHub calls (purely local D1 work, or dispatching an already-gated job type) — see the new negative-assertion test.
  • All five now yield to shouldWaitForGitHubRateLimit at dequeue time and coalesce a repeated enqueue into an already-pending/processing row.
  • maintenance-admission.ts's header comment is corrected to describe which job types get GitHub-budget admission (the subset making real GitHub calls) vs. only local-load admission (the purely-internal sweeps), rather than claiming universal coverage.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused (one root cause, one mechanical fix pattern applied everywhere it was confirmed to apply) and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (fix(queue): reconcile-open-prs has no GitHub rate-limit admission or job dedup #4505).

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run typecheck
  • npm run test:coverage locally (unsharded) — zero uncovered statements/branches in the diff
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — invariant test (reconcile-open-prs pre-yields at the dequeue-time throttle while the shared budget is exhausted, end-to-end via worker.queue()), regression tests (jobCoalesceKey now returns a stable per-repo key for generate-review-recap instead of falling through to null; the two previously-simple-return-type cases refresh-installation-health/selftune still coalesce correctly), and a positive+negative isGitHubBudgetBackgroundJob matrix covering all 5 newly-gated types plus every confirmed-clean type from the audit, per fix(queue): reconcile-open-prs has no GitHub rate-limit admission or job dedup #4505's requirements

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests — N/A, no such change.
  • API/OpenAPI/MCP behavior is updated and tested where needed — N/A, no external behavior change (internal scheduling/dedup only).
  • UI changes use live API data — N/A, no UI change.
  • UI Evidence — N/A, no visible/UI change.
  • Public docs/changelogs updated — N/A.

…ntenance job types (#4505)

reconcile-open-prs was missing from GITHUB_BUDGET_BACKGROUND_TYPES and had no
jobCoalesceKey case, so once GITTENSORY_PR_RECONCILIATION is enabled its real
paginated per-repo GitHub REST calls would never yield to an exhausted shared
budget, and every 10-min tick would insert a duplicate row instead of
coalescing into one still-pending scan.

Fixing this one prompted a systematic audit of every MAINTENANCE_JOB_TYPES
member against GITHUB_BUDGET_BACKGROUND_TYPES (the module's own header
comment claims universal coverage), which found four more of the same gap:

- refresh-installation-health: getAppInstallation (a direct, unprotected
  `GET /app/installations/{id}` REST call) per installation, plus
  resolveRepositorySettings per installed repo. Runs every 30 min,
  UNCONDITIONALLY -- the most severe of the five, since it is exercised in
  every deployment today, not just after an operator opts into a flag.
- backlog-convergence-sweep: resolveRepositorySettings per repo in both the
  fan-out and per-repo handler. Runs every 30 min, unconditional for
  self-hosted runtimes.
- selftune: resolveRepositorySettings per registered repo. Hourly,
  flag-gated.
- generate-review-recap: loadRepoFocusManifest directly. Not yet
  cron-enqueued (manual/API trigger only today), and had no jobCoalesceKey
  case at all -- every trigger inserted a fresh duplicate row.

All five now yield to shouldWaitForGitHubRateLimit at dequeue time and
coalesce a repeated enqueue into an already-pending/processing row. The
maintenance-admission.ts header comment is corrected to describe which job
types get GitHub-budget admission (the subset making real GitHub calls) vs.
only local-load admission (the purely-internal sweeps), rather than claiming
universal coverage.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.10%. Comparing base (3f23c61) to head (7d6e723).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4570   +/-   ##
=======================================
  Coverage   94.10%   94.10%           
=======================================
  Files         427      427           
  Lines       38022    38022           
  Branches    13877    13877           
=======================================
  Hits        35779    35779           
+ Misses       1586     1585    -1     
- Partials      657      658    +1     
Files with missing lines Coverage Δ
src/selfhost/maintenance-admission.ts 100.00% <ø> (ø)
src/selfhost/queue-common.ts 95.44% <ø> (ø)
🚀 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 10, 2026
@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-10 04:45:19 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/selfhost/maintenance-admission.ts (matched src/selfhost/**), src/selfhost/queue-common.ts (matched src/selfhost/**).

Review summary
This PR closes #4505 by adding five maintenance job types (reconcile-open-prs, backlog-convergence-sweep, selftune, refresh-installation-health, generate-review-recap) to GITHUB_BUDGET_BACKGROUND_TYPES and adding missing jobCoalesceKey cases for reconcile-open-prs and generate-review-recap, which previously fell through to null and inserted duplicate rows on every trigger. The fix is well-scoped to the audited root cause (all five types are legitimately in MAINTENANCE_JOB_TYPES per the full file content shown, and each is documented as making real, unprotected GitHub REST calls), and the new tests exercise the real production path — index.test.ts drives worker.queue() end-to-end with a genuine recordGitHubRateLimitObservation-exhausted budget rather than fabricating an impossible state. The reconcile-open-prs coalesce key correctly uses the global `return type` arm since the job payload has no repoFullName (a single scan across all watched repos), while generate-review-recap correctly gets a per-repo key matching its payload shape.

Nits — 5 non-blocking
  • The inline audit comments added to GITHUB_BUDGET_BACKGROUND_TYPES in queue-common.ts:98-121 are quite long (multi-paragraph per entry) — consider trimming to one line per job type with a link to the fix(queue): reconcile-open-prs has no GitHub rate-limit admission or job dedup #4505 discussion for the full rationale, since this file is already flagged as long (974 lines).
  • The negative-assertion test in selfhost-queue-common.test.ts lists 10 job types confirmed to make no GitHub calls; consider a short comment noting how future additions to MAINTENANCE_JOB_TYPES should extend this test so the audit doesn't silently go stale.
  • Consider adding a lint/test rule that fails when a MAINTENANCE_JOB_TYPES entry has no corresponding jobCoalesceKey case, to prevent the generate-review-recap-style 'silent fallthrough to null' gap from recurring.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4505
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 353 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 353 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Linked issue satisfaction

Partially addressed
The diff adds reconcile-open-prs to GITHUB_BUDGET_BACKGROUND_TYPES, adds the jobCoalesceKey case, corrects the maintenance-admission.ts header comment, and includes an invariant test for GitHub rate-limit yield — covering the first three deliverables and one of the three required tests. However, the issue's non-negotiable regression test explicitly for a second reconcile-open-prs enqueue coalescin

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: not available
  • Official Gittensor activity: 48 PR(s), 353 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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 10, 2026
@JSONbored
JSONbored merged commit 3faad4f into main Jul 10, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/github-budget-admission-audit-4505 branch July 10, 2026 04:50
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.

fix(queue): reconcile-open-prs has no GitHub rate-limit admission or job dedup

1 participant