feat: add PR/issue anchor dynamic updates #88
Conversation
WalkthroughThis PR centralizes issue event metadata and standardizes event handling. It adds getIssueEventEmoji and getIssueEventHeader to src/formatters/shared.ts, refactors src/formatters/events-api.ts and src/formatters/webhook-events.ts to use those helpers, and makes isThreadReply a required boolean parameter for three formatter functions. EventProcessor.processEvent signature is changed to accept a direct branch string (renamed from branchContext) and a new private handleAnchorEvent consolidates PR/issue anchor handling. Message delivery now passes explicit entityContext fields instead of spreading. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/services/message-delivery-service.ts (1)
215-219: Minor redundancy: reusing already-destructured variables.
githubEntityTypeandgithubEntityIdare already destructured at line 174. You could simplify by reusing those local variables for the first two fields. However, the explicit field listing pattern is consistent withhandleCreate(lines 247-251), so this is acceptable for uniformity.await this.storeMapping({ spaceId, channelId, repoFullName, - githubEntityType: entityContext.githubEntityType, - githubEntityId: entityContext.githubEntityId, + githubEntityType, + githubEntityId, parentType: entityContext.parentType, parentNumber: entityContext.parentNumber, - githubUpdatedAt: entityContext.githubUpdatedAt, + githubUpdatedAt, townsMessageId: existingMessageId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/formatters/events-api.ts(2 hunks)src/formatters/shared.ts(2 hunks)src/formatters/webhook-events.ts(6 hunks)src/github-app/event-processor.ts(9 hunks)src/services/message-delivery-service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/formatters/shared.tssrc/services/message-delivery-service.tssrc/formatters/webhook-events.tssrc/formatters/events-api.tssrc/github-app/event-processor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/formatters/shared.tssrc/services/message-delivery-service.tssrc/formatters/webhook-events.tssrc/formatters/events-api.tssrc/github-app/event-processor.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.463Z
Learning: All event handlers receive a base payload including userId, spaceId, channelId, eventId, and createdAt - use eventId as threadId/replyId when responding to maintain event threading
🧬 Code graph analysis (3)
src/formatters/webhook-events.ts (1)
src/formatters/shared.ts (2)
getIssueEventEmoji(79-84)getIssueEventHeader(89-94)
src/formatters/events-api.ts (1)
src/formatters/shared.ts (2)
getIssueEventEmoji(79-84)getIssueEventHeader(89-94)
src/github-app/event-processor.ts (4)
src/services/message-delivery-service.ts (1)
EntityContext(21-28)src/constants.ts (2)
BRANCH_FILTERABLE_EVENTS_SET(56-58)EventType(34-34)src/types/webhooks.ts (2)
PullRequestPayload(14-14)IssuesPayload(15-15)src/formatters/webhook-events.ts (2)
formatPullRequest(49-72)formatIssue(74-91)
🔇 Additional comments (11)
src/formatters/shared.ts (2)
76-94: LGTM! Good extraction of issue event helpers.The new
getIssueEventEmojiandgetIssueEventHeaderfunctions follow the same pattern as the PR equivalents and properly handle theopened,closed, andreopenedactions. This centralization eliminates duplication acrossevents-api.tsandwebhook-events.ts.
61-73: LGTM! Reopened state support added.The "reopened" action handling for PRs is consistent with the new issue event helpers, using the same 🔄 emoji.
src/github-app/event-processor.ts (5)
58-68: LGTM! Simplified signature.The refactored
processEventsignature is cleaner - replacingbranchContextobject with a directbranchstring parameter, and makingisThreadReplyexplicitly required in the formatter callback.
262-274: LGTM! Clean delegation to anchor handler.The
onIssueshandler correctly delegates tohandleAnchorEventwith appropriate parameters. Theissue.state ?? "open"fallback is good defensive coding.
226-239: LGTM! PR handler refactored to use anchor handler.The
onPullRequesthandler cleanly delegates tohandleAnchorEvent, passingpull_request.base.reffor branch filtering. See the earlier comment about merged state handling.
110-119: LGTM! Clean destructuring.Explicitly destructuring
{ channelId, spaceId }from the channel objects makes the code more readable and clear about which fields are being used.
196-217: No action required — merged PR state is correctly preserved during edits.The formatter receives the full
pull_requestpayload, including themergedflag. When a merged PR is edited,getPrEventHeader()checks both the action ("closed") and themergedflag to display "Pull Request Merged" rather than "Pull Request Closed". The implementation already handles this case correctly.Likely an incorrect or invalid review comment.
src/formatters/events-api.ts (1)
72-91: LGTM! Consistent use of shared helpers.The
IssuesEventhandling now usesgetIssueEventEmojiandgetIssueEventHeaderfrom the shared module, aligning with howPullRequestEventalready uses the PR equivalents. The early return guard at line 80 correctly handles unknown actions.src/formatters/webhook-events.ts (3)
57-60: LGTM! Stats now always included in PR anchors.This change ensures diff stats (
+additions -deletions) are present in all PR anchor messages regardless of action, providing consistent context for opened, closed, merged, and reopened states.
74-91: LGTM! formatIssue uses shared helpers.The issue formatter now uses the centralized
getIssueEventEmojiandgetIssueEventHeaderhelpers, consistent with the Events API formatter refactor.
162-165: LGTM! Required isThreadReply parameter.Making
isThreadReplya required parameter across these three formatters (formatIssueComment,formatPullRequestReview,formatPullRequestReviewComment) improves type safety and ensures callers always explicitly specify the threading context.
When a PR or issue is edited, closed, or reopened: - Update the anchor message to reflect the new state - For close/reopen: post a thread reply before updating anchor Other changes: - Add handleAnchorEvent to consolidate PR/issue event handling - Add reopened state support for PR and issue formatters - Extract getIssueEventEmoji/Header to shared.ts - Simplify branchContext parameter to just branch string - Make isThreadReply parameter required - Always include diff stats in PR anchor messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
shuhuiluo
force-pushed
the
feat/pr-anchor-updates
branch
from
338cdaa to
b903bde
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/github-app/event-processor.ts (2)
405-408: Fix formatter signature to match processEvent requirements.The inline formatter doesn't match the required signature.
processEventexpects(event: T, isThreadReply: boolean) => string, but this formatter only accepts the event parameter. The underlyingformatCreateandformatDeletefunctions also don't accept anisThreadReplyparameter.Apply this diff to fix the signature:
const formatter = (e: CreatePayload | DeletePayload) => - eventType === "create" - ? formatCreate(e as CreatePayload) - : formatDelete(e as DeletePayload); + const formatter = (e: CreatePayload | DeletePayload, isThreadReply: boolean) => + eventType === "create" + ? formatCreate(e as CreatePayload) + : formatDelete(e as DeletePayload);Note:
formatCreateandformatDeletedon't useisThreadReply, so it's ignored. Branch events don't support threading, so this parameter won't be meaningful here, but the signature must match.
245-256: Update formatter signatures to match processEvent requirements.The
processEventmethod expects formatters with signature(event: T, isThreadReply: boolean) => string(line 61), but these formatters are missing theisThreadReplyparameter:
formatPushformatReleaseformatWorkflowRunformatForkformatWatchEach formatter must accept
isThreadReply: booleanas a second parameter to match the expected signature, even if the parameter is unused for non-threaded events.
🧹 Nitpick comments (2)
src/formatters/webhook-events.ts (1)
77-80: LGTM! Consider logging unsupported actions.The refactoring to use centralized helpers improves maintainability. The guard clause correctly handles unsupported actions by returning an empty string. Consider adding a debug log when
emojiorheaderis empty to help identify unexpected actions during development.Apply this diff to add debug logging for unsupported actions:
export function formatIssue(payload: IssuesPayload): string { const { action, issue, repository } = payload; const emoji = getIssueEventEmoji(action); const header = getIssueEventHeader(action); - if (!emoji || !header) return ""; + if (!emoji || !header) { + console.debug(`Unsupported issue action: ${action}`); + return ""; + } return buildMessage({src/github-app/event-processor.ts (1)
73-79: Consider error handling strategy for missing branch.The validation ensures branch-filterable events have the required
branchparameter, which prevents silent failures. However, throwing an error will crash the webhook handler for that event.Consider whether this should:
- Log an error and return early (current approach crashes the handler)
- Use a runtime assertion during development but gracefully handle in production
- Remain as-is if the intention is to fail fast on configuration errors
If you prefer graceful degradation, apply this diff:
// Validate branch is provided for branch-filterable events const isBranchFilterable = BRANCH_FILTERABLE_EVENTS_SET.has(eventType); if (isBranchFilterable && !branch) { - throw new Error( - `${eventType} is branch-filterable but no branch provided` - ); + console.error( + `${eventType} is branch-filterable but no branch provided - skipping event` + ); + return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/formatters/events-api.ts(2 hunks)src/formatters/shared.ts(2 hunks)src/formatters/webhook-events.ts(6 hunks)src/github-app/event-processor.ts(9 hunks)src/services/message-delivery-service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/formatters/events-api.ts
- src/formatters/shared.ts
- src/services/message-delivery-service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/formatters/webhook-events.tssrc/github-app/event-processor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/formatters/webhook-events.tssrc/github-app/event-processor.ts
🧬 Code graph analysis (2)
src/formatters/webhook-events.ts (1)
src/formatters/shared.ts (2)
getIssueEventEmoji(79-84)getIssueEventHeader(89-94)
src/github-app/event-processor.ts (4)
src/services/message-delivery-service.ts (1)
EntityContext(21-28)src/constants.ts (2)
BRANCH_FILTERABLE_EVENTS_SET(56-58)EventType(34-34)src/types/webhooks.ts (2)
PullRequestPayload(14-14)IssuesPayload(15-15)src/formatters/webhook-events.ts (2)
formatPullRequest(49-72)formatIssue(74-91)
🔇 Additional comments (9)
src/formatters/webhook-events.ts (3)
20-26: LGTM! Centralized event metadata helpers.The addition of
getIssueEventEmojiandgetIssueEventHeaderimports aligns with the refactoring to use shared helpers for consistent issue event formatting.
57-60: No changes needed. The comment "Always include stats for anchor messages" is accurate—formatPullRequestis used exclusively withinhandleAnchorEvent(line 235 of event-processor.ts) and is never called for non-anchor PR messages. The code correctly implements what the comment describes.Likely an incorrect or invalid review comment.
162-165: All call sites verified — no action needed.The three formatters are correctly integrated. They're all invoked through
processEvent()(line 117 in event-processor.ts), which properly passes theisThreadReplyparameter:formatter(event, isThreadReply). This matches the required signature in webhook-events.ts.
formatIssueCommentcalled at line 325 (onIssueComment)formatPullRequestReviewcalled at line 356 (onPullRequestReview)formatPullRequestReviewCommentcalled at line 383 (onPullRequestReviewComment)src/github-app/event-processor.ts (6)
64-67: LGTM! Simplified branch parameter.The refactoring to pass
branchas a string instead ofbranchContext: { branch: string }simplifies the API. MakingisThreadReplyrequired in the formatter signature ensures consistent threading behavior.
110-119: LGTM! Cleaner destructuring and explicit parameters.The changes improve code clarity by:
- Destructuring
channelIdandspaceIddirectly from subscribed channels- Passing explicit parameters instead of nested object access
- Properly threading
isThreadReplythrough the formatter callback
226-239: LGTM! Clean refactoring using handleAnchorEvent.The refactoring simplifies PR event handling by delegating to the new
handleAnchorEventhelper. All parameters are correctly passed, including the base branch for filtering.
262-274: LGTM! Defensive handling of issue state.The refactoring mirrors the PR handling and correctly uses
issue.state ?? "open"to provide a default value. Issues are correctly handled without branch filtering.
245-256: LGTM! Consistent branch parameter handling across all event handlers.All event handlers have been updated to pass
branchas a string:
- Push events extract from
ref- Workflow runs use
head_branchwith fallback- PR-related events use
base.ref- Branch events use
refdirectlyThe changes are consistent with the simplified signature and correctly apply branch filtering where appropriate.
Also applies to: 294-305, 344-368, 374-395, 401-419
124-221: The anchor event handling logic is sound.The implementation correctly handles PR/issue anchor events with the following verified behavior:
VirtualAction logic (lines 196-201): The derivation correctly maps all combinations to supported formatter actions. For "closed"/"reopened", it uses those values directly. For "edited", it maps to the entity's current state ("opened" if open, "closed" if closed). All resulting values ("opened", "closed", "reopened") are handled by
getIssueEventEmoji/HeaderandgetPrEventEmoji/Headerfunctions, so formatters will work correctly.Type assertion (line 204): Spreading the event and asserting
as Tis safe because T is only constrained to requirerepository.full_nameandrepository.default_branch. The modified action field is not part of this constraint. Since virtualAction values are always from the valid set {"opened", "closed", "reopened"}, there are no runtime type issues.EntityId for thread replies (line 187): The composite format
${anchorNumber}-${action}(e.g., "123-closed", "123-reopened") correctly creates unique identifiers for close/reopen thread replies without overwriting anchor mappings. The MessageDeliveryService treats this as a lookup key with no special parsing required.
When a PR or issue is edited, closed, or reopened:
Other changes:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.