Skip to content

Hold a group's deletes until its snapshots are written, from an attached viewer - #887

Merged
SimonCropp merged 2 commits into
mainfrom
fix-group-accept-deletes
Sep 23, 2026
Merged

SimonCropp merged 2 commits into
mainfrom
fix-group-accept-deletes

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

Two commits: removing the done items from todo.md, then the one open data-loss item.

The data loss

A snapshot moving inline arrives as a patch plus a delete of the verified file it replaces. "Accept all in " in a viewer attached to someone else's queue went wrong like this:

  • DispatchGroup posted one Accept per member, the delete included.
  • The owner, usually the tray, carried each out as asked.
  • A patch it couldn't write, because the call site moved since the run, was dropped the way a stale patch is.
  • The verified file was deleted anyway, so no copy of the snapshot was left.

The owning viewer's group accept, and every accept-all since #878, already hold deletes for this. This path didn't.

The fix

  • The attached viewer now sends a group accept as one ordered step (OwnerLink.PostAcceptGroup). It accepts moves, then snapshots, and only then the deletes, and only if every snapshot is in the source.
  • The reply's ok can't say whether a snapshot is in the source, because a stale patch gets ok too. A listing can't either: a stale patch is gone from it just as an applied one is. So an accept reply now carries written, set from the applier's own answer by both owners: the tray (OwnedInlineHost) and an owning viewer (MessageHandler).
  • Compatibility: it's a new optional line, which older readers skip. A reply without it, from an owner that predates it, holds the deletes. They stay queued, to be accepted on their own, and the status line says why.

Tests

  • AttachedViewerTests.AGroupAcceptHoldsItsDeletesWhenASnapshotWasNotWritten and AGroupAcceptDeletesOnceItsSnapshotsLanded go through the real menu, a real OwnerLink and an owning viewer. The first fails against main's DispatchGroup, with the delete carried out.
  • TrayViewerSyncTest.AViewerGroupAcceptHoldsItsDeletesWhenTheTrayCouldNotWriteASnapshot and AViewerGroupAcceptCarriesOutItsDeletesOnceTheTrayWroteTheSnapshots use the tray as owner, the arrangement it sets up at login.
  • ViewerProtocolTests.AnAcceptSaysWhetherTheSnapshotWasWritten covers the new field on the wire, and a reply without it.

The full solution passes locally in Release: 2,040 passed, 24 skipped.

…hed viewer

A snapshot moving inline arrives as a patch plus a delete of the verified file
it replaces. "Accept all in <solution>" in a viewer attached to someone else's
queue posted one accept per member, the delete included, and the owner carried
each out as asked. A patch it could not write - the call site moved since the
run - was dropped as a stale one is, and the verified file went anyway, leaving
no copy of the snapshot. The owning viewer's group accept and every accept-all
already hold deletes for this; this path did not.

The attached viewer now sends the group as one ordered step: moves, then
snapshots, then the deletes only once every snapshot is in the source. Whether
one is cannot be read from the reply's ok, which a stale patch also gets, nor
from a listing, where a stale patch is gone just as an applied one is. So an
accept reply now says so: written, set by the tray and by an owning viewer from
the applier's own answer. A reply without it, from an owner that predates it,
holds the deletes, which stay queued to be accepted on their own.
@SimonCropp SimonCropp added this to the 20.5.0 milestone Sep 23, 2026
@SimonCropp
SimonCropp merged commit 1135222 into main Sep 23, 2026
10 checks passed
@SimonCropp
SimonCropp deleted the fix-group-accept-deletes branch September 23, 2026 04:17
This was referenced Sep 23, 2026
This was referenced Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant