Add SettleAppliedInline for applied patches

Introduce `DiffRunner.SettleAppliedInline(InlinePatch)` to retire inline queue entries after a patch is applied outside the test run (IDE/plugin/staged fallback). Unlike `SettleInline`, it does not stamp the caller’s framework, avoiding silent misses when queued entries were created by another target framework, and it uses patch metadata (including `MemberName`) to keep settling reliable when line numbers move. Docs and inline engine tests were updated to explain and lock in this behavior.

Author: SimonCropp

claude.md

@@ -74,8 +74,10 @@ stdin and binds the port itself; if no viewer resolves (or `DiffEngine_InlineVie
 Verify stages `received`/`expected`/`.inlinepatch` files and the IDE plugin or a text diff tool
 becomes the review surface. Accepting anywhere runs `InlineApplier` against the source file
 (per-file cross-process mutex — safe concurrently from any process). A passing re-run calls
-`SettleInline`, and any surface that applies a patch itself must settle too, or the queue owner
-keeps offering a snapshot that is already in the source.
+`SettleInline`, and any surface that applies a patch itself must call `SettleAppliedInline` — not
+`SettleInline`, whose framework label is the running process's own and so never matches an entry
+some other process queued, missing silently — or the queue owner keeps offering a snapshot that is
+already in the source.
 
 The source may be C# or F#, decided by the file's extension (`SourceLanguage.ForFile`) rather than
 stated on the patch. `InlinePatcher` walks the same structure either way — a name, an argument

docs/inline.md

@@ -76,7 +76,7 @@ DiffEngineViewer --inline --source <source file> --line <number> < the.inlinepat
 For the producing side — a test library with a failing inline snapshot:
 
 * `DiffRunner.AddInlineAsync(patch)` queues a patch with whatever owns the port, launching the bundled viewer when nothing does. Returns `Queued`, `Disabled` (build servers, continuous testing and AI CLIs included), or `NoViewerFound` — the caller's cue to stage files and fall back to a text diff.
-* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry.
+* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry. That framework is the running process's, which makes this the test run's verb and only the test run's: a surface applying a patch of its own wants `SettleAppliedInline`, [below](#applying-a-patch-from-another-surface).
 * `AddInlineAsync` stamps `patch.Framework` with the consuming project's target framework ("net9.0", "net48") unless the caller already set it, which is what lets the owner tell a re-run from another framework disagreeing. The value is the `$(TargetFramework)` the package's build targets stamp into the project's runtimeconfig, read back rather than asked of the process — in a hosted test run the entry assembly is the runner (testhost, ReSharperTestRunner), whose framework is not the project's — with the running runtime's version as the fallback for consumers without the targets. Callers may also set `patch.TestName`, which the viewer uses to group and label the queue; without it, items are labeled by call site.
 * Set `patch.OriginalExpression` from `CallerArgumentExpression` where the language supplies one, and `patch.OriginalValue` — the previous expected argument's value — where it does not. One of the two is what stops a patch rewriting the wrong call site when the file has moved since the run. `patch.MemberName` from `CallerMemberName` narrows it further, and is supported everywhere including F#.
 * Setting `DiffEngine_InlineViewer` to `false` reports `NoViewerFound` without probing, which is how a user opts into reviewing in their IDE instead of a window.
@@ -207,7 +207,7 @@ Listing waits half a second, since an owner that cannot answer one in that time
 For the staging fallback, where no viewer could be resolved and the patch is a file on disk rather than an entry in a queue: read it with `InlinePatchFile.TryRead`, apply it with `InlineApplier.Apply`, and honour two rules.
 
 * **InlineApplier owns all locking.** A per file cross process mutex (up to a ten second wait) plus an in process gate serialise every writer, so applying beside a concurrently accepting tray or viewer is safe, and callers must not add locking of their own. The file's encoding, BOM and line endings are preserved, and its extension picks the language.
-* **Settle what was applied.** The same test run that staged the files may also have queued the patch with the port owner, and that queue outlives both the window and the run. After `Applied` or `AlreadyApplied`, call `DiffRunner.SettleInline(patch.SourceFile, patch.LineHint)` — otherwise the tray keeps offering a snapshot that is already in the source.
+* **Settle what was applied.** The same test run that staged the files may also have queued the patch with the port owner, and that queue outlives both the window and the run. After `Applied` or `AlreadyApplied`, call `DiffRunner.SettleAppliedInline(patch)` — otherwise the tray keeps offering a snapshot that is already in the source. Not `SettleInline`: that one labels the settle with the running process's framework, which here is the applier's rather than the test project's, so the owner finds no variant to strip and does nothing — and answers no differently than if it had, so the miss is silent. `SettleAppliedInline` carries no framework at all, which is the right statement to make: replacing the literal the source held leaves every framework's variant anchored to text that has gone, so none of them can apply any more. It also takes the whole patch, so `MemberName` goes along and the entry is still found once an earlier accept has pushed its line down the file.
 
 `Apply` returns `Applied`, `AlreadyApplied` (the literal already matches), `NotFound` (the source changed since the test run — tell the user to re-run rather than retrying), or a failure with a message (locked file, unreadable source), which is retryable.
 

docs/mdsource/inline.source.md

@@ -69,7 +69,7 @@ DiffEngineViewer --inline --source <source file> --line <number> < the.inlinepat
 For the producing side — a test library with a failing inline snapshot:
 
 * `DiffRunner.AddInlineAsync(patch)` queues a patch with whatever owns the port, launching the bundled viewer when nothing does. Returns `Queued`, `Disabled` (build servers, continuous testing and AI CLIs included), or `NoViewerFound` — the caller's cue to stage files and fall back to a text diff.
-* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry.
+* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry. That framework is the running process's, which makes this the test run's verb and only the test run's: a surface applying a patch of its own wants `SettleAppliedInline`, [below](#applying-a-patch-from-another-surface).
 * `AddInlineAsync` stamps `patch.Framework` with the consuming project's target framework ("net9.0", "net48") unless the caller already set it, which is what lets the owner tell a re-run from another framework disagreeing. The value is the `$(TargetFramework)` the package's build targets stamp into the project's runtimeconfig, read back rather than asked of the process — in a hosted test run the entry assembly is the runner (testhost, ReSharperTestRunner), whose framework is not the project's — with the running runtime's version as the fallback for consumers without the targets. Callers may also set `patch.TestName`, which the viewer uses to group and label the queue; without it, items are labeled by call site.
 * Set `patch.OriginalExpression` from `CallerArgumentExpression` where the language supplies one, and `patch.OriginalValue` — the previous expected argument's value — where it does not. One of the two is what stops a patch rewriting the wrong call site when the file has moved since the run. `patch.MemberName` from `CallerMemberName` narrows it further, and is supported everywhere including F#.
 * Setting `DiffEngine_InlineViewer` to `false` reports `NoViewerFound` without probing, which is how a user opts into reviewing in their IDE instead of a window.
@@ -200,7 +200,7 @@ Listing waits half a second, since an owner that cannot answer one in that time
 For the staging fallback, where no viewer could be resolved and the patch is a file on disk rather than an entry in a queue: read it with `InlinePatchFile.TryRead`, apply it with `InlineApplier.Apply`, and honour two rules.
 
 * **InlineApplier owns all locking.** A per file cross process mutex (up to a ten second wait) plus an in process gate serialise every writer, so applying beside a concurrently accepting tray or viewer is safe, and callers must not add locking of their own. The file's encoding, BOM and line endings are preserved, and its extension picks the language.
-* **Settle what was applied.** The same test run that staged the files may also have queued the patch with the port owner, and that queue outlives both the window and the run. After `Applied` or `AlreadyApplied`, call `DiffRunner.SettleInline(patch.SourceFile, patch.LineHint)` — otherwise the tray keeps offering a snapshot that is already in the source.
+* **Settle what was applied.** The same test run that staged the files may also have queued the patch with the port owner, and that queue outlives both the window and the run. After `Applied` or `AlreadyApplied`, call `DiffRunner.SettleAppliedInline(patch)` — otherwise the tray keeps offering a snapshot that is already in the source. Not `SettleInline`: that one labels the settle with the running process's framework, which here is the applier's rather than the test project's, so the owner finds no variant to strip and does nothing — and answers no differently than if it had, so the miss is silent. `SettleAppliedInline` carries no framework at all, which is the right statement to make: replacing the literal the source held leaves every framework's variant anchored to text that has gone, so none of them can apply any more. It also takes the whole patch, so `MemberName` goes along and the entry is still found once an earlier accept has pushed its line down the file.
 
 `Apply` returns `Applied`, `AlreadyApplied` (the literal already matches), `NotFound` (the source changed since the test run — tell the user to re-run rather than retrying), or a failure with a message (locked file, unreadable source), which is retryable.
 

src/DiffEngine/DiffRunner_Inline.cs

@@ -129,6 +129,41 @@ public static void RetireInline(string sourceFile, int line, string? memberName
         ViewerClient.TrySend(new(ViewerVerb.Settle, InlineKey.For(sourceFile, line), null, memberName));
     }
 
+    /// <summary>
+    /// Drops a pending inline snapshot whose literal is now in the source, put there by a surface
+    /// that applied the patch itself rather than asking the queue owner to: the IDE plugin, or a
+    /// tool applying what a test run staged. Call it after <see cref="InlineApplier" /> reports
+    /// <see cref="InlineApplyStatus.Applied" /> or <see cref="InlineApplyStatus.AlreadyApplied" />.
+    /// <para>
+    /// Carries no framework, for the same reason <see cref="RetireInline" /> does not. Every
+    /// variant of a call site is anchored to the literal the source was holding, so replacing that
+    /// literal leaves none of them able to apply - the frameworks whose content differed included.
+    /// The statement is "this call site has been written", which is true for all of them at once.
+    /// </para>
+    /// </summary>
+    /// <remarks>
+    /// <see cref="SettleInline" /> is the wrong verb here and fails silently at it. That one stamps
+    /// the running process's own framework as the origin, which is right for the test run it was
+    /// written for and wrong for an applier, because an applier is not the test run: an IDE backend
+    /// or a dotnet tool reports its own moniker while the entry is labelled with the test project's.
+    /// The owner then finds no variant carrying that label, strips nothing, and answers no
+    /// differently than if it had - so the entry stays pending against source that already holds
+    /// the snapshot, and nothing anywhere says so.
+    /// </remarks>
+    /// <param name="patch">
+    /// The patch that was applied, which names the call site and the member it sits in.
+    /// </param>
+    public static void SettleAppliedInline(InlinePatch patch)
+    {
+        if (Disabled)
+        {
+            return;
+        }
+
+        ViewerClient.TrySend(
+            new(ViewerVerb.Settle, InlineKey.For(patch.SourceFile, patch.LineHint), null, patch.MemberName));
+    }
+
     static InlineResult CheckInline()
     {
         if (Disabled)

src/DiffEngineViewer.Tests/EngineInlineTests.cs

@@ -118,6 +118,73 @@ public async Task ARemovePatchIsRefused()
         await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty();
     }
 
+    /// <summary>
+    /// The trap <see cref="EngineRunner.SettleAppliedInline" /> exists for. A settle names the
+    /// running process's framework, which is the test run's for the caller that verb was written
+    /// for, and something else entirely for a surface that applies a patch of its own. The owner
+    /// finds no variant carrying that label and answers no differently than if it had.
+    /// </summary>
+    [Test]
+    public async Task SettleMissesAnEntryQueuedByAnotherFramework()
+    {
+        using var scope = new EngineScope();
+        var patch = Patch("Sample.cs", 42, "\"old\"", "new");
+        // A moniker no process can report, standing in for a test project this one is not.
+        patch.Framework = "net99.0";
+        await EngineRunner.AddInlineAsync(patch);
+
+        EngineRunner.SettleInline("Sample.cs", 42);
+
+        await Assert.That(scope.Fixture.Host.State.Queue).HasSingleItem();
+    }
+
+    [Test]
+    public async Task SettleAppliedDropsTheEntryWhateverFrameworkQueuedIt()
+    {
+        using var scope = new EngineScope();
+        var patch = Patch("Sample.cs", 42, "\"old\"", "new");
+        patch.Framework = "net99.0";
+        await EngineRunner.AddInlineAsync(patch);
+        await EngineRunner.AddInlineAsync(Patch("Other.cs", 7, "\"old\"", "new"));
+
+        EngineRunner.SettleAppliedInline(patch);
+
+        var queue = scope.Fixture.Host.State.Queue;
+        await Assert.That(queue).HasSingleItem();
+        await Assert.That(queue[0].Name).IsEqualTo("Other.cs:7");
+    }
+
+    /// <summary>
+    /// Applying one call site moves every later one in the file, so the line an applier reports is
+    /// no longer the line the entry was queued at. The member is what survives that, and the patch
+    /// is carrying it.
+    /// </summary>
+    [Test]
+    public async Task SettleAppliedFindsAnEntryWhoseLineHasMoved()
+    {
+        using var scope = new EngineScope();
+        var queued = Patch("Sample.cs", 42, "\"old\"", "new");
+        queued.MemberName = "TheTest";
+        await EngineRunner.AddInlineAsync(queued);
+
+        var applied = Patch("Sample.cs", 48, "\"old\"", "new");
+        applied.MemberName = "TheTest";
+        EngineRunner.SettleAppliedInline(applied);
+
+        await Assert.That(scope.Fixture.Host.State.Queue).IsEmpty();
+    }
+
+    [Test]
+    public async Task SettleAppliedForAnUnknownCallSiteIsHarmless()
+    {
+        using var scope = new EngineScope();
+        await EngineRunner.AddInlineAsync(Patch("Sample.cs", 42, "\"old\"", "new"));
+
+        EngineRunner.SettleAppliedInline(Patch("Nothing.cs", 1, "\"old\"", "new"));
+
+        await Assert.That(scope.Fixture.Host.State.Queue).HasSingleItem();
+    }
+
     [Test]
     public async Task TheOptOutDoesNotReachTheViewer()
     {