Repository navigation
mailcore: a draft stays addressable by its local: id after reconcile_id (#1884, #1881) - #1891
Merged
Merged
Conversation
#1881's retried send needs a route that fails once and then succeeds. `FakeServer::start_limited` takes a use limit per route; a used-up route is skipped, so the next matching route answers. `start` keeps every route unlimited, so the existing `Route` literals are untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A compose window left open across SaveDraft, or a queued SendDraft
retried after a 503, still holds the draft's local: id. Once the drain
reconciled the draft to its Graph id, every call with that id touched no
row, so later edits were lost, the next SaveDraft/SendDraft failed "no
local draft stored" until it was quarantined, and the message was never
sent.
reconcile_id now keeps the local: id on the row (messages.local_id, with
a migration and a unique index), so the alias goes with the message on
every delete path. Every draft call resolves the id inside its one
statement, atomic against the sync thread's own connection, and
Store::current_id exposes the same rule. ensure_draft_on_graph decides
create vs update by the row's current id, so a reconciled local: id is
patched, never created twice. Delete resolves before the engine's local
delete and queues the resolved id; a local: id that never reached Graph
deletes nothing there instead of being quarantined.
The SaveDraft/SendDraft payloads and Sent { id } keep the id the caller
used. Five existing assertions that draft(local) is None after a
reconcile now assert it returns the Graph-id row.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A script can hand mail.delete a draft's local: id after the draft has been reconciled. The local delete removes the alias along with the row, so the id is resolved before it, and SyncCommand::Delete names the Graph id. The reply still echoes the id the caller passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The lookxy list can still hold a draft's local: id after the sync thread
reconciled it, since a SaveDraft drain reloads nothing. Deleting that row
then removed the Graph row along with its alias, and SyncCommand::Delete
{ local: } resolved to itself. OutboxOp::Delete returned Ok without any
Graph call, so the draft was gone locally but stayed on Graph, with no
error.
Store::delete_message_resolved runs DELETE ... RETURNING id, so the match
and the delete are one statement that a reconcile_id on the sync thread's
connection cannot split. Callers send Delete with the id it returns.
That covers lookxy's three list deletes (delete_and_send), mail.delete
(no longer two steps), and the engine's Delete arm; when the UI already
deleted the row, the arm falls back to current_id. The outbox comment and
CLAUDE.md now say that an id still resolving to local: is treated as
never having reached Graph, not that it is known to be.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wait for PendingOps(1) returned at once: enqueue_and_drain emits it before draining. A 503 is Throttled, so the engine emits no event at all and leaves the op's attempts untouched. A FetchBody of an unknown id, queued behind the SendDraft, is the barrier instead. Its 404 error arrives only once the send has been handled, and only then does the test check that nothing was sent and that the op is still queued. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SyncCommand::SaveDraft's doc still said a local: id is always created, and Compose::draft_id's doc said it becomes the Graph id. Neither holds any more. A local: id already reconciled to a Graph id is patched in place, and a compose window keeps the local: id it opened under, which the store resolves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Open
5 tasks
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Owner
Author
|
[linux] Merged automatically (auto-merge is on for this checkout): review stopped: round 2 had no Major; all findings fixed; merge rounds: none; whole suite green on 4933814 ( Follow-ups: the leftovers issue #1892, with the plan's five out-of-scope items:
No finding was disputed or deferred; no duplicate reports. |
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
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.
A draft stays addressable by its
local:id afterStore::reconcile_idrenames it to its Graph id. A compose window left open across aSaveDraftkeeps working: later edits, attachments, a secondSaveDraftandSendDraftreach the draft. A queuedSendDraft { local: }retried after a failed send finds its draft again.Closes #1884
Closes #1881
What changed
messages.local_id:reconcile_idkeeps thelocal:id there, only for alocal:id. The issue proposed an alias table; a column is used instead. It goes with the message on every delete path (delete_message, a delta@removed, an FK cascade), so no trigger or cleanup is needed and no orphan alias is possible. Migration: the swallowedALTER TABLElike the other columns, then aUNIQUEindex created inStore::init.draft_row_sql!(onmessages) andcurrent_id_sql!(forbodies/outbound_attachments) are spliced into the one SQL statement that acts. That keeps it atomic against areconcile_idthe sync thread commits on its own connection. Graph ids are never looked up (substr(?1,1,6) = 'local:'guard).draft(now oneJOIN bodiesstatement; the returned row'sidis the current id),update_draft_fields,mark_sent,move_message,delete_message,add/remove/clear_outbound_attachment(s),outbound_attachments.Storecalls:current_id(id);delete_message_resolved(id) -> Option<String>:DELETE … RETURNING id, one statement.ensure_draft_on_graphdecides create vs update from the stored row's current id, so an already-reconciledlocal:id is patched, never created twice. A queuedDeleteresolves its id; one stilllocal:makes no Graph call (it never reached Graph, short of a delete racing the create).SyncCommand::Delete, lookxy's UI deletes (App::delete_and_send: the DeleteThread confirm and bothdelete_selectedpaths) and themail.deleteautomation verb delete throughdelete_message_resolved. They sendSyncCommand::Deletewith the id of the row actually deleted. The list can still hold alocal:id after the sync thread reconciled it.SaveDraft/SendDraftpayloads keep the id they were addressed with, and so doesSyncEvent::Sent { id }, so a UI trackingpending_opsby its id still finds them.FakeServer::start_limited(routes with a use limit;startis unchanged). Also the 202/503 reason phrases.CLAUDE.md: "Mail drafts keep theirlocal:id (mailcore: a draft stays addressable by its local: id after reconcile_id #1884, mailcore: a queued draft op keeps its local: id after reconcile_id, so it can never be retried #1881)".The four assertions the issue named as intentionally changing (
draft(local)wasNoneafter reconcile) now assert the row with the Graph id. A fifth, thatoutbound_attachments(local)was empty after reconcile, now asserts the stored key is the Graph id and thelocal:id lists the same row.Testing
cargo fmt --check;cargo clippy -p mailcore -p lookxy --all-targets -- -D warnings;cargo test -p mailcore -p lookxy: mailcore 284 passed, lookxy 332 passed (24 new).Acceptance cases from #1884 (test server)
local:id, the edit through a secondStoreconnection: 1 POST, 1 PATCH carrying the edit, 1 send,Sent { id: local }.Throttled). A command barrier confirms the op is still queued; thenRefresh: 1 POST, 1 PATCH, 2 sends, oneSent { id: local }.local:id: the message is sent.Other tests
local_idfile, anddelete_message_resolved.local:delete.local:id sends the Graph id;mail.deletelikewise.Review record
revmux, 2 rounds (Claude-only profile).
r1:
local:id deleted the draft locally but never on Graph. Fixed withdelete_message_resolvedeverywhere.mail.deleteresolved and deleted in two statements.GraphError::Throttled, which emits no state change, so the test now waits on a command barrier.r2:
SyncCommand::SaveDraftdoc.Compose::draft_id's doc claimed it becomes the Graph id.Nothing disputed. review stopped: round 2 had no Major; all findings fixed
Out of scope, filed as follow-ups in #1892:
local:id;local:ids;busy_timeouton the store's connections.🤖 Generated with Claude Code