fix(selfhost): wire GITHUB_CACHE_TTL_SECONDS or remove the dead config - #2573
Conversation
GITHUB_CACHE_TTL_SECONDS (threaded into createRedisResponseCache as the fallback ttlSeconds) was dead beyond its >0/=0 enable/disable gate: every real call site into GitHubResponseCache.set() (client.ts, graphql-cache.ts) already resolves its own per-class TTL env var (GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS, GITHUB_COMMIT_CACHE_TTL_SECONDS, GITHUB_METADATA_CACHE_TTL_SECONDS, GITHUB_GRAPHQL_CACHE_TTL_SECONDS) and always passes it explicitly, so ttlOverrideSeconds ?? ttlSeconds never fell through to the configured value. An operator setting GITHUB_CACHE_TTL_SECONDS=5 or =120 (per .env.example's own "short default Redis TTL" documentation) got no change in actual cache freshness -- only the gate had any effect. Chose removal over wiring it in as an actual fallback default: the per-class TTLs default to 10-20 minutes, but docker-compose.yml already sets GITHUB_CACHE_TTL_SECONDS=20 unconditionally in every self-host deployment's environment -- wiring it in as a fallback would silently collapse every existing deployment's caching from 10-20 minutes down to 20 seconds on upgrade, a surprising regression for a "fix the docs" change. Make ttlSeconds a required parameter on GitHubResponseCache.set() and createRedisResponseCache() instead of an optional one with a dead fallback, and correct the misleading .env.example/docker-compose.yml/ server.ts documentation to describe the actual enable/disable-gate behavior.
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-02 09:24:35 UTC
✅ Suggested Action - Approve/Merge
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 #2573 +/- ##
==========================================
+ Coverage 95.95% 95.98% +0.02%
==========================================
Files 226 229 +3
Lines 25425 25813 +388
Branches 9244 9390 +146
==========================================
+ Hits 24397 24777 +380
- Misses 417 425 +8
Partials 611 611
🚀 New features to boost your workflow:
|
Summary
GITHUB_CACHE_TTL_SECONDS(threaded intocreateRedisResponseCacheas the fallbackttlSeconds) was dead beyond its>0/=0enable/disable gate: every real call site intoGitHubResponseCache.set()(src/github/client.ts,src/github/graphql-cache.ts) already resolves its own per-class TTL env var (GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS,GITHUB_COMMIT_CACHE_TTL_SECONDS,GITHUB_METADATA_CACHE_TTL_SECONDS,GITHUB_GRAPHQL_CACHE_TTL_SECONDS) and always passes it explicitly, sottlOverrideSeconds ?? ttlSecondsnever fell through to the configured value. An operator settingGITHUB_CACHE_TTL_SECONDS=5or=120(per.env.example's own "short default Redis TTL" documentation) got no change in actual cache freshness — only the gate had any effect. A staleserver.tscomment ("Default 20s") reinforced the false impression.docker-compose.ymlalready setsGITHUB_CACHE_TTL_SECONDS=20unconditionally in every self-host deployment's environment — wiring it in as a fallback would silently collapse every existing deployment's caching from 10–20 minutes down to 20 seconds on upgrade, a surprising regression for what should read as a docs/dead-code fix.ttlSecondsa required parameter onGitHubResponseCache.set()andcreateRedisResponseCache()instead of an optional one with a dead fallback (every real caller already supplies it), and corrected the misleading.env.example/docker-compose.yml/server.tsdocumentation to describe the actual enable/disable-gate behavior.test/unit/selfhost-redis-response-cache.test.tsfor the new required-argument signature (100% coverage retained on the changed file).Closes #2505.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. (redis-response-cache.tsis 100% on the changed lines;client.ts's two changed lines land inclient.ts's existing, unrelated uncovered ranges — unchanged by this diff.)npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateSafety
UI Evidencesection below — N/A, no UI changes.Notes