feat(gmail): wire draft writeback to the Gmail drafts API - #257
Conversation
Draft writes were declared everywhere but dispatched nowhere. `resources.ts`,
the discovery doc, the writeback-path catalog, and
`scripts/writeback-discovery-data.mjs` all advertise `/gmail/drafts`, but
`resolveWritebackRequest` collapsed every non-lifecycle path to
`resource: "threads"` with the `messages/{messageId}/modify` endpoint —
`parseRelayfilePath` could only return `object | lifecycle | unknown`. A draft
create was therefore dispatched as a label modify.
Changes:
- `queries.ts` declares the two missing provider actions: `draftCreate`
(`/gmail/v1/users/{account}/drafts`) and `draftWrite`
(`/gmail/v1/users/{account}/drafts/{draftId}`). The existing `gmail.modify`
scope already covers drafts, so no consent change is required.
- `parseRelayfilePath` recognizes a `drafts` resource under both addressing
forms — rootless `/gmail/drafts/<id>.json` (writeback-discovery) and
account-scoped `/gmail/<account>/drafts/<id>.json` (mounted) — across the
canonical and legacy roots.
- `resolveWritebackRequest` routes drafts to the drafts endpoints: create POSTs
to the collection, update PUTs the addressed draft (Gmail's drafts.update
replaces, unlike messages.modify's PATCH), delete addresses the draft id.
Thread and watch resolution are unchanged.
Testing:
- New `packages/gmail/src/writeback.test.ts` pins action, method, and endpoint
for drafts create/update/delete plus the unchanged thread and watch routes,
and asserts a draft update does NOT land on `objectWrite`.
- `integration.test.ts` now asserts the draft writeback's action, method, and
endpoint rather than only its operation.
- `scripts/storage-bridge-smoke.mjs`: renamed `resolveWriteback` to
`expectedWriteback` and documented that it is a fixture-consistency check.
It returned `item.createOperation` verbatim, so its
`createOperation: "gmail.drafts.create"` assertion compared the fixture to
itself and exercised no adapter code — which is why this gap survived.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughGmail writeback now supports draft creation, updates, and deletion. Draft paths are recognized across supported root forms. Routing selects draft-specific endpoints and methods while preserving thread and watch behavior. ChangesGmail draft writeback
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Writeback as writeback.ts
participant PathMapper as parseRelayfilePath
participant Queries as providerQueries
Writeback->>PathMapper: parse Gmail resource path
PathMapper-->>Writeback: return drafts, threads, or lifecycle resource
Writeback->>Queries: resolve resource-specific endpoint
Queries-->>Writeback: return draftCreate or draftWrite endpoint
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gmail/src/writeback.test.ts`:
- Around line 11-24: Remove the magic “new.json” create convention from the
draft resolver and its test. Update the draft create/update logic used by
resolveWritebackRequest so any non-canonical filename in the resource directory
resolves to a create, while canonical draft paths continue resolving as updates.
Replace the “new.json” test case with a neutral non-canonical filename and
preserve the expected POST/create assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 291b425e-cc41-4737-8124-78185b42a21b
📒 Files selected for processing (7)
CHANGELOG.mdpackages/gmail/src/__tests__/integration.test.tspackages/gmail/src/path-mapper.tspackages/gmail/src/queries.tspackages/gmail/src/writeback.test.tspackages/gmail/src/writeback.tsscripts/storage-bridge-smoke.mjs
![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: 3c4c846efe
ℹ️ 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".
| export const RELAYFILE_ROOT = GMAIL_PATH_ROOT; | ||
| export const OBJECT_RESOURCE_PATH = `${RELAYFILE_ROOT}/{account}/threads`; | ||
| export const LIFECYCLE_RESOURCE_PATH = `${RELAYFILE_ROOT}/watches`; | ||
| export const DRAFTS_RESOURCE_PATH = `${RELAYFILE_ROOT}/drafts`; |
There was a problem hiding this comment.
The new draft support exports only a collection constant, so consumers still have to hand-build both /gmail/drafts/<id>.json and /gmail/<account>/drafts/<id>.json; even the added tests use literal paths. Export a typed draft record-path helper and cover compose-to-parse round trips so the two accepted shapes cannot drift from the parser.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Review feedback: the draft resolver inherited the legacy reserved-prefix
heuristic (`draft|create|new|upload|tmp|temp`) plus a permissive
`^[A-Za-z0-9_-]+$` canonical check. A neutral filename that matched neither —
`ask-storebrand.json` — resolved as canonical and issued a PUT for a draft id
that does not exist. Routing drafts to the real drafts endpoint turned that
latent misclassification into a live bad request.
Drafts now follow the file-native writeback contract: a filename matching the
resource's declared `idPattern` edits that draft, anything else creates one, and
`new.json` has no special privilege.
- `writeback-discovery-normalizer.mjs` declares gmail's drafts id pattern as
`^r-?\d+$` — the real Gmail draft id shape (`r-4692061400304996596`). The
permissive default classified prose filenames as canonical. Regenerated
`resources.ts` and `.adapter.md` from it; no other adapter's artifacts are
touched.
- `resolveWritebackRequest` reads canonicality from `resources.ts` for drafts.
Threads and watches keep the legacy heuristic: gmail is absent from the
file-native migration table, and changing their create/update split would
alter behavior this change has no reason to touch.
- `path-mapper.ts` exports `toDraftRelayfilePath({ account?, id })` so both
accepted shapes are composed, not hand-built (AGENTS.md adapter contract).
Account segments keep a literal `@`; percent-encoding it would compose a path
that never matches a mounted file.
Tests: `new.json` replaced with neutral non-canonical names (including one with
a space, per the migration doc); added canonical-id-implies-update, a
compose-to-parse round trip for both path shapes, and direct id-pattern
coverage. 25 tests pass, typecheck clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Addressed both review comments in a630b2e. CodeRabbit — magic Drafts now derive create vs update from the resource's declared
Scope note: threads and watches keep the legacy heuristic. gmail is absent from the file-native migration table in cubic — typed draft path composer (P1). Added One real bug fell out of writing it:
|
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/gmail/src/resources.ts">
<violation number="1" location="packages/gmail/src/resources.ts:23">
P1: Existing drafts whose provider ID falls outside this undocumented numeric format will now be treated as create filenames, so saving them POSTs a duplicate rather than updating the draft. Avoid inferring canonicality from a private Gmail ID shape; carry an explicit operation/provider-ID marker, or preserve a compatible identifier strategy.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| path: "/gmail/drafts", | ||
| pathPattern: /^\/gmail\/drafts(?:\/[^\/]+(?:\.json)?)?$/, | ||
| idPattern: /^[A-Za-z0-9_.:-]+$/, | ||
| idPattern: /^r-?\d+$/, |
There was a problem hiding this comment.
P1: Existing drafts whose provider ID falls outside this undocumented numeric format will now be treated as create filenames, so saving them POSTs a duplicate rather than updating the draft. Avoid inferring canonicality from a private Gmail ID shape; carry an explicit operation/provider-ID marker, or preserve a compatible identifier strategy.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/gmail/src/resources.ts, line 23:
<comment>Existing drafts whose provider ID falls outside this undocumented numeric format will now be treated as create filenames, so saving them POSTs a duplicate rather than updating the draft. Avoid inferring canonicality from a private Gmail ID shape; carry an explicit operation/provider-ID marker, or preserve a compatible identifier strategy.</comment>
<file context>
@@ -20,7 +20,7 @@ export const resources = [
path: "/gmail/drafts",
pathPattern: /^\/gmail\/drafts(?:\/[^\/]+(?:\.json)?)?$/,
- idPattern: /^[A-Za-z0-9_.:-]+$/,
+ idPattern: /^r-?\d+$/,
schema: "discovery/gmail/drafts/.schema.json",
createExample: "discovery/gmail/drafts/.create.example.json",
</file context>
…d pattern Review feedback on the composer and the canonical pattern. - `encodeSingleSegment` replaces the slash-preserving encoder for draft stems and account segments. `encodePathSegment` restores `%2F` to `/`, so a stem like `q3/budget reply` expanded into two segments and shifted the id the parser reads off the end of the path — a write or delete could address a different draft. An account containing `/` was worse: the extra segment made the parser classify the path as `object`, misrouting the writeback away from drafts entirely. `@` is still restored for accounts so composed paths match the mounted tree. - Documented the drafts id pattern in the file-native migration table, scoped explicitly to drafts, and noted in `writeback.ts` that inference is a default a caller can bypass by passing `operation` outright. Tests pin both slash cases end to end: composition, parse-back, and the resulting writeback operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Second round addressed in 3a817b9. cubic P2 — slash in a draft stem ( cubic P2 — slash in an account ( cubic P1 — inferring canonicality from the Gmail id shape (
Happy to switch to an explicit marker if you'd rather gmail diverge from the shared contract — just say so and I'll change it. cubic P3 ( |
1 participant
Problem
Gmail drafts were declared everywhere but dispatched nowhere.
resources.ts, the discovery doc (.adapter.md: "Creates a Gmail draft."), the writeback-path catalog, andscripts/writeback-discovery-data.mjs(/gmail/drafts/new.json) all advertise a drafts resource — butresolveWritebackRequestcollapsed every non-lifecycle path toresource: "threads"with themessages/{messageId}/modifyendpoint, becauseparseRelayfilePathcould only returnobject | lifecycle | unknown.A draft create was therefore dispatched as a label modify.
queries.tsconfirms it from the other direction: it declared exactly two provider actions (objectWrite→messages/modify,lifecycleWrite→watch). There was no drafts endpoint anywhere in the adapter.Why this survived
scripts/storage-bridge-smoke.mjsappears to assertcreateOperation: "gmail.drafts.create". It doesn't. Its local helper was:It returns the fixture's own expected value, so the assertion compared the fixture to itself and exercised no adapter code.
integration.test.tsasserted only.operation === 'create'— never the action or endpoint, which is exactly where the bug lived.Changes
queries.ts— declares the two missing actions:draftCreate(/gmail/v1/users/{account}/drafts) anddraftWrite(/gmail/v1/users/{account}/drafts/{draftId}). The adapter already requestsgmail.modify, which covers draft creation, so no scope or consent change is needed.path-mapper.ts—parseRelayfilePathrecognizes adraftsresource under both addressing forms: rootless/gmail/drafts/<id>.json(writeback-discovery) and account-scoped/gmail/<account>/drafts/<id>.json(mounted), across canonical and legacy roots.writeback.ts— drafts route to the drafts endpoints: createPOSTs the collection, updatePUTs the addressed draft (Gmail'sdrafts.updatereplaces, unlikemessages.modify'sPATCH), delete addresses the draft id. Thread and watch resolution are byte-for-byte unchanged.Testing
packages/gmail/src/writeback.test.tspins action, method, and endpoint for drafts create/update/delete plus the unchanged thread and watch routes, and explicitly asserts a draft update does not land onobjectWrite.integration.test.tsnow asserts the draft writeback's action, method, and endpoint.scripts/storage-bridge-smoke.mjs— helper renamed toexpectedWritebackand documented as a fixture-consistency check, pointing at the adapter test for real resolution. No behavior change.Not in this PR
Cloud must also declare the resource.
cloud/packages/core/src/relayfile/provider-contracts.ts→GOOGLE_MAIL_RESOURCESlistslabels, filters, send-as, messages, threads, watch-renewals— nodrafts— so drafts won't materialize until that side lands too. Companion PR to follow.The
/gmailvs/google-mailroot is deliberately left alone.identity.tsdocuments a considered migration (canonicalWrites: canonical-only,legacyReads: supported,retireLegacyAfter: zero-references-and-explicit-cutover), andidentity.test.tspins it. Cloud still materializes/google-mail; that's the migration in progress, not a defect, so this PR doesn't touch it.Context
Found while benchmarking mail surfaces (Gmail MCP vs Superhuman MCP vs relayfile google-mail) for a set of personal-assistant personas. The claude.ai Gmail connector can't archive (missing modify scope) and relayfile couldn't draft — this closes the relayfile half.
🤖 Generated with Claude Code