chat: clear deleted unsequenced sends pinned to bottom of scroller - #6123
chat: clear deleted unsequenced sends pinned to bottom of scroller#6123jamesacklin wants to merge 12 commits into
Conversation
A message deleted in the window after the server acknowledged the send but before the sequenced addPost echo arrived (deliveryStatus: 'sent', sequenceNum: 0) survived as an isDeleted tombstone in the pending merge layer. mergePendingPosts sorts unconfirmed rows first, so the tombstone rendered pinned to the bottom of the chat scroller — below newer messages — indefinitely, since the confirmed delete means the sequenced addPost that would replace the cached row never arrives. - deletePost: once the server confirms the delete, hard-delete a still-unsequenced top-level row via clearUnsentPost instead of leaving the tombstone. Re-reads the row first so a sequenced echo that lands during the delete round trip keeps the normal in-place tombstone. - db.clearGhostPosts + syncStart sweep: repair rows poisoned before this fix (isDeleted + sequenceNum 0 + deliveryStatus set + delete confirmed) and repoint affected channel heads. - PinnedPostBanner: don't render a pinned post that has been deleted. Fixes TLON-5911 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28745e5e49
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex review: `clearGhostPosts` matched any non-null deliveryStatus, so after a restart it could hard-delete a deleted-but-still-in-flight send (`enqueued`/`pending` with `deleteStatus: 'sent'`) that `getDeliveryPendingPosts` deliberately keeps for delivery polling to reconcile. Match the deletePost-side condition instead: only `sent` / `needs_verification` rows are ghosts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jamesacklin
changed the title
|
The |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4609854282
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Addressed both Codex findings in f8b18b3: hard-deleted rows now emit a distinct live removal state so stale newPosts snapshots disappear immediately, and the chat-list read path performs a bounded non-destructive repair for inconsistent null-id/non-null-timestamp previews. Added regression coverage; 155 focused tests, typecheck, and lint pass. @codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8b18b344f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 862c632d04
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 203ac02687
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1e780c429
ℹ️ 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".
The develop->feature merge (b1e780c) spliced the TLON-5911 "sent-but-unsequenced post" and TLON-6133 "pinned post" describe blocks into each other, corrupting both. Restore each block verbatim from its parent so both test suites are intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two P2 findings from the latest Codex review: - postActions.ts: the guarded hard-delete only ran on the delete poke's success path. When the ack was lost but the server confirmed the delete (serverPost.isDeleted or 404), the verification branches only set deleteStatus:'sent', leaving an acknowledged sequenceNum:0 row and its live isDeleted overlay behind. Extract settleConfirmedDelete() and run it in both verification branches so a confirmed delete never leaves a ghost, however it was confirmed. - queries.ts/dbHooks.ts: getSettledDeletedGhostChannelIds flagged a channel if it held any settled-delete ghost, without checking the ghost matched the stale head. An unrelated ordinary delete (lastPostId nulled with a partial cache) in a channel that also held an older ghost could trigger recomputeChannelLastPost and blank/rewind the preview. Match the ghost row's receivedAt against the channel's stale lastPostAt so only the actual settled-delete head is repaired. Adds regression coverage for both (unsequenced pinned lost-ack via isDeleted and 404; receivedAt-mismatch exclusion). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca9c6a83d6
ℹ️ 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".
When a user deletes an optimistic post while its original send is still enqueued/pending, the queued delete can be acknowledged before delivery flips the row to `sent`. At delete time `deleteUnsequencedAcknowledgedPost` matches nothing (delivery isn't acknowledged yet), so the guarded hard-delete never runs and no `'removed'` overlay is emitted. Once `markPostSent` later flips delivery to `sent`, `getPendingPosts` hides the DB row but a mounted channel's live `newPosts` snapshot keeps rendering it as a tombstone until remount. Add `deleteSettledUnsequencedDeletedPost` (atomic guard: isDeleted + deleteStatus 'sent' + sequenceNum 0 + top-level + deliveryStatus 'sent', so it can never touch a normally delivered post) and run it from the `markPostSent` handler, broadcasting `removeFromChannelPosts` and recomputing the channel head when it fires. Adds DB coverage (fires on delivered settled delete; leaves live posts, in-flight sends, and sequenced echoes untouched) and a sync-handler test proving markPostSent removes the row and emits the 'removed' overlay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c7a207c08
ℹ️ 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".
Delivery reaches `sent` through two paths, not one: `markPostSent` subscription events and `verifyPostDelivery()` after a send times out to `needs_verification`. The previous commit only finished settled deletes on the `markPostSent` path, so a post deleted while still `pending` (delete settles first, delete-time guard finds nothing) that later verified as delivered would strand its settled-delete row as a live tombstone. Extract the cleanup into `finishSettledDeleteOnDelivery` (neutral module, no import cycle) and call it from both delivery-resolve paths. Adds verifyPostDelivery coverage: confirmed delivery hard-deletes a settled delete and emits the 'removed' overlay, while a live post is only advanced to 'sent'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3 participants
Problem
TLON-5911: a deleted message stays rendered as a "Message deleted" tombstone pinned to the very bottom of the chat scroller — below newer messages — indefinitely. Highly visible on desktop where the inverted list bottom-anchors content.
Root cause
When a message is deleted in the window after the server acknowledged the send but before the sequenced
addPostecho arrived (deliveryStatus: 'sent',sequenceNum: 0):markPostAsDeletedstampsisDeleted: trueon the cached row.getPendingPostsintentionally keeps deleted'sent'rows as a tombstone source (only'failed'rows are excluded).mergePendingPostssorts unconfirmed (sequenceNum === 0) rows first → bottom of the inverted list.addPostthat would normally replace the cached row (deleteReplacedCachedPosts) never arrives — so the ghost is permanent.Fix
deletePost(packages/shared/src/store/postActions/postActions.ts): once the server confirms the delete, re-read the row and hard-delete it via the existingclearUnsentPostpath if it is still an unsequenced top-level'sent'/'needs_verification'row. A sequenced echo that lands during the delete round trip keeps the normal in-place tombstone behavior; in-flightenqueued/pendingrows are left to the delivery machinery.db.clearGhostPosts(packages/shared/src/db/queries.ts) + asyncStartsweep: repairs rows already poisoned in users' local DBs (like the May 9th ghost in the issue screenshot) —isDeleted+sequenceNum: 0+deliveryStatusset +deleteStatus: 'sent'— and repoints affected channel heads.PinnedPostBanner(packages/app/ui/components/Channel/PinnedPostBanner.tsx): secondary bug from the investigation — don't render the banner for a pinned post that has been deleted.Tests
postActions.test.ts: new suite covering the hard-delete after confirmed delete (including pending-layer/channel-head assertions), the mid-round-trip sequenced-echo race, and rollback on poke failure.queries.test.ts:clearGhostPostsremoves only the ghost shape (confirmed tombstones, in-flight deletes, un-delete-confirmed rows, and replies all survive) and repoints the channel head.All 344 shared-package tests pass;
tscclean.Fixes TLON-5911
🤖 Generated with Claude Code