Make a repeat channel delete a no-op instead of a second announcement and audit row - #691
Merged
davidmckayv merged 8 commits intoOct 2, 2026
Merged
Conversation
… and audit row softDelete's deletedAt guard kept the timestamp, but the member announcement and the route's channel.deleted audit row still ran on every repeat, so a retry or a second tab told every member again and wrote a row to the append-only trail for a deletion that did not happen. softDelete now reports whether this call deleted the channel, announces only then, and the route records only then. The response is still 204.
Chebaleomkar
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso,
mxmzb and
tylerslaton
as code owners
October 1, 2026 09:00
davidmckayv
previously approved these changes
Oct 2, 2026
davidmckayv
enabled auto-merge (squash)
October 2, 2026 03:56
auto-merge was automatically disabled
October 2, 2026 11:12
Head branch was pushed to by a user without write access
davidmckayv
previously approved these changes
Oct 2, 2026
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
The current head 99277ff matches the accepted source disposition from the refreshed triage. Required CI must pass before landing.
davidmckayv
enabled auto-merge (squash)
October 2, 2026 16:52
auto-merge was automatically disabled
October 2, 2026 18:59
Head branch was pushed to by a user without write access
# Conflicts: # CHANGELOG.md
softDelete now says whether it deleted anything, which the reset hook's Promise<void> type refused, so the server no longer typechecked. The reset ignores the answer, so the hook takes any result.
Contributor
|
I pushed one commit to fix the server typecheck. |
# Conflicts: # CHANGELOG.md
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.
What this changes
softDelete's comment says itsdeleted_atguard "is what makes a repeat call a no-op rather than a new stamp", and the existing test is "deleting again is a no-op, not an error". The guard kept the timestamp, but the rest of a delete still ran on every repeat:pg_notifytold every member the channel was deleted, again;recordDeleted, which wrote anotherchannel.deletedrow toaudit_events. That table is append-only, so a retry, a second tab or a double click left rows for deletions that did not happen. The comment aboverecordDeletedsays "The trail records acts, not attempts."softDeletenow returns whether this call deleted the channel. The update uses.returning(); on zero rows it returnsfalsebefore reading members or notifying. The route records only when the store did not answerfalse, and still answers 204 either way, so DELETE stays idempotent for the client.ChannelStore.softDeletechanges fromPromise<void>toPromise<boolean>. Test fakes that resolveundefinedare treated as "deleted", so existing route tests keep their behaviour. The test files are outsideserver/tsconfig.json'sinclude.Where it runs
UPDATE ... RETURNINGstamps one row once, so exactly one of them announces and records.pg_notify, now only for the call that deleted.Boundary and audit
channel.deletedrow. A repeat no longer writes one, because nothing happened.Changelog
Unreleased.Proof
routes.tschannel-events.integration, new: a repeat delete announces nothing to either member (real LISTEN/NOTIFY, with a 500 ms window)channel-routes, new: the route answers 204 and writes no audit row when the store answersfalsechannel-routes, existing "deleting again is a no-op", now assertingtruethenfalsechannel-routes,channel-events.integration,runtime-agents.integrationandroutines-store.integrationpass 165 of 165 againstpgvector/pgvector:pg17.tsc --noEmitexits 0, andbiomeis clean.