Skip to content

Conversation

@shuhuiluo
Copy link
Collaborator

@shuhuiluo shuhuiluo commented Dec 3, 2025

  • Add ActionsFor generic to extract action unions at compile-time
  • Export inspectable action types (PullRequestAction, IssuesAction, etc.)
  • Use actionsFor() to extract actions at runtime for Zod validation
  • Unknown actions now fail validation (rare since derived from Octokit)

Summary by CodeRabbit

  • Refactor
    • Improved webhook action validation across events, replacing static action lists with dynamically derived/validated action sets.
    • Exposed consistent action type definitions for common event categories to improve type safety.
    • Event payloads now validate their action values against the derived action sets, reducing mismatches and improving reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

This pull request adds compile-time and runtime utilities to derive and validate webhook action names from Octokit's emitter event names. It introduces WebhookEventName, a generic ActionsFor<TPrefix> type, runtime helpers actionsFor(prefix) and actionSchema(prefix, eventType), and exports action types for multiple event categories (pull_request, issues, release, workflow_run, issue_comment, pull_request_review, pull_request_review_comment, watch). Eight existing event payload schemas were updated to use actionSchema for validating action instead of fixed string enums. No other payload shapes or discriminated unions were changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify the ActionsFor<TPrefix> conditional type correctly extracts action suffixes from WebhookEventName (uses ${TPrefix}.${infer Action}).
  • Review actionsFor runtime helper to ensure it returns the expected union values and aligns with the compile-time type.
  • Inspect actionSchema to confirm correct Zod schema construction, runtime validation, and eventType usage.
  • Confirm the eight schema replacements pass correct prefix and eventType arguments and preserve payload discriminators.
  • Check exported action types (e.g., PullRequestAction, IssuesAction) map to the intended unions.

Pre-merge checks and finishing touches

✅ Passed checks (3 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: deriving event actions from @octokit/webhooks types, which aligns with the primary refactoring in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/events-api-actions

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 068ab25 and 5f0dfd3.

📒 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 (5)
src/types/events-api.ts (5)

9-21: Static typing from emitterEventNames looks solid and fixes the TSDoc warning

Deriving WebhookEventName and ActionsFor<TPrefix> from emitterEventNames gives you a single, strongly‑typed source of truth for actions, and the escaped \@octokit in the TSDoc resolves the earlier parser complaint. When you bump @octokit/webhooks, just re‑run type checks/tests to confirm the derived unions still match the expected action sets for each prefix.


22-28: actionsFor() correctly mirrors the compile‑time ActionsFor<T> helper

Filtering emitterEventNames by ${prefix}. and slicing off the prefix to build the action list matches the template‑literal extraction logic in ActionsFor<T>, and capturing actions in a closure keeps the per-parse cost low. If a future Octokit upgrade ever drops or renames a prefix, this will surface quickly as validation failures for that event family, so it’s worth exercising a couple of real webhooks in tests after such upgrades.


30-39: Exported *Action unions are a nice DX improvement

Exposing PullRequestAction, IssuesAction, etc. from the same ActionsFor<> source will make it much easier to strongly‑type handlers and business logic without duplicating action string literals.


41-51: Stricter actionSchema behavior: unknown actions now hard‑fail validation

The actionSchema(prefix, eventType) refine correctly restricts action values to the derived ActionsFor<T> set and provides a clear "Unknown ${eventType} action" error message. This does mean any action not present in emitterEventNames (e.g., a brand‑new GitHub action before Octokit is updated) will now cause validation to fail and the event to be dropped by validateGitHubEvent. That matches the PR description, but it’s worth double‑checking that you don’t rely on pass‑through handling of unknown actions anywhere upstream.


80-97: Consistent use of actionSchema(...) across payload schemas keeps actions in sync with Octokit

Replacing the hard‑coded z.enum/literal action fields in all payload schemas (PR, Issues, Release, WorkflowRun, IssueComment, PRReview, PRReviewComment, Watch) with actionSchema("<prefix>", "<EventType>") cleanly centralizes the valid‑action definition while preserving the existing discriminated union structure. This should reduce drift between TypeScript action unions and runtime validation as Octokit evolves.

Also applies to: 109-121, 155-167, 179-189, 201-217, 229-248, 296-320, 334-336


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

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c993895 and 068ab25.

📒 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
🪛 ESLint
src/types/events-api.ts

[error] 9-9: Unable to resolve path to module '@octokit/webhooks'.

(import-x/no-unresolved)


[error] 10-10: Unable to resolve path to module 'zod'.

(import-x/no-unresolved)

🪛 GitHub Actions: CI
src/types/events-api.ts

[warning] 12-12: tsdoc-characters-after-block-tag: The token "@octokit" looks like a TSDoc tag but contains an invalid character "/"; if it is not a tag, use a backslash to escape the "@"

🔇 Additional comments (4)
src/types/events-api.ts (4)

15-28: Excellent type-safe design!

The compile-time ActionsFor<TPrefix> and runtime actionsFor() work together elegantly to derive actions from Octokit's event names. The type assertion is safe given the filtering and slicing logic.


30-39: LGTM!

Exporting these action types improves discoverability and enables IDE hover inspection of the action unions.


81-81: LGTM! Consistent schema updates across all event types.

All eight event payload schemas have been systematically updated to use the new actionSchema() function. The consistent pattern of deriving actions from Octokit's type definitions improves maintainability and reduces the risk of drift between the codebase and upstream types.

Also applies to: 110-110, 156-156, 180-180, 202-202, 230-230, 297-300, 335-335


41-51: Remove suggestion to use z.enum() — the current refine() approach is correct for this use case.

The actionSchema function validates against a runtime-determined array derived from @octokit/webhooks. Zod's z.enum() requires a compile-time constant tuple (e.g., z.enum(['action1', 'action2'] as const)), but the actions array is computed at runtime by filtering emitterEventNames. The z.string().refine() approach with the type predicate is the idiomatic Zod solution for runtime validation and already provides appropriate error messages and type narrowing.

Likely an incorrect or invalid review comment.

- Add ActionsFor<T> generic to extract action unions at compile-time
- Export inspectable action types (PullRequestAction, IssuesAction, etc.)
- Use actionsFor() to extract actions at runtime for Zod validation
- Unknown actions now fail validation (rare since derived from Octokit)
@shuhuiluo shuhuiluo force-pushed the fix/events-api-actions branch from 068ab25 to 5f0dfd3 Compare December 3, 2025 11:10
@shuhuiluo shuhuiluo merged commit baba646 into main Dec 3, 2025
2 checks passed
@shuhuiluo shuhuiluo deleted the fix/events-api-actions branch December 3, 2025 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants