Repository navigation
The live-cleanup ratchet's exemption is a whole-file substring match, so a doc comment can silence it for every finally in the file #3014
Description
Activity
- added a commit that references this issue
on Sep 5, 2026 Closed by #3032.
The family census decided the scope: this was a one-file fix
Population swept: 65 files across
Darling.Tests(56) andLite.Tests(9) that walk toPerformanceMonitor.slnto read repo source. 30 enumerate more than one file and so can carry a per-file skip. Exactly two of the 30 skip on a raw-text substring:LiveCleanupConversionRatchetTests— this defect.LivePostgresCollectionHygieneTests(Six live Darling.Tests classes run outside the live-postgres collection, producing moving cross-class flakes on a shared store #1776's own sweep) — already hardened. PayloadDimensionLiveTests.DimensionGc_DefersWhenAFactFloorIsUnmeasurable fails first on a fresh live store: it reads timescaledb_information before any migration created the extension #1862 narrowed its attribute check to occurrences that open a line, after a prose mention exempted the store fixture; its#1776 own-storemarker reads the 25 lines above a class declaration rather than the whole file, and its remaining whole-file substring is an inclusion test that fails toward extra scanning.
Four others are comment-aware already, each via its own hand-rolled stripper — the #2913 duplication shape, a different problem, left alone.
This issue's "latent, not live" was right about the exemption and wrong one level down
The census here covered the two file-level tokens and concluded no file was mis-exempted. True. But the same defect class sits at the block level in the same function, and there it had a live instance.
DarlingObservabilityTests' repoint restore carried the sentence:Its own connection is not needed here the way LiveStoreCleanup gives one
A comment explaining why it did not need the helper — and that sentence is what
block.Contains("LiveStoreCleanup")read as compliance. The teardown still threw out of itsfinally. So the guard was concealing a real offender for as long as that comment existed. Converted toRunOwnedAsync, followingDarlingAlertingTests' precedent.The proof is a one-variable control rather than a plant: deleting only that sentence turns the identical file red on
dev, which establishes the teardown was genuinely non-compliant and the comment was genuinely what silenced it.Membership was prose-driven too, so the exemption was deleted rather than narrowed
[Collection("live-postgres")]is applied in 120 files. The raw substring matched 130. Every file the exemption silenced is among those extra ten, and no file that applies the attribute matches either exemption token.So membership moved onto the code construct and the exemption is gone, not narrowed — option 3 became moot. A new
[OwnStore]attribute was deliberately not introduced, because it would fork a convention the sibling guard keeps as a comment marker.Options 1 and 2 both taken. Option 2 cost a method call rather than new infrastructure, because
CSharpSourceWalker.StripCommentsAndStringslanded ondevearlier the same day in #3023 — andBraceBalanced's own doc comment states it is only correct over stripped text, so the brace-matching wanted the same treatment regardless. That is the second time in a day a "would need a real reader" cost assumption turned out already paid.Three variants, each reverting one half, each failing a different assertion type
Half reverted Reddens Assertion membership → raw substring AClassThatOnlyNamesTheAttributeIsNotScanned+ the tree factAssert.Equalon file countstripping → raw source both prose-compliance pins + ABraceInProseDoesNotEndTheBlockEarlyAssert.Singleon empty ×2,Assert.Emptyon non-emptyexemption restored ProseNamingAnExemptionDoesNotSilenceTheFileAssert.Equalon offender countDisjoint sets, and the assertion types differ — a half-fix cannot read as proven.
Population assertion in place of a set pin
No set was introduced; a two-token set was removed. The analogue is a vacuity floor: the
[Fact]now asserts files > 0 and blocks > 0, so a membership test that stops matching fails loudly rather than passing green over nothing.Population 120 files / 236 blocks, agreed by three independent methods — the code reflected,
grep -lE, and a standalone Python recount (Python as the third because this machine'sgrepisugrep). The block count is identical to dev's scanner at 236, which attributes the newly-found offender purely to the block-level hole rather than to a widened scan.Darling.Tests7612 → 7619, +7 exactly (5 Facts + 2 Theory cases), skipped unchanged at 313.Lite.Testsunchanged at 3323.A harness lesson this produced
The first red-proof run was invalid: the source was restored and the run used
--no-build, so the assembly still held the previous mutation. It was caught because plant B's failure named a fixture pin that a tree plant cannot touch — an impossible failure was the tell.The formulation worth keeping: an md5-verified restore does not make the last build valid. Every mutation cycle rebuilds.
Not verified, and two of these are now filed separately
- Nothing ran on Windows locally; all local runs are a
net10.0xunit host over the real files. - The live half of the converted teardown — proven to compile and to satisfy the ratchet, not proven to restore the row against PostgreSQL.
- The
Kill/File./Directory.tokens moved onto stripped text without narrowing.Killremains a bare substring matchingKilled/Killer— an over-broad exclusion, so it fails toward a missed offender. ScratchPostgresis not a throwaway cluster, contrary to this issue's own description of the exempt files: its summary says it mints a scratch database on the sharedDARLING_TEST_PGserver. FourScratchPostgresclasses carryfinallyblocks that no guard reads.- A recursion gap: this ratchet uses
SearchOption.TopDirectoryOnlywhile the sibling usesAllDirectorieswith an explicit "first file in a subfolder escapes silently" rationale.Darling.Testsis flat today (442.cs, none nested), so it is latent.
The last two are filed as their own issue rather than left in a closed issue's comment.
LiveCleanupConversionRatchetTests.Offendersdecides whether to scan a file with two whole-file substring checks over raw source — comments and string literals included:continueskips the entire file, not the member that mentioned the token. So one occurrence of either string anywhere — a doc comment, a<see cref>, a sentence explaining why the class is not exempt — silences the ratchet for everyfinallyblock in that file.How it surfaced
A lane adding a live test found its new file exempt because a doc comment in it contained the word
ScratchPostgres. Its teardown was a barecatchon hand-opened connections — exactly what this ratchet exists to catch — and the ratchet stayed green. The lane converted toLiveStoreCleanup.RunAsyncanyway rather than resting on the accidental exemption, which is why this is being reported rather than shipped.Current state: latent, not live
I checked the files that both carry
[Collection("live-postgres")]and match an exemption token. Only two have anyfinallyblocks — 8 and 13 respectively — and both are substantively entitled to the exemption: each stands up its own throwaway cluster from the bundled runtime rather than sharing the store. So no file is currently mis-exempted. The hole is in the mechanism, not in today's tree.Why it is still worth fixing
The two exempt files carry, in their own comments, a note that a substring search already misclassified them once:
The codebase has therefore already learned that substring matching misclassifies these classes — and the guard built afterwards uses substring matching for its exemption. The same file's own doc comment also records fixing a different comment-induced blind spot in this scanner ("a long explanatory comment pushes the actual statements past" a fixed line window, fixed by brace-matching). This is the third instance of the same category in one guard: prose is being read as though it were code.
The failure mode is silent and self-concealing. A file gains the exemption by discussing the exempt concept, the ratchet reports zero offenders, and a green ratchet is indistinguishable from a compliant tree.
What would fix it
The intent is "this class mints and drops its own database, so an abandoned teardown cannot reach anyone else." That is a property of what the class does, not of what its comments say. Options:
[OwnStore]) or an explicit opt-out list keyed by class name, so the exemption is declared once, deliberately, and cannot be acquired by accident.(1) is the smallest change that removes the whole class of problem, and it makes the exemption reviewable — an attribute in a diff is visible in a way a sentence in a comment is not.
Not verified
Whether the sibling guards in this family (
#1776's own sweep, and the other textual ratchets inDarling.Tests) use the same whole-file substring exemption shape. If they do, this fix wants to be applied across them rather than to this one file.