Skip to content

cmux NEXT Chief chat: delete my messages; the Chief forgets them (#36019) - #36057

Merged
austinywang merged 8 commits into
feat-cmux-nextfrom
feat-cmux-next-36019-chief-message-delete
Nov 5, 2026
Merged

austinywang merged 8 commits into
feat-cmux-nextfrom
feat-cmux-next-36019-chief-message-delete

Conversation

@austinywang

@austinywang austinywang commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #36019: in a cmux NEXT Chief chat the user could not delete their own messages.

What changed

Mac Home transcript (MessagesLabHome)

  • My committed message in a one-to-one Chief chat gets Delete in its context menu (last group, trash icon).
  • The row leaves at once: the projection shows the message retracted from the click on (HomeProjection.deleting), so no rebuild brings it back before the owner answers. The owner then retracts it (message.retract, the existing op). A refusal brings the row back and reports through onRefusal.
  • A Chief chat draws no row for a retracted message (HomeMapping.hidesRetracted, HomeDiff emits MessagesLab's .delete instead of .unsend). The message stays gone after reopening the chat and restarting the app, because the owner keeps the retraction. Other chats keep Messages' "You unsent a message" note.
  • The conversation list clears the deleted message's preview at the press too: HomeMirror.inboxRows overlays a pending message.retract on the newest message the way it already overlays pin, mute and read-cursor intents, and a refused delete shows the preview again. Before, the list kept the deleted text for about a second, until the owner's retraction arrived. iPhone Undo Send gets the same instant preview.
  • New string home.menu.deleteMessage ("Delete"), translated into all 21 locales (copied from the existing Not Delivered "Delete").

Chief (optchat-chief)

  • A retraction of a logged message is now logged as deleted <id>, the entry kind the view log already had, so the next turn's delta names the line as deleted.
  • The message's own text in the memory is written over with the tombstone (deleted by the user) through the Memory Inspector's change book (Editor::delete_message). That gives it a backup, a journal record, rebuilt summaries above it and an undo, exactly like an inspector delete. The Memory Inspector shows the tombstone and the delete in its change list.
  • The inspector and the brain now share one Editor, so there is one journal and one rebuild worker.
  • A replayed update (a reconnect's catch-up) does not delete again. The note's key is unchanged (:retracted), so a message an older build already logged is not logged twice.

iOS parity: the phone's Chief chat already offers Undo Send (2 minutes, the same message.retract), so the Chief's memory forgets those messages too. A Delete with no time limit that leaves no note on the phone is recorded as a partial row in cmux-next-ios/verify/parity-mac.md.

Tests

Regression tests were committed before the fix:

  • Native/OptChat/optchat-chief/tests/chat_delete.rs drives the brain. It checks that a retracted message's memory copy becomes the tombstone, that the change book journals a delete, that the next turn's input has |deleted <id> and no part of the text, that a replay is not a second delete, and that after a host restart the memory still holds only the tombstone.

    • Base (0a4e84d, test only), rbx lane nx-36019-red: fails at chat_delete.rs:60, where the memory still holds the deleted text.
    • Head: f93a011, rbx lane nx-36019-fix: both chat_delete tests pass (cargo test --locked --test chat_delete), along with delta_edit, reply_guarantee, memory_edit, inspect, chief_continue_retry and chief_inbox. A second test, a_message_deleted_while_the_chief_was_down_leaves_its_memory_on_reconnect_and_an_undo_holds, came from review: it also fails on the base and passes on the head.
  • Packages/Shared/CmuxMessagesLab/Tests/MessagesLabHomeTests/HomeChiefDeleteTests.swift drives the projection over the mock owner. It checks that Delete removes the row before the owner answers, that the owner retracts the message, and that a reopened chat shows no row and no unsent note. It also checks that the Chief's messages and ordinary chats offer no Delete.

    • Base (c5ad364, test only), fleet step c19e9f414251821263e15639 (scripts/ci/package-test-lane.sh suite Packages/Shared/CmuxMessagesLab Suites, Xcode 26.6): fails, because canDeleteMessage and deleteMessage do not exist.
    • Head (f93a011), fleet step 99018931a17a3086241892b4, the same command: HomeChiefDeleteTests, HomeChiefRetryTests and HomeRetrySendTests pass.
    • cedc76e adds only the DEBUG socket's menu choose path that the GUI dogfood below uses (one line in DebugHomeNativeFixture.swift, one pass-through); the tests above ran at f93a011.
  • Packages/Shared/CmuxHomeCore/Tests/CmuxHomeCoreTests/MirrorAndIntentLogTests.swift aPendingRetractClearsTheNewestMessagesPreviewUntilRefused checks that the list's preview is empty while the delete is pending and comes back on a refusal.

    • Base (bf58b72, test only), fleet step 55454276a155a675cf118ce7 (scripts/ci/package-test-lane.sh suite Packages/Shared/CmuxHomeCore MirrorAndIntentLogTests): fails at MirrorAndIntentLogTests.swift:71, where the preview still holds the deleted text.
    • Head (be92280), fleet step 3412dbb85084c7a04478075e, the same command: all 7 tests pass.
    • At the head, f77f606, the whole CmuxHomeCore package passes (... suite Packages/Shared/CmuxHomeCore CmuxHomeCoreTests, 288 tests in 51 suites, step 9724382a2665b889f4042730). HomeChiefDeleteTests, HomeChiefRetryTests and HomeRetrySendTests also pass at the head (step a6d780b59316818372aac960).

Changelog

  • Fixed: You can delete your own messages in a Chief chat; they leave the chat at once, stay gone after a restart, and the Chief forgets them.

Observed / Not run

Ran and saw:

  • The tests above, on the fleet and rbx, with each base failure and head pass as listed. Nothing was compiled or tested locally.
  • GUI dogfood on the real tagged app, fleet build a252943d07c19a4658db17a7 of the head f77f606, tag 36019-chief-chat-message-delete. It ran in an isolated scratch Chief home through the app's DEBUG socket, never the person's cmux. The steps:
    • I sent "dogfood N: my locker code is 4417" to the Chief and waited until the Chief's memory (optchat/memory.sqlite3) held it.
    • I read my bubble's real context menu. Its rows, in order: the two tapback palettes, Tapback Details…, Reply…, Attach Sticker…, Copy, Copy Link, Forward…, Share… and Delete. There is a separator before Copy, before Share… and before Delete.
    • I chose Delete through the menu item's own action (debug.home.drive menu {choose}).
    • The row left the transcript with MessagesLab's standard removal: a 0.2 s fade, then the ghost leaves the layout after 1.0 s. The row count went to 0 at 1.2 to 1.3 s in two runs (the ghost's lifetime).
    • The owner's snapshot holds the message retracted.
    • optchat/memory.sqlite3 no longer holds the text.
    • memory-edits.jsonl has the delete (message 0 changed to (deleted by the user)) and the rebuilds of summaries 0+1 and 0+2.
    • After quitting and relaunching, the chat shows no bubble, only the Chief's reply, and the memory still has no copy of the text.
    • I also ran this flow on the earlier builds. One of those runs found the list-preview bug fixed in be92280: the list kept the deleted text for about a second.
  • Copies of the text that stay, by design:
    • The change book's backup of the memory taken before the change (optchat/backups/memory-*.sqlite3). It is what makes the Memory Inspector's undo work, the same as an inspector delete.
    • The owner's append-only change log (tui/<chief>/conversations.sqlite3), which keeps the original send event. That is existing message.retract behavior and the same for Undo Send. Purging it is a follow-up, not in this PR.
  • One-off stall: once in five runs, the menu choose RPC took 13 s; the others took 13 to 678 ms. I could not reproduce it under sample (44 ms), and deleteMessage itself does one projection pass and hands off the op, so the cause is unknown.

Not run, or only read:

  • The open menu's look: not captured. Showing the real NSMenu needs a real right-click with the person's pointer. The row is the stock MenuAction with the trash symbol.
  • Fade-in-progress frames: not captured as timed frames (a window snapshot takes 0.2 to 1.6 s).
  • iPhone: not run. Its Chief chat keeps Undo Send (the same message.retract, so the Chief's memory forgets those too), and the list preview fix applies there as well. A Delete with no time limit on the phone is a partial row in cmux-next-ios/verify/parity-mac.md.
  • The first delete's backup cost on the brain thread: read, not measured. It happens once per Chief process, the same as an inspector delete.
  • Retract intents across an app quit: not run. They aren't saved in the client cache, so a quit before the owner's echo shows the old preview after relaunch until the first inbox fetch. That gap already existed for Undo Send.

🤖 Generated with Claude Code

austinywang and others added 3 commits November 4, 2026 18:57
…hief's memory (#36019)

The brain gets a memory editor seam (set/with_memory_editor) and the
change book a delete_message entry; nothing calls them yet, so the test
fails on the tombstone assertion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…6019)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ef's memory (#36019)

Mac Home transcript: my committed message in a Chief chat gets Delete in its
context menu. The row leaves at once (the projection shows it retracted
until the owner's echo), the owner retracts it (message.retract), and a
Chief chat draws no row for a retracted message, so it stays gone after
reopen and restart. A refusal brings the row back.

Chief: a retraction is logged as 'deleted <id>' (the next turn's delta
names the line), and the message's own text in the memory is written over
with the tombstone through the inspector's change book (backup, journal,
summaries above rebuilt, undoable), so the Memory Inspector and later
views no longer hold it. The inspector and the brain share one Editor.

iOS keeps its 2-minute Undo Send, which reaches the same memory deletion;
parity row added as partial.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang austinywang added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 5, 2026
- A deletion made while the Chief was not listening (restart, lost
  connection) reaches its memory on the reconnect's catch-up.
- The tombstone is written once per message, decided by the change book
  (has_deleted), so a failed write is tried again and an inspector undo
  holds across reconnects.
- Delete is not offered on read-only, agents, Feed or agent-sent rows.
- The projection forgets a delete once the store holds its retraction.
- The Swift test deletes a message it sent (the mock owner refuses to
  retract its generated rows), and names MessagesLabHome.Message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ow (#36019)

Drives the right-click's own menu path from the DEBUG socket, so the
Chief chat's Delete can be dogfooded on a background tagged app.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review (subagent, correctness-first) and resolution

Reviewed head: 329eecf. Fixes landed in f93a011.

# Finding Resolution
1 (blocking) The Swift test deleted a row the mock owner generates on the fly, so message.retract was refused with unknown_message and the row came back. Fixed. The test now sends a message, waits for its commit and deletes that one. It also names MessagesLabHome.Message, which the fleet compile caught as ambiguous. Green: fleet step 99018931a17a3086241892b4 at f93a011, where all 3 suites passed.
2 (blocking) A deletion the brain didn't hear live (Chief host restarting or disconnected) never reached the memory: catch-up skipped messages it had already handled. Fixed. catch_up now passes the newest page's retracted messages, not authored by the Chief, through the live path. New test a_message_deleted_while_the_chief_was_down_leaves_its_memory_on_reconnect_and_an_undo_holds fails on the base and passes on the head.
3 The comment claimed no later view holds the text, but summaries above the line keep it until they are rebuilt. Reworded. The line itself is tombstoned before the next turn; the summaries above it are marked stale and rebuilt by the change book's worker, the same path as an inspector delete.
4 The first delete in a process runs a full backup on the brain thread. Kept on purpose, so the tombstone is in place before the next turn renders. It happens once per process, only when the person deletes; the same cost an inspector delete pays.
5 A failed tombstone write was never retried, because it was gated on the note being fresh. Fixed. The gate is now the change book (Editor::has_deleted), so a failed write is tried again on the next update or catch-up, and an inspector undo holds across reconnects (covered by the new test).
6 Delete was offered on read-only, agents, Feed and agent-sent rows. Fixed. Each of those is excluded in deletableMessage.
7 deleting was never cleared. Fixed. An id leaves the set once the store holds its retraction.
8 Retracted messages from the Chief now disappear in Chief chats too. Intended: a Chief chat shows no "unsent" notes.

The reviewer also checked these and found no issue: no deadlock between the change book and the chat lock; every ChatIntents conformer still compiles thanks to the default implementations; .delete and .unsend stay consistent across rebuilds; read cursors still move.

austinywang and others added 2 commits November 4, 2026 20:01
…t the press (#36019)

Regression test for #36019: the
list kept the deleted message's text as its preview until the owner's
retraction arrived.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…36019)

The conversation list overlays a pending message.retract on the newest
message like pin, mute and read cursors, so Delete (and Undo Send) leaves
the preview in the same turn as the transcript row; a refusal shows it again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does (#36019)

Review: a refused retract is discarded, never left failed, so the
.failed skip was dead and the test drove a state production never reaches.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review 2 (subagent, correctness-first) and resolution

Reviewed: cedc76e166..be92280298, which makes the list's preview clear at the press. The GUI dogfood found that the conversation list kept the deleted text for about a second, until the owner's retraction arrived. Fixed in head f77f606.

# Finding Resolution
1 No flash on settle. A retract intent leaves the log only once the mirror holds the conversation revision of its acknowledgement, and settle() and rebuildRows() run in the same pass. The overlay gives the same row the owner's echo gives: empty preview, no attachments label, the same author and timestamp; homePreview returns before it reads reactions. No change.
2 (low) A refused retract is discarded from the log and never stays .failed, so the .failed skip was dead code, and the test drove a state that production never reaches. Fixed in f77f606: the skip is gone, and the test refuses the delete through log.discard, as HomeStore does.
3 Other inboxRows consumers (the iPhone's ConversationRowModel and HomeInboxMapping) read the same fields and match the echo. No change.
4 (informational) Retract intents aren't saved in the client cache. If the app quits before the echo, the cached list shows the old preview on relaunch until the first inbox fetch. Not changed here: the gap already existed (Undo Send has it), and the transcript and the Chief's memory follow the owner.

Fleet results:

  • Head f77f606:
    • CmuxHomeCoreTests: the whole package passes, 288 tests in 51 suites (step 9724382a2665b889f4042730).
    • MirrorAndIntentLogTests: fails on the base bf58b72 (step 55454276a155a675cf118ce7, line 71) and passes at the fix be92280 (step 3412dbb85084c7a04478075e).

@austinywang

Copy link
Copy Markdown
Contributor Author

Visual review (subagent) at f77f606: NEEDS-HUMAN-TASTE (advisory)

Evidence: window snapshots from the real tagged app, fleet build a252943d07c19a4658db17a7 of this exact head, dark appearance. The app ran in an isolated scratch Chief home and was driven through its DEBUG socket. Captured: the chat before Delete, about 2.5 s after it, and after a restart. Also an earlier build's capture shortly after Delete, from before be92280.

Result: the delete works.

  • After Delete, the bubble, its "Today" header and its "Read" line are gone. There's no leftover ghost or gap, and the list preview is empty.
  • After the relaunch, the message is still gone.
  • The earlier build's capture shows the bubble already fading while the list still showed the deleted text. This head fixes that state (be92280).
  • No clipping, contrast problems or half-drawn frames.
# Severity Finding Resolution
1 note (evidence) The open context menu was not captured: a real NSMenu needs a real right-click with the person's pointer. The rows were read from the real menu through the socket: the two tapback palettes, Tapback Details…, Reply…, Attach Sticker…, Copy, Copy Link, Forward…, Share… and Delete, with a separator before Copy, before Share… and before Delete. The row is the stock MenuAction with the trash symbol. Left for the person's dogfood of the tagged build.
2 taste After the delete, the list row keeps its two-line height with an empty preview line and the deleted message's time. The same as a retracted newest message today (Undo Send, after the owner's echo). Changing it (collapse the row, or show the previous message's time) is a product call, not in this PR.
3 note After the restart, the composer shows the emoji button instead of the stop square, and the avatar's activity ring is gone. That is the Chief run going idle after the relaunch (session state), not this change.

@austinywang
austinywang marked this pull request as ready for review November 5, 2026 04:20
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review f77f606: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr36057 -f ref=feat-cmux-next-36019-chief-message-delete -f platform=macos -f backend_mode=local

Shipped in:

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41990669105 at cf6dec5 failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at e025b48. This pull request is one of 5 merged since: #36262, #36236, #36057, #36285, #36207.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

For awareness: on feat-cmux-next at 76e7f91, optchat-chief's chat_delete test a_message_deleted_while_the_chief_was_down_leaves_its_memory_on_reconnect_and_an_undo_holds fails every run (3/3 on the fleet) at tests/chat_delete.rs:181 with the undo: (409, "its summaries are being rebuilt; try again in a moment"). That makes the Native/OptChat/optchat-chief and rust workspaces jobs red on PRs (e.g. #36411, which touches no Chief code). The undo seems to race the summary rebuild after the reconnect.

lawrencecchen added a commit that referenced this pull request Nov 5, 2026
…ild; tui-kit beacon clippy) (#36608)

* test(optchat-chief): the undo after a reconnect's delete waits for that delete's summaries to rebuild

Red on every full rust-workspaces run since #36057 (3deee62): the undo
raced the editor's rebuild worker and got Editor::undo's documented 409 'its
summaries are being rebuilt; try again in a moment'. The Memory Inspector and the
memory_edit tests wait (wait_rebuilt) before an undo; this test now does too.

* tui-kit: beacon::frame takes the run's Beacon (ws, pane, cell columns), so clippy's too_many_arguments passes

cargo clippy -D warnings in scripts/journeys/tui-kit/rust failed (frame took 8
arguments, from #31784); it shows on every full rust-workspaces run (#36427's
run changed .github/rust-workspaces.toml). The three values that name a beacon
for its whole run are one struct; tk-beacon builds it at the call.

* tui-kit: tk-beacon parses its command line into a named Options, so clippy's type_complexity passes

This branch was previously deployed

1 inactive deployment
ci-routing-agent — f77f6068 Deployed Nov 5, 2026 by austinywang via namespace autofix #14856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants