Skip to content

fix(agent-actions): honor ADMIN_GITHUB_LOGINS in close-eligibility - #2390

Merged
JSONbored merged 1 commit into
mainfrom
claude/admin-login-close-exemption
Jul 1, 2026
Merged

fix(agent-actions): honor ADMIN_GITHUB_LOGINS in close-eligibility#2390
JSONbored merged 1 commit into
mainfrom
claude/admin-login-close-exemption

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

The main planner's close-eligibility gate (isContributor/closeEligible/blacklistContributor in src/settings/agent-actions.ts) and the draft-dodge handler's duplicated owner check (src/queue/processors.ts) only recognized the literal repo-owner login as a trusted, never-auto-closed identity. The reopen-reclose path's hasMaintainerPermission already treats ADMIN_GITHUB_LOGINS members as trusted maintainers (login === repoOwner || admins.has(login)) — explicitly unified there for exactly this purpose. A fleet-operator admin login (not the literal repo owner) was therefore reopen-immune but still eligible for heuristic/blacklist auto-close on the very same PR: two independently drifting definitions of "maintainer" for the same identity.

Fix

  • src/settings/agent-actions.ts: added authorIsAdmin: boolean to the planner's input type, computed the same way authorIsOwner is (by the caller, kept out of this pure/dependency-injected module). Threaded it into isContributor, blacklistContributor, and closeEligible — treated identically to authorIsOwner throughout: never auto-closed by default, eligible only when closeOwnerAuthors is explicitly on (folded into the same per-repo toggle rather than inventing a second one, since it's the same "should our trusted-tier PRs be treated like contributor PRs" question).
  • src/queue/processors.ts: computed authorIsAdmin via the same parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS) helper hasMaintainerPermission already uses, at both the main maintenance planner's call site and the draft-dodge handler's independently-duplicated authorIsOwner computation — the two spots named in the issue.

Tests

  • 4 new tests in agent-actions.test.ts mirroring the existing owner-PR-guard group: an admin-authored noisy/failing PR is not auto-closed, still auto-merges when clean+approved, closes when closeOwnerAuthors is on, and is not closed on red CI.
  • Extended the existing blacklist-exemption test ("NEVER fires for the owner or an automation bot") to also assert an admin-authored PR is exempt from the blacklist short-circuit.
  • New integration test in queue.test.ts: the draft-dodge handler no-ops for an ADMIN_GITHUB_LOGINS-listed, non-owner author, mirroring the existing literal-owner no-op test.
  • npx tsc --noEmit clean — making authorIsAdmin a required field (matching authorIsOwner's own required-ness) surfaced every call site needing the update via the compiler, including 3 raw-literal test calls that bypass the shared input() test helper.
  • Scoped: agent-actions.test.ts — 99 passed (95 + 4 new).
  • Regression sweep: agent-action-executor.test.ts, agent-approval-queue.test.ts, outcomes-wire.test.ts, precision-breakers-chain.test.ts (all other importers of agent-actions.ts) — 89 passed; full queue.test.ts — 203 passed.
  • Diff-range coverage-gap check on both changed source files: fully covered.
  • Full unsharded npm run test:coverage: 5605 passed, 4 skipped (pre-existing/unrelated), 0 failed.
  • npm audit --audit-level=moderate: 0 vulnerabilities.

Advances #1936. Closes #2133.

The planner's isContributor/closeEligible/blacklistContributor gates
and the draft-dodge path's duplicated authorIsOwner computation only
recognized the literal repo-owner login. The reopen-reclose path's
hasMaintainerPermission already treats ADMIN_GITHUB_LOGINS members as
trusted maintainers, so a fleet-operator admin (not the literal owner)
was reopen-immune but still eligible for heuristic/blacklist auto-close
on the same PR — two independently drifting definitions of
"maintainer" for the same identity.

Thread a new authorIsAdmin field (computed via the same
parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS) helper) through
planAgentMaintenanceActions, treating it identically to authorIsOwner
everywhere: never auto-closed by default, eligible only when
closeOwnerAuthors is on. Apply the identical fix to the draft-dodge
handler's duplicate computation in processors.ts.
@dosubot dosubot Bot added the size:M label Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.71%. Comparing base (17d70b2) to head (4f98882).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2390   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files         222      222           
  Lines       24661    24663    +2     
  Branches     8949     8951    +2     
=======================================
+ Hits        23605    23607    +2     
  Misses        433      433           
  Partials      623      623           
Files with missing lines Coverage Δ
src/queue/processors.ts 90.23% <100.00%> (+0.01%) ⬆️
src/settings/agent-actions.ts 92.52% <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 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 20:09:50 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change extends the trusted-author exemption in the maintenance planner and draft-dodge close path from repo owner only to repo owner or configured admin login, and the call sites compute that flag with the existing login-list parser. The planner logic keeps automation-bot behavior unchanged and only makes admin-authored PRs close-eligible when closeOwnerAuthors is explicitly true, which matches the stated contract. The notable remaining gap is test depth at the main queue caller, not the core logic.

Nits — 6 non-blocking
  • nit: src/queue/processors.ts:1639 and src/queue/processors.ts:3376 still duplicate the trusted-author calculation, so the owner/admin definition can drift again unless this is centralized.
  • nit: test/unit/agent-actions.test.ts:372 covers the pure planner, and test/unit/queue.test.ts:9576 covers draft-dodge, but there is no visible queue-level test proving maybeRunAgentMaintenance threads ADMIN_GITHUB_LOGINS into authorIsAdmin on the main close-eligibility path.
  • Extract a small helper in src/queue/processors.ts for the owner-or-admin author check and use it at both src/queue/processors.ts:1635 and src/queue/processors.ts:3367.
  • Add a main maintenance processor test that sets ADMIN_GITHUB_LOGINS to a non-owner author and verifies an otherwise close-eligible PR is held open through the real maybeRunAgentMaintenance call path.
  • 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 #2133
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:M; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 67 registered-repo PR(s), 57 merged, 589 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 67 PR(s), 589 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Check active issues and PRs before submitting.
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 1, 2026
@JSONbored JSONbored self-assigned this Jul 1, 2026
@JSONbored
JSONbored merged commit 0ccd88d into main Jul 1, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/admin-login-close-exemption branch July 1, 2026 22:10
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
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.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

fix(agent-actions): owner-exemption logic does not recognize ADMIN_GITHUB_LOGINS

1 participant