fix(docs): write down the test ADR 0018 was applied under, and close five wrong keeps - #3475
Merged
Merged
Conversation
…five wrong keeps The sweep in c66f26c closed 16 cross-plugin path citations under ADR 0018 and kept 23 on a dividing test it stated only in its pull request. The test is not in the ADR. That omission is not academic: the same commit ruled a reach into a skill's private scripts/lib/ out of bounds in detector-findings 2.8.1 and left the byte-identical shape standing in shell-test-helpers, three files apart, on the same day. Five keeps re-derived and corrected, each verified against the tree first: - shell-test-helpers/README.md dropped the link to repo-hygiene:clean's scripts/lib/test-helpers.sh and names the copy instead, on 2.8.1's own remedy. The scripts/ entry-surface pointers later in that file resolve and stay. - plugin-data-report-keying/README.md drops three location pins (:97, :53-62, "steps 6 and 7"). All three resolved when checked, so none was broken yet; the pin is the part that rots, and detector-findings 2.7.1 already dropped a :414 pin as a class after it drifted. Each citation quotes its content inline, so the pin carried nothing. Patch bump to 1.0.1 per that contract's own rule. - docs/native-surfaces/records.json's note names /claude-ops:audit-native-overlap instead of addressing its private reference/canonical-pairs.json. The store is hand-editable by its own note, so the generation argument the sweep kept it on does not apply. overlap.py generate re-run: the rendered view is unchanged, as the note is store-only. ADR 0018 gains an amendment with three clauses it never carried: 1. The evidence/address test, stated. FIX a citation that is the doc's address for an obligation, since the binding fails exactly where it binds. KEEP one that is evidence about this tree at a moment in time. The ADR path-cites plugin skill privates five times on its own first page, so either it violates itself while stating the rule or clause 2 was always scoped to function rather than form. Its own five are recorded as evidence. 2. scripts/lib/ and any private subdirectory sit outside the entry-surface carve-out. Already ruled in detector-findings 2.8.1; a ruling that lives only in one convention's changelog is invisible to the next doc that needs it. 3. Plugin-level non-skill trees are outside the privacy question and inside clause 3. The population is recorded as measured, not swept. The sweep spec gains an open-remainder section: twelve citations c66f26c created and never rostered (each fix wrote a changelog entry quoting the path it removed), proven disjoint from the three already counted by differencing the tree at the fix commit's parent, so the population is 16 fixed and 35 kept rather than 16 and 23. Also three trees excluded by fiat with no carve-out authorizing it, and the dometrain security review whose re-trigger fired at 0.1.0 and has gone unrun to 0.2.7. One roster correction: the loop-lane citation was reported at :14 and is at :69 today, release 9.1.0 having landed above it. Re-derive by text, not line. Refs #3468, refs #3460. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UmrZGdp1dgbZuPCy7tcRJo
Round five of an adversarial claims audit. Both findings are the same defect family as rounds three and four: a status trusted instead of re-derived, in a document whose own decay rule forbids exactly that. "34 to 32" was wrong, and wrong in an instructive way. The claim appeared in the sweep record and in native-references' changelog: that closing V-review-13 and V-review-14 took the predecessor's L4 roster from 34 open rows to 32. Re-deriving all 34 against the roster's own text test -- is the cited private surface still named at the citing path? -- finds 22 of the other 32 already closed before this change set began. Twelve of them were closed by #3380, the same commit that last wrote the roster, which left its own line "All 34 path:line citations were re-verified on 2026-08-26 and every one resolves" standing over twelve citations it had just deleted. This change set read that status as fact and republished it, in a file that says at line 18 "the check is the text, never the status and never the line number". What actually remains of the 34 is Group 2's eight intra-plugin path-form defects, untouched here. The spec also still shipped the probe shape #3469 removed, presented as verified. It showed probe >/dev/null 2>&1 && probe | filter | head -N || echo TOKEN and stated it was verified by execution in three states: token outside a repository, nothing when the filter matches nothing, and the cap holding above it. Under set -o pipefail two of those three states print the token instead, which is why docs-hygiene 0.21.24 and code-tidying 0.14.13 replaced the form with the pipeline in a brace group closed by `:`. The plugin changelogs recorded that correctly; the spec is the resumable record a later pass reads, and it carried the superseded shape with no pipefail qualifier at all -- the word appears nowhere in docs/. Both the shape and the verification claim are corrected, and the qualifier is now stated as the lesson: a verification that fixes the shell's options and does not say so proves less than it appears to. The commit message of c66f26c carries the same "34 to 32" error and cannot be edited; this entry is the correction of record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UmrZGdp1dgbZuPCy7tcRJo
claude Bot
added a commit
that referenced
this pull request
Aug 28, 2026
…test, and record the pipefail question (#3476) The last five findings from an independent security review of #3460/#3468/ #3469. #3474 and #3475 closed the probe-shape and citation halves; this closes the filter half and writes the open question into the convention. Four probes silently under-reported any filename git status --porcelain C-quotes. A plain space is enough to defeat grep '\.md$' and awk '{print $NF}'. Executed against a repo holding seven files per extension (plain, space, single quote, double quote, semicolon, pipe, embedded newline), each of the four went from catching 4 of 7 to 7 of 7. Two correct forms already shipped in the same two plugins, so this was drift rather than a missing idea: the .md sites took audit-noise's grep form, which keeps their leading token and so keeps their grant valid, and dissolve-comments took audit-comment-residue's -z NUL parse. The residual is stated in the changelogs rather than hidden. audit-comment-residue's parity test could not see an over-reporting preview. Its loop only checked one direction, so mutating the SKILL.md awk to drop its rename skip made the preview emit a phantom path while detect.sh audited five real ones, and the suite still passed 53/53. A reverse loop now asserts every previewed path is one detect.sh audited: the same mutation fails 1/54 naming the phantom, and reverted it passes 54/54. The forward loop is byte-identical. claude-ops:observability rendered 'unknown' both outside a repo and inside one whose toplevel directory is literally named unknown. The failure case now renders a distinct token. Six labels asserted 'empty = none' in two states the probe does not establish: the brace group's : makes the outer || unreachable, which also masks a filter-stage failure and a guard/data double invocation. The labels now say 'empty = none matched or the probe returned nothing'. No plumbing restructured; the ~25 unfiltered probes were left alone because their claim is already true. The durable fix is in precompute-context.md: whether Claude Code runs ! injections under pipefail is undocumented, the brace group is correct under both settings while guard && pipeline || token is correct under only one, and the brace group's own price is that an internal failure renders empty, which is why the label must carry it. The recheck trigger now covers shell options. Two derivations unrelated to the review's table both return the same six filtered probes: the true set is six, four defective, not five and three. #3474 fixed audit-derivability's SIGPIPE shape hours earlier and left its filter untouched, so both defects landed on one line; this branch was rebuilt onto current main and carries both, since committing either alone would have reverted the other. 70 pipefail cells all exit 0; affected-tests real exit 0 with 17/17 suites; changelog parity including --check-preserved across 306 headings; 3061 plugin files validated; markdownlint clean over 173 files. No grant widened or broken. Refs #3474, refs #3475, refs #3469, refs #3468, refs #3460.
kyle-sexton
pushed a commit
that referenced
this pull request
Aug 28, 2026
…, and close the dometrain staleness #3468 closed the ADR 0018 encapsulation floor by writing a convention CHANGELOG entry for each fix, and every entry quotes the path it removed. That manufactured twelve new instances of the shape the pass was sweeping. The sweep record listed them and stated they were fine; nothing ruled on them one at a time, so the arithmetic (16 fixed and 35 kept, not 16 and 23) rested on an assumption. Ruled individually against the test #3475 wrote into ADR 0018's amendment. All twelve are keep-correct and none is edited. Each sits in a dated entry whose claim is what a named file contained on that date, quoted as the string the entry removed, with the replacing invocation named in the same sentence, so no reader is sent to any of them for a rule. Three sub-rulings the class needed are recorded, because a bare "all twelve are evidence" is the assumption this was meant to replace. The three config-cascade rows quote plugin-relative forms that resolve against nothing, and the entry says so: clause 3 does not fire where the entry asserts non-resolution, since making them resolve would delete the finding. The other nine resolve on disk from their own implied base, checked one at a time. None carries a line or step pin, the part the amendment says rots first. Re-derived with a second expression over the fix commit's added lines rather than a scan of the files at rest. Same twelve, plus one the roster excludes on purpose: plugins/review/reference/topic-docs.md, a plugin-level non-skill tree that belongs to the pass the amendment routes it to, and a declared keep rather than a quoted removal. Separately, MIGRATION-PLAYBOOK's dometrain record was reviewed at 0.1.0 and the manifest reads 0.2.7, eleven releases later. Whether any of them added a trust surface is what the re-trigger clause turns on and no record answers it, which takes a review rather than a reading. The staleness is now stated in place with the re-review logged as owed. The review itself is deliberately not performed here. Refs #3468, refs #3475, refs #3460. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UmrZGdp1dgbZuPCy7tcRJo
claude Bot
added a commit
that referenced
this pull request
Aug 28, 2026
…, and flag a fired security re-trigger (#3477) Every one of #3468's ADR-0018 citation fixes wrote a CHANGELOG entry quoting the path it removed, creating twelve new instances of the shape it was closing. The population at main is 16 fixed and 35 kept, not 16 and 23. #3475 wrote the governing test into ADR 0018; this applies it to those twelve so a later re-derivation does not re-open them. All twelve are keep-correct, established row by row and anchored on text rather than line numbers. A dated changelog entry quoting a citation it removed is the evidence case exactly. No CHANGELOG was edited, so no convention bump is owed. Three sub-rulings the class needed, now written into the spec rather than left implicit. config-cascade's three rows are the only clause-3 question in the set: they are plugin-relative and resolve against nothing, and the entry says so in the same sentence. Clause 3 governs addresses a doc offers, not strings it quotes in order to report them broken, and forcing resolution would delete the finding. The other nine resolve on disk today, verified one path at a time across eleven distinct targets. No row carries a line or step pin, the part the amendment says rots first. One excluded on purpose and recorded so it is not re-opened: plugins/review/reference/topic-docs.md, a plugin-level non-skill tree the amendment routes to its own pass, which the entry declares a keep rather than quoting as removed. Separately, MIGRATION-PLAYBOOK.md records a dometrain security review performed at 0.1.0 and states that a version bump adding a new trust surface re-triggers it. The manifest now reads 0.2.7, eleven releases later. The note added states the reviewed version, the shipping version, that the ACCEPT below it describes 0.1.0 only, and that the re-review is owed and deliberately not performed here. It stops short of asserting a trust surface was added: that is the condition the clause turns on and cannot be settled by reading the page. Two checks were run to avoid claiming otherwise and neither settles it, which the note says. The spec's dometrain entry pinned MIGRATION-PLAYBOOK.md:943, a line this change's own edit would have invalidated. Re-anchored on the record's heading text. That decay rule has been violated three times across this sweep; this is the first time it was caught before landing. Recorded because two earlier claims about it were wrong in opposite directions: this worker's definition declares the Agent tool and the harness withheld it at spawn, so nesting is gated by depth realized as pre-launch tool omission. There is no refusal string because there is no call to refuse. Refs #3476, refs #3475, refs #3474, refs #3469, refs #3468, refs #3460.
claude Bot
added a commit
that referenced
this pull request
Aug 28, 2026
…reopened it three times (#3478) The L4 roster's Group 2, eight intra-plugin path-form citations, was the last block anyone believed open. All eight were already closed, by 6c7a103 (#3380) on 2026-08-26 -- the same commit that wrote the roster. Its own message names what it did: eight citations written with an implied base of the plugin root while the real base was reference/, none of which resolved for any reader. Verified one at a time against the roster's own text test. All eight now read ${CLAUDE_PLUGIN_ROOT}/skills/<skill>/<path> and all seven distinct targets exist on disk. Zero plugins/** files edited, so no bump and no plugin changelog entry: nothing needed changing. The reason four separate passes each re-derived this roster from scratch is one sentence. #3380 fixed 32 of 34 rows and left its summary asserting that all 34 path:line citations were re-verified on 2026-08-26 and every one resolves -- standing over citations it had just deleted. A fifth-round audit caught that for Group 1, where 22 of 32 were already closed and twelve of those by #3380 itself. This closes the same hole for Group 2. The generalizable rule is now in the sweep spec: a record that fixes findings and updates its own summary in the same commit must update the summary, or the summary outranks the fix for every later reader. The roster's inventory rows are left verbatim, per its own decay rule that the inventory is the part that cannot be re-derived, and per the precedent that #3474 and #3475 both closed rows without touching the file. Only an additive closure stamp is appended. On whether ADR 0018 reaches this class, the honest answer is that its encapsulation half does not. Clause 1 legalises intra-plugin citations and names this exact citing surface. Clause 2 cannot apply, because both files ship inside one plugin so the runtime absence motivating it cannot occur, and the 2026-08-28 amendment's fix-an-address / keep-evidence test divides clause 2 applications only. Only clause 3 reaches Group 2, and clause 3 is a resolvability rule rather than an encapsulation one: had these been open, the remedy would have been path form and nothing else. The clause that earned its keep is the ADR's own observation that proximity did not prevent them. A second derivation making no reference to the roster resolved every citation token in every plugin-level reference/, context/ and agents/ tree plus every plugin README against the base its own form implies: 52 tokens, 0 clause-3 failures. Group 3's two anchors were also closed by #3380. 34 closed, 0 open. No fresh-context verifier reviewed this diff. Nested spawning is unavailable at this depth: the Agent tool is withheld pre-launch, so there is no call to refuse and no refusal string. A second mechanical derivation was substituted and every line reference re-checked against the live tree, which is weaker than a fresh context and is recorded as such. Refs #3477, refs #3476, refs #3475, refs #3474, refs #3469, refs #3468.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No linked issue
Summary
#3468 closed 16 cross-plugin path citations under ADR 0018 and kept 23 on a test it never wrote down. Its own verifier reported "3 keeps refuted; 4 cannot verify" — and that reasoning was unreadable, because the verifier ran as a remote session the agent could not read back. So three keeps were probably wrong and nobody knew which.
An independent verifier settled it. Five are wrong, not three. More usefully, it found why this recurred.
Fix
The root cause: the test was real but unwritten
The dividing test #3468 applied — fix a citation that is an address for an obligation, keep one that is evidence about this tree at a moment in time — appears nowhere in ADR 0018. Clause 2's last sentence is unconditional: "
docs/**and.claude/rules/**cite skills by slash invocation, never by path."Read literally, all 23 keeps are violations. But ADR 0018 is itself a
docs/**file and path-cites plugin skill privates five times, quoting their contents inline. Either the ADR violates itself on its own first page, or clause 2 is scoped to citations that function as an address. Only the second reading is coherent.An unwritten rule is exactly what let one commit rule two ways in two files. This PR writes it down.
The five wrong keeps
shell-test-helpers/README.md:27→clean/scripts/lib/test-helpers.sh/repo-hygiene:clean's bundled test-helper copy"plugin-data-report-keying/README.md:98→bugs/write/SKILL.md:97:108→unhobble/SKILL.md:53-62:123→ "steps 6 and 7"native-surfaces/records.json:3→audit-native-overlap/reference/canonical-pairs.jsonThe first is a self-contradiction inside
c66f26ce. That commit addeddetector-findings2.8.1, which fixed the byte-identical shape and stated the ground: a reach into a privatescripts/lib/directory "the entry-surface carve-out does not cover". One commit ruledscripts/lib/uncovered, fixed it in one file, and kept it in another. Both dated the same day, in the same commit.The three pins all resolve correctly today — verified — but
detector-findings2.7.1, dated the same day, removed a:414pin as a class because "the pin had already rotted". Each citation already quotes its content inline, so dropping the pin is lossless.records.json:3's keep-ground was simply false: the sweep kept it because hand-edits get overwritten by generation, but that store's own note says "Hand-editable and human-gated". It is exactly where an edit lands.Lines 50/55/57 of
shell-test-helpersstay — those arescripts/entry surface, not skill privates.The ADR amendment
Added as
## Amendment (2026-08-28), following ADR 0011's amendment convention, three clauses:scripts/lib/and any private subdirectory are outside the entry-surface carve-out. Already ruled indetector-findings2.8.1; it belongs here.Recorded as open remainder, not swept
c66f26cecreated itself. Every fix wrote a convention CHANGELOG entry quoting the path it removed. The population atmainis 16 fixed + 35 kept, not 16 + 23. All twelve are keep-correct on the test above, but nobody adjudicated them. Confirmed genuinely new by differencingc66f26ce^: exactly three such citations existed before, precisely the sweep's "three dated changelog entries".docs/specs/(~265),docs/topics/(~43),docs/adr/(25). The cheat sheet (162, generated and CI drift-checked) anddocs/upstream/(27, vendored) are defensible.MIGRATION-PLAYBOOK.md:943says thedometrainsecurity review was done at 0.1.0 and "a version bump adding a new trust surface re-triggers this review". The manifest reads 0.2.7. The trigger fired; nobody re-ran it.Round five: two more false claims, both stale statuses
"34 to 32" was wrong. Closing
V-review-13/V-review-14was said to take the predecessor's L4 roster from 34 open rows to 32. Re-deriving all 34 against the roster's own text test finds 22 of the other 32 already closed before this change set began — twelve of them by #3380, the same commit that last wrote the roster, which left its own line "All 34path:linecitations were re-verified on 2026-08-26 and every one resolves" standing over twelve citations it had just deleted. This change set read that status as fact and republished it, in a file whose line 18 reads "the check is the text, never the status and never the line number." What remains of the 34 is Group 2's eight intra-plugin defects.The spec still shipped the probe shape #3469 removed, presented as verified in three states — two of which print the failure token once
pipefailis on. The plugin changelogs superseded it correctly; the spec is the resumable record a later pass reads, and the wordpipefailappeared nowhere indocs/. Corrected, with the qualifier stated as the lesson: a verification that fixes the shell's options and does not say so proves less than it appears to.c66f26ce's commit message carries the same "34 to 32" error and cannot be edited; the changelog entry here is the correction of record.Verification
All 16 fixed citations resolve: each
/plugin:skillexists asplugins/<p>/skills/<s>/and<p>matches thatplugin.json'sname. All 24 kept targets exist on disk.Roster re-derived three ways — absolute
plugins/<p>/skills/…; the bareskills/<name>/token with no prefix; and all forms over convention changelogs including directory-only and plugin-relative shapes. Delta on the twelve: zero; derivation 3 recovered all twelve including threeconfig-cascadeplugin-relative forms and apermission-rule-hygienedirectory form that derivation 1's regex missed.Two things the verifier could not settle, stated rather than guessed: which three changelog citations the sweep meant by "three dated changelog entries" (arithmetic inference, not evidence), and whether the entry-surface carve-out reaches a prose convention doc at all — a stricter consumer-class reading would flip three more keeps.
Related
Refs #3468, refs #3469, refs #3460. Amends
docs/adr/0018.Sibling: #3474 (merged as
bd77698a) closes the probe-shape half of the same verification pass — elevenpipefailinversions, including thevideo-digestgate that would have refused to run on a machine that has the media toolchain. This PR is the citation half. They share no files.Generated by Claude Code