Skip to content

fix(review): preserve cached AI gate findings#1492

Merged
JSONbored merged 7 commits into
mainfrom
codex/propose-fix-for-ai-review-cache-vulnerability
Jun 27, 2026
Merged

fix(review): preserve cached AI gate findings#1492
JSONbored merged 7 commits into
mainfrom
codex/propose-fix-for-ai-review-cache-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The AI-review cache previously stored only public notes and reviewerCount, which allowed cached hits to skip re-applying gate-relevant findings (ai_consensus_defect, ai_review_split, ai_review_inconclusive) and could let block-mode PRs pass incorrectly.
  • The goal is to ensure cached reviews retain and replay the gate verdict so deterministic gate evaluation remains correct on cache hits.

Description

  • Persist findings with cached reviews by adding a findings_json column and wiring it into putCachedAiReview / getCachedAiReview in src/db/repositories.ts and a new migration migrations/0075_ai_review_cache_findings.sql.
  • Collect AI verdict findings locally in runAiReviewForAdvisory and include them in the returned cache payload (notes, reviewerCount, findings).
  • On a cache hit, replay cached findings into advisory.findings before the deterministic gate evaluation so cached blockers are not lost (src/queue/processors.ts).
  • Add/adjust regression tests to cover cache persistence of findings and the block-mode re-gate sweep behavior (test/unit/ai-review-cache.test.ts, test/unit/queue.test.ts).

Testing

  • Ran typecheck with npm run typecheck and it succeeded.
  • Verified migrations with npm run db:migrations:check and the new migration is contiguous and accepted.
  • Ran the targeted unit tests with vitest (test/unit/ai-review-cache.test.ts and the re-gate-related tests in test/unit/queue.test.ts) and they passed.

Codex Task

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 26, 2026
@superagent-security

Copy link
Copy Markdown

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

@JSONbored JSONbored self-assigned this Jun 26, 2026
@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix - worth 0.5x multiplier. label Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.44%. Comparing base (e81c5bc) to head (0abbdee).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1492      +/-   ##
==========================================
+ Coverage   95.43%   95.44%   +0.01%     
==========================================
  Files         202      202              
  Lines       21751    21754       +3     
  Branches     7860     7861       +1     
==========================================
+ Hits        20757    20764       +7     
+ Misses        416      414       -2     
+ Partials      578      576       -2     
Files with missing lines Coverage Δ
src/db/repositories.ts 96.13% <100.00%> (+<0.01%) ⬆️
src/queue/processors.ts 87.98% <100.00%> (+0.28%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored force-pushed the codex/propose-fix-for-ai-review-cache-vulnerability branch from 6ab074f to 318b459 Compare June 26, 2026 21:01
@gittensory-orb

gittensory-orb Bot commented Jun 27, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review — held for maintainer review

7 files · 1 AI reviewers · no blockers · readiness 48/100 · CI green · unknown

⏸️ Held for maintainer review

Review summary
The PR correctly addresses a security-relevant cache bug where block-mode gate findings (ai_consensus_defect, ai_review_split, ai_review_inconclusive) were not persisted, allowing cached hits to skip replaying blockers and letting block-mode PRs pass incorrectly. The local-accumulator pattern in runAiReviewForAdvisory is a clean refactor: findings are collected, bulk-pushed to advisory.findings (preserving the original side-effect), and returned for cache storage. The migration is backward-compatible (NOT NULL DEFAULT '[]'), and replay is correctly sequenced before gate evaluation in maybePublishPrPublicSurface.

Nits (5)

  • The putCachedAiReview call site inside maybePublishPrPublicSurface is not in the diff (file omitted as too large); verify it passes aiReview.findings rather than a narrower destructured object — findings is optional in the signature so TypeScript will not catch an accidental omission, and the queue test pre-seeds the cache directly rather than exercising this write path.
  • The ai_review_inconclusive branch has no new test coverage; only ai_review_split (ai-review-advisory.test.ts) and ai_consensus_defect (queue.test.ts) are exercised by the new cases.
  • The forced rename of 0076_orb_self_enrollment_disabled.sql to make room for the new 0075 slot is not mentioned in the PR description, which may confuse reviewers auditing migration sequence history.
  • Add an end-to-end test in queue.test.ts that lets runAiReviewForAdvisory execute with a mocked split verdict and then asserts the subsequent cache hit replays the finding — this closes the gap where the current test pre-seeds the cache directly and never validates the write path through the real processor.
  • Add a brief comment near advisory.findings.push(...cachedReview.findings) in processors.ts noting that this replay must precede gate evaluation; the ordering is load-bearing but currently implicit.
Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:M.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 26 open PR(s), 9 likely reviewable, 17 unlinked.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 81 PR(s), 297 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 2 non-blocking
  • Repository config was not parsed
  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 81 PR(s), 297 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (PR #1554)
  • Related work: Titles/paths share 7 meaningful terms. (PR #1537, PR #1546)
  • Related work: Titles/paths share 6 meaningful terms. (PR #1537, PR #1545)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The PR correctly addresses a security-relevant cache bug where block-mode gate findings (ai_consensus_defect, ai_review_split, ai_review_inconclusive) were not persisted, allowing cached hits to skip replaying blockers and letting block-mode PRs pass incorrectly. The local-accumulator pattern in runAiReviewForAdvisory is a clean refactor: findings are collected, bulk-pushed to advisory.findings (preserving the original side-effect), and returned for cache storage. The migration is backward-compatible (NOT NULL DEFAULT '[]'), and replay is correctly sequenced before gate evaluation in maybePublishPrPublicSurface.

Nits (5)

  • The putCachedAiReview call site inside maybePublishPrPublicSurface is not in the diff (file omitted as too large); verify it passes aiReview.findings rather than a narrower destructured object — findings is optional in the signature so TypeScript will not catch an accidental omission, and the queue test pre-seeds the cache directly rather than exercising this write path.
  • The ai_review_inconclusive branch has no new test coverage; only ai_review_split (ai-review-advisory.test.ts) and ai_consensus_defect (queue.test.ts) are exercised by the new cases.
  • The forced rename of 0076_orb_self_enrollment_disabled.sql to make room for the new 0075 slot is not mentioned in the PR description, which may confuse reviewers auditing migration sequence history.
  • Add an end-to-end test in queue.test.ts that lets runAiReviewForAdvisory execute with a mocked split verdict and then asserts the subsequent cache hit replays the finding — this closes the gap where the current test pre-seeds the cache directly and never validates the write path through the real processor.
  • Add a brief comment near advisory.findings.push(...cachedReview.findings) in processors.ts noting that this replay must precede gate evaluation; the ordering is load-bearing but currently implicit.

🟩 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.

  • Re-run Gittensory review

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 27, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 0abbdee Commit Preview URL

Branch Preview URL
Jun 27 2026, 01:25 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix - worth 0.5x multiplier. gittensor Gittensor contributor context size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant