Skip to content

fix(agent-actions): scope the global open-item cap query by installation - #2687

Merged
JSONbored merged 2 commits into
mainfrom
fix/global-contributor-cap-installation-scoping
Jul 3, 2026
Merged

fix(agent-actions): scope the global open-item cap query by installation#2687
JSONbored merged 2 commits into
mainfrom
fix/global-contributor-cap-installation-scoping

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • countOpenItemsForAuthorAcrossRepos (shipped in feat(agent-actions): add an install-wide contributor open-item cap across repos #2678, closing feat(agent-actions): add an install-wide contributor open-item cap across repos #2562) counts an author's open PRs/issues with no installation scoping at all — a plain author-login filter against the whole D1 database. On a database shared by multiple installations (the normal shape for the hosted product, and possible on self-host too if the same App is installed against more than one org/account), a contributor's open items on a completely unrelated installation can push another installation's count over GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP and trigger a wrongful close.
  • Fixes it by resolving the installation's repo set first (new listRepoFullNamesForInstallation, mirroring the existing markRepositoriesRemovedFromInstallation precedent), then scoping both the PR and issue count queries via inArray(...) — this codebase has no Drizzle joins to lean on instead.
  • Also fixes 5 AiReviewCacheInput test fixtures in queue.test.ts that were left broken by an unrelated, already-merged PR (security-focused review profile) adding a required securityFocus field without updating them — main's typecheck was red without this, which this PR's own CI would otherwise have inherited. (These were independently fixed on main during this PR's rebase; kept here as a no-op for safety.)

Scope

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run db:migrations:check — no new migration, no-op
  • npm run typecheck
  • npm run test:coverage locally — full coverage confirmed; added a direct regression test proving cross-installation isolation (countOpenItemsForAuthorAcrossRepos scoped to installation A does not see installation B's rows on the same D1), plus fixed 2 pre-existing integration tests whose setup relied on the unscoped (buggy) behavior to find a second repo's rows.
  • npm run test:workers (via npm run test:ci)
  • npm run build:mcp (via npm run test:ci)
  • npm run test:mcp-pack (via npm run test:ci)
  • npm run ui:openapi:check — no API-visible schema touched, no-op
  • npm run ui:lint (via npm run test:ci)
  • npm run ui:typecheck (via npm run test:ci)
  • npm run ui:build (via npm run test:ci)
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

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 surface touched; this is a same-database query-scoping fix.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API-visible change.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, backend-only.
  • Visible UI changes include a UI Evidence section below with screenshots. — N/A, no UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — N/A.

Notes

Regression found while independently implementing the same feature (#2562) in parallel — my own implementation caught this exact cross-installation scoping gap via the gate's own AI review before I discovered #2678 had already shipped without it. Filing as a direct fix rather than reopening the closed issue.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.14%. Comparing base (a2ea220) to head (0a74156).
⚠️ Report is 8 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/db/repositories.ts 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2687      +/-   ##
==========================================
- Coverage   96.15%   96.14%   -0.01%     
==========================================
  Files         240      240              
  Lines       26867    26875       +8     
  Branches     9751     9752       +1     
==========================================
+ Hits        25833    25839       +6     
- Misses        424      425       +1     
- Partials      610      611       +1     
Files with missing lines Coverage Δ
src/queue/processors.ts 92.68% <100.00%> (-0.05%) ⬇️
src/db/repositories.ts 96.56% <88.88%> (-0.05%) ⬇️
🚀 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 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-03 06:05:08 UTC

4 files · 1 AI reviewer · 1 blocker · readiness 93/100 · CI green · blocked

⏸️ Suggested Action - Manual Review

  • AI reviewers agree on a likely critical defect: src/db/repositories.ts:listRepoFullNamesForInstallation uses `.limit(20_000)`, so an installation with more than 20,000 tracked repositories will undercount open items beyond that arbitrary cutoff and can fail to enforce `GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP` for the same installation. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The change fixes the cross-installation leak by deriving the installation's repository set and scoping both PR and issue aggregates to those repo full names, with call sites updated to pass the current installation id. The regression tests cover the important shared-D1 case and the no-repositories case, so the main bug path is addressed. The notable remaining concern is that the helper silently caps the repo set at 20,000, which makes the install-wide cap incorrect for very large installations.

Blockers

  • src/db/repositories.ts:listRepoFullNamesForInstallation uses `.limit(20_000)`, so an installation with more than 20,000 tracked repositories will undercount open items beyond that arbitrary cutoff and can fail to enforce `GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP` for the same installation.
Nits — 5 non-blocking
  • nit: src/db/repositories.ts:listRepoFullNamesForInstallation could avoid the large `IN (...)` parameter fanout by counting via a repository-scoped subquery or batching, since the current shape can produce very large SQL statements for large installations.
  • nit: test/unit/global-contributor-cap.test.ts should include a fixture where a repo has open items but is not present in `repositories`, to pin the intended fail-closed/fail-open behavior for orphaned rows after this scope change.
  • Change `src/db/repositories.ts:listRepoFullNamesForInstallation` to remove the silent `.limit(20_000)`, or batch the repo names and sum the counts across batches so every tracked repository participates.
  • Add a test in `test/unit/global-contributor-cap.test.ts` that exercises a large or batched repo set, or at least proves the implementation does not truncate the installation scope.
  • Consider documenting in `countOpenItemsForAuthorAcrossRepos` that orphaned PR/issue rows without a matching `repositories` row are intentionally excluded, since the queue tests now depend on explicit repository registration.

Concerns raised — review before merging

  • src/db/repositories.ts:listRepoFullNamesForInstallation uses `.limit(20_000)`, so an installation with more than 20,000 tracked repositories will undercount open items beyond that arbitrary cutoff and can fail to enforce `GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP` for the same installation.
Signal Result Evidence
Code review ❌ 1 blocker 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 (size label size:M; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 61 registered-repo PR(s), 52 merged, 500 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 61 PR(s), 500 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
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: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 61 PR(s), 500 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.
  • 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

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 3, 2026
JSONbored added 2 commits July 2, 2026 23:16
countOpenItemsForAuthorAcrossRepos (merged via #2678, closing #2562)
counted an author's open PRs/issues across the ENTIRE D1 database with
no installation scoping at all -- on a database shared by multiple
installations (the hosted product's normal shape, and possible on
self-host too), a contributor's activity on one installation could
wrongly trigger GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP closes on a completely
unrelated installation that never gated them.

Scope the query through repositories.installationId first (matching
the existing markRepositoriesRemovedFromInstallation precedent), then
inArray(...) against the resulting repoFullNames -- this codebase has
no Drizzle joins to lean on instead.

Also fixes 5 AiReviewCacheInput test fixtures in queue.test.ts left
broken by an unrelated already-merged PR (#2675, security-focused
review profile) that added a required securityFocus field without
updating these fixtures -- main's typecheck was red without this,
which this PR's own CI would otherwise have inherited.
…rop it silently

listRepoFullNamesForInstallation's .limit(20_000) meant an installation
with more tracked repos than that would silently undercount toward
GLOBAL_CONTRIBUTOR_OPEN_ITEM_CAP with no signal anything was dropped.
Records an audit event on the rare install where the limit is still
hit, mirroring the same observability pattern already used for the
per-author item-count truncation in this file.

Addresses a gate review finding on #2687.
@JSONbored
JSONbored force-pushed the fix/global-contributor-cap-installation-scoping branch from 9e3afcc to 0a74156 Compare July 3, 2026 06:18
@JSONbored
JSONbored merged commit 96d2980 into main Jul 3, 2026
8 checks passed
@JSONbored
JSONbored deleted the fix/global-contributor-cap-installation-scoping branch July 3, 2026 06:34
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