Skip to content

fix(github): self-heal stale installation tokens on collaborator/user reads - #9401

Merged
loopover-orb[bot] merged 5 commits into
JSONbored:mainfrom
andriypolanski:fix/stale-retention-salvageability-tests-v2
Jul 27, 2026
Merged

fix(github): self-heal stale installation tokens on collaborator/user reads#9401
loopover-orb[bot] merged 5 commits into
JSONbored:mainfrom
andriypolanski:fix/stale-retention-salvageability-tests-v2

Conversation

@andriypolanski

@andriypolanski andriypolanski commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Closes #9315

Summary

  • Preserves existing contracts: 404 → null and non-token failures still throw (collaborator) or fail-open to null (user created_at).
  • Adds regression tests for 401 → re-mint → success on both reads, plus fail-open cases when retry cannot heal.

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 a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npx vitest run test/unit/github-app.test.ts -t "9315|collaborator permission|getGithubUserCreatedAt" — 8 passed
  • Full test/unit/github-app.test.ts — 103+ passed (incl. new cases)
  • npm run test:ci (full gate; run before push)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

Implementation + targeted regression tests complete. Full test:ci left for the opener.

Safety

  • No secrets, wallets, hotkeys, trust scores, or reward values.
  • Does not touch site/, CNAME, **/lovable/**, or root CHANGELOG.md.

UI Evidence

N/A — GitHub App token-retry plumbing only; no visible UI change.

Notes for reviewers / gate

  • Non-OK collaborator responses now attach status on the thrown Error so isGitHubBadCredentialsError recognizes empty-body 401s.
  • getGithubUserCreatedAt throws on 401/403 inside the retry wrapper (so a stale token can self-heal) and still fail-opens via the outer catch for anything that is not healed.

@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-27 16:28:13 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR routes getRepositoryCollaboratorPermission and getGithubUserCreatedAt through the existing withInstallationTokenRetry wrapper so a stale cached installation token self-heals on 401/403, matching the convention already used by createOrUpdateNamedCheckRun in the same file. The implementation is correct: both call sites now attach `status` on thrown errors so isGitHubBadCredentialsError/isGitHubInstallationPermissionError can classify them, the 404→null and non-token-failure contracts are preserved, and the new tests genuinely exercise the retry path with a real token-rejection-then-success fetch stub rather than fabricating an unreachable state. The included selfhost-pg-retention.test.ts comment-only change is unrelated scope but trivial (no code change, just doc clarification).

Nits — 5 non-blocking
  • src/github/app.ts:546 — getGithubUserCreatedAt now throws on 401/403 inside the retry wrapper but only checks `response.status === 401 || 403` directly rather than reusing isGitHubBadCredentialsError's bad-credentials-message detection path, so an empty-body 401 with no message still relies solely on status matching (which is fine here since status is set, but worth confirming this is intentionally narrower than the collaborator permission path's real-response 4xx handling).
  • The `orb(db): 3.4 GB in one month — no retention target has a usable index, and four caches have no delete path at all #9083` comment change in test/unit/selfhost-pg-retention.test.ts is unrelated to this PR's stated scope (installation-token retry) and would be cleaner in its own commit/PR.
  • Consider extracting the repeated `Object.assign(new Error(...), { status: response.status })` pattern (now duplicated at src/github/app.ts:499-ish and 546-ish) into a small helper to avoid drift between the two call sites.
  • src/github/app.ts: factor the `throw Object.assign(new Error(...), { status })` idiom into a shared helper (e.g. `throwGitHubStatusError`) used by both getRepositoryCollaboratorPermission and getGithubUserCreatedAt to keep the two retry-wrapped fetches in sync as this pattern likely spreads to more call sites.
  • Verify the unrelated selfhost-pg-retention.test.ts comment tweak isn't accidentally bundled from a different local branch — it doesn't affect behavior but is scope noise in an otherwise tightly-focused PR.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9315
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: 178 registered-repo PR(s), 115 merged, 22 issue(s).
Contributor context ✅ Confirmed Gittensor contributor andriypolanski; Gittensor profile; 178 PR(s), 22 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
Both getRepositoryCollaboratorPermission and getGithubUserCreatedAt now route their fetch calls through withInstallationTokenRetry, preserving the 404->null and non-token-failure/fail-open contracts, matching the pattern requested in the issue.

Review context
  • Author: andriypolanski
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, Rust, Cuda, JavaScript, Kotlin, MDX, Scala
  • Official Gittensor activity: 178 PR(s), 22 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.49%. Comparing base (995cd78) to head (569dcff).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9401       +/-   ##
===========================================
- Coverage   89.47%   75.49%   -13.98%     
===========================================
  Files         837      276      -561     
  Lines      109639    58322    -51317     
  Branches    26109     6293    -19816     
===========================================
- Hits        98097    44032    -54065     
- Misses      10279    14015     +3736     
+ Partials     1263      275      -988     
Flag Coverage Δ
backend 97.99% <100.00%> (+2.76%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/github/app.ts 97.99% <100.00%> (+0.02%) ⬆️

... and 699 files with indirect coverage changes

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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 2eb5d74 into JSONbored:main Jul 27, 2026
8 checks passed
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

None yet

Development

Successfully merging this pull request may close these issues.

fix(github): getRepositoryCollaboratorPermission/getGithubUserCreatedAt skip the installation-token self-heal every sibling call uses

2 participants