fix(github): cache the App JWT so App-level reads cache-hit (#1940) - #1981
Conversation
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-01 04:56:30 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 5 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1981 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 218 218
Lines 24306 24315 +9
Branches 8817 8818 +1
=======================================
+ Hits 23240 23249 +9
Misses 436 436
Partials 630 630
🚀 New features to boost your workflow:
|
9813ba7 to
792ac45
Compare
|
Fixed the blocker — the reviewers were right, thanks. |
createAppJwt re-signed a fresh App JWT on every call. Beyond the wasted RS256
CPU, the rotating JWT changes the auth-scoped response-cache key every call, so
the metadata cache class never hits for App-level reads (/app/installations/{id})
when two reads land more than a second apart — the heavy path used by the
per-repo backfill / refresh-installation-health.
Cache the minted JWT for a margin of its ~9-min validity (8 min). A Map keyed by
App id (so a process alternating between App identities keeps a JWT per App
instead of evicting one for another) with the private key held in the entry, so a
same-App CREDENTIAL ROTATION invalidates immediately and never serves a JWT
signed by the now-revoked old key. Cleared in clearInstallationTokenCacheForTest.
792ac45 to
94155a1
Compare
|
Also folded in the perf nit: the JWT cache is now a Map keyed by App id, so a process that alternates between App identities keeps a per-App JWT instead of thrashing a single entry (added a test asserting alternating |
Summary
createAppJwtminted a fresh App JWT (RS256 sign) on every call. Beyond the wasted CPU, the rotating JWT changes the auth-scoped response-cache key on every call (responseCacheKeyhashes theauthorizationheader), so themetadatacache class never hits for App-level reads (GET /app/installations/{id}) whenever two reads land more than a second apart — the exact heavy path used by the per-repo backfill /refresh-installation-health.This caches the minted JWT for a margin of its ~9-minute validity (
APP_JWT_REUSE_MS = 8 min), so repeated App-JWT reads share one signature and one stable cache key — the metadata cache class becomes effective, and the RS256 mint runs once per window instead of per call. Keyed by App id so a different App (or a test that swaps the private key) never reuses another's token, and cleared inclearInstallationTokenCacheForTest.Advances #1940 (rec #8 from the #1936 audit).
Scope
CONTRIBUTING.md; nosite//CNAME.Validation
git diff --checknpm run typechecknpm run test:coverage— added a regression test that advances fake time between twogetAppInstallationreads and asserts the second is served from the response cache (fetches === 1) instead of re-fetching (it fails without the JWT cache, since a rotated JWT would miss). The test also covers the re-mint-after-window and different-App branches; the changed region is 100% covered (statements + branches).npm run test:cinpm audit --audit-level=moderateIf any required check was skipped, explain why:
ui:openapi/cf-typegen/ migration regen: single-function backend change, no API/schema/binding/DB change.Safety
responseCacheKeyis unchanged); this only makes the App JWT itself stable within its validity window, so no cross-tenant cache widening.Notes