Repository navigation
Audit INVARIANT comments so each claims only what its oracle pins (ADR-0027 trigger) #524
Description
Summary
Audit every INVARIANT: comment in the repository and make each one claim only what its named oracle actually pins, per ADR-0027.
ADR-0027's promotion trigger has fired: a third review finding whose root was an INVARIANT: comment naming an oracle that does not pin the claim the comment makes. The ADR defers the corpus audit as a residual until that trigger fires. This issue is the promoted work.
Root cause
An INVARIANT: comment is a load-bearing prose assertion with nothing binding it to the behaviour it asserts. A comment can name a real, passing, relevant test that only pins part of its claim (or none of it). The reader trusts the broader claim; no build or test ever objects.
Instances that tripped the trigger
bca45c2— recorded in ADR-0027 as Instance update azure pipelines #1.75e64a32— local review finding on the Discarded messages — including Service Broker Error messages — commit the RECEIVE without ending the conversation #357/SQL error 208 classified as transient — a missing queue retries forever instead of failing fast #358 branch: theServiceBrokerMessageClassifierordering INVARIANT named a mutation that could not redden its oracle. Fixed in-branch.cb3be8b7— local review finding on the Discarded messages — including Service Broker Error messages — commit the RECEIVE without ending the conversation #357/SQL error 208 classified as transient — a missing queue retries forever instead of failing fast #358 branch: theServiceBrokerErrorPayloadINVARIANT claimed only the documented Error document could project a value, while its named mutation pinned only the namespace allowlist. Fixed in-branch by re-keying the comment onto what the code produces.
Scope
- Roughly 704
INVARIANT:occurrences across 237.csfiles (about 454 in production code, 250 in tests), across all modules. - For each comment: confirm the named oracle exists, apply the named mutation (or reason about it where a mutation is impractical), and confirm the oracle reddens. Where it does not, narrow the claim to what is pinned, name a real oracle, or state plainly that no test pins it.
- Per ADR-0027's trigger wording, the response is to question what each comment is keyed on, not to add a check. A lint remains rejected on the merits.
Bounded impact
Comments do not execute. A stale or overclaiming INVARIANT changes no runtime behaviour; the risk is a reader making a change on a false belief about what is protected. Low urgency, documentation accuracy.
Suggested approach
Module by module, one PR per module, so each audit stays reviewable and ships with the module it touches.
Activity
End discarded Service Broker conversations and fail fast on a missing…
brenpike commentedon Sep 24, 2026
Recorded residual from #395 (drop net8.0, single-target net10.0).
About 40 existing
INVARIANT:and mutation-evidence comments say a mutation was "measured on net8.0 and net10.0" or "on both target frameworks". After #395 only net10.0 exists. The main sites are inChatter.MessageBrokersandChatter.MessageBrokers.Reliability.EntityFramework:BrokeredMessageInbox.cs,BrokeredMessageOutbox.cs,OutboxProcessor.cs,InMemoryBrokeredMessageOutbox.cs,IPollableOutboxStore.cs,ReliabilityRetentionPurgeService.cs,UnitOfWork.cs, plus the EF integration suites.#395 deliberately leaves them alone:
Bounded impact: documentation accuracy only. No runtime behavior and no test oracle depends on the wording. This audit should cover them when it re-verifies each INVARIANT.
brenpike commentedon Sep 29, 2026
Further instances found during the #533 local review, recorded here as the existing home rather than as a new issue.
Branch-authored blocks (fixed on the #533 branch). Across four review iterations, the new
INVARIANT:blocks insrc/Chatter.MessageBrokers.RabbitMQ/src/Chatter.MessageBrokers.RabbitMQ/Receiving/RabbitMqReceiver.csrepeatedly claimed more than their oracles pin. Examples: a universal negative ("readsx-acquired-counton no delivery shape"), a bundled settlement block with one mutation for three claims, and a "derived once" clause that ignored a second read site inBuildClassicRedeliveryHeaders. Each round was fixed in-branch. A structural re-keying (native counters consumed at the receive capture point, one claim perINVARIANT:with measured mutations) closed the specific leak and the universal negative. It did not close the general class. Each rewrite of free prose can introduce a fresh clause broader than its oracles. That matches the root cause above, and ADR-0027 (amended on the #533 branch) records these as further instances of the trigger shape.Pre-existing blocks in scope for this audit. Codex also flagged six
INVARIANT:blocks in the same file that are byte-identical to master and untouched by #533. At the #533 branch HEAD they sit at roughly:270-278,:362-368,:544-553,:575-578,:605-608and:653-656. Per ADR-0027 they were not rewritten on that branch, and they belong to this audit.Bounded impact. Comment prose only. No runtime behaviour depends on these comments, and the full test suite is green on the #533 branch.