Repository navigation
feat(provider-gmail): delete drafts under the default gmail.compose scope - #219
Conversation
…cope delete_email on Gmail now deletes drafts through users.drafts.delete, which gmail.compose authorizes, so no new scope or restricted-scope verification is needed. It accepts a draft id or the draft's message id (resolved through drafts.list). The delete is permanent, as Gmail has no draft trash, and stays behind AGENT_EMAIL_DELETE_ENABLED plus user_explicitly_requested_deletion. Sent or received mail is refused with NOT_SUPPORTED naming gmail.modify. update_draft, getDraft, and getDraftReplyStatus accept a draft's message id too, so a draft found via list_emails(folder: drafts) can be edited. delete_email now returns provider failures as structured errors. Closes #218
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59f5870489
ℹ️ 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".
| if (err instanceof MessageNotFoundError) { | ||
| return { success: false, error: { code: 'MESSAGE_NOT_FOUND', message: err.message, recoverable: false } }; |
There was a problem hiding this comment.
Recognize not-found errors across package copies
When a provider resolves a different installed copy of @usejunior/email-core, its MessageNotFoundError fails this instanceof check and is returned as DELETE_FAILED instead of the promised MESSAGE_NOT_FOUND. This package-duplication scenario is explicitly handled for ProviderError by the structural isProviderError guard in providers/provider.ts, so the new not-found mapping needs an equivalent structural guard rather than relying solely on class identity.
Useful? React with 👍 / 👎.
Addresses peer review on #219. googleapis retries a DELETE after a 5xx, so a first attempt that removed the draft makes the retry see 404; that is now a successful delete rather than MESSAGE_NOT_FOUND. A client without drafts.list reports MessageNotFoundError for a non-draft id instead of the raw 404. Adds an email-security MODIFIED Delete Policy delta for the permanent-delete carve-out covering never-delivered items. Refs #218
Peer review: Codex
|
| Severity | Finding | Fix |
|---|---|---|
| should-fix | A permanent draft delete under the soft gate contradicts email-security "Default to soft delete" | Added email-security MODIFIED Delete Policy delta: never-delivered items a provider cannot trash may be deleted under the soft gates; hard_delete: true still needs the hard gate; delivered mail keeps the soft default |
| should-fix | googleapis retries DELETE after a 503. If the first attempt deleted the draft, the retry's 404 was reported as MESSAGE_NOT_FOUND |
A drafts.delete 404 after successful resolution now counts as success. Regression test added |
| nit | An old client without listDrafts let the raw 404 through as NOT_FOUND |
Normalized to MessageNotFoundError. Test added |
Probes passed for: stale and churned message ids failing closed, pagination, update_draft by message id, non-draft handling on every entry point, and no demonstrated wrong-draft deletion. A synthetic draft-id/message-id collision resolves to the draft namespace first.
Post-fix checks: build, lint, test:run (all green), spec coverage PASS, openspec validate --strict valid.
✅ Post-merge smoke passedMerged: Steps
Notes, for full disclosure:
Visual evidence: n/a (no UI) Cleanup
|
Releases #219 (Gmail draft-only delete under the default gmail.compose scope), which landed on main after the 0.3.4 tag. Agents run the installed server, not main, so draft deletion stays unavailable until this ships.
The change shipped in #219 and was released in 0.3.5. Archiving folds its requirements into the email-categorize, email-security and provider-gmail specs.
Closes #218
What
deleteMessage, drafts only. Callsusers.drafts.delete, whichgmail.composeauthorizes, so there's no new scope and no restricted-scope verification. It accepts a draft id (r…) or the draft's message id; a message id is resolved throughusers.drafts.list, following pagination. The delete is always permanent (Gmail has no draft trash), whateverhard_deletesays.NOT_SUPPORTED: "Only drafts can be deleted with the current Gmail scopes … requires gmail.modify, which Agent Email does not request." No modify endpoint is called.AGENT_EMAIL_DELETE_ENABLED=trueplususer_explicitly_requested_deletion: true. A Gmail draft delete doesn't needAGENT_EMAIL_HARD_DELETE_ENABLED: it's irreversible, but the draft was never sent. Passinghard_delete: truestill requires the hard gate.update_draft,getDraft, andgetDraftReplyStatuson Gmail also accept the draft's message id (the idlist_emails(folder: "drafts")returns), using the same resolver. A non-draft returnsNOT_A_DRAFT.delete_emailerrors are structured. Provider failures come back asNOT_SUPPORTED/MESSAGE_NOT_FOUND/DELETE_FAILEDinstead of being thrown.add-gmail-draft-deleteare updated.Out of scope:
send_draftstill takes only the draft id, because its duplicate-send guard keys on the id the caller passes, and accepting both ids would let the same draft get past the guard twice. The optionalretire_drafthelper is also left out.Verification
npm run build,npm run lint,npm run test:run(all green),check:spec-coveragePASS,openspec validate add-gmail-draft-delete --strictvalid.delete_email, andupdate_draftby message id. Also 2 googleapis-client tests and 3 core action error-mapping tests.update_draftby message id: success, subject updateddelete_emailby draft id:{"success":true}. Running it again:MESSAGE_NOT_FOUNDdelete_emailby message id:{"success":true}, and the draft no longer appears inlist_emails(folder: drafts)delete_emailon an inbox message:NOT_SUPPORTEDnaminggmail.modifyDELETE_DISABLEDdrafts.update, while the draft id stays stable. An old message id therefore getsMESSAGE_NOT_FOUNDafter an edit. This is now documented in the README and thedelete_emailid description.