fix(webhook): write-through target_cache on issues/pull_request events (#130) - #132
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extends webhook handling to keep target_cache fresh for issue/PR edits by writing through from issues.* and pull_request.* payloads, and adds regression tests to prevent future subscription/ordering regressions.
Changes:
- Subscribes to additional
issues.*andpull_request.*webhook actions to ensure cache write-through runs on all relevant mutations. - Adds write-through helpers (
writeIssueTargetCacheThrough,writePrTargetCacheThrough) and a transactional hard-delete helper (deleteTarget) forissues.deleted. - Hardens
upsertTargetwith monotonic update gating to prevent out-of-order webhook retries from clobbering newer cached fields, with new regression tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app.ts | Expands webhook subscriptions for issues/PR actions so cache write-through receives mutation events. |
| src/webhook/events/issues.ts | Adds issue target_cache write-through plus delete behavior for issues.deleted. |
| src/webhook/events/pull-request.ts | Adds PR target_cache write-through and runs it before action dispatch gating. |
| src/db/queries/conversation-store.ts | Adds monotonic field gating in upsertTarget and transactional deleteTarget. |
| test/webhook/events/issues-cache.test.ts | New regression tests for issue write-through behavior, subscription invariants, and ordering. |
| test/webhook/events/pull-request-cache.test.ts | New regression tests for PR write-through behavior, subscription invariants, and ordering. |
| ])("subscribes to %s", (action) => { | ||
| // Substring (not RegExp), same reasoning as `issue-comment-cache.test.ts`: | ||
| // dynamic-regex builds get flagged by CodeQL as escape footguns. | ||
| const hasDoubleQuoted = appSrc.includes(`"${action}"`); | ||
| const hasSingleQuoted = appSrc.includes(`'${action}'`); | ||
| expect(hasDoubleQuoted || hasSingleQuoted, `${action} not found as a quoted string`).toBe(true); | ||
| }); |
| ])("subscribes to %s", (action) => { | ||
| const hasDoubleQuoted = appSrc.includes(`"${action}"`); | ||
| const hasSingleQuoted = appSrc.includes(`'${action}'`); | ||
| expect(hasDoubleQuoted || hasSingleQuoted, `${action} not found as a quoted string`).toBe(true); | ||
| }); |
| const cacheCall = handlerSrc.indexOf("writeIssueTargetCacheThrough(payload)"); | ||
| expect(cacheCall, "cache write-through call missing").toBeGreaterThan(-1); | ||
|
|
||
| const unlabeledGuard = handlerSrc.indexOf(`payload.action === "unlabeled"`); | ||
| expect(unlabeledGuard, "unlabeled guard missing").toBeGreaterThan(-1); | ||
| expect( | ||
| cacheCall, | ||
| "cache write-through must appear BEFORE the unlabeled early-return", | ||
| ).toBeLessThan(unlabeledGuard); |
| it("calls writePrTargetCacheThrough BEFORE the first action branch", () => { | ||
| const cacheCall = handlerSrc.indexOf("writePrTargetCacheThrough(payload)"); | ||
| expect(cacheCall, "cache write-through call missing").toBeGreaterThan(-1); | ||
|
|
||
| const firstActionBranch = handlerSrc.indexOf(`payload.action === "labeled"`); | ||
| expect(firstActionBranch, "labeled action branch missing").toBeGreaterThan(-1); | ||
| expect( | ||
| cacheCall, | ||
| "cache write-through must appear BEFORE the first action-branch", | ||
| ).toBeLessThan(firstActionBranch); | ||
| }); |
# [1.13.0](v1.12.2...v1.13.0) (2026-05-21) ### Bug Fixes * **deps:** update dependency @anthropic-ai/bedrock-sdk to ^0.29.0 ([#147](#147)) ([eb95c64](eb95c64)) * **deps:** update dependency @anthropic-ai/claude-agent-sdk to ^0.3.0 ([#154](#154)) ([15add8e](15add8e)) * **docs:** anchor-verify src citations to catch silent line-shift rot ([#163](#163)) ([5a67863](5a67863)) * **webhook:** subscribe issue_comment.edited/.deleted for cache write-through ([#131](#131)) ([c84361d](c84361d)) * **webhook:** write-through target_cache on issues/pull_request events ([#130](#130)) ([#132](#132)) ([8b79c10](8b79c10)) ### Features * **prompt:** opt-in cacheable system/user prompt split ([#135](#135)) ([bb80ca7](bb80ca7)) * **review-learnings:** explicit [@bot](https://github.com/bot) remember + autonomous capture ([#160](#160)) ([#162](#162)) ([1c4c53a](1c4c53a)) * **review-learnings:** persistent per-repo review-policy directives ([#161](#161)) ([ba50972](ba50972)) * **scheduler:** scheduled actions via .github-app.yaml ([#159](#159)) ([142a5bc](142a5bc)) * **workflows:** comment-aware structured workflows via LLM discussion digest ([#148](#148)) ([7a6b315](7a6b315))
|
🎉 This PR is included in version 1.13.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Closes #130. Extends the cache write-through pattern shipped in #131 (which fixed
comment_cacheforissue_comment.edited/.deleted) to thetarget_cachetable. Before this PR,target_cachewas only written frombackfillFromGitHub, so any edit to an issue/PR title, body, state, or draft flag stayed stale until the next cold-miss backfill. The chat-thread executor could reason against pre-edit text the user thought they had changed.pull_request_review.edited/.dismissedwere called out in #130 but are deliberately not addressed here: review bodies are not cached anywhere today (comment_cache.surfaceonly permits'issue-comment'and'review-comment', andbackfillFromGitHubdoes not fetch top-level reviews), so there is no destination to write through to. Adding review-body caching requires a schema migration and is out of scope.Diagram
flowchart LR subgraph FLW["before vs after, target_cache freshness"] direction TB EdtBefore["user edits<br/>issue/PR body"]:::user WhBefore["webhook fires<br/>issues.edited or<br/>pull_request.edited"]:::wh DropBefore["NOT subscribed,<br/>event dropped"]:::stale CacheBefore["target_cache stale<br/>chat-thread reasons<br/>against old body"]:::stale EdtAfter["user edits<br/>issue/PR body"]:::user WhAfter["webhook fires<br/>issues.edited or<br/>pull_request.edited"]:::wh WriteAfter["writeIssueTargetCacheThrough or<br/>writePrTargetCacheThrough,<br/>monotonic upsert"]:::write CacheAfter["target_cache fresh<br/>chat-thread sees<br/>post-edit body"]:::fresh EdtBefore --> WhBefore --> DropBefore --> CacheBefore EdtAfter --> WhAfter --> WriteAfter --> CacheAfter end classDef user fill:#ecf0f1,color:#2c3e50,stroke:#2c3e50 classDef wh fill:#3498db,color:#ffffff,stroke:#1f618d classDef stale fill:#922b21,color:#ffffff,stroke:#641e16 classDef write fill:#1e8449,color:#ffffff,stroke:#0e6b3a classDef fresh fill:#27ae60,color:#ffffff,stroke:#196f3dChanges
src/app.ts: expandissuessubscription toopened/edited/closed/reopened/deleted/labeled/unlabeled; expandpull_requestsubscription toopened/edited/labeled/synchronize/closed/reopened/converted_to_draft/ready_for_review. Dispatch gates inside each handler are unchanged.src/webhook/events/issues.ts: addwriteIssueTargetCacheThrough(payload)called before any dispatch gate.deletedhard-deletes the row plus itscomment_cachechildren; all other actions upsert title/body/state frompayload.issue.src/webhook/events/pull-request.ts: addwritePrTargetCacheThrough(payload)called before any dispatch gate. Always upserts frompayload.pull_requestand collapsesmerged: truetostate = "merged"(matchesbackfillFromGitHub).src/db/queries/conversation-store.ts: adddeleteTargethelper that hard-deletescomment_cache+target_cacherows in a singledb.begintransaction so concurrentloadConversationcannot observe a partial view. HardenupsertTargetwith monotonicity gating: every mutable field (title,body,state,is_draft,base_ref,head_ref) now usesCASE WHEN EXCLUDED.updated_at >= target_cache.updated_at THEN EXCLUDED.x ELSE target_cache.x ENDso an out-of-order webhook retry cannot clobber a newer body.test/webhook/events/issues-cache.test.ts(new): behaviour coverage foropened/edited/closed/reopened/deleted, monotonicity regression (out-of-order edit), subscription invariant, and ordering invariant.test/webhook/events/pull-request-cache.test.ts(new): behaviour coverage foropened/edited,merged → state="merged"collapse,is_drafttoggle viaconverted_to_draft/ready_for_review, subscription invariant, and ordering invariant.Related Issues
Test plan
issues-cache.test.ts,pull-request-cache.test.tstest/webhook/events/,test/db/queries/) green in isolated runs; full-suite flakes are pre-existingbun run typecheckcleanbun run lint0 errorsbun run formatclean