Skip to content

sync: account for reply tombstones#5343

Merged
latter-bolden merged 4 commits into
developfrom
lb/deleted-thread-post-bug
Dec 5, 2025
Merged

sync: account for reply tombstones#5343
latter-bolden merged 4 commits into
developfrom
lb/deleted-thread-post-bug

Conversation

@latter-bolden
Copy link
Copy Markdown
Member

Summary

Reply parsing was throwing in cases where a thread message was deleted: this results in a tombstone, but wasn't accounted for in method type signatures.

While investigating, I discovered that reply deletes don't work at all. We weren't accounting for the different format and the action sheet fires a generic post delete when you try from a thread. The optimistic update would stick however, so it was deceptive.

Another issue that cropped up while working on it: we had a lingering bug from the @aura upgrade. #5342 should resolve this, but we'll need that to land first. That'll block functional review, but be otherwise fine for code review.

Changes

  • custom api method for handling reply deletes
  • conditionally call this method from deletePost in action module
  • add tombstones as a result type in the replies types of the urbit module
  • account for that type difference in the serializer

How did I test?

I deleted messages after fixing the API. Confirmed this creates the parse error that we saw breaking sync. Confirmed with the fix, we get no such parse error.

TODO: this is working for group channels, but I'm seeing a type issue with DMs

Risks and impact

  • Safe to rollback without consulting PR author? Yes
  • Affects important code area:
    • Message sync
    • Channel display

Operational Note

I think we should merge this into develop and then cherrypick onto the 8.0.3 hotfix branch

Fixes TLON-5128

@linear
Copy link
Copy Markdown

linear Bot commented Dec 5, 2025

Copy link
Copy Markdown
Member

@patosullivan patosullivan left a comment

Choose a reason for hiding this comment

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

One question, otherwise LGTM

Comment thread packages/shared/src/store/postActions.ts Outdated
@jamesacklin
Copy link
Copy Markdown
Member

Please target release/803, I'll open another PR to merge back into develop.

@latter-bolden latter-bolden merged commit 5290c0f into develop Dec 5, 2025
3 checks passed
@latter-bolden latter-bolden deleted the lb/deleted-thread-post-bug branch December 5, 2025 19:30
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.

3 participants