Skip to content

fix: remove a saved draft once its email is sent - #170

Open
LUCKYREDDY31 wants to merge 2 commits into
CopilotKit:mainfrom
LUCKYREDDY31:fix/remove-sent-draft
Open

LUCKYREDDY31 wants to merge 2 commits into
CopilotKit:mainfrom
LUCKYREDDY31:fix/remove-sent-draft

Conversation

@LUCKYREDDY31

@LUCKYREDDY31 LUCKYREDDY31 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Sending a saved draft leaves the draft behind. In sample mode: open Mail, tap Compose, write an email and tap Save draft, open it from Drafts, tap Review email, then Approve locally. The email appears in All messages, but Drafts still lists it. Nothing ever removes a draft: the server only lists and saves drafts, and the send request does not say which draft it came from. There is also no way to delete a draft by hand, so the person sees an email that looks unsent and can easily send the same email twice.

What changed

  • packages/domain/src/index.ts: an email.send proposal accepts an optional draftId (z.uuid()), and ActionProposal keeps it. It sits next to data, not inside emailDraftSchema, so the model's prepare_email tool, which builds data from emailDraftSchema, cannot set it.
  • apps/server/src/actions.ts: propose rejects a draftId that does not exist for the owner (404), saves the new review's ID on the draft, and cancels any older pending review of that draft, so one draft has at most one pending send. decide removes the draft only when the send succeeded and the draft still points to that review. A declined, expired, failed or uncertain send keeps the draft. The removal runs after the send is recorded; if it fails, the error is logged with backgroundFailure and the send still reports success.
  • apps/server/src/app.ts: saving an existing draft cancels its pending review, because that review no longer matches the draft. The saved draft no longer points to the old review, so even a send already in progress keeps the rewrite.
  • apps/server/src/db.ts: removeIf deletes a record only while it still contains the expected fields, the delete counterpart of compareAndSwap.
  • apps/mobile/src/details.tsx: Review email sends the draft's ID with the proposal. Edit details reopens the same draft, but only when this tap declined the review. If the review was already replaced or sent elsewhere, the screen shows that outcome instead of an editor. This check applies to calendar reviews too.
  • tests/actions.test.ts: the draft is removed after a send, and kept when the send is declined, fails, has an unknown outcome or expires. A rewrite saved after the review started is kept, a newer review replaces the older one so the email is sent once, a failed cleanup still reports the send, and a draft from another owner or an unknown ID is rejected.
  • tests/api.test.ts: saving a draft again through the API cancels its pending review, so the older text is never sent.

Screenshots

Sample mode, web, same steps before and after: save a draft, open it from Drafts, tap Review email, tap Approve locally, then Done.

BeforeAfter
Drafts
after Save draft
before-1-draft-saved
The saved draft
after-1-draft-saved
The saved draft
Review
after Review email
before-2-draft-review-approve
Waiting for approval
after-2-draft-review-approve
Waiting for approval
Sent
after Approve locally
before-3-draft-email-sent
Saved in local Sent mail
after-3-draft-email-sent
Saved in local Sent mail
Drafts
after Done
before-4-sent-draft-still-listed
The sent email is still a draft
after-4-sent-draft-removed
The draft is removed

Verification

  • On main, the draft-removal test fails. On the first version of this PR (e01080e), the five tests added for the review fail: rewrite kept, newer review replaces the older one, failed cleanup, owner check, and re-save cancels the review. The expired-review test passes on both, because expired reviews already kept the draft.
  • tests/actions.test.ts and tests/api.test.ts passed 20 of 20 repeated runs.
  • pnpm lint, pnpm typecheck, pnpm --dir apps/worker typecheck, pnpm test (491/491) and pnpm build:server pass on Node 24. The web, iOS and Android exports, pnpm test:browser, and both Docker container suites also pass.
  • In the running app (the screenshots above, taken on main at 73a7149), the server listed 1 draft after a successful send without the fix and 0 with it. The newer commits on main do not change the send, review or drafts code shown there.
  • Also in the running app: rewriting a draft after starting its review cancelled that review, left it with no buttons, and kept the rewrite. Two reviews of one draft left the first cancelled and sent one copy. Tapping Edit details on a review screen that a newer review had replaced showed "cancelled" instead of an editor.
  • feat: pluggable thread persistence with THREADS_BACKEND=local #166 also changes app.ts and packages/domain/src/index.ts, and applies together with this change without conflicts. feat: run chat fully offline with CopilotKit Intelligence optional #94 adds a route directly below the draft save route in app.ts, so the two touch neighboring lines; feat: run chat fully offline with CopilotKit Intelligence optional #94 already conflicts with main on its own.

