Skip to content

fix(drafts): stop clearing drafts on workspace switch and hide sent section #1708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wpfleger96 merged 3 commits into from
Jul 10, 2026
Merged

fix(drafts): stop clearing drafts on workspace switch and hide sent section #1708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fcaa0b1
fix(drafts): stop clearing drafts on workspace switch and hide sent s…
Jul 10, 2026
6eb0a36
fix(drafts): stop writing sent records and remove sent-section E2E test
Jul 10, 2026
759b1fa
fix(drafts): drop legacy sent records on read and clean up stale docs
Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 53 additions & 143 deletions desktop/src/features/messages/lib/useDrafts.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,12 @@ test("initDraftStore_resets_cache_on_pubkey_change_without_clearAllDrafts", () =
);
});

// ── status field: markDraftSent, getActiveDraftEntries, getSentDraftEntries ───
// markDraftSentEntry snapshots the draft into a distinct `sent:<key>:<ts>` key
// and removes the original active key so composer cleanup and new drafts are
// never affected by the sent record's lifecycle.
// ── markDraftSentEntry: clears active draft (no longer writes sent records) ──
// markDraftSentEntry now delegates to clearDraftEntry — it clears the active
// draft and writes nothing to the store. The sent-section UI was removed;
// these tests verify the new contract.

