Skip to content

fix(webhook): subscribe issue_comment.edited/.deleted for cache write-through - #131

Merged
chrisleekr merged 2 commits into
mainfrom
fix/issue-129
May 11, 2026
Merged

fix(webhook): subscribe issue_comment.edited/.deleted for cache write-through#131
chrisleekr merged 2 commits into
mainfrom
fix/issue-129

Conversation

@chrisleekr

@chrisleekr chrisleekr commented May 11, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #129.

src/app.ts:80 subscribed only issue_comment.created, so the chat-thread comment_cache write-through inside handleIssueComment (which handles all three actions: created/edited/deleted) never fired for edits or deletes. After a user edited a comment to clarify or retract content, the cache kept the original body forever; after a deletion, the soft-delete never ran. The chat-thread executor at src/workflows/ship/scoped/chat-thread.ts reads from comment_cache on every turn, so it kept reasoning against pre-edit text the user thought they had changed.

The subscription is now the array form ["issue_comment.created", ".edited", ".deleted"], mirroring pull_request_review_comment.* at src/app.ts:112-121. Dispatch behaviour is unchanged: the existing early-return at src/webhook/events/issue-comment.ts:36 still gates dispatchByIntent / dispatchCommentSurface to created-only, so editing a previously-mentioned comment cannot re-fire the workflow (would be surprising UX and double-bill the user).

Diagram

flowchart LR
  subgraph LIFE["issue_comment lifecycle"]
    direction TB

    GHE["User edits or deletes<br/>issue comment"] --> WHK["Webhook delivery"]

    WHK --> BFR["src/app.ts before<br/>'issue_comment.created' only"]:::bad
    BFR -->|"created"| BIN["handleIssueComment runs"]:::ok
    BFR -->|"edited or deleted"| BDR["delivery dropped<br/>cache stays stale"]:::bad
    BDR --> BST["chat-thread reads<br/>pre-edit body"]:::bad

    WHK --> AFR["src/app.ts after<br/>array subscription"]:::good
    AFR --> AIN["handleIssueComment runs<br/>for all three actions"]:::good
    AIN --> AGT["early-return at line 36<br/>still gates dispatch to created"]:::good
    AIN --> AWR["writeCommentCacheThrough<br/>updates comment_cache"]:::good
    AWR --> AFR2["chat-thread sees<br/>current GitHub state"]:::good
  end

  classDef bad fill:#7a1f1f,color:#ffffff,stroke:#400,stroke-width:1px
  classDef ok fill:#3d4f63,color:#ffffff,stroke:#1e2937,stroke-width:1px
  classDef good fill:#1f5d3a,color:#ffffff,stroke:#063,stroke-width:1px
Loading

Changes

  • src/app.ts — subscription extended from "issue_comment.created" to ["issue_comment.created", ".edited", ".deleted"].
  • src/webhook/events/issue-comment.ts — exported writeCommentCacheThrough for testability; added a terse WHY comment above the dispatch early-return to lock in the gate ordering.
  • test/webhook/events/issue-comment-cache.test.ts (new) — 8 tests across three suites:
    1. DB-backed behaviour for created / edited / deleted payloads asserting loadConversation reflects the post-edit body and the soft-delete hides the row.
    2. Source-level subscription invariant: scans src/app.ts and asserts every issue_comment action is present, with the regression shape app.webhooks.on("issue_comment.created", ...) explicitly forbidden.
    3. Source-level dispatch-gate ordering guard: asserts the early-return appears before any dispatch call site, so a future refactor that moves it below dispatch fails loudly.

Related Issues

Test plan

  • bun run typecheck clean
  • bun run lint clean on changed files (one pre-existing warning on issue-comment.ts:158 is from commit 698694b, not introduced here)
  • bun run format clean
  • bun test test/webhook/events/issue-comment-cache.test.ts — 8/8 pass
  • Full bun run test — 121/121 files pass with TEST_DATABASE_URL set
  • Verified regression-shape detection: temporarily reverting src/app.ts to the single-action form trips the subscription invariant tests

Summary by CodeRabbit

  • New Features

    • GitHub webhook now processes edited and deleted issue comments in addition to newly created ones, enabling more complete comment synchronization.
  • Tests

    • Added regression tests verifying issue comment cache behavior across creation, editing, and deletion events.

Review Change Stack

…-through

Closes #129.

`src/app.ts:80` only subscribed `issue_comment.created`, so the cache
write-through inside `handleIssueComment` (which handles all three
actions) never fired for edits or deletes. `comment_cache` rows kept
the original body after an edit, and soft-delete on `deleted` never
ran. Chat-thread (`src/workflows/ship/scoped/chat-thread.ts`) reads
those rows on every turn, so it kept reasoning against pre-edit text
the user thought they had retracted or rewritten.

