A cancelled page read gets the cancellation when the store connect times out first (#5678)

[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5678.

Why

When a browser leaves a page mid-read, its request token is cancelled. The store driver, given a token that is already cancelled, can throw a connect timeout instead of a cancellation. Code that only skips real cancellations then treats the person who left as a failure. It writes an Error line and a 500 for a request nobody is waiting on.

The first version of this fix patched one site at a time and tested the exception type before the token. Routes with no local catch were still affected (/api/fleet, /api/ag, /api/ag/count, /api/views/*, /api/alerts/*). They wrote a failure line for a gone caller. The sites that call DarlingWebFailureLog.Report directly were unguarded too. This round moves the rule to the two places every request passes through.

What changes

Part 1 (earlier commit)

DarlingServerResolver.LoadEnabledOrFaultAsync and the data-start note probe (WebDataStartNote) no longer turn a cancelled caller's connect timeout into a fault sentence or an unannotated grid. This round, both catches call a small method that takes the exception and the token. These are DarlingServerResolver.FaultOrCancel and WebDataStartNote.ProbeFailed. The rule is tested on a hand-built exception, with no connection attempt to time.

Part 2 (earlier commit)

DarlingWebEndpoints.ThrowIfCallerGone(result, token) ends a gone caller's tool-caught "Error during ..." sentence with a cancellation, in the /api/read/* loop. It was also called in the server-databases route. Round 2 removes that call.

This round (review round 1)

1. Backstop (DarlingWebFailureObserver): the quiet arm is now when (IsCallerGone(RequestAborted)). That is just RequestAborted.IsCancellationRequested. Round 2 dropped a BadHttpRequestException carve-out that round 1 had added. The token decides, not the exception type. Every exception the old OperationCanceledException or IOException filter swallowed is still swallowed. That includes BadHttpRequestException, which derives from IOException and was swallowed before its own arm. The one addition is the NpgsqlException a connect timeout throws. It writes no line and no body, as before.
2. DarlingWebFailureLog.Report (both overloads) takes a REQUIRED request-token parameter, so a missed site is a compile error. When the token fired, it writes ONE Debug line instead of the Warning or Error. The line holds the route, the elapsed time and the kind caller_gone. It also holds the exception type and SQLSTATE, or the same scrubbed sentence. It still marks the request as reported. A caller with no request in reach passes CancellationToken.None explicitly.
3. The two untokened ResolveOrErrorAsync calls (triage, DarlingTriageEndpoint.cs:618, and alert notebook, AlertNotebookEndpoint.cs:102) now get the request token. This does more than quiet a log line: those registry reads are now cancellable, so a gone caller's lookup stops. That is intended.
4. Triage RunSectionAsync: the throw added in part 2 is removed. Its two Report calls carry the token. Round 1 said this made its "Never throws" doc true again. The review showed that was wrong, and round 2 fixes it (see below).
5. Compose: /api/compose/run ends the request when outcome.Payload is null && !outcome.IsNotFound. That covers the panel author's own query error, which ComposeRunFailureResult logs as a Warning.
6. Reads versus writes: ThrowIfCallerGone stays in the /api/read/* loop (reads). It is removed from ServerTagToolResult, and its optional parameter is now a required token. The write routes' only trace for a gone caller is the Debug line from item 2.

The functions that reach Report now take the token too (ToHttpResult, ServerErrorResult, MuteRuleToolResult, ServerTagToolResult, ComposeRunFailureResult, DarlingWebStatementSweep.Refusal and JsonText).

Site list

Every Report call, and every helper that reaches it, with the token it passes:

Site	Token
DarlingWebFailureObserver (both Report calls)	context.RequestAborted
/api/read/* loop catch and ToHttpResult	context.RequestAborted
/api/compose/run	context.RequestAborted
/api/alert-templates (now takes the HttpContext), /api/alerts/test	context.RequestAborted
Mute-rule routes (4), server-tag routes (5)	context.RequestAborted
Server add, edit (/api/servers, /api/servers/{id}, admin read)	context.RequestAborted
Alert-history dismiss (/api/alert-history/dismiss)	context.RequestAborted
Server write slot (RunInServerWriteSlotAsync, 2 sites)	CancellationToken.None: the write core takes no request token on purpose, and the slot runs after the request is over
Server databases (2 sites)	context.RequestAborted
Triage: resolve-server (2), alert-history, sections (2), page sweep	context.RequestAborted
Alert notebook: resolve-server (2), alert-history, status-history, page sweep	context.RequestAborted
Alert notebook: scope-server, collector-freshness, prefetch (2)	the method's own token
Fleet sweep JSON helpers	CancellationToken.None: no logger, no request

Searched the web handlers and the host for any direct LogError or LogWarning in a catch with the request token in reach. Two remain: the compose author-error Warning (covered by item 5) and the server-write-slot timeout Warning (about a held slot, not a caller).

Test plan

• Backstop: a route throwing NpgsqlException under a cancelled token writes no failure line and no body, and the test shows the route ran. The control, with a live token, writes one Warning and answers 503 (timeout), or one Error and answers 500 (refused connect).
• Report under a cancelled token writes Debug only, on both overloads, and marks the request as reported.
• IsCallerGone and the two seams (FaultOrCancel, ProbeFailed) are tested directly on a hand-built exception and token. No timing: the old Thread.Sleep(1200) logger racing Timeout=1 is gone, with the three tests that rested on it.
• The roster census ("every web handler") is replaced. Each new pin names what it checks. They cover the read loop guard and the compose guard, and check that ServerTagToolResult has no guard. The compiler now enforces Report. The triage runner is covered by behavior tests (round 2), not a source pin.
• The web test server's data source is disposed by the test.
• RED-plant 1: reverted item 1 (type-based filter). TheBackstop_WritesNoLineAndNoBody_ForAStoreTimeoutUnderACancelledToken failed, nothing else did. Restored.
• RED-plant 2: reverted item 2 (Debug branch of Report). Report_UnderACancelledToken_WritesOneDebugLine_AndNoWarningOrError failed, nothing else did. Restored.
• DarlingWebFailureHandlingTests, DocCommentHygieneTests: 116 passed, 0 failed.
• First wide run: 2454 total, 2 failed, 186 skipped. It covered every *Census* and *Ratchet* class and the web exception text census. It also covered the WebDataStartNote*, triage, alert notebook, compose, server tag and statement filter classes. The endpoint, mute rule, fleet sweep and server databases classes ran too.
• Both failures were mine and are fixed. A new block comment in DarlingWebFailureLog.cs closed an old unmatched /* in a doc line. A parameter name in the registry seam missed the census allow-list entry.
• Re-run of the census, ratchet, triage, notebook, compose, server tag, statement filter and endpoint classes: 1733 total, 0 failed, 125 skipped.
• Darling and Darling.Tests build with 0 warnings.
• No PostgreSQL here, so live cases skipped. The full Darling.Tests suite was not run. CI covers it.

Round 2 (review round 2)

1. Triage runner (blocking). The review was right. The read tools resolve the server outside their own try (DarlingMcpDataTools.cs:79, 116). So a cancellation escaped the tool and then RunSectionAsync. So did a connect timeout that FaultOrCancel (DarlingServerResolver.cs:149) turned into one.
   • Fix: RunSectionAsync (DarlingTriageEndpoint.cs:821) has a new first catch, when (requestContext.RequestAborted.IsCancellationRequested). It writes one Debug line through Report and no Warning or Error. It adds no error text and returns the section.
   • A live token keeps its old path: a fixed sentence on the card and one failure line.
   • The method is internal so tests can call it.
   • The source pin is replaced by behavior tests. TheTriageSectionRunner_ReturnsQuietly_WhenASectionsToolThrowsForAGoneCaller has two cases, a tool throwing OperationCanceledException and one throwing the connect-timeout NpgsqlException. Both throw outside any try of their own, under a cancelled request token. The control is TheTriageSectionRunner_StillLogsAndDegradesTheCard_ForTheSameThrowUnderALiveToken.
   • RED at 95c9e36 (product code unchanged, only internal added): the two gone-caller cases failed, the first with System.OperationCanceledException : The operation was canceled., and 1 passed. GREEN with the fix: 3 of 3 pass.
2. BadHttpRequestException under a fired token. Restored the old behavior in code. IsCallerGone takes only the token, so the malformed-request type is swallowed again, as the old first arm did. The new test TheBackstop_StaysQuiet_ForAMalformedRequestUnderACancelledToken pins it (status 200, no Warning, no Error). The live-token test BadHttpRequestException_AnswersItsOwnStatus_WritesNoBody_LogsAtDebugNeverError still pins its own arm.
3. Server databases guard. The review is right. The only ServerError-shaped string ResolveOrErrorAsync returns is the registry-fault sentence. Under a fired token FaultOrCancel throws before that sentence is built (DarlingServerResolver.cs:149). I removed the ThrowIfCallerGone call and its comment in DarlingServerDatabasesEndpoint.cs. A comment on the store read's catch says why the lookup above needs none. The direct store read's ThrowIfCancellationRequested stays, because that is the live connect-timeout case. The two later context.RequestAborted arguments are left alone. They are harmless and keep the call shape the same as the other routes.
4. Triage and notebook registry reads are cancellable. This is said in item 3 of "This round" above.
5. Riskiest lines.
   • Observer swallow (DarlingWebFailureObserver.cs:53): kept on purpose. ASP.NET Core's own exception handler makes the same call for a client abort. The old arms were equally silent for OperationCanceledException and IOException. The addition is the connect-timeout NpgsqlException, whose cause here is the caller leaving.
   • One gap remains. If a route with no local catch hits a bug only while the caller's token is fired, the observer writes no line for it. Every handler that catches and logs itself writes the Debug line.
   • I did not add a Debug line to the observer arm. The earlier no-line tests assert that this arm writes nothing: /api/__test/abort, abort-io and the store-timeout test.
   • Resolver seam (DarlingServerResolver.cs:149): the review re-read every caller. It found each one under a when (ex is not OperationCanceledException) catch, or routed to the observer. The one that was not safe, the triage section runner, is fixed above with a behavior test. The seam itself is pinned by AFailedRegistryRead_SurfacesCancellation_ForACancelledCaller_AndTheFaultSentenceForALiveOne.
   • /api/read/* throw (DarlingWebEndpoints.cs:454): with a fired token nobody receives the body. Ending in a throw instead of a body changes nothing a caller can see. A 57014 formed before the caller left also ends there. Its latency sample is recorded first (:449-453), and ReadOutcomeClassifier.ClassifySentence says Cancelled for a fired token. So the sample is still right. It is pinned by ThrowIfCallerGone_OnlyEndsAServerErrorSentence_ForACancelledToken and the source pin on the loop.
6. The body no longer mentions a changelog.

Round 2 tests:

• DarlingWebFailureHandlingTests and WebDataStartNoteEventReadsTests: 55 passed, 0 failed.
• Every Census and Ratchet class in Darling.Tests, plus the alert notebook, triage, notification, server databases and database filter classes: 870 total, 0 failed, 7 skipped. The skips are live cases, because there is no PostgreSQL here.
• Darling and Darling.Tests build with 0 warnings.
• The full Darling.Tests suite was not run. CI covers it.

Round 2 handoff: done, items 1 to 6, pushed as 0a81b54. Not done: nothing.

[claude]

The fix is per-site, but the thing that is type-gated is the backstop

The PR body's own "Backstop check" paragraph states the mechanism precisely:

DarlingWebFailureObserver.InvokeAsync catches OperationCanceledException (or IOException) when RequestAborted is cancelled, and that arm writes no log line and no body.

That is the whole bug, in one filter — DarlingWebFailureObserver.cs:55-56 tests the exception TYPE first and the token second:

catch (Exception ex) when ((ex is OperationCanceledException or IOException)
    && context.RequestAborted.IsCancellationRequested)

Every route that does not have a local catch therefore still hands a gone caller's non-OperationCanceledException to the generic arm at line 75, which writes the Error line and writes a 500 body to a caller who left. Those routes are not in the body's table, and nothing in them is a tool string or a store sentence this PR's helper could intercept. Confirmed by reading, token threaded, no catch between the store and the observer:

• DarlingWebEndpoints.cs:375-380 — /api/ag/count → GetAvailabilityGroupCountAsync(postgres, null, context.RequestAborted). The chain (DarlingAgReader.cs:114 → DarlingAgStatesReader.cs:331 → GetNewestInstantsAsync) has no catch at all, so an NpgsqlException from the connect reaches the observer raw.
• DarlingWebEndpoints.cs:343-356 (/api/fleet) and 365-370 (/api/ag) — same shape.
• /api/views/* and /api/alerts/* — "Web viewer: tool errors still show the exception text in the browser #4283: no local catch" is the documented design (lines 572, 613, 635), and the store calls take context.RequestAborted (e.g. line 616).

Plus three sites that do hand a sentence or an exception to a failure log with the request token in reach, and are absent from the table:

• DarlingTriageEndpoint.cs:822 (and 705/708) — see the inline note; the arm right below the one this PR fixed.
• AlertNotebookEndpoint.cs:162-164, :187-189, :1046 — the alert-notebook page is the triage page's twin and is not mentioned anywhere in the PR. (:302 is covered, incidentally, because ResolveScopeServerAsync threads the token into LoadEnabledOrFaultAsync.)
• DarlingWebEndpoints.cs:1837-1839 — /api/alert-history/dismiss, token threaded into DismissKeysAsync.

None of this says the six converted sites are wrong — the helper and its placement after the latency sample read correctly, and the table's reasoning about mute rules, the write slot, the admin read, /api/alert-templates and /api/alerts/test all check out against the code (readServer is pinned to CancellationToken.None at line 1097; the mute-rule cores take no token at 960/977/1012/1028). It says the chosen shape needs one conversion per site forever, with a roster census behind it, where widening the observer's filter — the single place that already knows "the token fired, so nobody is waiting" — would cover the untouched routes above at once and make the per-site conversions redundant rather than load-bearing:

catch (Exception ex) when (context.RequestAborted.IsCancellationRequested
    && ex is not BadHttpRequestException)

That is a design call, not a one-line demand — but as it stands, the issue's symptom (a false failure line for a browser that left while the store connect timed out) is still reachable on /api/fleet, /api/ag, /api/ag/count, /api/views/*, /api/alerts/*, /api/alert-notebook, /api/alert-history/dismiss and the triage binding catch, and the new census asserts the opposite in its name.

Not blocking, for the record

The two parts of #5678 are also asymmetric about where the guard sits. Part 1 puts ThrowIfCancellationRequested() inside the catch of a shared seam (DarlingServerResolver.LoadEnabledOrFaultAsync, WebDataStartNote.AddNoteAsync), which changes the contract for every caller, not only the web ones: ResolveOrErrorAsync is the resolve-and-bail seam ~190 MCP tools use, and DiagnosticsBundleRunner.cs:255 is a CLI caller. Both end up throwing where they used to get a sentence. For a cancelled MCP request or a Ctrl-C'd bundle that is the better answer, so this is fine — but it is a wider blast radius than the table implies, and no test pins either caller under cancellation.

[claude]

Verdict: CHANGES REQUESTED

The helper is right and its placement after the latency sample is right; the table's reasoning about what it left alone checks out. What does not hold is the completeness the new census asserts in its name: the decision "a gone caller is not a failure" lives in the observer's TYPE-gated filter (DarlingWebFailureObserver.cs:55-56), so routes without a local catch still write an Error line and a 500 body for exactly the #5678 shape, and three sentence/exception sites are not in the table. Detail in the top-level note and four inline comments.

Claims

• VERIFIED — a tool-caught sentence went on to the error answer and logged a line — DarlingMcpFleetTools.cs:186-189 returns FormatError; ToHttpResult's ServerError arm (DarlingWebEndpoints.cs:6052-6056) calls ServerErrorResult, which calls Report (:6085); the old catch at :412 never saw it.
• VERIFIED — "The latency sample was already right" — ReadOutcomeClassifier.cs:66-71 returns Cancelled on a fired token, and the helper runs after both recorders (DarlingWebEndpoints.cs:449-454).
• VERIFIED — "ServerTagToolResult … All 5 callers pass it" — DarlingWebEndpoints.cs:2018, 2041, 2060, 2079, 2098.
• VERIFIED — mute-rule routes unchanged, the cores take no token — :960, :977, :1012, :1028.
• VERIFIED — server add/edit/admin read take no request token — :1097 (CancellationToken.None), :1455-1456.
• VERIFIED — /api/alert-templates and /api/alerts/test take no token — :833, :886.
• VERIFIED — the Backstop check paragraph — DarlingWebFailureObserver.cs:55-62 (quiet arm), :75-105 (the only Report calls), :111-118 (tail check gated on >= 500).
• VERIFIED — part 1's two catches — DarlingServerResolver.cs:140 and WebDataStartNote.cs:749, the latter inside AddNoteAsync, which AddAsync:609 awaits outside its own try.
• VERIFIED — four new cases, the helper's rule tested on its own — DarlingWebFailureHandlingTests.cs:466, 505, 528, 544; :528 covers all three branches of ThrowIfCallerGone (DarlingWebEndpoints.cs:6072-6078).
• UNVERIFIED — the suite counts (113 / 1059 / 593, 0 warnings) and the RED-plant — needs a build; this tree has no restored packages (Darling/Darling.Tests/obj absent).
• UNVERIFIED — "the driver, given a token that is already cancelled, can throw a connect timeout instead of a cancellation" — the premise of the change; a property of the pinned Npgsql version's connect path, provable only by running ASpentConnectBudget_ThrowsATimeout_NotACancellation_UnderACancelledToken.
• UNVERIFIED — "The store spends its 1 second connect budget deterministically" — the mechanism is Thread.Sleep(1200) in an Npgsql logger racing Timeout=1 (DarlingWebFailureHandlingTests.cs:456-462); determinism on a loaded runner needs repeated live runs.
• REFUTED — the census's summary, "every web handler that hands a tool's string, or a store fault, to a failure log with the request token in reach ends a gone caller's request first" — DarlingTriageEndpoint.cs:822 (and 705/708), AlertNotebookEndpoint.cs:162-164 and :187-189, and DarlingWebEndpoints.cs:1837-1839 all thread context.RequestAborted into the read, catch when (ex is not OperationCanceledException), and call Report. The pin checks five IndexOf windows and sees none of them.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs:454 — every /api/read/* answer now ends in a throw rather than a body when the token fired and the result is a ServerError sentence, a genuine 57014 formed before the user navigated away included — pinned by AToolCaughtFailure_ForABrowserThatLeft_WritesNoFailureLine, with AToolCaughtFailure_ForALiveBrowser_StillAnswersServerError_AndWritesOneLine as the control.
2. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingServerResolver.cs:140 — a shared seam: ResolveOrErrorAsync is the resolve-and-bail call in ~190 MCP tools and in DiagnosticsBundleRunner.cs:255, all of which now get a throw where they got a sentence — pinned for the seam by TheRegistryRead_SurfacesCancellation_WhenTheConnectBudgetIsSpent; UNPINNED for any caller.
3. Darling/PerformanceMonitor.Darling.Service/DarlingTriageEndpoint.cs:806 — makes RunSectionAsync throw against its own doc at :763 ("Never throws: a broken section is one card on the page, not a dead page") and leaves the sibling catch at :822 logging the PR's own exception shape — UNPINNED behaviourally; only the source pin at DarlingWebFailureHandlingTests.cs:561-563 covers it.

Traps near this diff

• "A roster-based census does not see the file it does not roster: EveryPagedTool_ObservesTruncation_… walked ten tools and missed three in another file (#3699). An anti-pattern census sweeps the class (every tool body); rosters are for the positive contract only." — applies — DarlingWebFailureHandlingTests.cs:541-564 is five lookups over three files under a universal name; the misses are under Claims.
• "A pin that encodes the lie … the >= limit inference as its own proof (#3699). Ask what a pin would fail ON." — does not apply — the control at DarlingWebFailureHandlingTests.cs:505 asserts the status and the one line a live token still gets, and WebDataStartNoteEventReadsTests.cs:76-85 pins the premise separately from the behaviour.
• "A test that reads the clock twice measures the CI scheduler … One anchor owns both ends of an interval (#3676)." — does not apply — nothing measures or asserts an interval; the assertions are exception types (WebDataStartNoteEventReadsTests.cs:84). The wall-clock dependence sits in the setup; see the checklist.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply — Lite ships no HTTP surface (grep -rl RequestAborted Lite/ returns nothing) and every changed file is under Darling/PerformanceMonitor.Darling.Service/.
• "An expression-bodied member holding a { } property pattern is read SHORT by the T-SQL convention guard's member walk (#3667)" — does not apply — the new member is block-bodied with no property pattern (DarlingWebEndpoints.cs:6072-6078) and the changed signature stayed a block body (:2292-2296).
• The SQL traps (ORDER BY ordinals, integer-division rates, page denominators, >= limit truncation) — do not apply — no SQL changed; DarlingServerResolver.LoadEnabledServersSql (:74-78) is untouched and no collector query is in scope.

Divergence (blind read vs PR body)

1. The table enumerates what was judged safe, not what logs. Routes with no local catch are absent, yet by the body's own Backstop reasoning they are the largest hole: /api/ag/count (DarlingWebEndpoints.cs:375-380 → DarlingAgReader.cs:114 → DarlingAgStatesReader.cs:331, no catch in the chain), /api/fleet, /api/ag, /api/views/*, /api/alerts/*. Three sites of the table's own shape are also absent: the triage binding catch, all of AlertNotebookEndpoint, and /api/alert-history/dismiss.
2. "/api/compose/run … when the outcome is a store fault or a server error" — ComposeRunFailureResult has three logging arms. The author-SQLSTATE Warning at :3291-3299 is reachable with Fault == null and IsServerError == false (FromRunException's remap arm, :2581-2585), so the guard at :684 lets it log for a gone caller.
3. "the data-start note read logged or answered a fault for a caller who was gone" — that probe only ever logged at Debug (WebDataStartNote.cs:751-753), so part 1 there is about not answering, not a false line. The registry half lands only where the token is threaded: DarlingTriageEndpoint.cs:618 and AlertNotebookEndpoint.cs:102 pass none, so a gone caller's registry fault still reaches Report at :629/:650 and :110.

Reviewer checklist

• Run the four new cases and WebDataStartNoteEventReadsTests repeatedly on a loaded runner: everything rests on a Thread.Sleep(1200) in Npgsql's logger outlasting a Timeout=1 budget, and on this Npgsql version not checking the token before the connect; the sleep also blocks a pool thread under xUnit parallelism.
• On a live host, abort an /api/ag/count or /api/fleet read against a store slow to accept connections and see whether an Error line and a 500 body still appear — the residual case this shape cannot reach.
• On a live host, abort a POST /api/server-tags whose core then fails and confirm an operator can still tell the write failed (LogServerTagWrite returns early for a non-2xx envelope at :2145-2148, and the ServerErrorResult line is now suppressed).
• UnreachableStore() / ConnectBudgetSpent() build an NpgsqlDataSource the web tests never dispose (only the TestServer is in a using).

Claude posting for Erik Darling. Stale: every finding is fixed in 95c9e36 and each thread has a reply. The round-2 review on that head reached its turn limit without a verdict.

[erikdarlingdata]

Claude posting for Erik Darling. This answers item 2 under "Riskiest lines" in review round 1, about the resolver change that reaches every MCP tool and the diagnostics bundle.

No caller gets a new kind of outcome. FaultOrCancel throws only when the caller's own token has already fired (cancellationToken.ThrowIfCancellationRequested(), DarlingServerResolver.cs:149). The catch above it has let OperationCanceledException through to every caller since #4203 (:130). This change sends the connect timeout that the driver can raise under a fired token down that same path. A caller whose token is live still gets the fault sentence, as before.

[claude]

Round 2 reads as the right shape; two of the body's sentences do not hold

Round 1's finding is genuinely fixed, and fixed in the place that makes the per-site conversions redundant rather than load-bearing: IsCallerGone at DarlingWebFailureObserver.cs:53-54 is the token first and the type second, so /api/ag, /api/ag/count, /api/fleet, /api/views/* and /api/alerts/* — the routes with no local catch that round 1 named — are covered without a line of their own. Making the token a REQUIRED parameter on both Report overloads is the better trade than the roster census it replaces, and the three timing-dependent tests (the Thread.Sleep(1200) logger racing Timeout=1) are gone from the end state, along with the undisposed NpgsqlDataSource the round-1 checklist flagged — BuiltServer now owns it (DarlingWebFailureHandlingTests.cs:173-200).

I walked the site-list table row by row against grep -rn "DarlingWebFailureLog.Report(" and the seven result helpers: every row matches, the counts match (server databases 3, triage 6, notebook 9), and the two CancellationToken.None rows are right for the stated reasons — the fleet-sweep helpers pass a null logger, so Refusal cannot log at all (DarlingFleetSweepEndpoints.cs:265, 272), and RunInServerWriteSlotAsync genuinely has no HttpContext in reach by its own doc (DarlingWebEndpoints.cs:1442). "Two direct LogWarnings remain" is exact: :1493 and :3297 are the only two in the five web-surface files, and item 5's guard at :686-689 does close the second one for a gone caller.

Two sentences in the body are not true of this tree.

1. "Triage RunSectionAsync: … so its doc (Never throws) is true again." The explicit throw is gone, but the method still propagates a gone caller's cancellation, and part 1 of this same PR is what widened the set of exceptions that make it do so. Chain and the smallest honest fix are in the inline note on TheTriageSectionRunner_DoesNotThrowForAGoneCaller — in short, read tools resolve the server outside their own try (DarlingMcpDataTools.cs:79, 116), so FaultOrCancel's new throw (DarlingServerResolver.cs:149) exits the tool, and RunSectionAsync's catch filter at DarlingTriageEndpoint.cs:820 excludes OperationCanceledException. The behaviour is fine — nobody is reading that page — but the doc at :763, the test's name and the body all assert a contract the code does not keep, and the pin is five IndexOf lookups that would stay green either way.

2. "The changelog line now says no failure is logged. It adds that a store timeout arriving after the browser left leaves one Debug line." No changelog file is in this PR (gh pr diff 5679 --name-only lists 20 files, all under Darling/), and neither CHANGELOG.md nor docs/changelog/3.10.md mentions #5678 or any of this wording — grep -rn "5678\|caller_gone\|browser left\|Debug line" CHANGELOG.md docs/changelog/3.10.md is empty, and that file's newest entry is #5637. The line referenced is #4281's, in the already-released docs/changelog/3.9.md:485. Given build.yml:777-783 describes a separate changelog-splice lane, the entry may legitimately belong to a splice PR rather than this one — I am not asking for a changelog edit here, only noting that the sentence describes an edit that is not in the tree.

Three non-blocking notes are inline: the BadHttpRequestException carve-out is a behaviour change rather than the no-op item 1 implies (it derives from IOException, so the old first-arm filter swallowed it under a fired token and never set its status); the observer's quiet arm keeps no trace at all while Report's new branch deliberately keeps one Debug line, which now matters more because the arm covers every exception type; and ThrowIfCallerGone at DarlingServerDatabasesEndpoint.cs:77 is unreachable once FaultOrCancel is in place.

Everything else I checked held: the LoadEnabledOrFaultAsync blast radius is safe at all ~130 resolve sites (every one either sits under a when (ex is not OperationCanceledException) catch or routes to the observer, and DarlingMcpTools.cs:1356 still passes no token, so MuteAnalysisFindingScopeTests.cs:255's source pin still matches); AlertNotebookEndpointTests.cs:560-563 is a DoesNotContain pin and stays satisfied; WebDataStartNoteTests.cs:515-525's ordering pin still finds its three anchors; the ex.Message census's allow-list entry for RegistryReadFaultPrefix}{ex.Message} still matches the one occurrence after the move into FaultOrCancel; no stale cref parameter list survives; and ThrowIfCallerGone's doc claim about the latency sample is accurate (ReadOutcomeClassifier.cs:66-71 returns Cancelled on a fired token, and the call sits after both recorders at DarlingWebEndpoints.cs:449-454).

[claude]

Verdict: CHANGES REQUESTED

Round 1's finding is fixed, in the one place that covers the routes it named. Item 4 does not hold: RunSectionAsync still propagates a gone caller's cancellation against the doc the body says it restored, and the pin named for that property cannot fail on it. Detail in the top-level note and three inline comments.

Claims

• VERIFIED — the quiet arm is now the token, not the exception type — DarlingWebFailureObserver.cs:53-54, 64; pinned by IsCallerGone_IsTheTokenNotTheType_ExceptForABadHttpRequest (DarlingWebFailureHandlingTests.cs:540-550) and TheBackstop_WritesNoLineAndNoBody_ForAStoreTimeoutUnderACancelledToken (:218-236) with the live-token Theory control at :244-257.
• VERIFIED — both Report overloads take a required token, write one Debug line, and still mark the request reported — DarlingWebFailureLog.cs:223-237, 261-273 (MarkReported() precedes the check on both); Report_UnderACancelledToken_WritesOneDebugLine_AndNoWarningOrError (:556-575).
• VERIFIED — the two untokened ResolveOrErrorAsync calls now get the request token — DarlingTriageEndpoint.cs:618, AlertNotebookEndpoint.cs:102.
• VERIFIED — item 5 covers the compose author-error Warning — the guard at DarlingWebEndpoints.cs:686-689 sits on the only path to logger.LogWarning at :3297.
• VERIFIED — ThrowIfCallerGone removed from ServerTagToolResult, its optional parameter now a required token — DarlingWebEndpoints.cs:2295-2304, pinned at DarlingWebFailureHandlingTests.cs:604-607.
• VERIFIED — the site-list table, row by row — every Report( call and all seven helper call sites match the stated token, counts included (server databases 3, triage 6, notebook 9); both CancellationToken.None rows hold (DarlingFleetSweepEndpoints.cs:265, 272 pass a null logger; DarlingWebEndpoints.cs:1442 forbids the HttpContext).
• VERIFIED — "two direct LogWarnings remain" — a sweep of the five web-surface files returns exactly DarlingWebEndpoints.cs:1493 and :3297.
• VERIFIED — the Thread.Sleep(1200) tests are gone and the test server's data source is disposed — no Thread.Sleep(1200)/ConnectBudgetSpent/AToolCaughtFailure_ForABrowserThatLeft survives; BuiltServer owns it (DarlingWebFailureHandlingTests.cs:173-200, 293).
• VERIFIED — ThrowIfCallerGone's doc claim that the latency sample is already right — ReadOutcomeClassifier.cs:66-71 returns Cancelled on a fired token; the call sits after both recorders (DarlingWebEndpoints.cs:449-454).
• UNVERIFIED — every suite count, "0 warnings", and both RED-plants — needs a restored tree and a .NET SDK, neither reachable here.
• UNVERIFIED — "the driver can throw a connect timeout instead of a cancellation under an already-cancelled token" — still the premise; the tests now hand-build that exception, so nothing in the tree demonstrates it on the pinned Npgsql version.
• REFUTED — "the throw added in part 2 is removed, so its doc (Never throws) is true again" — read tools resolve outside their own try (DarlingMcpDataTools.cs:79, 116), so FaultOrCancel's new throw (DarlingServerResolver.cs:149) leaves the tool, and RunSectionAsync's filter excludes OperationCanceledException (DarlingTriageEndpoint.cs:820). A gone caller whose section hits a connect timeout ends the whole triage page, not one card.
• REFUTED — "The changelog line now says no failure is logged…" — no changelog file is in the diff, and 5678/caller_gone/browser left/Debug line appear nowhere in CHANGELOG.md or docs/changelog/3.10.md (newest entry #5637); the #4281 line referenced is in the released docs/changelog/3.9.md:485. The splice lane (build.yml:777-783) may own it, so a wrong sentence rather than a missing deliverable.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Hosting/DarlingWebFailureObserver.cs:53 — widens a silent swallow from two exception types to every type but one, so a server-side bug coinciding with a fired token leaves no line at any level on the no-local-catch routes — pinned by IsCallerGone_IsTheTokenNotTheType_ExceptForABadHttpRequest and TheBackstop_WritesNoLineAndNoBody_…; the cancelled-token-plus-BadHttpRequestException case is UNPINNED.
2. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingServerResolver.cs:149 — a fault sentence becomes a throw at the seam ~130 MCP and web sites share; I re-read all of them and each sits under a when (ex is not OperationCanceledException) catch or routes to the observer — pinned for the seam by AFailedRegistryRead_SurfacesCancellation_ForACancelledCaller_AndTheFaultSentenceForALiveOne, UNPINNED for any caller, and the mechanism behind the REFUTED triage claim.
3. Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs:454 — an /api/read/* answer now ends in a throw rather than a body whenever the token fired and the result is a ServerError sentence, a genuine 57014 formed before the caller left included — pinned by ThrowIfCallerGone_OnlyEndsAServerErrorSentence_ForACancelledToken (:579-589) and the source pin at :597-607; UNPINNED end-to-end.

Traps near this diff

• "A pin that encodes the lie … the >= limit inference as its own proof (#3699). Ask what a pin would fail ON." — applies — TheTriageSectionRunner_DoesNotThrowForAGoneCaller (DarlingWebFailureHandlingTests.cs:624-634) is one Assert.DoesNotContain over the method's own source span, named for a behaviour that arrives from DarlingServerResolver.cs:149 one frame down, which the pin cannot see.
• "A roster-based census does not see the file it does not roster … rosters are for the positive contract only." — does not apply — the roster census round 1 faulted is gone; the positive contract is the compiler (DarlingWebFailureLog.cs:223, 261) and the four survivors each name one wiring spot. The compiler enforces that a token is passed, not the request's.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding." — does not apply — Lite ships no web dashboard and no counterpart to DarlingWebFailureLog (a grep for it over Lite/ and Lite.Tests/ is empty); its only HTTP surface is the MCP transport at Lite/Mcp/McpHostService.cs:121, with no request-scoped failure log.
• "An expression-bodied member holding a { } property pattern is read SHORT by the T-SQL convention guard's member walk (#3667)." — does not apply — the one new expression-bodied member holds a type pattern (DarlingWebFailureObserver.cs:53-54); the other three new methods are block-bodied.
• "A test that reads the clock twice measures the CI scheduler (#3676)." — does not apply, and this round removed it — the new seams take a hand-built exception and token (DarlingServerResolver.cs:147, WebDataStartNote.cs:755) and nothing in the diff measures an interval.
• The SQL traps (ORDER BY ordinals, integer-division rates, page denominators, >= limit truncation) — do not apply — no SQL changed; DarlingServerResolver.LoadEnabledServersSql:74-78 is untouched.

Divergence (blind read vs PR body)

1. Item 4 claims a restored contract that is not restored, and its test cannot fail on it — the blocking item.
2. The final changelog sentence describes an edit nowhere in the tree.
3. Item 1 is presented as preserving the old arms exactly. It also changes BadHttpRequestException: that type derives from IOException and the old swallow arm ran first, so under a fired token a malformed request was swallowed with its status never set; the carve-out now routes it to :73-84, which answers bad.StatusCode. An improvement framed as a no-op, untested on that path.
4. Item 3 reads as logging-only, but passing the token to ResolveOrErrorAsync at DarlingTriageEndpoint.cs:618 and AlertNotebookEndpoint.cs:102 also makes those registry reads cancellable for the first time — the mechanism behind divergence 1.
5. "The write routes' only trace for a gone caller is the Debug line" holds for server-tag and mute-rule routes, not for the write routes with no local catch (DarlingWebEndpoints.cs:572, 613, 755), which reach the observer's quiet arm and leave nothing.

Reviewer checklist

• Build and run Darling.Tests: every count, the "0 warnings" claim and both RED-plants are unverifiable here, and four source pins' IndexOf windows shift with the new hunks.
• On a live host, abort an /api/triage read while the store is slow to accept connections and confirm the log is quiet and the page simply ends — nothing in the tree exercises RunSectionAsync under a fired token.
• On a live host, send a malformed or oversized body and abort it in the same breath; confirm the response carries the 413/400 rather than the old silent 200.
• Confirm the premise once on the pinned Npgsql version: a connect under an already-cancelled token throwing NpgsqlException(TimeoutException), not OperationCanceledException.
• Decide whether #5678 wants a docs/changelog/3.10.md entry here or in the splice lane, and correct the body's last sentence either way.

Claude posting for Erik Darling. Stale: every round-2 item is fixed or answered in 0a81b54, each inline thread has a reply and is resolved, and the PR body answers the summary items. The follow-up review on that head reached its turn limit without a verdict.