-
Notifications
You must be signed in to change notification settings - Fork 0
fix: add Events API specific actions to validation schemas #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
GitHub's Events API uses different action names than Webhooks API: - PR: adds 'merged' action (webhooks uses 'closed' with merged flag) - PR Review: adds 'created'/'updated' (webhooks uses 'submitted'/'edited') Also improves error logging to show the actual action value on validation failure.
WalkthroughReplaces the previous single-string action validators with explicit per-event action constant arrays and a new unified actionSchema that validates against those arrays and returns detailed allowed-action errors. Updates action fields on PullRequest, Issues, Release, WorkflowRun, IssueComment, PullRequestReview, PullRequestReviewComment, and Watch payload schemas to use the new action sets. Adds private const arrays for each event's allowed actions and an actionSchema implementation. Enhances validateGitHubEvent error logging to include eventType, eventId (defaulting to "unknown"), and the payload action when available. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas for attention:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (8)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/types/events-api.ts (1)
35-47: Consider enhancing error messages with valid actions list.The error message could be more helpful by including the list of valid actions when validation fails, improving the developer experience during debugging.
Apply this diff to enhance the error message:
function actionSchema<T extends readonly string[]>( actions: T, eventType: string ) { return z .string() .refine( (action): action is T[number] => (actions as readonly string[]).includes(action), - { message: `Unknown ${eventType} action` } + { + message: `Unknown ${eventType} action. Valid actions: ${actions.join(", ")}` + } ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/types/events-api.ts(9 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/types/events-api.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/types/events-api.ts
🔇 Additional comments (2)
src/types/events-api.ts (2)
523-535: LGTM! Excellent error logging enhancement.The enhanced error reporting safely extracts event ID and action with proper type guards and fallbacks, providing clear diagnostic information when validation fails.
81-81: LGTM! Consistent schema updates.All payload schemas correctly use the new
actionSchemafunction with their respective action constants and event type names. The implementation is consistent and type-safe across all event types.Also applies to: 130-130, 187-187, 214-214, 239-239, 278-278, 348-350, 389-389
- Clarified comments for PR and Issue actions to specify Events API context. - Removed unused actions from PR_ACTIONS. - Simplified PR_REVIEW_ACTIONS to focus on relevant actions for Events API. - Enhanced documentation for better understanding of action mappings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/types/events-api.ts (1)
35-47: LGTM! Consider enhanced error messaging.The
actionSchemafunction correctly validates actions using Zod 4'srefinemethod with a type predicate. The implementation is sound.Optional enhancement: Zod 4 supports a unified
errorparameter that can provide richer context:function actionSchema<T extends readonly string[]>( actions: T, eventType: string ) { return z .string() .refine( (action): action is T[number] => (actions as readonly string[]).includes(action), - { message: `Unknown ${eventType} action` } + { + error: (issue) => { + const value = issue.input; + return `Unknown ${eventType} action: "${value}". Expected one of: ${actions.join(", ")}`; + }, + } ); }Based on library documentation for Zod 4's unified error customization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/types/events-api.ts(9 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/types/events-api.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/types/events-api.ts
🔇 Additional comments (9)
src/types/events-api.ts (9)
49-71: LGTM! Action list correctly updated.The PR actions array properly includes Events API-specific actions like
"merged"and excludes merge-queue specific actions ("enqueued","dequeued") that were flagged in previous reviews. The array now aligns with GitHub Events API documentation.Based on past review verification against GitHub's official Events API documentation.
102-122: LGTM! Complete action list.The issues actions array now includes the previously missing
"typed"and"untyped"actions (lines 120-121), matching the GitHub Events API documentation.Based on past review verification.
170-179: LGTM!Release actions array matches GitHub Events API documentation per previous verification.
205-206: LGTM!Workflow run actions correctly include the three event types:
"requested","in_progress", and"completed".
230-231: LGTM!Issue comment actions are correct per GitHub Events API documentation.
261-262: LGTM! Actions correctly match Events API, but PR description is inconsistent.The PR review actions array correctly contains only
"submitted","edited", and"dismissed", which aligns with GitHub Events API documentation verified in previous reviews.However, the PR description claims to add
"created"and"updated"actions for pull request reviews, which contradicts the actual code changes. The current implementation is correct—the PR description appears to be outdated or incorrect.Based on past review verification against GitHub's official Events API documentation.
331-332: LGTM!Pull request review comment actions are correctly defined. Note these differ from PR review actions (submitted/edited/dismissed) because review comments and reviews are distinct event types.
372-373: LGTM!Watch event correctly uses only the
"started"action per GitHub Events API documentation.
513-524: LGTM! Robust error handling.The enhanced error logging correctly:
- Safely extracts event ID with a fallback to
"unknown"for non-string values- Extracts action from payload when available with proper type checking
- Provides clear, contextual error messages including event type, ID, and action
The defensive type checking prevents runtime errors when processing malformed events.
…tion - Updated the error message in actionSchema to include the actual input value and expected actions for better debugging.
GitHub's Events API uses different action names than Webhooks API:
Also improves error logging to show the actual action value on validation failure.
Summary by CodeRabbit
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.