Integration limits

  • Tested in sample mode only. In live mode the same decide path removes the draft after the Gmail send succeeds, but I have not run it with a real Google account.
  • Drafts left behind by sends before this change are not cleaned up.
  • Edit details has no automated test, because the app has no component tests. I checked it by hand in the web app. The failed-cleanup path is covered by a test that forces a database error, not by the running app.

Security review

  • No new data flows, dependencies, configuration or environment variables.
  • The draft is removed through the existing owner-scoped store, and propose only accepts a draftId that exists for the same owner, so a proposal can only touch that owner's draft.
  • draftId must be a UUID. Proposals prepared by agent tasks never carry one, because the model only supplies the email fields.
  • Older reviews are cancelled with the existing compare-and-swap on their status, so a review that is already being approved is never changed.
  • A draft is removed only after the provider reports success, so a failed or uncertain send never loses the person's text.

Sending a saved draft left it in Drafts after the email went out. The
send request did not say which draft it came from, and nothing removed
drafts, so the person saw an email that looked unsent and could send it
twice.

The email.send proposal now carries an optional draftId, and the action
service removes that draft only after the send succeeds. A declined,
expired, failed or uncertain send keeps the draft. Edit details on the
review reopens the same draft instead of creating a copy.

@NathanTarbert NathanTarbert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LUCKYREDDY31. Removing a draft once its email goes out is the right behavior, and keeping the draft when a review is denied or fails makes sense too. A few situations can still lose someone's work or send an email twice, so I'd like to sort those out before merging.

Edits made after starting a review can be lost. Say someone opens a draft and taps Review email, then backs out without deciding. They reopen the draft, rewrite it and save. Later they approve the original review from Activity. The old text gets sent and the draft is deleted, so the rewrite is gone. Nothing checks that the draft still matches what was reviewed before removing it.

A failed cleanup looks like a failed send. The draft is removed after the email is already saved as sent (actions.ts:207), and nothing catches an error there. If removing the draft fails, for example because of a brief database error, the user sees an error even though the email went out. The draft is also still there. The natural next step is to send it again, which is the duplicate this PR is trying to prevent. Treating the removal as best effort, with a try/catch and a log line, would keep a cleanup problem from looking like a send problem.

One draft can be sent twice. Tapping Review email on the same draft twice creates two pending sends. Approving the first sends the email and removes the draft, but the second is still waiting in Activity, and approving it sends the same email again.

Edit details can break afterward. In that same case, opening the second pending send and tapping Edit details opens the editor for a draft that no longer exists. Saving then fails with "Draft not found," and the user has to start over.

I think all four come from the same place. The draft and its pending send aren't linked on the server, so each case needs its own special handling. One way to link them would be to store the pending send's id on the draft. A new review or a new save would then replace the older pending send, and sending would only remove the draft if it's still the version that was reviewed. Happy to talk through other approaches if you see a simpler one.

Two smaller things. propose accepts any draftId without checking that the draft exists for this user, so it's worth validating it there. And the new field uses z.string().uuid(), while the rest of the schema file uses Zod 4's z.uuid() style.

It would also be good to add tests for the cases above, and for an expired review keeping its draft, which the description mentions but no test covers yet.

A draft now records its pending review. Starting a new review or
saving the draft again cancels the older review, so one draft has at
most one pending send and older text cannot be sent over a rewrite.
A send removes the draft only if it still points to that review, and
a failed cleanup is logged instead of reported as a failed send.

propose rejects a draft that does not exist for the owner, and Edit
details opens the editor only when its own tap declined the review.
@LUCKYREDDY31

Copy link
Copy Markdown
Contributor Author

Thanks @NathanTarbert, this was a really useful review. I reproduced the first three cases on the previous commit and went with the linked approach you suggested. The draft now records its pending review:

  • Edits after starting a review: saving the draft again cancels its pending review, and a send removes the draft only if the draft still points to that review. Approving an older review can no longer send old text or delete a rewrite.
  • Failed cleanup: the removal runs after the send is recorded, and an error there is logged with backgroundFailure instead of failing the request, so the send still reports success.
  • One draft sent twice: starting a new review cancels any older pending review of the same draft, so a draft has at most one pending send.
  • Edit details after the draft is gone: a replaced review is cancelled and loses its buttons. If a review screen is already open when that happens, Edit details now shows the outcome instead of opening an editor. This check applies to calendar reviews too.
  • propose now rejects a draftId that does not exist for the owner, and the field uses z.uuid().
  • Tests: added tests for each case, plus an expired review keeping its draft. The five new behavior tests fail on the previous commit; the expired one passes on both, since expired reviews already kept the draft. Edit details has no automated test because the app has no component tests, so I checked it by hand in the web app.

pnpm test is now 491/491, and lint, types, the exports, the browser suite and both Docker suites pass. I updated the description to match.

This branch has not been deployed

No deployments
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