Skip to content

fix(selfhost): stop exempting agent-regate-sweep from installation concurrency admission - #2989

Merged
JSONbored merged 2 commits into
mainfrom
claude/installation-concurrency-sweep-gap
Jul 4, 2026
Merged

fix(selfhost): stop exempting agent-regate-sweep from installation concurrency admission#2989
JSONbored merged 2 commits into
mainfrom
claude/installation-concurrency-sweep-gap

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • PR feat(selfhost): add per-installation GitHub-fetch concurrency admission #2970 (merged) added per-installation GitHub-fetch concurrency admission, gated at the sqlite-queue.ts/pg-queue.ts call sites via isForegroundJobPriority(job.priority) ? null : installationConcurrencyKeyForJob(message).
  • agent-regate-sweep's own row priority (8, PRIORITY_BY_TYPE in src/selfhost/queue-common.ts) exactly equals FOREGROUND_QUEUE_PRIORITY_FLOOR (also 8), so isForegroundJobPriority(8) returns true and the limiter was silently inert for agent-regate-sweep — exactly the sweep/backfill fan-out this policy was added to bound (per the feature's own motivating narrative: "one installation's background sweep/backfill claim every available background slot"). A pre-existing test comment already documented this as a known gap and worked around it with a different fixture instead of fixing it.
  • This moves the exclusion into installationConcurrencyKeyForJob itself, keyed by job type (agent-regate-pr only) instead of by priority-floor comparison at the call sites, so it can never collide with another type's tuned priority again.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused 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 an issue, or this is small enough that the summary explains why an issue is not needed. (No issue: a small, scoped follow-up fix to an already-merged feature (feat(selfhost): add per-installation GitHub-fetch concurrency admission #2970), found and fixed same-day.)

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage
  • 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

If any required check was skipped, explain why:

  • Ran typecheck plus the full targeted test files (test/unit/selfhost-installation-concurrency-admission.test.ts, test/unit/selfhost-sqlite-queue.test.ts, test/unit/selfhost-pg-queue.test.ts — 286/286 passing, confirmed via v8 branch coverage that the new type-based branch is hit both ways) rather than the full suite, per this repo's convention of scoping local verification to the affected area for small fixes and letting CI verify the rest.

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 auth/session/CORS changes.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/OpenAPI/MCP changes.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Visible UI changes include a UI Evidence section below with screenshots. (N/A — no visible UI changes.)
  • Public docs/changelogs are updated where needed. (N/A — no docs/changelog changes.)

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

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-04 09:02:23 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change moves installation-concurrency foreground exemption from a fragile priority-floor check at the queue call sites into `installationConcurrencyKeyForJob`, keyed specifically on `agent-regate-pr`, and adds regression coverage for `agent-regate-sweep` in both queue backends. That directly addresses the priority-8/floor-8 collision described in the PR and keeps the limiter applied to sweep fan-out while preserving foreground PR handling. The new tests exercise the actual queue claim path instead of only the pure resolver, which is the important part of this fix.

Nits — 6 non-blocking
  • nit: `src/selfhost/queue-common.ts:409` and the new test comments repeat a long explanation of the same priority collision; consider shortening the queue-backend comments and leaving the detailed rationale in the resolver test or helper doc comment.
  • nit: `test/unit/selfhost-pg-queue.test.ts:3202` hard-codes `priority: 8`; importing or deriving the sweep priority from the same source as production would keep the regression test from drifting if the priority is intentionally retuned.
  • nit: `test/unit/selfhost-sqlite-queue.test.ts:3629` uses a broad `payload LIKE '%agent-regate-sweep%'` lookup; filtering by `last_error` or job key as well would make the assertion less sensitive to unrelated pending sweep rows in this fixture.
  • In `test/unit/selfhost-pg-queue.test.ts:3202`, replace the magic `priority: 8` with the production sweep priority constant if it is exported, or add a focused assertion that documents the intended equality with `FOREGROUND_QUEUE_PRIORITY_FLOOR`.
  • In `test/unit/selfhost-sqlite-queue.test.ts:3629`, narrow the pending-row lookup so the test proves the deferred row is the second sweep job rather than any pending sweep payload.
  • 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 ✅ No-issue rationale PR body explains why no issue is linked.
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: 60 registered-repo PR(s), 51 merged, 421 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 421 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 60 PR(s), 421 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
  • 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

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2989   +/-   ##
=======================================
  Coverage   96.06%   96.06%           
=======================================
  Files         260      260           
  Lines       28684    28685    +1     
  Branches    10437    10437           
=======================================
+ Hits        27555    27556    +1     
  Misses        493      493           
  Partials      636      636           
Files with missing lines Coverage Δ
src/selfhost/queue-common.ts 95.43% <100.00%> (+0.01%) ⬆️
src/selfhost/sqlite-queue.ts 99.58% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added 2 commits July 4, 2026 01:58
…ncurrency admission

installationConcurrencyKeyForJob was gated at the sqlite-queue.ts/pg-queue.ts
call sites by isForegroundJobPriority(job.priority), but agent-regate-sweep's
own row priority (8, PRIORITY_BY_TYPE) equals FOREGROUND_QUEUE_PRIORITY_FLOOR
(also 8), so that guard misclassified it as foreground and silently exempted
it from the new per-installation concurrency limiter -- exactly the
sweep/backfill fan-out this policy was added to bound.

Move the exclusion into installationConcurrencyKeyForJob itself, by job type
(agent-regate-pr only) rather than by priority, so it can never collide with
another type's tuned priority again. Add regression coverage exercising
agent-regate-sweep through the limiter in both queue backends.
The fabricated agent-regate-sweep row carried no priority column, which
is indistinguishable from a real background job under the old
priority-based exclusion guard (isForegroundJobPriority(NaN) is false
either way) -- the test passed against both the buggy and fixed code,
proving nothing. Construct the row with the real priority (8,
PRIORITY_BY_TYPE) via enqueueResult so the test only passes under the
new type-based exclusion. Verified by temporarily reintroducing the old
priority-based guard: the test now fails against it as expected.

Also lock the type-based split at the pure resolver level: assert
agent-regate-sweep gets a key despite the same priority-8/floor-8
collision that excludes agent-regate-pr, in the same test.
@JSONbored
JSONbored force-pushed the claude/installation-concurrency-sweep-gap branch from 63f2610 to c9f5cf4 Compare July 4, 2026 08:58
@JSONbored
JSONbored merged commit 6e1872f into main Jul 4, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/installation-concurrency-sweep-gap branch July 4, 2026 09:06
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.

Development

Successfully merging this pull request may close these issues.

1 participant