feat(review): add durable webhook-invalidated CI-state snapshot cache - #2984
Conversation
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-04 09:28:09 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 1 non-blocking
Concerns raised — review before merging
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 #2984 +/- ##
==========================================
+ Coverage 96.06% 96.08% +0.01%
==========================================
Files 260 260
Lines 28689 28750 +61
Branches 10441 10471 +30
==========================================
+ Hits 27560 27623 +63
+ Misses 493 492 -1
+ Partials 636 635 -1
🚀 New features to boost your workflow:
|
e0a1c5e to
1fa1a66
Compare
check_run/check_suite completions currently force a fresh REST/GraphQL CI read on every readiness check and disposition pass, even when the same (repo, PR, head_sha) was already read moments earlier in the same or a prior job. Extend the existing pull_request_detail_sync_state cache (#2537) with CI-state columns: a fresh row (TTL-capped, invalidated on check_run/check_suite completed) is served without a GitHub call; a miss falls through to a live fetch and write-throughs the result. The act-boundary re-check (refreshLiveCiAggregate) always forces a live read regardless of cache state, preserving its existing always-fresh contract.
…te on legacy CI events Two correctness gaps in the durable CI-state cache: 1. cachedFetchLiveCiAggregate persisted the live aggregate under the normal cache key even when cachedRequiredStatusContexts's lookup had failed (fail-open null required contexts), so a transient branch-protection read error could mask a repo's real required-context state for every reader until TTL expiry. Track whether the lookup actually resolved and skip the durable write-through when it didn't -- the live-fetched aggregate is still used for the current pass's own decision either way. 2. status/workflow_run webhook events weren't wired to invalidate the cache at all (only check_run/check_suite were), so a real legacy CI transition could leave prReadyForReview reading a stale pre-transition aggregate for up to the full TTL. Add a narrower invalidation-only handler for these two event types (re-review triggering stays out of scope, per the existing MVP boundary) that resolves affected PRs via the fast stored-DB head-SHA lookup and invalidates their cache entries. Both fixes verified by temporarily reintroducing the old behavior and confirming the new regression tests fail against it.
1fa1a66 to
69bc6d6
Compare
…exts (#3036) cachedFetchLiveCiAggregate (#2984) keyed its durable pull_request_detail_sync_state row from the raw, unresolved settings.expectedCiContexts config instead of the actually-resolved required-contexts set that cachedRequiredStatusContexts returns (branch protection merged with config, via mergeRequiredCiContexts). If branch protection changes server-side while expectedCiContexts config and head_sha stay the same, the durable key was unchanged, so the readiness path could keep serving a stale aggregate computed against the old required-context set for up to the cache's TTL. Derive the key from the resolved set instead. Also hardens deserializeCachedCiAggregate: JSON.parse succeeding does not guarantee the parsed value is an array, so a corrupted/malformed row's failingDetails/nonRequiredFailingDetails now fail open to a cache miss via an explicit Array.isArray check rather than handing callers a wrong shape.
Summary
PR #2847 ("prioritize live PR work under GitHub rate pressure") explicitly deferred this as needing "a dedicated design pass": every readiness check and disposition pass does a fresh REST/GraphQL CI read (check-runs/status/check-suites), even when the same
(repo, PR, head_sha)was already read moments earlier in the same or a prior job — the VPS evidence that originally motivated the fix#3 work showed ~1,720 check-runs reads and ~1,211 status reads in a 2-hour window.This extends the existing durable
pull_request_detail_sync_statecache (#2537's PR-state cache) with 10 CI-state columns, following the exact same pattern: a fresh row (60s TTL, invalidated on acheck_run/check_suitecompletedwebhook for the matching head SHA) is served without a GitHub call; a miss falls through to a live fetch and write-throughs the result for the next reader.status/workflow_runevents are explicitly not wired to invalidation today (neither is handled for re-review triggering either) — the 60s TTL is the sole backstop for those two event types, documented with a test rather than left silently uncovered.The act-boundary re-check (
refreshLiveCiAggregate, used right before a merge/close decision) always forces a genuinely live read regardless of cache state via aforceRefreshflag threaded through the call chain — mirroring the pre-existingrefreshLiveMergeState"never durable-cached" contract exactly. The two services that call the raw CI-aggregate fetcher directly at the actual act boundary (agent-approval-queue.ts,agent-action-executor.ts) are unaffected — they never import the new cached path.Generic self-host engine behavior only; no repo-specific logic. Follow-up to #2970 (per-installation GitHub-fetch concurrency admission), the other item PR #2847 deferred — independent files, independent risk profile, no dependency between them.
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 actionlint(not run — no workflow files touched)npm run typechecknpm run test:coveragelocally (unsharded); precise diff-coverage cross-reference againstcoverage/coverage-final.jsonconfirms 100% branch coverage on every changed line across all touchedsrc/**filesnpm run test:workers(not run — no Cloudflare-Workers-pool-specific code touched)npm run build:mcp/npm run test:mcp-pack(not run — no MCP package changes)npm run ui:openapi:check/npm run ui:lint/npm run ui:typecheck/npm run ui:build—ui:openapi:checkwas run and passes (no API/OpenAPI surface changed); the others not run — noapps/gittensory-uisource changes, only the generated env-reference file was regeneratednpm audit --audit-level=moderatenpm run selfhost:env-reference:check(regenerated; stale only becauseorigin/mainmoved since PR feat(selfhost): add per-installation GitHub-fetch concurrency admission #2970, unrelated to this PR's own changes)npm run db:migrations:check(new migration0108_pull_request_ci_state_cache.sql, confirmed contiguous, next-free 0109) andnpm run db:schema-drift:check(59 Drizzle tables OK)If any required check was skipped, explain why:
validatejob runs them as a backstop.Safety
UI Evidencesection below with screenshots. (N/A — no visible UI changes.)Notes