test("markDraftSent_writes_sent_record_under_distinct_key_and_removes_active_key", () => {
test("markDraftSent_clears_active_draft_and_writes_no_sent_record", () => {
setup();
persistDraftEntry("chan-1", "sent message content", "chan-1", [IMG_A], []);
markDraftSentEntry("chan-1", "sent message content", "chan-1", [IMG_A], []);
Expand All @@ -448,185 +448,67 @@ test("markDraftSent_writes_sent_record_under_distinct_key_and_removes_active_key
undefined,
"active key must be cleared after markDraftSent",
);
// Sent record must exist under a sent: key.
// No sent records written.
const sent = getSentDraftEntries();
assert.equal(sent.length, 1, "one sent entry must exist");
assert.equal(sent[0].draft.status, "sent", "status must be 'sent'");
assert.equal(
sent[0].draft.content,
"sent message content",
"content preserved",
);
assert.equal(sent[0].draft.pendingImeta.length, 1, "image preserved");
assert.equal(sent[0].draft.pendingImeta[0].url, IMG_A.url);
assert.ok(
sent[0].key.startsWith("sent:chan-1:"),
"sent key must have sent: prefix",
);
assert.equal(sent.length, 0, "no sent entries written");
// All-entries also empty.
assert.equal(getAllDraftEntries().length, 0, "store is empty after send");
});

test("markDraftSent_writes_sent_record_even_when_active_key_already_cleared", () => {
// The never-persisted boundary is enforced at the call site (sentDraftKey
// is only set when a draft was actually persisted). This function writes
// unconditionally so a navigation-during-send race cannot cause data loss:
// if the active key was already cleared before send success, the snapshot
// content still produces a sent record (createdAt falls back to now).
test("markDraftSent_is_a_no_op_when_active_key_absent", () => {
setup();
// Call without any prior persistDraftEntry — simulates the race where the
// active key was deleted by a composer cleanup before markDraftSent ran.
// active key was already cleared before markDraftSent ran.
markDraftSentEntry("no-such-key", "content", "chan-x", [], []);
assert.equal(
loadDraftEntry("no-such-key"),
undefined,
"active key still absent",
);
const sent = getSentDraftEntries();
assert.equal(
sent.length,
1,
"sent record is written even without a live active key",
);
assert.equal(sent[0].draft.content, "content");
assert.equal(sent[0].draft.status, "sent");
assert.ok(
sent[0].key.startsWith("sent:no-such-key:"),
"sent key has correct prefix",
);
});

test("markDraftSent_send_then_cleanup_preserves_sent_record", () => {
// Simulate the full composer lifecycle:
// 1. Draft exists on key A.
// 2. User sends -> markDraftSent(A) snapshots under sent:A:ts and clears A.
// 3. Composer cleanup calls persistDraft(A, "", ...) -> clearDraftEntry(A).
// The sent record under sent:A:ts must still exist after step 3.
setup();
persistDraftEntry("chan-A", "my draft", "chan-A", [IMG_A], []);
markDraftSentEntry("chan-A", "my draft", "chan-A", [IMG_A], []);
// Simulate composer cleanup: empty persist on the now-absent active key.
persistDraftEntry("chan-A", "", "chan-A", [], []);
const sent = getSentDraftEntries();
assert.equal(sent.length, 1, "sent record must survive composer cleanup");
assert.equal(sent[0].draft.content, "my draft");
});

test("markDraftSent_navigation_during_async_send_still_creates_sent_record", () => {
// Regression test for the async-send/navigation race (Thufir Pass-2 finding):
// 1. Persisted draft A exists at submit time.
// 2. Composer clears the editor (clearContent) then awaits onSend.
// 3. While onSend is in flight, user switches channel. MessageComposer
// cleanup runs persistDraftEntry(A, empty) -> clearDraftEntry(A) — active
// key is gone before send success.
// 4. Send succeeds; markDraftSentEntry(A, savedContent, ...) runs.
// The sent record MUST still be written from the passed-in snapshot.
setup();
persistDraftEntry("chan-race", "race draft", "chan-race", [IMG_A], []);
// Simulate step 3: active key cleared by navigation-during-send cleanup.
persistDraftEntry("chan-race", "", "chan-race", [], []);
assert.equal(
loadDraftEntry("chan-race"),
undefined,
"active key should be cleared (simulating race)",
);
// Simulate step 4: send succeeds, mark sent with full snapshot.
markDraftSentEntry("chan-race", "race draft", "chan-race", [IMG_A], []);
const sent = getSentDraftEntries();
assert.equal(
sent.length,
1,
"sent record must be written despite active key being gone",
);
assert.equal(
sent[0].draft.content,
"race draft",
"snapshot content preserved",
);
assert.equal(
sent[0].draft.pendingImeta.length,
1,
"snapshot image preserved",
);
assert.equal(sent[0].draft.status, "sent");
// Still no sent record written.
assert.equal(getSentDraftEntries().length, 0, "no sent entries");
assert.equal(getAllDraftEntries().length, 0, "store still empty");
});

test("markDraftSent_new_active_draft_after_send_is_independent", () => {
// After sending, a new draft typed in the same channel must appear in
// getActiveDraftEntries() as active, and the sent record must remain in
// getSentDraftEntries() -- they coexist under distinct keys.
// getActiveDraftEntries() — markDraftSent must not affect it.
setup("pubkey-coexist");
persistDraftEntry("chan-X", "original draft", "chan-X", [], []);
markDraftSentEntry("chan-X", "original draft", "chan-X", [], []);
// New draft in the same channel.
persistDraftEntry("chan-X", "new draft after send", "chan-X", [IMG_B], []);
const active = getActiveDraftEntries();
const sent = getSentDraftEntries();
assert.equal(active.length, 1, "one active draft");
assert.equal(active[0].draft.content, "new draft after send");
assert.equal(active[0].draft.status, "active");
assert.equal(sent.length, 1, "one sent record");
assert.equal(sent[0].draft.content, "original draft");
assert.equal(sent[0].draft.status, "sent");
});

test("markDraftSent_double_send_in_same_channel_creates_two_distinct_sent_records", () => {
// Sending twice from the same channel must produce two independent sent
// records -- the timestamp suffix prevents key collision.
setup("pubkey-double-send");
persistDraftEntry("chan-Y", "first draft", "chan-Y", [], []);
markDraftSentEntry("chan-Y", "first draft", "chan-Y", [], []);
// Second draft in the same channel.
persistDraftEntry("chan-Y", "second draft", "chan-Y", [], []);
markDraftSentEntry("chan-Y", "second draft", "chan-Y", [], []);
const sent = getSentDraftEntries();
assert.equal(sent.length, 2, "two distinct sent records");
const contents = sent.map((e) => e.draft.content).sort();
assert.deepEqual(contents, ["first draft", "second draft"]);
const keys = sent.map((e) => e.key);
assert.notEqual(keys[0], keys[1], "sent keys must be distinct");
// No sent records.
assert.equal(getSentDraftEntries().length, 0, "no sent records");
});

test("getActiveDraftEntries_returns_only_active_drafts", () => {
test("getActiveDraftEntries_excludes_cleared_drafts", () => {
setup("pubkey-active");
persistDraftEntry("chan-active", "active draft", "chan-active", [], []);
persistDraftEntry("chan-sent", "sent draft", "chan-sent", [], []);
markDraftSentEntry("chan-sent", "sent draft", "chan-sent", [], []);
const active = getActiveDraftEntries();
assert.equal(active.length, 1, "only one active draft");
assert.equal(active.length, 1, "only one active draft remains");
assert.equal(active[0].key, "chan-active");
assert.equal(active[0].draft.status, "active");
});

test("getSentDraftEntries_returns_only_sent_drafts", () => {
test("getSentDraftEntries_returns_empty_after_markDraftSent", () => {
setup("pubkey-sent");
persistDraftEntry("chan-active2", "still drafting", "chan-active2", [], []);
persistDraftEntry("chan-sent2", "already sent", "chan-sent2", [], []);
markDraftSentEntry("chan-sent2", "already sent", "chan-sent2", [], []);
// markDraftSentEntry now just clears the active entry — no sent record.
const sent = getSentDraftEntries();
assert.equal(sent.length, 1, "only one sent draft");
assert.ok(sent[0].key.startsWith("sent:chan-sent2:"), "sent key has prefix");
assert.equal(sent[0].draft.status, "sent");
});

test("getActiveDraftEntries_and_getSentDraftEntries_partition_all_entries", () => {
setup("pubkey-partition");
persistDraftEntry("ch-a", "draft a", "ch-a", [], []);
persistDraftEntry("ch-b", "draft b", "ch-b", [], []);
persistDraftEntry("ch-c", "draft c", "ch-c", [], []);
markDraftSentEntry("ch-b", "draft b", "ch-b", [], []);
const all = getAllDraftEntries();
assert.equal(sent.length, 0, "no sent entries");
// The remaining active draft is still there.
const active = getActiveDraftEntries();
const sent = getSentDraftEntries();
// ch-a, ch-c still active; sent:ch-b:ts is the sent record.
assert.equal(all.length, 3);
assert.equal(active.length + sent.length, all.length, "active + sent = all");
assert.ok(
active.every((e) => e.draft.status === "active"),
"all active entries have status active",
);
assert.ok(
sent.every((e) => e.draft.status === "sent"),
"all sent entries have status sent",
);
assert.equal(active.length, 1, "one active draft");
assert.equal(active[0].key, "chan-active2");
});

// ── status migration: pre-status entries read as "active" ────────────────────
Expand Down Expand Up @@ -682,3 +564,31 @@ test("pre_status_entry_appears_in_getActiveDraftEntries_after_migration", () =>
assert.equal(active[0].key, "chan-old");
assert.equal(active[0].draft.status, "active");
});

test("pre_status_sent_entry_is_dropped_on_read", () => {
// Entries previously written with status "sent" (by the old markDraftSentEntry)
// used a "sent:" key prefix. readStore now skips those keys entirely so legacy
// sent records cannot resurface as ghost drafts.
setup("pubkey-normalise");
const oldSentEntry = {
content: "a message I sent a while ago",
selectionStart: 27,
selectionEnd: 27,
channelId: "chan-z",
createdAt: "2025-09-01T00:00:00.000Z",
updatedAt: "2025-09-01T00:00:00.000Z",
pendingImeta: [],
spoileredAttachmentUrls: [],
status: "sent",
};
localStorage.setItem(
"buzz-drafts.v1:pubkey-normalise",
JSON.stringify({ "sent:chan-z:1725148800000-1": oldSentEntry }),
);
clearAllDrafts();
initDraftStore("pubkey-normalise");
// The sent: key is skipped — the entry must NOT appear as an active draft.
const active = getActiveDraftEntries();
assert.equal(active.length, 0, "old sent: entry is dropped, not promoted");
assert.equal(getSentDraftEntries().length, 0, "no entries read as sent");
});
86 changes: 25 additions & 61 deletions desktop/src/features/messages/lib/useDrafts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ export type DraftState = {
/** URLs of imeta attachments marked as spoilered. */
spoileredAttachmentUrls: string[];
/**
* Lifecycle status of this draft.
* - "active": draft is in progress (not yet sent).
* - "sent": draft was sent; kept for the Drafts inbox "Sent" subsection.
* Lifecycle status of this draft. Always `"active"` at runtime.
* The `"sent"` value is not written by any production path; legacy `sent:`
* keyed records from older builds are dropped on read by `readStore`.
* Entries persisted before this field was added have no status field —
* the read path treats absent status as "active" (see `isValidDraftState`).
* the read path treats absent status as `"active"` (see `isValidDraftState`).
*/
status: "active" | "sent";
};
Expand All @@ -77,10 +77,6 @@ const MAX_DRAFTS = 100;
/** Module-level pubkey set by `initDraftStore`. Empty string = no identity. */
let currentPubkey = "";

/** Monotonically-incrementing counter used to guarantee unique sent-record keys
* even when two sends happen within the same millisecond (e.g. in tests). */
let _sentSeq = 0;

function storageKey(): string {
return `${DRAFT_STORE_KEY_PREFIX}:${currentPubkey}`;
}
Expand Down Expand Up @@ -140,6 +136,12 @@ function readStore(): Map<string, DraftState> {
!Array.isArray(parsed)
) {
for (const [key, value] of Object.entries(parsed as StoredDrafts)) {
// Drop legacy sent: records — they were written by the old
// markDraftSentEntry and have no role now that the Sent section is
// removed. Skipping here compacts them out on the next flush.
if (key.startsWith("sent:")) {
continue;
}
if (isValidDraftState(value)) {
map.set(key, value);
}
Expand Down Expand Up @@ -169,11 +171,13 @@ function isValidDraftState(v: unknown): v is DraftState {
return false;
}
// Migration: entries written before the status field was introduced have no
// status. Treat absent/invalid status as "active" rather than rejecting the
// entry — this avoids data loss on first run after the upgrade.
// status field. Treat absent status as "active" to avoid data loss on the
// first run after the upgrade.
// Legacy sent: keys are skipped by readStore before reaching this function;
// reject any remaining entry whose status is not "active".
if (d.status === undefined || d.status === null) {
(d as DraftState).status = "active";
} else if (d.status !== "active" && d.status !== "sent") {
} else if (d.status !== "active") {
return false;
}
return true;
Expand Down Expand Up @@ -291,7 +295,7 @@ export function getActiveDraftEntries(): Array<{

/**
* Returns only sent drafts, sorted most-recently-updated first.
* Used by the "Sent" subsection of the Drafts inbox panel.
* Returns empty — sent records are dropped on read. Kept for test assertions.
*/
export function getSentDraftEntries(): Array<{
key: string;
Expand All @@ -301,60 +305,20 @@ export function getSentDraftEntries(): Array<{
}

/**
* Mark a draft as sent by writing its content to a durable sent-record key.
*
* The active draft key is simultaneously cleared so the composer can create
* a fresh draft in the same channel without inheriting the sent status, and so
* the composer's empty-content cleanup can never delete the sent record.
* Clear the active draft entry for a sent draft.
*
* The sent record is stored under `sent:<draftKey>:<timestamp>` — a key the
* composer never writes to — so active and sent records for the same channel
* can coexist in the store independently.
*
* The "never-persisted draft writes no sent record" boundary is enforced at
* the call site: callers only invoke this function when `sentDraftKey` is
* non-null, which only holds for drafts that were persisted before submit.
* This function writes unconditionally so the sent record is created even
* when the active key was already cleared by a composer cleanup that raced
* the async send (e.g. the user switched channels while send was in flight).
* Kept as a named export so callers (`useMentionSendFlow`) don't need
* updating. Previously wrote a visible sent-record to the store; the
* sent section has been removed, so we now just clear the active draft.
*/
export function markDraftSentEntry(
draftKey: string,
content: string,
channelId: string,
pendingImeta: ImetaMedia[],
spoileredAttachmentUrls: string[],
_content: string,
_channelId: string,
_pendingImeta: ImetaMedia[],
_spoileredAttachmentUrls: string[],
): void {
const map = readStore();
const existing = map.get(draftKey);
const now = new Date().toISOString();
// Use the live entry's createdAt when available; fall back to now when the
// active key was already cleared by a navigation-during-send race. Either
// way the sent record is written — the race cannot cause data loss.
const createdAt = existing?.createdAt ?? now;
// Write the sent record under a stable, distinct key so it can never be
// overwritten by the composer's active-draft persist path.
// The `Date.now()-seq` suffix guarantees uniqueness even if two sends in the
// same channel happen within the same millisecond.
const sentKey = `sent:${draftKey}:${Date.now()}-${++_sentSeq}`;
map.set(sentKey, {
content,
selectionStart: content.length,
selectionEnd: content.length,
channelId,
createdAt,
updatedAt: now,
pendingImeta,
spoileredAttachmentUrls,
status: "sent",
});

// Clear the active draft key (if still present) so the composer starts fresh
// and any subsequent empty-content persist doesn't encounter the sent record.
map.delete(draftKey);
evictOldest(map);
flushStore(map);
notifySubscribers();
clearDraftEntry(draftKey);
}

// ── Reactive hooks ────────────────────────────────────────────────────────────
Expand Down
Loading