The subscription is now the array form
`["issue_comment.created", ".edited", ".deleted"]`, mirroring the
review-comment block at `src/app.ts:112-121`. The dispatch path is
unchanged: the existing early-return at
`src/webhook/events/issue-comment.ts:36` still gates
`dispatchByIntent` / `dispatchCommentSurface` to created-only, so
editing a previously-mentioned comment cannot re-fire the workflow.

`writeCommentCacheThrough` is now exported so the new test can drive
it directly. Tests cover all three actions plus two source-level
invariants (subscription shape and dispatch-gate ordering) that fail
loudly if a future refactor reintroduces either side of the bug.

Follow-up #130 audits the analogous gap on `issues.*` and
`pull_request_review.*` subscriptions.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 08:55
@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@chrisleekr has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 15 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 34e90d78-de38-435b-aac1-b287ad75a6c0

📥 Commits

Reviewing files that changed from the base of the PR and between 30e6c94 and 51a37e6.

📒 Files selected for processing (1)
  • test/webhook/events/issue-comment-cache.test.ts
📝 Walkthrough

Walkthrough

This PR extends GitHub webhook subscriptions for issue comment events to handle edited and deleted actions alongside created, enabling comment cache write-through to fire for all three actions. The handler's dispatch path remains gated to created only. A new regression test suite validates cache insertion, update, soft-deletion, and source-level invariants.

Changes

Issue Comment Webhook Multi-Action Support

Layer / File(s) Summary
Webhook Subscription
src/app.ts
Expand webhook listener from issue_comment.created to ["issue_comment.created", "issue_comment.edited", "issue_comment.deleted"] array form, matching the review-comment subscription pattern.
Handler Gating & Export
src/webhook/events/issue-comment.ts
Add inline documentation that writeCommentCacheThrough runs before the created-only dispatch gate; export writeCommentCacheThrough function to enable test calls.
Test Infrastructure & Fixtures
test/webhook/events/issue-comment-cache.test.ts
Introduce DB reachability detection, skip helpers, cleanup routines, and basePayload factory for constructing IssueCommentEvent objects for all three actions.
Cache Behavior Tests
test/webhook/events/issue-comment-cache.test.ts
Verify writeCommentCacheThrough inserts comment on created, updates body on edited, and soft-deletes on deleted, validating each transition via loadConversation.
Source Invariant Tests
test/webhook/events/issue-comment-cache.test.ts
Assert app.ts registers all three issue_comment actions in array form; assert handleIssueComment gates dispatch to created only before any dispatch calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: extending webhook subscriptions to handle edited and deleted issue comments for cache write-through.
Linked Issues check ✅ Passed The PR comprehensively addresses all coding requirements from #129: extends webhook subscription to include edited/deleted actions, exports writeCommentCacheThrough, preserves created-only dispatch gating, and adds extensive test coverage for all three actions.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #129: webhook subscription extension, handler export, and regression tests. No unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread test/webhook/events/issue-comment-cache.test.ts Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a webhook subscription gap where issue_comment.edited and issue_comment.deleted deliveries were never handled, preventing the chat-thread comment_cache write-through from reflecting edits and deletions (and causing the chat-thread executor to reason against stale comment bodies).

Changes:

  • Expand issue_comment webhook registration to subscribe to created, edited, and deleted actions (matching the existing review-comment pattern).
  • Keep workflow dispatch behavior unchanged by preserving the created-only dispatch gate inside handleIssueComment, while still running cache write-through for all actions.
  • Add regression tests covering DB-backed cache behavior plus source-level invariants guarding subscription shape and dispatch-gate ordering.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/app.ts Subscribes to issue_comment.{created,edited,deleted} using the multi-action array form so cache write-through fires for edits/deletes.
src/webhook/events/issue-comment.ts Exports writeCommentCacheThrough (testability) and documents why dispatch remains created-only while cache write-through runs first.
test/webhook/events/issue-comment-cache.test.ts Adds regression coverage for cache insert/update/soft-delete and guards against subscription/ordering regressions via source checks.

CodeQL flagged the `new RegExp(`['"]${action.replace(/\./g, "\\.")}['"]`)`
construction as "incomplete string escaping or encoding" because it does
not escape backslashes in the input. The action names in the it.each
table are hardcoded literals with no backslashes, so the alert is
defensive only, but a substring check is simpler and removes the entire
class of regex-escape footguns.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chrisleekr
chrisleekr merged commit c84361d into main May 11, 2026
22 checks passed
@chrisleekr
chrisleekr deleted the fix/issue-129 branch May 11, 2026 09:17
chrisleekr pushed a commit that referenced this pull request May 21, 2026
# [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))
@chrisleekr

Copy link
Copy Markdown
Owner Author

🎉 This PR is included in version 1.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(webhook): subscribe issue_comment.edited and .deleted so chat-thread cache write-through actually fires

3 participants