Repository navigation
Add package email event subscriptions - #290
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughInbound email storage is now authoritative and triggers package subscription dispatches (email.message.received). Packages receive metadata-only payloads and must fetch message bodies or attachment bytes on-demand via new MCP capabilities (email_message_get, email_attachment_get). Legacy authored-package email config schemas were removed in favor of subscriptions. Changes
Sequence DiagramsequenceDiagram
actor MailSender
participant Inbound as Inbound Handler
participant EmailDB as Email DB
participant Dispatcher as Subscription Dispatcher
participant PackageDB as Package DB
participant Runtime as Package Runtime
participant Repo as Repo Functions
MailSender->>Inbound: forward message
Inbound->>EmailDB: store message + attachments
Inbound->>Dispatcher: trigger dispatch (waitUntil)
Dispatcher->>PackageDB: load saved packages & manifests
Dispatcher->>PackageDB: find subscriptions for email.message.received
Dispatcher->>Runtime: invokePackageSubscription(params: message metadata)
Runtime->>Repo: getEmailMessageWithAttachmentsById(messageId)
Repo->>EmailDB: query message + attachment records
Runtime->>Repo: getEmailAttachmentById(attachmentId)
Repo->>EmailDB: fetch rawMime & parse part -> return data_base64
Runtime-->>Dispatcher: subscription result
Dispatcher-->>Inbound: dispatch complete
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
kentcdodds
marked this pull request as ready for review
April 30, 2026 16:36
Contributor
|
🔎 Preview deployed: https://kody-pr-290.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
cursor
Bot
force-pushed
the
cursor/package-email-event-subscriptions-06d9
branch
from
April 30, 2026 17:52
4bef71f to
ad154cc
Compare
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 37f40bc. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/mcp/run-codemode-registry.ts (1)
431-438:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward
emailToolswhen routing module-syntax code.Line 431 delegates to
runModuleWithRegistry, butemailToolsis not passed, so module-syntax executions invoked viarunCodemodeWithRegistrylose email helper support.Proposed fix
if (hasTopLevelModuleSyntax(moduleSource)) { return runModuleWithRegistry(env, callerContext, moduleSource, params, { executorExports: options?.executorExports, additionalTools: options?.additionalTools, storageTools: options?.storageTools, serviceTools: options?.serviceTools, packageContext: options?.packageContext, + emailTools: options?.emailTools, executorTimeoutMs: options?.executorTimeoutMs, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/run-codemode-registry.ts` around lines 431 - 438, The call in runCodemodeWithRegistry that delegates to runModuleWithRegistry is missing forwarding of emailTools, so module-syntax executions lose email helper support; update the options passed to runModuleWithRegistry to include emailTools: options?.emailTools (alongside executorExports, additionalTools, storageTools, serviceTools, packageContext, executorTimeoutMs) so runModuleWithRegistry receives and can use the email helper utilities.
🧹 Nitpick comments (1)
packages/worker/src/mcp/run-codemode-registry.node.test.ts (1)
1092-1256: ⚡ Quick winAdd a regression test for
runCodemodeWithRegistry(..., { emailTools })with module syntax.Current additions validate
runBundledModuleWithRegistryandrunModuleWithRegistry, but not the module-routing branch insiderunCodemodeWithRegistry. A focused test here would catch dropped option forwarding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/run-codemode-registry.node.test.ts` around lines 1092 - 1256, Add a regression test that mirrors the "runModuleWithRegistry injects email helpers for module syntax" case but calls runCodemodeWithRegistry (with module-style code string) to verify emailTools are forwarded for the module-routing branch; spy/mock getCapabilityRegistryForContext and createExecuteExecutor (as in the other tests), mock buildKodyModuleBundle if needed, capture providerFns from the executor, invoke runCodemodeWithRegistry with an emailTools object (getMessage/getAttachment), assert the result and that providerFns.email_message_get and providerFns.email_attachment_get resolve to the expected values, and restore mocks (use the same symbols: runCodemodeWithRegistry, buildKodyModuleBundle, getCapabilityRegistryForContext, createExecuteExecutor, providerFns, email_message_get, email_attachment_get).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/use/email-primitives.md`:
- Around line 31-34: Add the missing capability name email_attachment_get to the
primary capabilities list so it appears alongside the other email primitives
(e.g., email_message_get, import { email } from 'kody:runtime') in the top
capabilities section; update the capabilities bullet list to include
"email_attachment_get" for discoverability and ensure wording/formatting matches
the existing entries (same casing and style).
In `@packages/worker/src/email/inbound.ts`:
- Around line 169-173: The handler currently awaits dispatchPromise when _ctx is
falsy, which can let subscription-dispatch failures bubble up and cause the
inbound ingestion to fail despite storage succeeding; change the logic around
_ctx and dispatchPromise so dispatchPromise is not awaited directly when _ctx is
absent—instead attach a non-throwing error handler (e.g.,
dispatchPromise.catch(err => processLogger.error('subscription dispatch failed',
err))) or otherwise swallow/log errors so the handler returns success; update
the block that references _ctx and dispatchPromise in inbound.ts to call
_ctx.waitUntil(dispatchPromise) when _ctx exists and to attach a .catch(...) (or
equivalent safe handling) to dispatchPromise when _ctx is missing so failures do
not propagate.
In `@packages/worker/src/email/inbound.workers.test.ts`:
- Around line 462-465: The test is building the manifest snapshot KV key with
publishedCommit but the key builder buildPublishedSourceManifestSnapshotKvKey
expects the versionHash; update the call in the test to pass the versionHash
value (e.g., versionHash: 'commit-1' or the variable holding the version hash)
along with sourceId so the generated key matches the real snapshot key and the
subscription dispatch assertion becomes reliable.
In `@packages/worker/src/mcp/capabilities/email/email-attachment-get.ts`:
- Around line 42-45: The guard in email-attachment-get that currently uses a
truthiness check on attachment.contentBase64 will reject legitimate zero-byte
attachments; update the check in the code path (the block that throws the Error
using args.attachment_id) to explicitly test for null/undefined (e.g.,
attachment.contentBase64 == null or attachment.contentBase64 === undefined)
instead of using !attachment.contentBase64 so empty string payloads are allowed
to pass through and return successfully.
---
Outside diff comments:
In `@packages/worker/src/mcp/run-codemode-registry.ts`:
- Around line 431-438: The call in runCodemodeWithRegistry that delegates to
runModuleWithRegistry is missing forwarding of emailTools, so module-syntax
executions lose email helper support; update the options passed to
runModuleWithRegistry to include emailTools: options?.emailTools (alongside
executorExports, additionalTools, storageTools, serviceTools, packageContext,
executorTimeoutMs) so runModuleWithRegistry receives and can use the email
helper utilities.
---
Nitpick comments:
In `@packages/worker/src/mcp/run-codemode-registry.node.test.ts`:
- Around line 1092-1256: Add a regression test that mirrors the
"runModuleWithRegistry injects email helpers for module syntax" case but calls
runCodemodeWithRegistry (with module-style code string) to verify emailTools are
forwarded for the module-routing branch; spy/mock
getCapabilityRegistryForContext and createExecuteExecutor (as in the other
tests), mock buildKodyModuleBundle if needed, capture providerFns from the
executor, invoke runCodemodeWithRegistry with an emailTools object
(getMessage/getAttachment), assert the result and that
providerFns.email_message_get and providerFns.email_attachment_get resolve to
the expected values, and restore mocks (use the same symbols:
runCodemodeWithRegistry, buildKodyModuleBundle, getCapabilityRegistryForContext,
createExecuteExecutor, providerFns, email_message_get, email_attachment_get).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d49d6eda-2d2d-47da-ac7c-f5dd0b4ec2df
📒 Files selected for processing (19)
docs/use/email-primitives.mdpackages/worker/src/email/inbound.tspackages/worker/src/email/inbound.workers.test.tspackages/worker/src/email/package-subscriptions.tspackages/worker/src/email/repo.tspackages/worker/src/mcp/capabilities/email/domain.tspackages/worker/src/mcp/capabilities/email/email-attachment-get.node.test.tspackages/worker/src/mcp/capabilities/email/email-attachment-get.tspackages/worker/src/mcp/capabilities/email/email-domain.node.test.tspackages/worker/src/mcp/run-codemode-registry.node.test.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-invocations/service.tspackages/worker/src/package-registry/manifest.node.test.tspackages/worker/src/package-registry/types.tspackages/worker/src/package-runtime/published-bundle-artifacts.node.test.tspackages/worker/src/package-runtime/published-bundle-artifacts.tspackages/worker/src/package-runtime/subscription-artifacts.tspackages/worker/src/repo/checks.ts
💤 Files with no reviewable changes (1)
- packages/worker/src/package-registry/types.ts
Comment on lines
+462
to
+465
| buildPublishedSourceManifestSnapshotKvKey({ | ||
| sourceId, | ||
| publishedCommit: 'commit-1', | ||
| }), |
There was a problem hiding this comment.
Use versionHash when building the manifest snapshot KV key.
Line 464 uses publishedCommit, but the snapshot key builder is keyed by versionHash. This can seed the wrong key and make the subscription dispatch assertion less reliable.
Suggested fix
bundleKv.set(
buildPublishedSourceManifestSnapshotKvKey({
sourceId,
- publishedCommit: 'commit-1',
+ versionHash: 'commit-1',
}),
JSON.stringify({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| buildPublishedSourceManifestSnapshotKvKey({ | |
| sourceId, | |
| publishedCommit: 'commit-1', | |
| }), | |
| buildPublishedSourceManifestSnapshotKvKey({ | |
| sourceId, | |
| versionHash: 'commit-1', | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/email/inbound.workers.test.ts` around lines 462 - 465,
The test is building the manifest snapshot KV key with publishedCommit but the
key builder buildPublishedSourceManifestSnapshotKvKey expects the versionHash;
update the call in the test to pass the versionHash value (e.g., versionHash:
'commit-1' or the variable holding the version hash) along with sourceId so the
generated key matches the real snapshot key and the subscription dispatch
assertion becomes reliable.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This was referenced Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
email.message.receivedundefinedvalues against storednullmetadata, with focused regression coverageemailToolsthrough the module-syntaxrunModuleWithRegistrypath so runtime email helpers behave consistently across execute entry stylesExecutionContextdoes not let post-storage package-dispatch failures bubble back into the email ingestion path, and allow zero-byte attachments throughemail_attachment_getValidation
npx vitest run --project node-unit --project workers-unit packages/worker/src/package-registry/manifest.node.test.ts packages/worker/src/package-runtime/published-bundle-artifacts.node.test.ts packages/worker/src/email/inbound.workers.test.ts packages/worker/src/mcp/capabilities/email/email-domain.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts packages/worker/src/mcp/capabilities/email/email-attachment-get.node.test.ts packages/worker/src/package-invocations/service.node.test.tsnpx vitest run --project workers-unit packages/worker/src/email/inbound.workers.test.ts && npx vitest run --project node-unit packages/worker/src/mcp/capabilities/email/email-attachment-get.node.test.tsnpx vitest run --project node-unit packages/worker/src/package-invocations/service.node.test.tsnpx vitest run --project node-unit packages/worker/src/mcp/run-codemode-registry.node.test.tsnpm run typechecknpm run buildSummary by CodeRabbit
New Features
Documentation
Tests