From 9031351fe201687ce0fd51369fad1d2d0369a33f Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Tue, 22 Sep 2026 16:38:40 +1000 Subject: [PATCH] Add DiffRunner.SettleDelete to withdraw a pending delete A delete is raised for a file that looked stale, and waits in the tray or the viewer for someone to accept it. Nothing could withdraw one when the file came back into use - a target that came back, or a snapshot that moved inline and then back to its file - so accepting it removed a file a passing test depends on. The owners already drop a tracked delete on a settle for its key; there was just no way for a test run to send one. SettleDelete sends that settle to the queue owner, as SettleDiff does for a pending move. It reaches the delete in a viewer, and in a tray that owns the queue, which keeps the deletes that arrived over the piper port in the same tracked files. Nothing is deleted. It answers to DiffRunner.Disabled, and nothing owning the queue is silent, with the unowned port remembered like every other settle. A tray that does not own the queue keeps its own deletes and cannot be reached, the same limit SettleDiff has: the piper format is frozen at moves and deletes. Pinned at each end. The client sends the tracked delete key, sends nothing while disabled, and is silent with no owner. A tray that owns the queue drops the delete it tracked from the raw path when the settle names the folded key, and leaves the file on disk. The viewer's session drops the delete and keeps the rest of the queue. --- docs/mdsource/tray.source.md | 2 + docs/tray.md | 2 + src/DiffEngine.Tests/PendingFilesDiffTests.cs | 55 +++++++++++++++++++ src/DiffEngine/DiffRunner.cs | 20 +++++++ src/DiffEngine/Tray/PendingFiles.cs | 16 ++++++ .../TrayViewerSyncTest.cs | 21 +++++++ .../TrackedFileTests.cs | 17 ++++++ 7 files changed, 133 insertions(+) diff --git a/docs/mdsource/tray.source.md b/docs/mdsource/tray.source.md index 626a5cfb..7ae89ae8 100644 --- a/docs/mdsource/tray.source.md +++ b/docs/mdsource/tray.source.md @@ -43,6 +43,8 @@ A test can produce multiple resulting snapshots. If the accepted versions has a Clicking "file1" or "file2" will delete file1 or file2 respectively. The drop down will expose extra actions for that change. +A delete is withdrawn when a later test run verifies against its file again, since the file is then in use rather than stale: `DiffRunner.SettleDelete(file)` drops the pending delete and leaves the file alone. It reaches a tray that owns the inline queue, which is the usual arrangement since the tray starts at login. + ### Pending snapshots diff --git a/docs/tray.md b/docs/tray.md index 7cae7824..da96be82 100644 --- a/docs/tray.md +++ b/docs/tray.md @@ -50,6 +50,8 @@ A test can produce multiple resulting snapshots. If the accepted versions has a Clicking "file1" or "file2" will delete file1 or file2 respectively. The drop down will expose extra actions for that change. +A delete is withdrawn when a later test run verifies against its file again, since the file is then in use rather than stale: `DiffRunner.SettleDelete(file)` drops the pending delete and leaves the file alone. It reaches a tray that owns the inline queue, which is the usual arrangement since the tray starts at login. + ### Pending snapshots diff --git a/src/DiffEngine.Tests/PendingFilesDiffTests.cs b/src/DiffEngine.Tests/PendingFilesDiffTests.cs index cd440f3e..d0459a29 100644 --- a/src/DiffEngine.Tests/PendingFilesDiffTests.cs +++ b/src/DiffEngine.Tests/PendingFilesDiffTests.cs @@ -128,6 +128,60 @@ public async Task SettlingWithNoOwnerIsSilent() await Assert.That(() => PendingFiles.SettleDiff(Temp)).ThrowsNothing(); } + /// + /// The other end of a delete: the file it was raised for is in use again, because a later run + /// verified against it. The delete goes, by the tracked key it is listed under, and nothing + /// reaches the file. + /// + [Test] + public async Task SettlingADeleteSendsTheDeleteKey() + { + using var owner = new Recording(); + var previousDisabled = DiffRunner.Disabled; + // DisabledChecker turns this on for build servers and AI CLIs, and this drives the real + // DiffRunner entry point + DiffRunner.Disabled = false; + try + { + DiffRunner.SettleDelete(Stale); + } + finally + { + DiffRunner.Disabled = previousDisabled; + } + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Settle}:{TrackedKeys.ForDelete(Stale)}:"]); + } + + /// + /// A settle answers to the same switch the delete it settles did. + /// + [Test] + public async Task SettlingADeleteWhileDisabledSendsNothing() + { + using var owner = new Recording(); + var previousDisabled = DiffRunner.Disabled; + DiffRunner.Disabled = true; + try + { + DiffRunner.SettleDelete(Stale); + } + finally + { + DiffRunner.Disabled = previousDisabled; + } + + await Assert.That(owner.Heard).IsEmpty(); + } + + [Test] + public async Task SettlingADeleteWithNoOwnerIsSilent() + { + using var absent = new NoOwner(); + + await Assert.That(() => PendingFiles.SettleDelete(Stale)).ThrowsNothing(); + } + /// /// The tray works the arguments out for itself when a move arrives without them, and used to /// take the viewer's declared ones - two plain paths, which open a window of its own for a @@ -176,6 +230,7 @@ static ResolvedTool Other(bool isMdi) => const string Temp = @"c:\temp\Sample.Test.received.png"; const string Target = @"c:\code\Sample.Test.verified.png"; + const string Stale = @"c:\code\Sample.Stale.verified.txt"; /// /// Carries the identity the route branches on. Never started: an owner answers every time. diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs index c415cc62..bf5ae719 100644 --- a/src/DiffEngine/DiffRunner.cs +++ b/src/DiffEngine/DiffRunner.cs @@ -192,6 +192,26 @@ public static Task AddDeleteAsync(string file) return DiffEngineTray.AddDeleteAsync(file); } + /// + /// Withdraws a pending delete, for when the file it was raised for is in use again: a later + /// run verified against it, so accepting the delete would remove a file that run depends on. + /// Nothing is deleted. + /// + /// Does nothing when no tray or viewer holds the queue - and cheaply, the way + /// does, since a port found with nothing listening is not + /// connected to again for a while. + /// + /// + public static void SettleDelete(string file) + { + if (Disabled) + { + return; + } + + PendingFiles.SettleDelete(file); + } + public static Task LaunchAsync(ResolvedTool tool, string tempFile, string targetFile, Encoding? encoding = null) { GuardFiles(tempFile, targetFile); diff --git a/src/DiffEngine/Tray/PendingFiles.cs b/src/DiffEngine/Tray/PendingFiles.cs index efedfda8..11f16bbb 100644 --- a/src/DiffEngine/Tray/PendingFiles.cs +++ b/src/DiffEngine/Tray/PendingFiles.cs @@ -218,6 +218,22 @@ await ViewerLaunchGate.LaunchAsync( public static void SettleDiff(string tempFile) => ViewerClient.TrySend(new(ViewerVerb.Settle, TrackedKeys.ForMove(tempFile))); + /// + /// The other end of : the file a delete was raised for is in use + /// again, so the delete goes. Nothing is deleted. + /// + /// To the queue owner, as is, which reaches the delete wherever it + /// is held: in a viewer, or in a tray that owns the queue, which keeps the deletes that arrived + /// over the piper port in the same tracked files. A tray that does not own the queue keeps its + /// own, and the piper format that would reach it is frozen at moves and deletes. + /// + /// + /// Silent when nobody answers: no owner means no row, which is the state this was asking for. + /// + /// + public static void SettleDelete(string file) => + ViewerClient.TrySend(new(ViewerVerb.Settle, TrackedKeys.ForDelete(file))); + /// /// Whether a pending file should take the route rather than the plain /// tracking one, which is exactly whether the tool that would have opened a window for it is diff --git a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs index 4d947374..df69ce9d 100644 --- a/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs +++ b/src/DiffEngineTray.Tests/TrayViewerSyncTest.cs @@ -771,6 +771,27 @@ public async Task AnOwningTrayTracksWhatArrivesOnTheViewerPort() await Assert.That(pair.Pump().Keys()).IsEquivalentTo([TrackedKeys.ForDelete(delete)]); } + /// + /// A delete raised for a file that a later run verified against again, and settled by that + /// run. The settle names the file through its folded key while the tray tracked it as it + /// arrived, and the two have to meet: the delete goes, and the file it would have removed + /// stays where it is. + /// + [Test] + public async Task ASettledDeleteLeavesTheTrayAndKeepsTheFile() + { + await using var pair = new TrayOwned(); + var delete = pair.AddDelete(); + await Assert.That(pair.Pump().Keys()).IsEquivalentTo([delete.Key]); + + var response = pair.Send(new(ViewerVerb.Settle, TrackedKeys.ForDelete(delete.File))); + + await Assert.That(response.Ok).IsTrue(); + await Assert.That(pair.Tracker.Deletes).IsEmpty(); + await Assert.That(pair.Pump().Queue).IsEmpty(); + await Assert.That(File.Exists(delete.File)).IsTrue(); + } + /// /// The tray check is cached, so this is the state a test process is in, not a property of the /// machine. Set explicitly rather than assumed, because another test in this project sets it diff --git a/src/DiffEngineViewer.Tests/TrackedFileTests.cs b/src/DiffEngineViewer.Tests/TrackedFileTests.cs index 38e9bad7..fceec7da 100644 --- a/src/DiffEngineViewer.Tests/TrackedFileTests.cs +++ b/src/DiffEngineViewer.Tests/TrackedFileTests.cs @@ -113,6 +113,23 @@ await Assert.That(settled.Queue.Select(_ => _.Kind)) await Assert.That(done).IsEmpty(); } + /// + /// A delete leaving because the file it was raised for is in use again: a later run verified + /// against it. Settling drops it, and a settle is a change to the queue alone, so the file + /// accepting would have removed stays. + /// + [Test] + public async Task SettlingADeleteDropsItAndKeepsTheRest() + { + var state = Owned(Fixtures.Move(), Fixtures.Delete()); + state = ViewerSession.EnqueueInline(state, Fixtures.Patch()); + + var settled = ViewerSession.Settle(state, Fixtures.Delete().Key); + + await Assert.That(settled.Queue.Select(_ => _.Kind)) + .IsEquivalentTo([QueueEntryKind.Inline, QueueEntryKind.Move]); + } + [Test] public async Task SettlingAPairThatIsNotQueuedChangesNothing() {