feat(docs-hygiene): add container-position pattern forms to rename-references - #1386
Conversation
…ferences Six stale references survived three sweep passes during the re-anchor -> discipline plugin rename. All six were two syntactic shapes the pattern library did not cover, and the gap is structural rather than incidental: Forms 1-12 all assume the renamed token is a skill or mode identifier. When a CONTAINER renames, the token also occupies positions none of them reach. Form 1 anchors on `/<old>`, so it cannot fire on `/plugin configure <old>` where the slash belongs to `plugin` and the token sits downstream in argument position. Form 3 needs a path. Form 2 matches, but cannot separate the container sense from the verb sense at any triage setting when the token is also a verb in the consuming codebase. Forms 13-15 anchor on syntax that admits only the naming sense: a management verb immediately before the token, a `$`-anchored heading whose entire content is the token, and the possessive clitic or a noun-class appositive. Each stays Certain regardless of blocklist membership. Validated on both axes against the real fixture rather than asserted. Recall came from the removed lines of the commit that fixed the references; precision from the whole pre-fix tree. Over that tree, bare-token Form 2 matched 134 lines for 8 real defects; Forms 13-15 matched 9 -- the 8 defects plus one frozen CHANGELOG-history line the existing rule already excludes. Phase 6 now requires that both-axis validation for any future form, because recall alone is not evidence: Form 2 already has perfect recall on every form in the library and is still unusable. triage.md records why extending the English-verb blocklist is the wrong remedy for this collision class. The blocklist holds tokens that are verbs in general; a token that is a verb in the consuming codebase fails both ways -- omitted, every hit is rated Certain; added, every hit lands ambiguous and per-match confirmation turns a handful of defects into hundreds of prompts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
…rm 2 flood The P1 review finding is correct and it invalidated this change's own claim. Forms 13-15 were added as the remedy for a token that is a verb in the consuming codebase, but nothing suppressed Form 2 — so every line the new forms caught was ALSO still a Form 2 hit, and the prompt flood the forms exist to avoid remained fully intact. The forms only added a lens. Precedence is what makes the remedy real. Forms 13-15 are strictly more specific than Form 2: every line they match, Form 2 matches too. The sweep now deduplicates by (file, line) after collecting and before triage — a container-position match takes the Certain path and its bare-token duplicate for that line is dropped as the same reference seen through a weaker lens, not a second finding. Only lines the container forms did not match fall through to the blocklist rule. On the measured fixture the sweep still runs Form 2 and still collects its 134 lines; precedence turns those into 8 Certain container-position findings plus 126 ordinary verb uses that were never candidates, rather than 134 confirmation prompts. The audit report carries the superseded count so the suppression is visible rather than inferred. Two regex gaps from the same review, both verified against real input before and after: - Form 14 missed single-quoted YAML (`name: '<old>'`). The fix uses a paired alternation rather than `["']?`, which would have matched the invalid mismatched form `"<old>'`. - Form 15 missed the token as inline code before the clitic. In markdown that is the common case, not the exception — when the token is a code span the literal `<old>'s` sequence never appears, so the form missed its own motivating example. Eval 7's prompt said "every /re-anchor: invocation", where the bare trailing colon reads as a malformed invocation; it now names the qualified `/re-anchor:<skill>` form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
The second P1 is correct: precedence resolved the overlap but left the residue. Deduplicating by (file, line) only helps lines a container form ALSO matched — 8 of Form 2's 134 on the measured fixture. The other 126 are ordinary verb uses no container form touches, so they still fell through to Form 2's Certain default. My claim that they "were never candidates" was wrong; nothing in the precedence rule made that true. The sweep now declares a MODE at Phase 0, decided by what is being renamed. For a container — a plugin, a marketplace entry, a package — the renamed thing is a proper name, so a bare-token occurrence is evidence of nothing: as likely the ordinary word as the container. In that mode the residue is excluded from Certain regardless of blocklist membership, reported as one aggregate count rather than per-match prompts, and surfaced only behind an explicit widen and then as Ambiguous. Mode succeeds where the blocklist cannot because it is a property of the rename, not of whether someone remembered to list the token in advance. The second finding is also real, and worse under precedence. Form 14's premise — "a heading that IS the token can only be naming it" — holds for a coined or hyphenated name and fails for an ordinary-word one. Verified against this repository: renaming a `testing` plugin matches README.md:86 (`### Testing`, a marketplace category heading), and renaming `architecture` matches plugins/miro/README.md:39 (`## Architecture`, an unrelated design section). A false Certain there is worse than a plain Form 2 hit, because precedence discards the safer classification. Title matches are now Certain only in plausibly container-owned files, and always Ambiguous when the token is a common English word. Fixture result with mode and precedence together: 8 Certain findings, 126 reported-not-proposed, 0 confirmation prompts, against Form 2's unaided 134. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a618c31dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Claude finished @kyle-sexton's task in 1m 48s —— View job Security Review
ScopeThis PR is entirely documentation and configuration changes: three new markdown instruction sections ( FindingsIMPORTANT — Form 13 email-address collision in apply modeConfidence: HIGH \b<old>@[\w.-]+This standalone alternative matches
The combination is: a Certain-rated pattern that matches email addresses + an apply mode that auto-rewrites Certain hits → contact addresses in documentation get silently renamed to the new plugin identifier, with no confirmation prompt and no false-positive accounting. The false-positive note in the form's own definition ("prose does not accidentally say Mitigation options: Constrain the second alternative to known marketplace registries ( SUGGESTION —
|
|
Claude finished @kyle-sexton's task in 4m 31s —— View job PR Review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a618c31dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two of these are self-contradictions this change introduced, both caught by review and both verified against the files before fixing. The precedence rule said a deduplicated line "enters the Certain bucket" unconditionally, while Form 14's scope rule demotes an out-of-scope title match to Ambiguous. Applied literally, precedence would route every Form 14 hit to Certain and drop the Form 2 match that would have been Ambiguous — using precedence to launder a demotion, producing exactly the false positive the scope rule was added to prevent, and making eval 11's own expectation unachievable. A deduplicated line now enters the owning form's bucket AFTER its scope rules apply. Precedence decides which form owns a line, never how safely that line is rated. Container-rename mode also broke apply mode's termination invariant. apply.md Phase 6 completes at count == 0, but the residue the mode rule deliberately leaves unrenamed still matches the token forever, so the count never reaches zero and Outcome B loops indefinitely. Phase 6 now evaluates the ACTIONABLE count — the survey after precedence and mode — and reports residue in the hand-off summary rather than looping on it. The other two are gaps rather than contradictions: Phase 0b named --include-bare-token as the only documented way to inspect suppressed residue, but the flag was registered in neither SKILL.md's argument-hint nor audit-modes.md's override table, whose contract errors on unknown flags. Registered in both, audit-mode only, always Ambiguous. Form 13's qualified-id alternative had no management verb anchoring it, so for a container named info/admin/support it matched contact addresses on a Certain-rated form — a silent auto-rewrite. The discriminator is structural: a marketplace slug is kebab-case with no dots, an email domain carries a TLD dot. The form now accepts [\w-] with a (?![\w.-]) lookahead. Verified info@melodic-software matches while info@melodicsoftware.com and info@example.co.uk do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
|
Claude finished @kyle-sexton's task in 3m 28s —— View job PR Review
The four findings from the two prior review rounds are all resolved in this HEAD ( Prior findings — confirmed resolvedP1 — Form 2 was never suppressed. Fixed. Phase 0b container-rename mode excludes bare-token residue from Certain and reports it as an aggregate rather than per-match prompts. P1 (second round) — precedence left the residue. Fixed. Phase 0b is correctly documented as a second, independent rule that must run after precedence. P2 — Form 14 over-reached on ordinary-word names. Fixed. The scope rule demotes to Ambiguous for non-container-owned files or common-word tokens, and the precedence rule now preserves that demotion ("Precedence changes WHICH form owns the line, never the safety of its rating."). P2 — regex gaps. Fixed. Form 14 now includes single-quoted YAML ( Security: email-address collision. Fixed. Form 13's Finding 3 — Finding 2 — Phase 6 non-termination under container-rename. Fixed. Finding 1 — HIGH: Form 13 hyphenated-plugin-ID false positiveFile: Two separate false positives, both Certain-rated (auto-apply):
Command-argument form: The trailing The fixture ( The Cross-platform note prohibits lookbehinds. A no-lookbehind fix for both alternatives is to require a non-word, non-hyphen character before An eval case covering Finding 2 — LOW: SKILL.md Phase 6 summary still says "count == 0"File:
Minor notes
Verdict: Finding 1 is a correctness gap that needs fixing before merge — a Certain auto-apply hit on a different plugin's qualified ID is exactly what the form's management-verb anchor was supposed to prevent, and the fixture didn't catch it. Finding 2 creates a direct inconsistency between SKILL.md's inline summary and apply.md's authoritative definition. Everything from the prior review rounds is cleanly addressed. |
|
Claude finished @kyle-sexton's task in 3m 33s —— View job Security Review
ScopeAll nine changed files are documentation and configuration: Markdown instruction files ( Previously reported findings that are resolved in this PR: The prior review identified Form 13's email-address collision (
FindingsSUGGESTION —
|
All four findings CONFIRMED and fixed in
|
| # | Finding | Verdict | Fix |
|---|---|---|---|
| 1 | Precedence says Certain unconditionally, contradicting Form 14's demotion | CONFIRMED — HIGH | Deduplicated line enters the owning form's bucket after its scope rules |
| 2 | Apply-mode Phase 6 never terminates under container-rename mode | CONFIRMED — HIGH | Phase 6 evaluates the actionable count |
| 3 | --include-bare-token unregistered; flag contract errors on unknown flags |
CONFIRMED | Registered in SKILL.md + audit-modes.md, audit-only |
| 4 | Form 13's @-form matches email addresses |
CONFIRMED | Dot-excluding slug pattern with a lookahead |
Finding 1 — the sharpest of the four
You identified it exactly: precedence would be used to launder a demotion. Form 14 demotes an out-of-scope title match to Ambiguous precisely because it might be a category heading rather than the container; precedence then re-promoted it to Certain and dropped the Form 2 match that would have been the safer classification. Net effect: the scope rule I added in the previous commit to prevent auto-applying ### Testing was silently defeated by the precedence rule I added in the commit before it. Eval 11 asserted the correct behavior against a document that made it unreachable.
The rule now reads: a deduplicated line "enters that form's own triage bucket after its scope rules are applied", with an explicit statement that precedence decides WHICH form owns a line, never how safely it is rated.
Finding 2 — the invariant I broke without noticing
Container-rename mode leaves the residue unrenamed by design, but those lines still match <old> forever. apply.md Phase 6 completes at count == 0, so the count could never reach zero and Outcome B would loop indefinitely. I added the mode rule to patterns.md and never checked what it did to apply mode's termination condition.
Phase 6 now defines "count" as the actionable count — the survey result after both precedence and mode — with the reasoning stated inline so the invariant is not re-broken. Residue is reported in the Phase 7 hand-off summary as the same aggregate the audit reports. Eval 12 holds it.
Finding 3
Verified: grep -c 'include-bare-token' returned 0 in both SKILL.md and audit-modes.md. A user following Phase 0b's own instruction would hit the unknown-flag error. Registered in the argument-hint, added to the override table as a fourth flag, marked AUDIT-MODE ONLY and always Ambiguous — widening the report does not change that bare-token position carries no signal for a container rename.
Finding 4
Confirmed against real input, and the fix is structural rather than a caveat. A marketplace slug is kebab-case with no dots; an email domain always carries a TLD dot. So the @-form accepts [\w-] with a negative lookahead:
\b<old>@[\w](?:[\w-]*[\w])?(?![\w.-])Verified on real strings:
/plugin install info@melodic-software MATCH (real qualified id)
"info@melodic-software": true MATCH (enabledPlugins key)
Contact info@melodicsoftware.com no match
mail to info@example.co.uk no match
Eval 13 holds it. I did not take the "add a False-positives note" option alone — a note does not stop an auto-apply on a Certain-rated form.
Minor notes
- PR description said "two eval cases" — stale, carried over from the first revision. There are now 13 (7–13). Corrected in the body.
- Form 14 case-sensitivity — real gap, deliberately not fixed here. Case-insensitive matching would make
# Testingmatch atestingrename in every document, which is the over-reach Finding 1's scope rule exists to bound; the safe version needs the scope rule to be load-bearing first. Left as-is rather than half-fixed, and worth its own issue if you want it. - Apply-mode eval for container mode — added as eval 12, per your suggestion. It is the direct test for Finding 2.
Gates after the fix: claude plugin validate . passes, markdownlint-cli2 0 errors across 7 files, changelog parity passes, evals.json parses with 13 cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7038a0d4bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…xamples plugin-gate failed on the previous commit: plugins/docs-hygiene/skills/rename-references/context/patterns.md: reusable skill content must not require publisher-specific runtime identifiers The email-collision fix illustrated the dot-exclusion rule with this marketplace's own name. `rename-references` ships to consumers with their own marketplaces, so a portable skill must not encode the publisher's — the gate is correct, and the same slip reached the two eval cases and the CHANGELOG entry, which the failure did not name. Every example now uses a neutral `acme-tools` / `acmetools.com` pair. The distinction being demonstrated is unchanged and re-verified against real input: `info@acme-tools` matches, `info@acmetools.com` and `info@example.co.uk` do not. `node scripts/validate-plugin-contracts.mjs` — the gate that failed — now passes locally: 43 setup skills and 2097 plugin files checked, exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
|
Claude finished @kyle-sexton's task in 6m 39s —— View job PR Review
What the latest commit (d35e5b9) fixedThe Prior-round findings confirmed resolvedFour findings from the first review round and all security findings marked resolved in the author's
Finding 1 — HIGH: Form 13 hyphenated-plugin-ID false positives (from second review round, still open)File: These were identified in the second reviewer's Finding 1 (HIGH) and were not addressed in either
Command-argument form: The trailing An eval case covering Finding 2 — MEDIUM: Form 13's
|
|
|
Claude finished @kyle-sexton's task in 4m 54s —— View job Security Review
ScopeAll nine changed files are documentation and configuration: Markdown instruction files, a JSON eval fixture, and Previously reported findings confirmed resolved at this HEAD (d35e5b9): The prior round's email-collision fix ( Finding 1 — IMPORTANT | Confidence: HIGHForm 13's File: (^|[^\w/])/plugins?\s+(install|uninstall|configure|enable|disable|update|add|remove)\s+`?<old>\b
\b<old>@[\w](?:[\w-]*[\w])?(?![\w.-])
Command-argument form — trailing Qualified-ID form — leading Neither case appeared in the This finding was raised in the 20:06 PR review (Finding 1, HIGH) and was not addressed by d35e5b9, which only changed example text. Fix options (no lookbehinds — per the cross-platform constraint):
Finding 2 — SUGGESTION | Confidence: MEDIUMForm 13's File: The email-collision fix depends on the negative lookahead Two failure modes:
The email-collision protection introduced in the prior round only works if the lookahead actually executes. Fix: Either rewrite the boundary without a lookahead, or add an explicit instruction that the Example without lookahead: \b<old>@[\w](?:[\w-]*[\w])?(?=$|[\s"'`,;)\]])This requires the match to terminate at a whitespace, punctuation, or end-of-line — positions where an email domain's TLD dot never appears — rather than relying on a negative lookahead to exclude following dots. Fix this →%20so%20the%20match%20must%20terminate%20at%20whitespace%2C%20punctuation%2C%20or%20end-of-line%20--%20positions%20where%20a%20TLD%20dot%20cannot%20appear%20without%20a%20space%20or%20quote%20first.&repo=melodic-software/claude-code-plugins) Finding 3 — SUGGESTION | Confidence: HIGH (gap exists); LOW (exploitable impact)
File:
Blast radius is limited: This was flagged in the 20:06 security review and is unaddressed in d35e5b9. No other security issues found
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d35e5b9a28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…mmits Each verified against the files or against real input before fixing. Two of these were introduced BY the fixes for the earlier round. Deduplication keyed on (file, line), so a line carrying two independent references lost one of them: `Use <old> via /plugin install <old>@mkt` has a bare occurrence and a command-argument occurrence. Because Phase 5 replaces a single span at a time, the survivor was then reclassified as residue, excluded by container mode, and the re-sweep declared completion with a live stale reference in the file — a false completion, not merely a missed report. A weaker match is now suppressed only when its span is COVERED BY a more-specific match's span. The email-collision fix used a negative lookahead. ripgrep's default engine — the engine this skill instructs — rejects look-around without -P/--pcre2, and patterns.md's own cross-platform note already bans lookbehinds for the same class of reason, so the form I added would have been rejected rather than evaluated. It now consumes a terminator, ([^\w.@-]|$), the same shape Forms 4 and 5 use. Verified equivalent across six cases including bare end-of-line. The other two are over-promises rather than breakage: --include-bare-token was advertised for all audit sub-modes, but Orphans sweeps only Forms 1 and 3 and therefore has no bare-token residue to surface — the flag silently returned the default result there. Now scoped explicitly and reported as not-applicable. The actionable-count rule promised users a residue aggregate, but neither Phase 7 success template had a field for it and the default hand-off still said `0 stragglers`. The fix prevented the loop while hiding the number it committed to. Both templates now carry the count, emitted only under container-rename mode and only when non-zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
|
Claude finished @kyle-sexton's task in 5m 3s —— View job PR Review
|
|
Claude finished @kyle-sexton's task in 4m 17s —— View job Security Review
ScopeAll nine changed files are documentation and configuration: Markdown instruction files ( What
|
Round 3 — four new findings CONFIRMED and fixed in
|
| Anchored to | Count | Status |
|---|---|---|
9a618c31 |
9 | Already fixed in 7038a0d4 / d35e5b9a — re-reports of the same four findings against a superseded revision |
7038a0d4 / d35e5b9a |
4 | Genuinely new, and fixed in a3138a5d |
Two of the four new ones were introduced BY the earlier fixes. Fixing a review round created its own defects; that is worth stating plainly rather than presenting this as steady convergence.
1. Dedup by line dropped a second reference — a FALSE COMPLETION, not a missed report
The sharpest of the four. Keying on (file, line) meant a line carrying two independent references lost one:
Use <old> via /plugin install <old>@marketplace
^bare ^command-argument
Because Phase 5 replaces a single span at a time (replace_all: false), the surviving bare reference was then reclassified as residue, excluded by container mode, and the re-sweep declared completion with a live stale reference still in the file. The failure mode is a false green, not an omission from a report.
Dedup now keys on the occurrence span (file, line, start, end), and a weaker match is suppressed only when its span is covered by a more-specific match's span. Eval 14 holds it, and names the false-completion path specifically.
2. The lookahead would have been rejected, not evaluated
My email-collision fix in 7038a0d4 used (?![\w.-]). patterns.md's own Cross-platform note says "Do not use lookbehinds — Form 1 uses \B instead" — I added a lookaround 147 lines above the rule banning that class of construct, and ripgrep's default engine (the one this skill instructs) rejects look-around without -P/--pcre2.
The boundary now consumes a terminator instead — ([^\w.@-]|$) — which is the same shape Forms 4 and 5 already use. Verified equivalent across six cases:
/plugin install info@acme-tools MATCH
`info@acme-tools` MATCH
"info@acme-tools": true MATCH
info@acme-tools (bare EOL) MATCH
Contact info@acmetools.com no match
mail info@example.co.uk no match
Documented on the form with the reasoning, so the next author does not reintroduce the lookahead. Eval 15 holds it.
3. --include-bare-token over-promised on Orphans
Confirmed: Orphans sweeps only Forms 1 and 3, so it produces no bare-token residue and the flag silently returned the default result there. Scoped explicitly in both the prose and the override table, and reported as not-applicable rather than silently ignored — a flag that appears to work is worse than one that says it does not apply.
4. The residue count was promised and never emitted
Also confirmed by reading both templates: neither Phase 7 success template had a field for it, and the default hand-off still said 0 stragglers. The actionable-count fix prevented the infinite loop while hiding the aggregate it committed to — arguably worse than the loop, since it presents a raw-zero sweep. Both templates now carry the count, emitted only under container-rename mode and only when non-zero.
On the nine re-reports
Marked 👀 rather than 👍/👎. They are correct as written against 9a618c31 and were fixed two commits ago; re-reporting them against a superseded revision is a review-tooling artifact, not a disagreement. If any of them is still reproducible at a3138a5d, that is a real finding and I would rather hear it again than have it dropped.
Gates at a3138a5d: node scripts/validate-plugin-contracts.mjs (the gate that failed earlier) passes — 43 setup skills, 2097 plugin files, exit 0. claude plugin validate . passes, markdownlint-cli2 0 errors across 6 files, changelog parity passes, evals.json parses with 15 cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3138a5dc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tered Three review rounds each found real defects, and six of them were one class: a rule changed in one file while other files kept asserting its opposite. This skill states the same contract across SKILL.md, five context/ files, and evals/evals.json by design, so changing patterns.md alone was never enough. I had already written that lesson into this PR after the plugin-gate failure and then did not apply it to the rules themselves. Swept the whole skill directory for every claim the earlier commits altered, rather than waiting for a fourth round to name them one at a time: - SKILL.md gated the re-sweep on `count == 0` in TWO places. That is the always-loaded surface, so it would have shipped the non-terminating loop the apply.md fix exists to close, regardless of what apply.md said. - An eval asserted the raw count and would have FAILED against the corrected skill, reading as a regression rather than as the stale assertion it was. - triage.md's bucket criteria never learned that Forms 13-15 can be demoted by their own scope rules — the file that owns bucketing did not know about the demotion that makes the title form safe. - audit.md defined Certain as "high-precision form" without the scope qualifier, and flagged an empty Ambiguous bucket as unusual when under container-rename mode it is the designed outcome. patterns.md Phase 6 now requires this reconciliation for any future rule change: grep the skill directory for the claim just changed and fix every hit, evals included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f
|
Claude finished @kyle-sexton's task in 3m 53s —— View job PR Review
What
|
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 9s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5127aa0285
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…owlist
Round 11, four findings.
Form 14 missed a catalog KEYED by the container. Ladder rule 3 names both
shapes -- the name/id field of a manifest, or a key in a marketplace/registry
catalog -- but only the field shape had a pattern, so a registry written as
`plugins: { "<old>": { ... } }` selected container mode while its own key
stayed unmatched Form 2 residue. The key-position alternative requires the
value to OPEN an object or array, which distinguishes a catalog entry from an
ordinary scalar setting.
That alternative MUST carry the manifest/catalog condition rather than merely
benefit from it: `"<key>": {` is the commonest line shape in JSON and this
repository carries 569 of them, so unconditioned it would rate a large slice
of that Certain for an ordinary-word container name. Outside a manifest or
registry catalog it is Ambiguous unconditionally.
A YAML block-mapping catalog key is recorded as a deliberate gap, not
silently covered: in YAML the entry opens with nothing, so the discriminator
has nothing to bind to and the only pattern that reaches it would match every
nested YAML key -- a worse trade than the one it fixes.
Form 14's YAML declaration was anchored at column zero. A manifest or catalog
that nests its entries indents them, and the mode ladder recognizes that
field as container evidence either way, so the nested shape was excluded
residue under the mode it selected. The JSON and TOML alternatives were
indentation-agnostic from the start; the YAML one now matches them.
Container mode's Certain rule was enforced only against Form 2. Rule 1 is an
ALLOWLIST -- Forms 1, 3 and 13-15 are the whole eligible set -- so filtering
just the bare-token residue left Forms 8 and 12 on the auto-apply path, both
Certain by default. Renaming a `context` plugin would rewrite the unrelated
dotted key `context.timeout` and a `{a,context,b}` glob enumerating skills.
Both forms anchor on syntax proving the token is an IDENTIFIER, which is not
what a container rename is asking. Every non-allowlisted form now demotes to
Ambiguous, reported per match rather than folded into the aggregate, because
they are few and a container name genuinely can appear in a glob set.
The per-occurrence survey rescanned line by line, which reproduces nothing
for the two multiline forms. Form 7 and Form 14's Setext alternative match
only against a block, so the rescan emitted no record and the reference
vanished between survey and triage -- silently, on exactly the two forms
added because their references were being missed. The block is now kept
intact, the pattern re-run against it, and the captured span converted back
to (line, start, end) via the block's first-line number.
Verified verbatim through the Grep tool: the key-position alternative matches
`"<old>": {` and rejects `"<old>-extra": {`, a mid-line `"<old>":` with a
scalar value, and an indented YAML `<old>:`. The widened YAML declaration
matches `name:`, `title:` and `id:` bare, double- and single-quoted, and
still rejects `notes: <old> is used here as prose`.
Phase 6 reconciliation grep on the eligible-form list, the rescan unit and
Form 14's alternative count updated five sites: audit.md's Phase 3 Certain
definition and Phase 4 breakdown, triage.md's Form 8 entry and closing
paragraph, and two evals.
Gates: validate-plugin-contracts (43 setup skills, 2098 files), claude plugin
validate, markdownlint-cli2 0 errors over 36 files, changelog parity
--check-bump, evals parse at 36 cases with no duplicate ids.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 811696be7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…he delimiter
Round 12, two findings.
A confirmed skip was counted as actionable, so apply mode could not
terminate. The count already excluded container mode's residue but not the
matches a user declines at Phase 4, so choosing "skip this" on an unrelated
`context.timeout` re-presented the same match at Outcome B on every re-sweep;
the only exits were rewriting a known false positive or aborting with a
partial result. That is the identical non-terminating loop the residue rule
closes, reached through the other door -- and the previous commit's allowlist
change made it routine rather than rare, because demoting Forms 4-12 to
Ambiguous turns unrelated matches into per-match prompts whose correct answer
is to skip.
Phase 4 now records each decline as (file, line, start, end) -- the same
occurrence key precedence and dedup use -- and Phase 6 subtracts those spans
before testing for completion. Keyed by span rather than by file or form,
because skipping one occurrence is not consent to skip another on the same
line. Scoped to the sweep that asked: if Phase 6 finds a NEW form and the
library is extended, the question changed and the user is re-asked. Reported
on its own hand-off line, never folded into the residue aggregate -- residue
was never proposed, a skip was proposed and declined.
The catalog-key alternative failed its own motivating example. Its `^\s*`
anchor required the renamed key to begin the line, so it reached only the
pretty-printed rendering -- not the compact `plugins: { "<old>": { ... } }`
used by eval 34 and by the bullet documenting the form. A left anchor that
misses the example it was written for is the recurring failure in this file's
history. The anchor is now `(^|[{,])`: a JSON key follows a line start, an
opening brace, or a comma, and nothing else, so both renderings match with no
widening beyond them.
Verified verbatim through the Grep tool: `plugins: { "<old>": {`, an indented
`"<old>": {`, `{"a":1,"<old>":{}}` and an array-valued `"<old>": [` all
match; `"other": { "<old>": "scalar" }`, `"<old>-extra": {`, an indented YAML
`<old>:` (the documented gap) and prose all do not.
Phase 6 reconciliation grep on the actionable-count definition found it
restated as residue-only in three more places, including the always-loaded
SKILL.md in both its workflow step and its gotchas entry -- the same surface
that shipped the previous non-terminating-loop bug. Those plus eval 12 are
corrected here.
Gates: validate-plugin-contracts (43 setup skills, 2098 files), claude plugin
validate, markdownlint-cli2 0 errors over 36 files, changelog parity
--check-bump, evals parse at 37 cases with no duplicate ids.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8969f0a659
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ation
Round 13, three findings.
Skip spans did not survive the edits they coexist with. `<old>` and `<new>`
differ in length in the general case, so applying an accepted occurrence
shifts every LATER occurrence on that same line. On
`/plugin configure <old>; use <old>.timeout` the accepted Form 13 match moves
the skipped Form 12 match's columns, the Phase 6 rescan reports different
(start, end), the stored span fails to subtract, and the user is prompted for
the deliberate skip again -- the same non-terminating loop, defeated by the
bookkeeping added last commit to close it. Each Edit now shifts the start and
end of every stored skip span on its line whose start is greater than the
edited start; other lines and earlier spans are untouched, because a rename
replaces in place and adds no lines. The matched snippet is carried alongside
and a post-remap mismatch is reported rather than silently dropping the skip.
The JSON declaration alternative required the field to occupy a whole line.
A minified or merely compact manifest -- `{"name":"<old>","version":"1"}` --
is perfectly valid, and filesystem evidence still selects container mode for
it, so the registration went unmatched and was suppressed as residue while
apply mode reported completion. It now uses the same `(^|[{,])` opener and
`(,|}|$)` terminator as the key-position shape: in JSON a member starts after
a line start, a brace, or a comma and ends at a comma, a brace, or end of
line.
The YAML and TOML alternatives deliberately keep their `$` anchor. Both
grammars are line-oriented for the shapes manifests actually use, so
end-of-line is a real discriminator there; JSON is the one of the three with
no line structure at all. Flow-mapping and inline-table variants are recorded
as hand-resolve cases rather than a reason to widen those two into
every-nested-key patterns.
The mode-ladder lead-in still said "stop at the first rule that fires" while
the conflict rule directly below the list said to collect rules 2-4 in full
and compare. Both statements stood in one file. Only rule 1 short-circuits;
the list ordering is check order, not verdict precedence.
That third one was in scope for the previous round's reconciliation grep and
was missed because the grep filtered out the line range containing it. The
filter, not the pattern, was the defect.
Verified verbatim through the Grep tool: the JSON alternative matches
minified, compact, indented, spaced, sibling-after and `title`/`id` forms,
and rejects `-extra`, `description`, and a prose line quoting the pair
mid-sentence. Precision on this repository is unchanged at exactly
marketplace.json:192 and plugin.json:3.
Reconciliation also corrected eval 15, which still explained the YAML
alternative's miss by its column-zero anchor -- superseded two commits ago;
the real reason is that it requires an unquoted key.
Gates: validate-plugin-contracts (43 setup skills, 2098 files), claude plugin
validate, markdownlint-cli2 0 errors over 36 files, changelog parity
--check-bump, evals parse at 40 cases with no duplicate ids.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5bd7e020
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Round 14, one finding, and a real interaction between two earlier decisions.
Forms 3, 13, 15 and both delimiter-anchored Form 14 alternatives CONSUME a
trailing delimiter rather than asserting it, because ripgrep's default engine
rejects look-around. The consumed character is frequently the LEADING
delimiter the next occurrence needs, so the per-occurrence rescan -- which
resumed after the whole match -- silently dropped the second of two ADJACENT
references.
Reproduced exactly: on `{"name":"<old>","id":"<old>"}` a global `rg -o`
returns ONE match, `{"name":"<old>",`, because the first match ate the comma
the `id` member needed as its opener. Insert `"version":"1"` between them and
both appear, which is what makes this specifically an adjacency collision --
and a compact manifest is precisely where adjacency happens. Form 2 still
found the lost token, but under container mode that is suppressed residue, so
the sweep could report completion with the second declaration stale.
Fixed once in the survey rather than in each regex, since every
consume-the-delimiter form shares the hazard: Phase 2 now advances the rescan
cursor to the end of the CAPTURED `<old>` span. Verified that this recovers
the case -- one occurrence record becomes two on the adjacent input.
Two details recorded because the obvious variants are both wrong. Advance a
cursor, do not slice the string: re-running the pattern against a substring
lets `^` match at the cursor and would admit a member with no delimiter in
front of it. And advance by the captured token's end rather than match start
plus one, or the same occurrence is re-found through another alternative and
the record doubles.
Dropping the consumed terminator is explicitly NOT the fix, and patterns.md
says so at the site: without it the boundary is unenforced and `<old>@slug`
matches inside `<old>@slug.com` again -- the finding that put the terminator
there.
Gates: validate-plugin-contracts (43 setup skills, 2098 files), claude plugin
validate, markdownlint-cli2 0 errors over 36 files, changelog parity
--check-bump, evals parse at 41 cases with no duplicate ids.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bcc2bdc8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… skip in handoff Round 15, three findings. The survey lost occurrences inside a wide match. Form 7's pattern swallows a whole frontmatter field, and its greedy prefix binds the captured group to a single occurrence, so `description: "first <old> and then <old>"` produced ONE whole-pattern match for two references. Reproduced directly. Last round's cursor rule cannot recover the other one either: re-matching from inside the field cannot reproduce the `description:` prefix the pattern requires, so the fix had to be enumeration rather than a different cursor. Phase 2 now scans each match's text for every occurrence of `<old>` and emits one record per occurrence, all attributed to the matching form. The whole-pattern match establishes THAT the form applies and over what extent; the token spans inside it are the references. The cursor rule survives as the step that finds the NEXT match, advancing past the LAST enumerated span -- past the whole match is the adjacency bug, past only the first token re-emits what was already enumerated. Form 14's ATX alternative rejected the closed form. `# <old> #` is valid ATX and the trailing hash run is decoration rather than content, so the heading's entire content is still the token and the Certain reasoning applies unchanged; without the optional closing-hash group the title's only hit was Form 2, which container mode excludes. The run is not part of the reference: only the captured span is replaced. Verified that discrimination is unchanged -- `## How <old> works`, `# <old> extra` and `# <old>-extra #` are still rejected. The default hand-off template omitted the skipped count. Phase 7 requires reporting it separately from container residue, but the success template at the bottom of apply.md carried only `<R>`, so a normal run following the template reported "0 actionable stragglers" without disclosing that stale occurrences were preserved by request. The zero is true and insufficient on its own; a reader cannot otherwise distinguish it from a sweep that found nothing. Gates: validate-plugin-contracts (43 setup skills, 2098 files), claude plugin validate, markdownlint-cli2 0 errors over 36 files, changelog parity --check-bump, evals parse at 44 cases with no duplicate ids. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 306dac4fc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nifest declarations (#1435) Follow-up to #1386. Two **P1** findings were raised on that PR after it had already merged, so they never reached `main`. Closes #1439 ## Form 1 rewrote unrelated slash commands `\b` treats a hyphen as a word boundary, so `\B/<old>\b` prevented `/confirm` matching in `/confirmation` but **not** in `/confirm-changes`. Renaming `context` matched the unrelated `/context-guard`. Form 1 is rated Certain **and** sits on container mode's Certain allowlist, so this went through the default auto-apply path and renamed another command. Slash-command and container names are kebab-case, so it fires constantly rather than rarely. Now uses the consumed `([^\w-]|$)` terminator — the shape Forms 13 and 15 already use for the same reason. Verified against this tree: the nine `/docs-hygiene:<skill>` namespaced invocations still match, a colon being a valid terminator, while `/context-guard`, `/contextual` and `path/context` do not. ## Manifest declarations could not carry an inline comment `name: <old> # package name` and `name = "<old>" # package name` are ordinary self-documenting manifests, and the end-anchored declaration alternatives rejected the whole line — while filesystem evidence still selected container mode, so the registration went unmatched and was suppressed as residue while apply mode reported completion. The whitespace rule differs between the two deliberately: - **YAML requires whitespace before `#`.** YAML starts a comment only after whitespace, so `name: <old>#x` is the single scalar `<old>#x` and must not match. - **TOML allows optional whitespace,** its value being quoted, so the closing quote already ends the string unambiguously. - **JSON is excluded entirely,** having no comment syntax. All six positive shapes and both negatives verified. ## Reconciliation The Phase 6 reconciliation grep on Form 1's boundary found it restated in four more places — `triage.md`'s Certain criteria, `audit-modes.md`'s Orphans sweep, `apply.md`'s word-boundary-trap gotcha, and `audit.md`'s consume-the-delimiter form list — all corrected here. Two eval cases added (45, 46). `docs-hygiene` 0.9.0 → 0.9.1. ## Gates - `node scripts/validate-plugin-contracts.mjs` — 43 setup skills, 2122 files - `claude plugin validate .` - `markdownlint-cli2` — 0 errors over 36 files - `scripts/check-changelog-parity.sh --check-bump origin/main` - `evals.json` parses, 46 cases, no duplicate ids ## Related - #1386 — the PR these findings were raised on; it merged before they landed, so neither fix reached `main` - #1283 — the original pattern-library gap #1386 closed - #1394 — a separate, deliberately deferred Form 14 gap, untouched here 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ATCcexm8GPTaNntu2yrGMk --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Closes #1283
Summary
Six stale references survived three sweep passes during the
re-anchor→disciplineplugin rename (#1276). All six were two syntactic shapesrename-references' pattern library did not cover, and the gap is structural rather than incidental: Forms 1–12 all assume the renamed token is a skill or mode identifier.When a CONTAINER renames — a plugin, a marketplace entry — the token occupies positions none of them reach:
/<old>, so it cannot fire on/plugin configure <old>: the slash belongs toplugin, and the token sits downstream in argument position.Forms 13–15 anchor on syntax that admits only the naming sense:
/plugin install <old>@mkt,/plugin configure <old># <old>, frontmattername:$anchor: a heading that contains the token may be verb usage, but one that IS the token can only be naming it<old>'s,the <old> pluginCommand-argument hits are flagged as functional breaks, not cosmetic: a reader following
/plugin install <old>@marketplacegetsplugin-not-found. Four of the six missed references were this shape, including the README's own install block.Fix
context/patterns.md— Forms 13–15, each with the five documented fields the existing forms carry, under a short section explaining why container position is its own class.context/triage.md— records the collision class the English-verb blocklist cannot serve. The blocklist holds tokens that are verbs in general; a token that is a verb in the consuming codebase fails both ways: omitted → every bare-token hit is rated Certain and the sweep proposes rewriting the verb uses; added → every hit lands ambiguous, where the per-match confirmation rule turns a handful of real defects into hundreds of prompts. Extending the blocklist swaps one unusable bucket for another. The remedy is position.context/patterns.mdPhase 6 — now requires validating any new form on both axes. Recall alone is not evidence: Form 2 already has perfect recall on every form in the library and is still unusable.context/audit.md— pattern-form breakdown lists 13–15, so an audit report accounts for every form the sweep runs.Verification
Validated on both axes against the real fixture rather than asserted. Recall came from the removed lines of
930c97a4(the commit that fixed the references — its deletions are the defect set); precision from the whole pre-fix tree at930c97a4^.930c97a4^underplugins/disciplineThe 9th hit is a frozen CHANGELOG-history line, which the existing "Frozen historical records" auto-exclusion already handles. The one defect line Forms 13–15 do not match is the frontmatter
descriptiontrigger-phrase block — deliberately kept in that rename, so matching it would have been a false positive.Gates run locally:
claude plugin validate .— passesmarkdownlint-cli2on all four changed markdown files — 0 errorsscripts/check-changelog-parity.sh --check-bump origin/main— passes (docs-hygiene0.8.6 → 0.9.0 with a matching entry)scripts/check-changelog-parity.sh --check— passesevals.jsonparses; diff is additive only (25 insertions, 0 deletions to existing cases)Review rounds (this PR supersedes #1335 for branch reasons only — same work, rebased)
Four findings across two reviewers, all verified against real input before fixing; two were correct about defects in this PR's own claims:
(file, line)after the sweep, before triage.testingplugin matchesREADME.md:86(### Testing), renamingarchitecturematchesplugins/miro/README.md:39. Under precedence a false Certain there is worse than a Form 2 hit — it discards the safer classification. Form 14 is now scoped to container-owned files and always Ambiguous for common-word tokens.Final fixture result: 8 Certain findings, 126 reported-not-proposed, 0 confirmation prompts, against Form 2 unaided at 134.
Related
930c97a4is the reference commit🤖 Generated with Claude Code
https://claude.ai/code/session_01GSXnCLnmzk8y4cKv2y1z9f