feat(gmail): add isHtml param and email outreach specs#46
Merged
AojdevStudio merged 7 commits intomainfrom Mar 10, 2026
Merged
Conversation
…pecs The Gmail module already supported HTML emails internally (types, utils, reply, forward, attachments) but the SDK spec didn't advertise isHtml for sendMessage and createDraft. Now both operations explicitly document isHtml with examples showing inline-styled HTML for templating and brand identity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds two planning documents for email outreach capabilities: - outreach-features.md: v2 feature spec for template rendering, batch send, and tracking - outreach-gap-analysis.md: codebase readiness assessment against v4.0.0-alpha Co-Authored-By: AOJDevStudio
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a v2 email outreach feature set: Gmail templating, dry-run preview, sendFromTemplate/sendBatch with throttling and per-recipient variables, reply detection, Sheets readAsRecords/updateRecords helpers, a Cloudflare Worker tracking pixel with KV-backed schema, SDK spec/runtime wiring, and unit tests. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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. Comment |
📊 Type Coverage ReportType Coverage: 98.59% This PR's TypeScript type coverage analysis is complete. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gdrive-mcp | 32cc3a6 | Commit Preview URL Branch Preview URL |
Mar 10 2026, 10:13 PM |
🔒 Security Scan SummaryGenerated on: Tue Mar 10 22:14:17 UTC 2026 Scan Results
Summary
Recommendations
Security report generated by Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea87f4176b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| example: "const sent = await sdk.gmail.sendMessage({\n to: 'recipient@example.com',\n subject: 'Hello',\n body: 'This is the message body.'\n});\nreturn sent.messageId;", | ||
| signature: "sendMessage(options: { to: string | string[], subject: string, body: string, isHtml?: boolean, cc?: string | string[], bcc?: string | string[], inReplyTo?: string }): Promise<{ messageId, threadId, labelIds }>", | ||
| description: "Send an email immediately. Use isHtml for rich HTML emails.", | ||
| example: "// Plain text email\nconst sent = await sdk.gmail.sendMessage({\n to: 'recipient@example.com',\n subject: 'Hello',\n body: 'This is the message body.'\n});\n\n// HTML email with branding\nconst htmlSent = await sdk.gmail.sendMessage({\n to: 'client@example.com',\n subject: 'Welcome to Our Service',\n body: '<div style=\"font-family: Arial; max-width: 600px;\"><h1 style=\"color: #2563eb;\">Welcome!</h1><p>Thank you for signing up.</p></div>',\n isHtml: true\n});\nreturn htmlSent.messageId;", |
There was a problem hiding this comment.
The new example now issues two sdk.gmail.sendMessage calls (plain text + HTML) before returning, so running it verbatim from the search output can send duplicate real emails and burn quota unexpectedly. Since this spec is exposed as executable guidance to agents/users, each example should show a single send path (or clearly separate non-runnable alternatives) to prevent accidental duplicate outreach.
Useful? React with 👍 / 👎.
Performance Comparison ReportOperation Performance
Memory Usage
Summary
Performance report generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sdk/spec.ts`:
- Around line 419-429: The SDK_SPEC entries for the Gmail methods are
incomplete: update the signature and params for createDraft and sendMessage in
the SDK spec so they expose the same supported options as in the runtime types
(add from and references to createDraft; add from, references, and threadId to
sendMessage), and ensure the params dictionary documents each added field (types
and brief descriptions, e.g., "from: string (optional) — sender override",
"references: string | string[] (optional) — message IDs for threading",
"threadId: string (optional) — existing thread to append to"). Reference the
SDK_SPEC entries named createDraft and sendMessage and keep
descriptions/examples consistent with existing style.
- Around line 419-427: The spec advertises to/cc/bcc as "string | string[]" and
shows scalar examples, but the runtime (src/modules/gmail/types.ts) and
buildEmailMessage() require arrays; update the SDK spec for the Gmail API to use
array-only recipient types and array examples: change the createDraft
signature/params to use string[] for to, cc, bcc (and mark required/optional
consistent with types.ts), and update both example snippets to pass arrays
(e.g., ['team@company.com']) so the spec matches the runtime contract; apply the
same fixes in the other spec block that documents these fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7e6d78da-8b59-4816-b444-5463c0e9011a
📒 Files selected for processing (3)
docs/specs/outreach-features.mddocs/specs/outreach-gap-analysis.mdsrc/sdk/spec.ts
Comment on lines
+419
to
429
| signature: "createDraft(options: { to: string | string[], subject: string, body: string, isHtml?: boolean, cc?: string | string[], bcc?: string | string[], inReplyTo?: string }): Promise<{ draftId, messageId, threadId }>", | ||
| description: "Create an email draft (not sent). Use isHtml for rich HTML emails. Use sendDraft() to send it.", | ||
| example: "// Plain text draft\nconst draft = await sdk.gmail.createDraft({\n to: 'team@company.com',\n subject: 'Weekly Update',\n body: 'Hi team,\\n\\nHere is this week\\'s update...'\n});\n\n// HTML draft with branding\nconst htmlDraft = await sdk.gmail.createDraft({\n to: 'client@example.com',\n subject: 'Your Invoice',\n body: '<div style=\"font-family: Arial;\"><h1>Invoice #1234</h1><p>Amount due: <strong>$500</strong></p></div>',\n isHtml: true\n});\nreturn htmlDraft.draftId;", | ||
| params: { | ||
| "to": "string | string[] (required) — recipient email(s)", | ||
| subject: "string (required)", | ||
| body: "string (required) — email body text or HTML", | ||
| body: "string (required) — email body (plain text or HTML)", | ||
| isHtml: "boolean (optional, default false) — whether body is HTML", | ||
| "cc": "string | string[] (optional)", | ||
| "bcc": "string | string[] (optional)", | ||
| inReplyTo: "string (optional) — message ID to reply to (for threading)", |
There was a problem hiding this comment.
Expose the rest of the supported Gmail options here.
While adding isHtml, these signatures are still behind the actual surface in src/modules/gmail/types.ts: createDraft is missing from and references, and sendMessage is missing from, references, and threadId. Since SDK_SPEC is what agents discover first, those supported paths stay effectively invisible.
Proposed spec alignment
- signature: "createDraft(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], inReplyTo?: string }): Promise<{ draftId, messageId, threadId }>",
+ signature: "createDraft(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], from?: string, inReplyTo?: string, references?: string }): Promise<{ draftId, messageId, threadId }>",
...
+ from: "string (optional) — send from a specific send-as alias",
inReplyTo: "string (optional) — message ID to reply to (for threading)",
+ references: "string (optional) — References header for threading",
...
- signature: "sendMessage(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], inReplyTo?: string }): Promise<{ messageId, threadId, labelIds }>",
+ signature: "sendMessage(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], from?: string, inReplyTo?: string, references?: string, threadId?: string }): Promise<{ messageId, threadId, labelIds }>",
...
+ from: "string (optional) — send from a specific send-as alias",
inReplyTo: "string (optional) — message ID to reply to",
+ references: "string (optional) — References header for threading",
+ threadId: "string (optional) — add the message to an existing thread",Also applies to: 434-444
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/spec.ts` around lines 419 - 429, The SDK_SPEC entries for the Gmail
methods are incomplete: update the signature and params for createDraft and
sendMessage in the SDK spec so they expose the same supported options as in the
runtime types (add from and references to createDraft; add from, references, and
threadId to sendMessage), and ensure the params dictionary documents each added
field (types and brief descriptions, e.g., "from: string (optional) — sender
override", "references: string | string[] (optional) — message IDs for
threading", "threadId: string (optional) — existing thread to append to").
Reference the SDK_SPEC entries named createDraft and sendMessage and keep
descriptions/examples consistent with existing style.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- renderTemplate() handles {{variable}} replacement with HTML escaping
- dryRunMessage() previews rendered email without sending (pure function)
- All outreach types added (DryRunOptions, SendFromTemplateOptions, BatchSendOptions, etc.)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- sendFromTemplate renders {{variable}} placeholders then sends via sendMessage
- sendBatch sends to multiple recipients with configurable delay
- sendBatch supports dryRun mode for previewing all renders without sending
- Continues on individual send failures with per-recipient status reporting
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ords - Add tracking pixel endpoint in worker.ts (GET /track/:campaignId/:recipientId/pixel.gif) - Add src/server/tracking.ts with KV-backed open tracking (summary record pattern, 90-day TTL) - Add gmail.detectReplies — checks threads for replies from external participants - Add sheets.updateRecords — updates rows by key column match - Add gmail.getTrackingData — query tracking data (CF Workers only, requires KV) - Register all 3 operations in SDK spec, runtime, and types - Add kv? optional property to FullContext for Worker runtime - Update rate limiter scope test (66 ops × 2 = 132) - Include implementation plan document Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 Type Coverage ReportType Coverage: 98.60% This PR's TypeScript type coverage analysis is complete. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/sdk/spec.ts (1)
443-469:⚠️ Potential issue | 🟠 MajorKeep
createDraft/sendMessagealigned with the real Gmail contract.These entries still advertise scalar
to/cc/bccvalues and still omit supported sender/threading fields. The runtime Gmail types are array-only for recipients and already supportfrom,references, andthreadIdwhere applicable, so agents following this spec will either send the wrong shape or miss supported options.Suggested direction
- signature: "createDraft(options: { to: string | string[], subject: string, body: string, isHtml?: boolean, cc?: string | string[], bcc?: string | string[], inReplyTo?: string }): Promise<{ draftId, messageId, threadId }>", + signature: "createDraft(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], from?: string, inReplyTo?: string, references?: string }): Promise<{ draftId, messageId, threadId }>", ... - \"to\": \"string | string[] (required) — recipient email(s)\", + \"to\": \"string[] (required) — recipient email(s)\", ... - \"cc\": \"string | string[] (optional)\", - \"bcc\": \"string | string[] (optional)\", + \"cc\": \"string[] (optional)\", + \"bcc\": \"string[] (optional)\", + from: \"string (optional) — send-as alias email address\", + references: \"string (optional) — References header for threading\", ... - signature: "sendMessage(options: { to: string | string[], subject: string, body: string, isHtml?: boolean, cc?: string | string[], bcc?: string | string[], inReplyTo?: string }): Promise<{ messageId, threadId, labelIds }>", + signature: "sendMessage(options: { to: string[], subject: string, body: string, isHtml?: boolean, cc?: string[], bcc?: string[], from?: string, inReplyTo?: string, references?: string, threadId?: string }): Promise<{ messageId, threadId, labelIds }>",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdk/spec.ts` around lines 443 - 469, The spec for createDraft and sendMessage is out of sync with the runtime Gmail contract: change the recipient params to array-only (to/cc/bcc as string[] not string | string[]), and add the supported sender/threading fields (add optional from: string, references: string[] and threadId: string where applicable) in both the signature and params sections for createDraft and sendMessage (update the signature strings, params entries, and examples to use array recipients and include from/references/threadId so agents generate the correct request shape).
🧹 Nitpick comments (4)
src/modules/sheets/update.ts (1)
341-354: Performance: Consider batching cell updates into a single API call.Currently, this makes an individual
values.updateAPI call for each cell being updated. For a batch of N records with M columns each, this results in O(N×M) API calls, which could hit rate limits or cause significant latency.Consider using
batchUpdatewith multipleUpdateCellsRequestentries, or accumulating all cell updates and issuing fewer API calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/sheets/update.ts` around lines 341 - 354, The loop calling context.sheets.spreadsheets.values.update per cell (iterating over update.values using headers, columnToLetter, sheetPrefix, rowIndex) causes many API calls; instead build a single rectangular values matrix for the contiguous range of updated columns (or collect all updates into a single sparse matrix) and send one batched request—either use spreadsheets.values.update once with the combined range and requestBody.values matrix, or use spreadsheets.values.batchUpdate to submit multiple ranges in one call; modify the code around update.values, headers, columnToLetter, sheetPrefix and rowIndex to compute the minimal start/end range, populate the 2D values array at the proper column offsets, and replace the per-cell await context.sheets.spreadsheets.values.update calls with a single batched update call.src/modules/sheets/__tests__/updateRecords.test.ts (1)
32-80: Tests cover basic scenarios; consider adding edge case coverage.The current tests validate:
- Successful update by key column match
- Reporting of not-found keys
Consider adding tests for:
- Multiple updates in a single call
- Unknown columns in
values(should be silently skipped)- Missing key column in headers (should throw)
- Empty data in range (should throw)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/sheets/__tests__/updateRecords.test.ts` around lines 32 - 80, Add unit tests for edge cases around updateRecords: add a test that sends multiple updates in one call and asserts the updated count and that values.update was called for each matching row; add a test where updates include unknown column names and assert those keys are ignored (no error, only known headers written); add a test where the headers returned by mockSheetsApi.spreadsheets.values.get do not contain the keyColumn and assert updateRecords throws; and add a test where get returns no data/empty values and assert updateRecords throws; use the same mockSheetsApi.spreadsheets.values.get and .update mocks and reference updateRecords in each test to locate where to add them.src/modules/gmail/types.ts (1)
717-726:BatchSendItemResultrequiresmessageId/threadIdeven for failed sends.When
statusis'failed', there's no actual message ID or thread ID. The type currently requires these fields unconditionally, which may lead to empty strings or misleading values in failure cases.Consider making these fields optional or using a discriminated union:
♻️ Option 1: Make fields optional
export interface BatchSendItemResult { to: string; - messageId: string; - threadId: string; + messageId?: string; + threadId?: string; status: 'sent' | 'failed'; error?: string; }♻️ Option 2: Discriminated union (more type-safe)
export type BatchSendItemResult = | { to: string; status: 'sent'; messageId: string; threadId: string } | { to: string; status: 'failed'; error: string };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/gmail/types.ts` around lines 717 - 726, The BatchSendItemResult type currently requires messageId and threadId even when status === 'failed'; change BatchSendItemResult to a discriminated union (e.g., one variant for status: 'sent' including messageId and threadId, and one for status: 'failed' including error) to make the shape explicit and type-safe. Update the type declaration for BatchSendItemResult and then fix any code that constructs or reads BatchSendItemResult (look for usages of BatchSendItemResult, places that build batch send results, and consumers that read messageId/threadId) to handle both variants (narrow on status before accessing messageId/threadId or error).src/modules/gmail/detect-replies.ts (1)
39-88: LGTM with one operational consideration.The implementation is solid with proper error handling per thread and case-insensitive email comparison. The date parsing and From header extraction handle both "Name " and bare email formats correctly.
Operational note: As per
src/sdk/runtime.ts:251-253, the rate limiter wraps the entiredetectRepliescall, meaning thegetProfilecall plus allthreads.getcalls (one per thread ID) execute as a single rate-limited operation. For largethreadIdsarrays, this could hit Gmail API rate limits internally. Consider documenting a recommended batch size or implementing internal throttling for production use with large datasets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/gmail/detect-replies.ts` around lines 39 - 88, detectReplies currently does the profile fetch and all context.gmail.users.threads.get calls in one shot which can hit Gmail rate limits for large options.threadIds arrays; change detectReplies to fetch the profile first (context.gmail.users.getProfile) and then process threadIds in configurable batches (e.g., add an options.batchSize or internal constant) and either await a short delay between batches or use a simple concurrency limiter (e.g., process N threads in parallel then await completion) when calling context.gmail.users.threads.get; ensure the exception handling and threads.push behavior remain unchanged and surface the batchSize as a recommendation in docs or logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/gmail/send.ts`:
- Around line 188-190: The three one-line conditional branches that set
properties on sendOpts (e.g., "if (options.cc) sendOpts.cc = options.cc;") need
braces to satisfy the curly rule; update each such statement that assigns to
sendOpts from options (the cc, bcc, from lines around the send logic and the
similar occurrence near line 258) to use block form (if (options.xxx) {
sendOpts.xxx = options.xxx; }) so lint passes—look for the sendOpts and options
usage in the send function to locate and fix all occurrences.
In `@src/modules/sheets/update.ts`:
- Line 344: The linter wants braces on the single-line if; replace the
single-line statement "if (colIndex === -1) continue;" with a braced block using
the same condition and action (e.g. "if (colIndex === -1) { continue; }") where
colIndex is used in src/modules/sheets/update.ts so the control flow remains
identical but satisfies ESLint.
- Around line 359-361: The cache invalidation call in updateRecords uses a
wildcard key (context.cacheManager.invalidate(`sheet:${spreadsheetId}:*`)), but
WorkersKVCache.invalidate (see WorkersKVCache in src/storage/kv-store.ts) treats
patterns with '*' as a no-op, leaving stale entries; fix by constructing and
invalidating the precise cache keys written by the sheets logic (e.g., the exact
keys for sheet list and individual records used elsewhere in update.ts or the
cache wrapper), or add a KV-specific invalidation path in WorkersKVCache to
enumerate and delete matching keys, or at minimum add a clear comment in
updateRecords referencing WorkersKVCache.invalidate’s limitation so future
readers know why wildcard invalidation won’t work.
In `@src/sdk/runtime.ts`:
- Around line 255-263: createSDKRuntime currently unconditionally registers
getTrackingData (via limiter.wrap('gmail', ...)) even when context.kv is absent,
causing deterministic throws; change the runtime construction so getTrackingData
is only added when context.kv is present (e.g., wrap the limiter.wrap
registration in an if (context.kv) branch) or move getTrackingData into a
Worker-only runtime/extension so non-Worker runtimes produced by
createSDKRuntime do not expose it; ensure the implementation still imports
../server/tracking.js and calls getTrackingData(opts, context.kv) only in the
guarded path and update any callers/factory code to stop expecting
getTrackingData on runtimes without KV.
In `@src/sdk/spec.ts`:
- Around line 631-643: The spec currently documents gmail.dryRun as synchronous;
update the signature string for dryRun to return a Promise (e.g.,
"dryRun(options: {...}): Promise<DryRunResult>") and change the returns
description to "Promise<{ to: string[], subject: string, body: string, isHtml:
boolean, wouldSend: false }>", then update the example to call it with await
(e.g., "const preview = await sdk.gmail.dryRun({...});") and ensure the example
context reflects async usage (async function or top-level await).
In `@src/server/tracking.ts`:
- Around line 85-87: The three one-line guard clauses checking parts (if
(parts.length !== 4), if (parts[0] !== 'track'), if (parts[3] !== 'pixel.gif'))
must be wrapped with braces to satisfy the repo's curly lint rule; update each
to use block form (e.g., if (condition) { return null; }) and do the same for
the similar single-line guard at the later location around line 92 so all guards
in the parsing logic (the checks using the parts array in this
tracking/path-parsing code) consistently use braces.
- Around line 118-148: The current read-modify-write in handleTrackingRequest
that loads TrackingSummary from kv.get and then kv.put causes lost updates under
concurrency; replace this with an atomic update model: move mutation/aggregation
into a single-authority (e.g., a Durable Object like CampaignTracker) that
receives the pixel hit and serializes updates, or else use per-key atomic
counters (campaign:{campaignId}:total and
campaign:{campaignId}:recipient:{recipientId}) and have the DO increment those
and update unique-open logic centrally; update handleTrackingRequest to call the
Durable Object (or increment per-key counters) instead of reading/modifying the
whole TrackingSummary blob, keep TTL behavior (KV_TTL_SECONDS) when persisting
final state, and stop using the shared summary.recipients read-modify-write
pattern in handleTrackingRequest.
In `@worker.ts`:
- Around line 162-167: parseTrackingPath currently returns
campaignId/recipientId without validation; add a strict ID validation (e.g.
const ID_PATTERN = /^[A-Za-z0-9_-]{1,64}$/) inside parseTrackingPath and reject
(return null) if parts[1] or parts[2] fail the pattern so malformed/oversized
IDs never propagate; ensure handleTrackingRequest checks for a null parse result
and returns an appropriate 4xx response rather than using the IDs in KV keys
like "tracking:summary:${campaignId}" or as object keys
summary.recipients[recipientId].
---
Duplicate comments:
In `@src/sdk/spec.ts`:
- Around line 443-469: The spec for createDraft and sendMessage is out of sync
with the runtime Gmail contract: change the recipient params to array-only
(to/cc/bcc as string[] not string | string[]), and add the supported
sender/threading fields (add optional from: string, references: string[] and
threadId: string where applicable) in both the signature and params sections for
createDraft and sendMessage (update the signature strings, params entries, and
examples to use array recipients and include from/references/threadId so agents
generate the correct request shape).
---
Nitpick comments:
In `@src/modules/gmail/detect-replies.ts`:
- Around line 39-88: detectReplies currently does the profile fetch and all
context.gmail.users.threads.get calls in one shot which can hit Gmail rate
limits for large options.threadIds arrays; change detectReplies to fetch the
profile first (context.gmail.users.getProfile) and then process threadIds in
configurable batches (e.g., add an options.batchSize or internal constant) and
either await a short delay between batches or use a simple concurrency limiter
(e.g., process N threads in parallel then await completion) when calling
context.gmail.users.threads.get; ensure the exception handling and threads.push
behavior remain unchanged and surface the batchSize as a recommendation in docs
or logs.
In `@src/modules/gmail/types.ts`:
- Around line 717-726: The BatchSendItemResult type currently requires messageId
and threadId even when status === 'failed'; change BatchSendItemResult to a
discriminated union (e.g., one variant for status: 'sent' including messageId
and threadId, and one for status: 'failed' including error) to make the shape
explicit and type-safe. Update the type declaration for BatchSendItemResult and
then fix any code that constructs or reads BatchSendItemResult (look for usages
of BatchSendItemResult, places that build batch send results, and consumers that
read messageId/threadId) to handle both variants (narrow on status before
accessing messageId/threadId or error).
In `@src/modules/sheets/__tests__/updateRecords.test.ts`:
- Around line 32-80: Add unit tests for edge cases around updateRecords: add a
test that sends multiple updates in one call and asserts the updated count and
that values.update was called for each matching row; add a test where updates
include unknown column names and assert those keys are ignored (no error, only
known headers written); add a test where the headers returned by
mockSheetsApi.spreadsheets.values.get do not contain the keyColumn and assert
updateRecords throws; and add a test where get returns no data/empty values and
assert updateRecords throws; use the same mockSheetsApi.spreadsheets.values.get
and .update mocks and reference updateRecords in each test to locate where to
add them.
In `@src/modules/sheets/update.ts`:
- Around line 341-354: The loop calling
context.sheets.spreadsheets.values.update per cell (iterating over update.values
using headers, columnToLetter, sheetPrefix, rowIndex) causes many API calls;
instead build a single rectangular values matrix for the contiguous range of
updated columns (or collect all updates into a single sparse matrix) and send
one batched request—either use spreadsheets.values.update once with the combined
range and requestBody.values matrix, or use spreadsheets.values.batchUpdate to
submit multiple ranges in one call; modify the code around update.values,
headers, columnToLetter, sheetPrefix and rowIndex to compute the minimal
start/end range, populate the 2D values array at the proper column offsets, and
replace the per-cell await context.sheets.spreadsheets.values.update calls with
a single batched update call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60b24636-3866-44eb-9b0b-9750bee6e863
📒 Files selected for processing (24)
docs/plans/2026-03-10-email-outreach-features.mdsrc/__tests__/sdk/runtime-rate-limiter-scope.test.tssrc/modules/gmail/__tests__/detectReplies.test.tssrc/modules/gmail/__tests__/dryRun.test.tssrc/modules/gmail/__tests__/sendBatch.test.tssrc/modules/gmail/__tests__/sendFromTemplate.test.tssrc/modules/gmail/__tests__/templates.test.tssrc/modules/gmail/compose.tssrc/modules/gmail/detect-replies.tssrc/modules/gmail/index.tssrc/modules/gmail/send.tssrc/modules/gmail/templates.tssrc/modules/gmail/types.tssrc/modules/sheets/__tests__/readAsRecords.test.tssrc/modules/sheets/__tests__/updateRecords.test.tssrc/modules/sheets/index.tssrc/modules/sheets/read.tssrc/modules/sheets/update.tssrc/sdk/runtime.tssrc/sdk/spec.tssrc/sdk/types.tssrc/server/__tests__/tracking.test.tssrc/server/tracking.tsworker.ts
✅ Files skipped from review due to trivial changes (1)
- src/modules/gmail/tests/sendFromTemplate.test.ts
Comment on lines
+359
to
+361
| // Invalidate cache | ||
| await context.cacheManager.invalidate(`sheet:${spreadsheetId}:*`); | ||
| context.performanceMonitor.track('sheets:updateRecords', Date.now() - context.startTime); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that WorkersKVCache.invalidate returns early for wildcard patterns
rg -n -A5 "async invalidate\(pattern" src/storage/kv-store.tsRepository: AojdevStudio/gdrive
Length of output: 373
🏁 Script executed:
# Get the full invalidate method implementation in kv-store.ts
cat -n src/storage/kv-store.ts | sed -n '115,126p'Repository: AojdevStudio/gdrive
Length of output: 530
🏁 Script executed:
# Get the cache invalidation code at lines 359-361 in update.ts
cat -n src/modules/sheets/update.ts | sed -n '359,361p'Repository: AojdevStudio/gdrive
Length of output: 266
🏁 Script executed:
# Check if there's a broader context around the invalidate call to understand the caching strategy
cat -n src/modules/sheets/update.ts | sed -n '350,370p'Repository: AojdevStudio/gdrive
Length of output: 688
Cache invalidation with wildcard patterns is a no-op in Workers KV environments.
At line 360, context.cacheManager.invalidate(sheet:${spreadsheetId}:*) passes a wildcard pattern. The WorkersKVCache.invalidate method returns early without action for patterns containing * (see src/storage/kv-store.ts:120-122), leaving stale data in cache after updateRecords completes.
Consider either:
- Building and invalidating the exact cache key instead of using a wildcard
- Implementing a different cache invalidation strategy for KV environments
- Adding a comment noting this is a known limitation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/sheets/update.ts` around lines 359 - 361, The cache invalidation
call in updateRecords uses a wildcard key
(context.cacheManager.invalidate(`sheet:${spreadsheetId}:*`)), but
WorkersKVCache.invalidate (see WorkersKVCache in src/storage/kv-store.ts) treats
patterns with '*' as a no-op, leaving stale entries; fix by constructing and
invalidating the precise cache keys written by the sheets logic (e.g., the exact
keys for sheet list and individual records used elsewhere in update.ts or the
cache wrapper), or add a KV-specific invalidation path in WorkersKVCache to
enumerate and delete matching keys, or at minimum add a clear comment in
updateRecords referencing WorkersKVCache.invalidate’s limitation so future
readers know why wildcard invalidation won’t work.
Comment on lines
+255
to
+263
| getTrackingData: limiter.wrap('gmail', async (opts: unknown) => { | ||
| if (!context.kv) { | ||
| throw new Error( | ||
| 'getTrackingData is only available in the Cloudflare Workers runtime (requires KV namespace)' | ||
| ); | ||
| } | ||
| const { getTrackingData } = await import('../server/tracking.js'); | ||
| return getTrackingData(opts as { campaignId: string }, context.kv); | ||
| }), |
There was a problem hiding this comment.
Don't expose getTrackingData on runtimes that can never satisfy it.
createSDKRuntime() adds this method unconditionally, but src/server/factory.ts never populates context.kv, so every stdio/server instance discovers an operation that deterministically throws. Either register it only when KV is available or split the Worker-only surface from the shared runtime contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/runtime.ts` around lines 255 - 263, createSDKRuntime currently
unconditionally registers getTrackingData (via limiter.wrap('gmail', ...)) even
when context.kv is absent, causing deterministic throws; change the runtime
construction so getTrackingData is only added when context.kv is present (e.g.,
wrap the limiter.wrap registration in an if (context.kv) branch) or move
getTrackingData into a Worker-only runtime/extension so non-Worker runtimes
produced by createSDKRuntime do not expose it; ensure the implementation still
imports ../server/tracking.js and calls getTrackingData(opts, context.kv) only
in the guarded path and update any callers/factory code to stop expecting
getTrackingData on runtimes without KV.
Comment on lines
+631
to
+643
| dryRun: { | ||
| signature: "dryRun(options: { to: string[], subject: string, template: string, variables: Record<string, string>, isHtml?: boolean }): DryRunResult", | ||
| description: "Preview a rendered templated email without sending. Renders {{variable}} placeholders in subject and body, validates recipients, returns the fully rendered email. Pure function — no API calls.", | ||
| example: "const preview = sdk.gmail.dryRun({\n to: ['amy@todaysdental.com'],\n subject: '{{firstName}}, quick follow-up',\n template: 'Hey {{firstName}},\\n\\n{{personalNote}}',\n variables: { firstName: 'Amy', personalNote: 'We rebuilt your claims sheet' },\n});\nconsole.log(preview.subject); // 'Amy, quick follow-up'\nconsole.log(preview.wouldSend); // false", | ||
| params: { | ||
| to: "string[] (required) — recipient email addresses", | ||
| subject: "string (required) — subject line with {{variable}} placeholders", | ||
| template: "string (required) — email body with {{variable}} placeholders", | ||
| variables: "Record<string, string> (required) — key-value map for placeholder replacement", | ||
| isHtml: "boolean (optional, default false) — when true, variable values are HTML-escaped", | ||
| }, | ||
| returns: "{ to: string[], subject: string, body: string, isHtml: boolean, wouldSend: false }", | ||
| }, |
There was a problem hiding this comment.
Advertise dryRun() as async in the spec.
The runtime wrapper still returns a Promise, but this signature and example present sdk.gmail.dryRun() as synchronous. Agents using the discovery spec will call it with the wrong shape unless this is changed to Promise<...> and the example uses await.
Suggested fix
- signature: "dryRun(options: { to: string[], subject: string, template: string, variables: Record<string, string>, isHtml?: boolean }): DryRunResult",
+ signature: "dryRun(options: { to: string[], subject: string, template: string, variables: Record<string, string>, isHtml?: boolean }): Promise<DryRunResult>",
...
- example: "const preview = sdk.gmail.dryRun({\n to: ['amy@todaysdental.com'],\n subject: '{{firstName}}, quick follow-up',\n template: 'Hey {{firstName}},\\n\\n{{personalNote}}',\n variables: { firstName: 'Amy', personalNote: 'We rebuilt your claims sheet' },\n});\nconsole.log(preview.subject); // 'Amy, quick follow-up'\nconsole.log(preview.wouldSend); // false",
+ example: "const preview = await sdk.gmail.dryRun({\n to: ['amy@todaysdental.com'],\n subject: '{{firstName}}, quick follow-up',\n template: 'Hey {{firstName}},\\n\\n{{personalNote}}',\n variables: { firstName: 'Amy', personalNote: 'We rebuilt your claims sheet' },\n});\nconsole.log(preview.subject); // 'Amy, quick follow-up'\nconsole.log(preview.wouldSend); // false",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/spec.ts` around lines 631 - 643, The spec currently documents
gmail.dryRun as synchronous; update the signature string for dryRun to return a
Promise (e.g., "dryRun(options: {...}): Promise<DryRunResult>") and change the
returns description to "Promise<{ to: string[], subject: string, body: string,
isHtml: boolean, wouldSend: false }>", then update the example to call it with
await (e.g., "const preview = await sdk.gmail.dryRun({...});") and ensure the
example context reflects async usage (async function or top-level await).
Comment on lines
+118
to
+148
| // Read existing summary (or start fresh) | ||
| const raw = await kv.get(kvKey); | ||
| const summary: TrackingSummary = raw | ||
| ? JSON.parse(raw) | ||
| : { | ||
| campaignId, | ||
| totalOpens: 0, | ||
| uniqueOpens: 0, | ||
| recipients: {}, | ||
| updatedAt: now, | ||
| }; | ||
|
|
||
| // Update recipient tracking | ||
| const existing = summary.recipients[recipientId]; | ||
| if (existing) { | ||
| existing.openCount += 1; | ||
| existing.lastOpenedAt = now; | ||
| } else { | ||
| summary.recipients[recipientId] = { | ||
| openCount: 1, | ||
| firstOpenedAt: now, | ||
| lastOpenedAt: now, | ||
| }; | ||
| summary.uniqueOpens += 1; | ||
| } | ||
|
|
||
| summary.totalOpens += 1; | ||
| summary.updatedAt = now; | ||
|
|
||
| // Write back to KV with 90-day TTL | ||
| await kv.put(kvKey, JSON.stringify(summary), { expirationTtl: KV_TTL_SECONDS }); |
There was a problem hiding this comment.
This KV update pattern will undercount concurrent opens.
handleTrackingRequest() reads the whole campaign summary, mutates it, and writes it back as one blob. Two pixel hits for the same campaign arriving close together can read the same snapshot and overwrite each other, corrupting totalOpens, uniqueOpens, and per-recipient counts. This needs an atomic storage model instead of a shared read-modify-write summary record.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/tracking.ts` around lines 118 - 148, The current read-modify-write
in handleTrackingRequest that loads TrackingSummary from kv.get and then kv.put
causes lost updates under concurrency; replace this with an atomic update model:
move mutation/aggregation into a single-authority (e.g., a Durable Object like
CampaignTracker) that receives the pixel hit and serializes updates, or else use
per-key atomic counters (campaign:{campaignId}:total and
campaign:{campaignId}:recipient:{recipientId}) and have the DO increment those
and update unique-open logic centrally; update handleTrackingRequest to call the
Durable Object (or increment per-key counters) instead of reading/modifying the
whole TrackingSummary blob, keep TTL behavior (KV_TTL_SECONDS) when persisting
final state, and stop using the shared summary.recipients read-modify-write
pattern in handleTrackingRequest.
Comment on lines
+162
to
+167
| // Tracking pixel route: GET /track/:campaignId/:recipientId/pixel.gif | ||
| // Handled before auth — tracking pixels are fire-and-forget from email clients. | ||
| if (request.method === 'GET' && url.pathname.startsWith('/track/')) { | ||
| const { handleTrackingRequest } = await import('./src/server/tracking.js'); | ||
| return handleTrackingRequest(request, env.GDRIVE_KV); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current validation in tracking.ts
rg -n -A10 'parseTrackingPath' src/server/tracking.tsRepository: AojdevStudio/gdrive
Length of output: 893
🏁 Script executed:
#!/bin/bash
# Check handleTrackingRequest function body
rg -n -A30 'function handleTrackingRequest|export.*handleTrackingRequest' src/server/tracking.tsRepository: AojdevStudio/gdrive
Length of output: 991
🏁 Script executed:
#!/bin/bash
# Check TrackingParams type definition
rg -n -B5 -A5 'type TrackingParams|interface TrackingParams' src/server/tracking.tsRepository: AojdevStudio/gdrive
Length of output: 308
Unauthenticated tracking route lacks input validation for path parameters.
The tracking pixel route bypasses authentication (intentionally for email client compatibility), but campaignId and recipientId are extracted from the URL path in parseTrackingPath (lines 81-90) and used directly without validation—both in KV key construction (tracking:summary:${campaignId} at line 116) and as object keys (summary.recipients[recipientId] at line 131).
Risks from missing validation:
- Unbounded length: No maximum length limit enables storage abuse and potential DoS
- Arbitrary characters: Accepting any string could cause data consistency issues or collisions with crafted IDs
- No format enforcement: No regex or type constraints on ID format
Add validation in parseTrackingPath to enforce ID format and length:
const ID_PATTERN = /^[a-zA-Z0-9_-]{1,64}$/;
if (!ID_PATTERN.test(parts[1]) || !ID_PATTERN.test(parts[2])) {
return null; // Reject invalid identifiers
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@worker.ts` around lines 162 - 167, parseTrackingPath currently returns
campaignId/recipientId without validation; add a strict ID validation (e.g.
const ID_PATTERN = /^[A-Za-z0-9_-]{1,64}$/) inside parseTrackingPath and reject
(return null) if parts[1] or parts[2] fail the pattern so malformed/oversized
IDs never propagate; ensure handleTrackingRequest checks for a null parse result
and returns an appropriate 4xx response rather than using the IDs in KV keys
like "tracking:summary:${campaignId}" or as object keys
summary.recipients[recipientId].
- Fix 9 ESLint `curly` errors across send.ts, update.ts, tracking.ts - Add recipient validation (to/cc/bcc) in sendFromTemplate matching dryRunMessage - Unwrap dryRun from rate limiter — pure function with zero API calls - Refactor sendBatch loop to for...of with entries() for type safety - Fix spec: to field uses string[] (not bare string), dryRun shows await - Document readAsRecords null behavior for sparse cells in JSDoc and spec - Add race condition comment on tracking KV read-modify-write - Update rate limiter test count (132 → 130) for dryRun unwrapping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 Type Coverage ReportType Coverage: 98.61% This PR's TypeScript type coverage analysis is complete. |
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
1 participant
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
isHtmlparameter insendMessageandcreateDraftSDK specs, enabling rich HTML email composition with clear opt-in semanticsdocs/specs/outreach-features.md) covering templates, sequences, personalization, and analyticsdocs/specs/outreach-gap-analysis.md) mapping current capabilities vs. required featuresType of Change
Test Plan
src/sdk/spec.tscorrectly documentsisHtmlparam for bothsendMessageandcreateDraftnpm run type-checkto ensure no TypeScript regressionsChecklist
Co-Authored-By: AOJDevStudio
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
API
Tests