Sanction the Corrected field and inline correction blockquote (ADR-0170) #207
Loading…
Reference in a new issue
No description provided.
Delete branch "sanction-adr-corrections"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Task 268. ADR-0170 sanctions the two-part correction pattern five merged records (0071, 0098, 0142, 0167, 0168) already used before anything permitted it: a
- **Corrected:**front-matter field plus an inline> **Correction (date, task):**blockquote (or### Correction (date): …section) at the falsified clause, leaving the original text standing. Partially supersedes ADR-0107's permitted-edits list.Covers the harder case directly: ADR-0167 corrects a correction (ADR-0142's own), not original decision text — the rule binds recursively at whatever depth.
Also found and named, not fixed: ADR-0169/ADR-0104 already used a third, weaker, unsanctioned shape — a one-directional
Correctsfield plus an unstructured inline edit on ADR-0104's own page, invisible to the mechanical check this PR adds. Left as a known gap for a future ticket (task 270), the same way ADR-0107 named the ADR-0002/ADR-0032 gap it didn't fix either.Mechanical check:
AdrIndex.CorrectedFieldGaps()/AdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirections— flags aCorrectedfield with no inline marker, or an inline marker with noCorrectedfield, in either direction, mirroringCrossRecordSupersessionGaps(ADR-0084). The field is read only from a record's own front matter, the marker only from its own body, with every fenced code block and every HTML comment excluded from both (AdrIndex.FencedLineMask,AdrIndex.HtmlCommentLineMask, unioned byAdrIndex.ExcludedLineMask).Two review rounds found real evasions, both now closed:
HtmlCommentLineMaskalongside the fence mask.A real bug was caught by its own tests, not shipped: the first version of
HtmlCommentLineMasknever re-entered "in comment" state after an opening<!--, so a multi-line comment was only masked on its first line. Its own new tests (HtmlCommentLineMask_MultiLineComment_...,HtmlCommentLineMask_UnterminatedComment_...) failed on first run, exactly as the watched-to-fail discipline is meant to catch. Fixed before this shipped.One known limit is named, not fixed: an unterminated fence or comment opened in a record's front matter masks everything to end of file, which could hide a genuine field or marker the same way it hides a decoy. Left as a documented limit (ADR-0170 Consequences) rather than specially handled, because unlike the comment decoy it isn't silent — it visibly breaks the page's own rendering, which the record's own author would see.
Watched to fail, restored before shipping:
<!-- -->elsewhere in the file) — reddened, naming the file and the missing-marker direction.docs/adr/README.md's generated index anddocs/adr/template.mdare updated to match.Verdict: changes needed
Re-derived independently (fresh clone, not the author's worktree): 16/16 green at
dcc7b2d; re-ran the ADR-0071 field-removal mutation and it reddens as reported; job-level CI (build/e2e/container-images, run 853 / actions/runs/761) all success againstdcc7b2d, no drift.Precedent list (0071, 0098, 0142, 0167, 0168) verified correct by grep. 0167's "correction of a correction" is genuinely covered in the Decision section's "What a correction may do", not just mentioned. ADR-0107's reciprocal
Partially superseded by/Status/README update matches the existing convention (compare 0105). Test placement inPlaceMark.Architecture.Testsfollows established precedent (AnsweredByGaps,CrossRecordSupersessionGapsalready live there).One finding blocks, detailed inline: the Consequences claim that the check's blind spot is "never a real correction disguised well enough to hide from it" doesn't hold. I mutated ADR-0142 myself — broke both its genuine inline markers (renamed them so they no longer match), left its
Correctedfield in place — then added a fenced code block elsewhere in the same file quoting the marker syntax purely as a documentation snippet (exactly the same thing this ADR's own page does to itself, just deliberately).CorrectedFieldGaps()still reported no gap: 16/16 stayed green. A genuine missing marker was masked by unrelated text anywhere in the file, including inside a fence. That's the same evasion family CLAUDE.md already catalogues for source-text guards (a comment merely mentioning the marker it checks for) — not a new risk, but this ADR's Consequences asserts the check is immune to it, and it isn't.On the second self-raised point (0104/0169 left unfixed): agree with leaving it. 0104's correction is real but genuinely unstructured (checked directly — no
Correctedfield, no marker), 0169'sCorrectsfield is one-directional; naming it without fixing it matches the project's own precedent of ADR-0107 naming the 0002/0032 gap it didn't close either. Worth a Vikunja follow-up ticket if one doesn't exist yet, but not a reason to block this PR.British English and no AI attribution both clean.
@ -0,0 +155,4 @@a plausible slip rather than a contrived one — did *not* get through: the check correctly reddened,naming the file and demanding the field either match the sanctioned syntax or be removed. The check'sblind spot is only ever the first kind: two matching shapes with no genuine relationship betweenthem, never a real correction disguised well enough to hide from it. Nor does it stop the actual failure this ADR responds to: nothingThis overstates what the check can't be evaded by. It's not just self-quotation in a worked example — any text matching the marker regex anywhere in the file masks a genuine gap, including inside a fenced code block used only for illustration. Verified: broke both of 0142's real inline markers, added an unrelated fenced-code-block snippet elsewhere in the same file quoting
> **Correction (as a syntax example, andCorrectedFieldGaps()still reported no gap. Either scope the scan (skip fenced code blocks; restrict the field match to the front-matter block) or narrow this claim to what's actually true: the check can be defeated by any matching text in the file, not just this page's own self-reference.Both
hasCorrectedFieldandhasInlineMarkerscan every line of the file with no regard for fenced code blocks or section (front matter vs body). That's what makes the self-match on ADR-0170's own page possible, and it's also what lets a genuine gap be masked by unrelated matching text anywhere else in the same file — see the inline comment on 0170 for a worked mutation. Consider skipping lines inside ``` fences before matching, at minimum for the marker regex.Verdict: changes needed
Reproduced the round-1 blocking evasion exactly (ADR-0142 markers broken + decoy fence quoting the marker syntax elsewhere in the file): it now correctly reddens
AdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirections, naming ADR-0142's file and the missing-marker direction. Suite is 20/20 green at rest.Also confirmed: all five precedent records (0071, 0098, 0142, 0167, 0168) are detected as correction-carrying, and stripping every genuine inline marker from each of 0098, 0167 and 0168 independently reddens the test (only 0071/0142 were demonstrated in the PR body). The two evasions the author reports as correctly failing (single-backtick span, body-not-front-matter decoy) check out by inspection of the regex anchor and
FrontMatterLines.Found a working evasion the fix doesn't cover: a decoy marker inside an HTML comment. Same recipe as the round-1 bug — delete ADR-0142's two real markers, then append:
FencedLineMaskonly masks backtick/tilde fences, not HTML comment blocks, so this decoy is read as a genuineBodyLinesmarker and the test stays green — 1/1 passed with the real correction gone. This is a materially worse hole than the two disclosed-and-failed attempts: an HTML comment is invisible when the page renders, so unlike the fenced-block evasion this round fixed, a reader glancing at the rendered ADR sees nothing amiss either. It's the same class of bug that blocked round 1, just moved to different Markdown syntax the mask doesn't cover.Recommend closing it the same way the fence was closed — exclude
<!-- ... -->blocks (single- and multi-line) fromFrontMatterLines/BodyLinesmatching, alongsideFencedLineMask— or, if that's out of scope for this round, add it to the Consequences record of tried-and-succeeded evasions rather than leaving the two tried-and-failed ones as the only account, since the honesty bar the rewrite sets for itself is specifically about not overstating what's been tried.Secondary, lower-severity finding, not blocking on its own: an unterminated fence opened in front matter (e.g. inserting a lone
```line before theCorrectedfield, with no closing fence anywhere in the file) masks everything from that point to EOF — the field and every later marker — and the check reports no gap regardless of what's actually there. Also verified working. Less concerning than the HTML-comment case because it visibly wrecks the page's own rendering (everything after becomes a literal code block), so a reader looking at the rendered ADR — not just the check — would notice something is badly wrong.Separately: ADR-0170's own page no longer self-matches (its worked examples are inside excluded fenced blocks), and its rewritten Consequences/Decision text is honest and appropriately unfalsifiable-avoiding — it names exactly what was tried (including the reviewer's own round-1 reproduction) and states plainly "this is not a proof that no evasion exists, only a record of the ones tried," matching the ADR-0066 precedent it cites. That framing is good; it's the completeness of this round's specific record that the HTML-comment finding undercuts.
CI: run #871 (job ids 1578/1579/1580) —
buildandcontainer-imagessucceeded,e2efailed. Pulled the job log directly: all 59 E2E tests failed with the identical fixture-start error (WaitUntilReadyAsync, WebUI never answeredlocalhost:5169within 60s), not any individual assertion. The diff for this PR touches onlydocs/adr/*.mdandtests/PlaceMark.Architecture.Tests/*.cs— no runtime code, no WebUI/API/Infrastructure/Domain project, nothing that could plausibly affect WebUI process start-up timing. I agree this reads as unrelated infra contention rather than anything this change did. What would distinguish the two causes: this PR could only plausibly cause an E2E failure by breakingbuild(it doesn't —buildsucceeded, and the architecture-test suite is green) or by a genuinely new per-test flake (it isn't — the failure is uniform at fixture start, before any journey runs). A clean rerun on the same SHA, once the runner isn't contended, is what I'd expect to show, and would confirm it; I would not read this red run as blocking on its own, but it still needs a green rerun before merge per the standing rule on red jobs.@ -0,0 +184,4 @@through: a decoy marker wrapped in a single backtick span at the start of a line — already excluded,since the anchor requires `>` as the line's first character, with no separate inline-code handlingneeded — and a decoy `- **Corrected:**` line written in a record's body rather than its front matter,excluded by the front-matter/body split. **None of this is a proof that no evasion exists, only aThis paragraph names two evasions tried and both failing. Verified a third — a decoy marker inside an HTML comment (
<!-- > **Correction (...):** ... -->) after removing the real markers — and it succeeds:CorrectedFieldGaps()reports no gap with a genuine correction actually deleted. Worth either fixing (mask HTML comment blocks alongside fenced ones) or naming here, since this record's own honesty bar is about not overstating what's been tried, and right now only the two failed attempts are on record.@ -414,0 +553,4 @@/// shipped./// </para>/// </remarks>internal static bool[] FencedLineMask(string[] lines)Confirmed working evasion: wrap a decoy
> **Correction (...):**in an HTML comment block (<!-- ... -->) after deleting the real markers elsewhere in the same file.FencedLineMaskonly masks backtick/tilde fences, so the decoy is read fromBodyLinesas a genuine marker andCorrectedFieldGaps()reports no gap with the real correction actually gone — same severity as the round-1 fenced-code-block bug, and worse in one respect: an HTML comment is invisible when the page renders, so a reader glancing at the rendered ADR sees nothing wrong either. Recommend excluding HTML comment blocks fromFrontMatterLines/BodyLinesthe same way fenced blocks are excluded, or disclosing this as a tried-and-succeeded evasion in ADR-0170's Consequences.Verdict: mergeable
Re-derived independently in the author's worktree (read-only, restored to a clean tree afterwards; SDK confirmed at 10.0.100):
> **Correction (...)**wrapped in<!-- -->elsewhere in the file): reddensAdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirections, naming ADR-0142's file and the missing-marker direction.- **Corrected:**field and a line-anchored inline marker at rest — checked directly, not inferred from the mutation reddening alone — so the guard is a true positive, not vacuously green.HtmlCommentLineMaskbug is real and load-bearing, not just narrated. Removing theinComment = closeIndex < 0reassignment (so the state machine never re-enters "in comment" after an opening<!--) reproduces the exact bug shape described and reddens bothHtmlCommentLineMask_MultiLineComment_...andHtmlCommentLineMask_UnterminatedComment_.... 146/146 green at rest once restored.HtmlCommentLineMask's XML remarks, ADR-0170's Decision, ADR-0170's Consequences, andAdrIndexTests' own remarks all give the same reasoning (visibly wrecks the page's own rendering, therefore a lesser risk than a silent decoy) and none overstates it as fixed.76fff92viahead_shafilter (avoiding the id/number trap) — success.Precedent list, ADR-0107's reciprocal supersession, README/template updates, British English and AI-attribution were all untouched by this round's commit (
1d3b30d..76fff92only touchesAdrIndex.cs,AdrIndexTests.csand ADR-0170 itself) — not rechecked, per the brief.No blocking findings.