Skip to content

fix(queue): opportunistically refresh installation health on a PR-write 403 - #2410

Merged
JSONbored merged 2 commits into
mainfrom
claude/installation-health-refresh-on-403
Jul 2, 2026
Merged

fix(queue): opportunistically refresh installation health on a PR-write 403#2410
JSONbored merged 2 commits into
mainfrom
claude/installation-health-refresh-on-403

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

installationPermissions is read once from the installations table at the top of the action loop and threaded through the whole pass. That DB row only self-heals via a consented permission-upgrade webhook (GitHub sends none for a maintainer-initiated downgrade) or the ~30-minute health-refresh cron. A live downgrade of pull_requests write access can therefore go undetected by the readiness gate for up to 30 minutes — during which the executor keeps attempting merge/close/review/update_branch calls that GitHub correctly rejects with 403.

This is a wasted-attempt/staleness gap, not an authorization bypass: GitHub enforces the live scope server-side on the actual mutation and returns 403, which the existing terminal-403 handling already catches and holds correctly.

Fix

When a PR-write mutation (request_changes/approve/merge/close/update_branch) fails with a 403, opportunistically call refreshInstallationHealthForInstallation (already used by the 30-minute cron) so the DB row self-heals immediately instead of waiting for the next tick — every later sweep/webhook read of it, for this or any other PR on the installation, then reflects the corrected permissions. A failed refresh is swallowed (best-effort; GitHub's own enforcement is already the real backstop).

Exported the existing githubErrorStatus helper from github/app.ts to detect the 403 rather than duplicating status-reading logic.

Tests

  • New test: a 403 from a PR-write mutation calls refreshInstallationHealthForInstallation(env, installationId).
  • New test: a non-403 mutation failure does NOT trigger the refresh.
  • New test: a 403 from label (uses issues:write, not pull_requests:write — not in the write-permission gate) does NOT trigger the refresh.
  • New test: a failed refresh itself is swallowed and does not affect the recorded action outcome.
  • npx tsc --noEmit clean.
  • npx vitest run test/unit/agent-action-executor.test.ts — 33/33 pass.
  • Diff-range coverage check: both changed files fully covered, zero gaps.
  • Regression sweep across executor/app/approval-queue/merge-failure/queue suites — 360/360 pass.
  • Full unsharded npm run test:coverage — 309 files / 5661 tests pass.
  • npm audit --audit-level=moderate — 0 vulnerabilities.

Advances #1936. Closes #2265.

…te 403

The local installations.permissions snapshot is read once at the top of
each sweep/webhook pass and self-heals only via a maintainer-consented
permission-upgrade webhook (GitHub sends none for a downgrade) or the
~30-minute health-refresh cron. A maintainer-initiated downgrade of
pull_requests write access can therefore go undetected by the readiness
gate for up to 30 minutes, during which the executor keeps attempting
merge/close/review/update_branch calls that GitHub correctly rejects with
403 — wasted attempts, not a security gap (GitHub's own server-side
enforcement is already the real backstop).

When a PR-write mutation fails with a 403, opportunistically trigger a
per-installation health refresh so the DB row (and therefore every later
sweep/webhook read of it, for this or any other PR on the installation)
self-heals immediately instead of waiting for the next cron tick. Exports
the existing githubErrorStatus helper from github/app.ts to detect the
403 without duplicating status-reading logic.
@dosubot dosubot Bot added the size:S label Jul 1, 2026
@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-02 01:07:13 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change correctly routes 403 failures from PR-write action classes through the existing installation-health refresh path, which addresses the stale local permissions snapshot without changing the terminal action outcome. Exporting and reusing `githubErrorStatus` keeps status extraction consistent with the existing GitHub error handling. The added tests cover the important boundaries: PR-write 403, non-403 failures, non-PR-write 403s, and best-effort refresh failure swallowing.

Nits — 3 non-blocking
  • src/services/agent-action-executor.ts:181: Consider adding a focused test for a 403 carried on `error.response.status`, since the production helper supports both shapes and GitHub client errors commonly use the nested response form.
  • 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 #2265
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:S; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 563 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 563 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: 65 PR(s), 563 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Triage stale or unlinked PRs.
  • No action.
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
@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.90%. Comparing base (1e456b7) to head (a3f42e7).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2410   +/-   ##
=======================================
  Coverage   95.90%   95.90%           
=======================================
  Files         224      224           
  Lines       25217    25219    +2     
  Branches     9169     9170    +1     
=======================================
+ Hits        24185    24187    +2     
  Misses        419      419           
  Partials      613      613           
Files with missing lines Coverage Δ
src/github/app.ts 98.00% <ø> (ø)
src/services/agent-action-executor.ts 92.79% <100.00%> (+0.13%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ealth-refresh-on-403

# Conflicts:
#	src/services/agent-action-executor.ts
#	test/unit/agent-action-executor.test.ts
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui a3f42e7 Commit Preview URL

Branch Preview URL
Jul 02 2026, 01:01 AM

@JSONbored
JSONbored merged commit 51fef84 into main Jul 2, 2026
13 checks passed
@JSONbored
JSONbored deleted the claude/installation-health-refresh-on-403 branch July 2, 2026 01:10
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 2, 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(queue): the PR-write permission-readiness snapshot can lag a live downgrade by up to 30 minutes

1 participant