Skip to content

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

Closed
andriypolanski wants to merge 5 commits into
JSONbored:mainfrom
andriypolanski:fix/stale-retention-salvageability-tests
Closed

fix(github): self-heal stale installation tokens on collaborator/user reads (#9315)#9378
andriypolanski wants to merge 5 commits into
JSONbored:mainfrom
andriypolanski:fix/stale-retention-salvageability-tests

Conversation

@andriypolanski

@andriypolanski andriypolanski commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

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.

JSONbored#9085

Update the Postgres retention mock for PK-ordered deletes and flip the
confidence-less salvageability assertion to match CONFIDENCE_WHEN_UNSTATED.

Co-authored-by: Cursor <cursoragent@cursor.com>
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-27 16:10:40 UTC

3 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This PR wraps `getRepositoryCollaboratorPermission` and `getGithubUserCreatedAt` in `withInstallationTokenRetry`, matching the existing convention used by other GitHub helpers (`createOrUpdateNamedCheckRun`, `createInstallationToken` retry callers per #6191/#8892). The collaborator-permission fix attaches `.status` to the thrown error so `isGitHubBadCredentialsError`/`isGitHubInstallationPermissionError` (which check `githubErrorStatus`) can detect a 401/403 even with an empty/non-matching body — this is a real, reachable fix since the prior code threw a bare `Error` with no status field. The `getGithubUserCreatedAt` change correctly narrows the fail-open behavior to only non-auth failures, explicitly re-throwing on 401/403 so the retry wrapper gets a chance to re-mint before the outer try/catch fails open to null. Tests cover the 401→re-mint→success path for both functions plus persistent-401 and non-retryable-403 fail-open cases, and the diff traces cleanly against the shown `withInstallationTokenRetry` implementation.

Nits — 3 non-blocking
  • src/github/app.ts:499/546 — the 401/403 status checks are magic numbers scattered across the file; consider extracting shared HTTP status constants if this pattern keeps growing.
  • The unrelated `test/unit/selfhost-pg-retention.test.ts` comment-only change (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 reference) is out of scope for this PR's stated intent and should probably be a separate commit/PR.
  • Consider a small shared helper like `throwWithStatus(message, status)` since the same `Object.assign(new Error(...), { status })` pattern is now duplicated across both functions in this file.

Why this is blocked

📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue does not appear to be satisfied: AI assessment: this PR does not appear to satisfy its linked issue's scope. This PR is explicitly test-only housekeeping for already-merged \#9083/\#9085 behavior and touches only two unrelated test files, with the PR itself noting it is 'kept separate from \#9315 \(installation-token retry\)'; it does not touch src/github/app.ts, wrap getRepositoryCollaboratorPermission or getGithubUserCreatedAt in withInstallationTokenRetry, or add the required regression tests in test/unit/ — Confirm this PR actually addresses the linked issue's scope, or link the correct issue.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard 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
Linked issue satisfaction

Not yet addressed
This PR is explicitly test-only housekeeping for already-merged #9083/#9085 behavior and touches only two unrelated test files, with the PR itself noting it is 'kept separate from #9315 (installation-token retry)'; it does not touch src/github/app.ts, wrap getRepositoryCollaboratorPermission or getGithubUserCreatedAt in withInstallationTokenRetry, or add the required regression tests in test/unit/

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
  • Keep the PR focused and include validation evidence before maintainer review.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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.

Decision record
  • action: hold · clause: linked_issue_scope_mismatch
  • config: 03a7f8b529a91e8b1d96f173ed0ef78b45024abd943f40fe00690bf780df9757 · pack: oss-anti-slop
  • record: d303c3a27e339c80ae4147045670d3cd0754b193015d32b9dfadbc8bbe6c43c5 (schema v3, head 66fc88a)

🟩 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 (c658557) to head (66fc88a).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9378      +/-   ##
==========================================
+ Coverage   75.38%   75.49%   +0.11%     
==========================================
  Files         275      276       +1     
  Lines       58023    58322     +299     
  Branches     6181     6293     +112     
==========================================
+ Hits        43739    44032     +293     
- Misses      14014    14015       +1     
- Partials      270      275       +5     
Flag Coverage Δ
backend 97.99% <100.00%> (?)

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%> (ø)

@andriypolanski andriypolanski changed the title test: align retention and salvageability suites with #9083 and #9085 fix(github): self-heal stale installation tokens on collaborator/user reads (#9315) Jul 27, 2026
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 27, 2026
@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Linked issue does not appear to be satisfied). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 27, 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. manual-review Gittensor contributor context

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

3 participants