fix(docs): scope three setup-contract rules to what backs them - #3183
Conversation
Post-merge verification of #3139 found three defects in the prose it shipped, each the same class that PR existed to correct: a claim or rule reaching past its evidence. - The org-agnosticism fetch prohibition stated a cause ("has made the publisher a runtime dependency") that does not hold for the third-party documentation URLs its own sanction covers, condemning a practice already shipping in the tree. The line now turns on who owns the target rather than on cite-versus-fetch, which also settles a coherence gap: for content an agent reads, "cited for a reader to consult" and "instructed to fetch" are not cleanly separable, so a rule keyed on that distinction leaves cases undecidable. - The evidence-bearing bullet demanded the "effective value" observed, while the same section pins effective value to fresh-session behaviour. A same-session run can only observe the stored value, so the bullet was unsatisfiable as worded by the surface other docs restate in isolation. - The hook-plugin eval skip restated its own rationale as "no model-facing skill at all" where the prior text said "no model-invoked skill". A setup skill sets disable-model-invocation: true and so is genuinely not model-invoked, meaning the original term did cover the excluded case: a substantive narrowing wearing the word clarification. The original term is restored and the exclusion now argues its own merits. Also: name the read-back location at the first of the two paired sites so it agrees with the second; correct a cross-reference that named Reintegration's own step 3 rather than Fresh-consumer onboarding's; name the axes on which the github sweep is wider and narrower; and correct the pr-issue-linkage header comment, which still described the gate as validating one section when the pinned reusable requires four. Closes #3182 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 1s —— View job Code review complete ✅
Ran the CODE-REVIEW lane ( SummaryThis is a self-correcting prose PR, so I verified each of its own factual claims against the referenced code/docs rather than just reading the diff in isolation:
No correctness or maintainability defects found in the touched lines. The PR's own verification claims (markdownlint, |
|
Claude finished @kyle-sexton's task in 46s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." All three changed files are prose-only:
No injection surfaces, authorization/access-control changes, secret/token handling, or GitHub Actions permission/trigger changes are present in this PR. Per the security-review skill's skip gate, no further findings are reported. |
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd113e57d9
ℹ️ 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".
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…n mode Review caught that the previous fix was internally contradictory. It restored "no model-invoked skill" as the skip *condition* while arguing in the next sentence that a hook plugin shipping a setup skill is not a skip — but a setup skill sets disable-model-invocation: true, so it satisfies that condition. The literal rule admitted exactly the plugins the surrounding prose excludes, and the text conceded as much rather than resolving it. Both candidate wordings fail the same way: "no model-invoked skill" and "no model-facing skill at all" are each satisfied by a plugin whose only skill is a manually-invoked setup skill. The defect was never the choice between them — it was stating the condition in terms of invocation mode at all. The condition is now the absence of a judgment-bearing contract, which is what the rationale always turned on and what the warrant rule two sentences above already uses as its test. The outcome is unchanged: all 19 hook plugins shipping a setup skill carry setup evals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 53s —— View job Code review complete ✅
Ran the Codex's P2 finding is already resolvedCodex flagged that the hook-plugin eval skip ( Verified claimsSpot-checked the PR's own factual claims against the referenced code (not just the diff in isolation), since this PR's stated purpose is fixing prose that reached past its evidence:
FindingFile: This diff corrects the file's header comment (lines 3-7) from "a non-empty # dependabot PR bodies cannot carry the closing-keyword + `## Related`
# markers this gate requires; exempt the login so its PRs are mergeable.
exempt-authors: 'dependabot[bot]'Before this PR, both comments agreed (each said only Not independently re-verifiedPer lane policy, the PR body's own verification claims ( |
Fresh-context verification returned FAIL on the item-1 rewrite with three blocking findings, all correct. The rule is a conjunction — publisher-owned AND fetch/poll/write — so cite-versus-fetch remains the sole discriminator across the whole publisher-owned row. "The line is drawn by who owns the target, not by whether the content merely cites it" therefore denied a discriminator that demonstrably operates, two clauses after the sanctioned-use sentence that depends on it. Stated as a conjunction now, with both conditions named. The rationale also claimed to remove an undecidability it had only relocated. Ownership narrows the prohibition — third-party targets leave it entirely, so cite-versus-fetch never has to be answered for them — but for publisher-owned targets the question still governs and is still hard. topic-docs.md is named as the standing undecided case, undecided on two axes since it is also not skill, agent, or schema content. Third: only the permitted half was ever checked against the tree. The prohibiting half — the rule's entire novel content — was not. Measured: 41 files under plugins/*/skills/ across 17 plugins cite a publisher-owned URL and 14 carry an explicit fetch instruction, the strongest being ai-slop's persist-findings, which is fail-closed on fetching a publisher-owned contract. The count is now stated so the prohibition is read against what it binds rather than against its one worked permitted case. Also from the same pass: the identical stale gate description 33 lines below the header comment this PR already corrected; a claim about "model-facing" — a term defined nowhere and asserted against the reading in #3182 — dropped, since the model-invoked half alone carries the argument; and a modifier left attached to a noun that cannot be stale after the effective/stored change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 9s —— View job Code review in progress
|
|
Claude finished @kyle-sexton's task in 50s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." Per the last automated review's tracking comment, the last security-reviewed head was
No injection surfaces, authorization/access-control changes, secret/token handling, or GitHub Actions permission/trigger changes are present in the commits since the last security review. Per the security-review skill's skip gate, no further findings are reported. |
…alify storage Second fresh-context verification returned FAIL with four blocking findings. All four were correct; two were introduced by the commit that answered the first FAIL. The conjunction scoped the wrong prohibition. Following the paragraph that establishes "tokens do not appear in skill, agent, or schema content", "the prohibition is a conjunction ... both conditions must hold" made fetch-on-publisher-owned necessary for nonconformance — contradicting the retained sentence that bare organization names in skill prose are nonconforming, and contradicting the MELODIC_* gate this same passage cites approvingly. It scopes the sanctioned-use exception, not the token rule, and now says so. The counts did not resolve. "41 files ... 14 of them" reproduced under no stated method: the glob matches 42 including a .py and a .js, and the 14 landed between eight different plausible criteria without matching any. Worse, the 41 counted citations, which conform under this document's own rule, so leading with it as the gap overstated the nonconformance. Replaced with the five files that carry an actual imperative, named individually so the claim is checkable rather than counted. The previous commit also introduced a fresh unqualified harness claim — "where non-sensitive options land irrespective of enable scope" — while its own sibling 55 lines later correctly qualifies the same proposition as observed only at --scope local. The primary record (extensibility-contract-smoke-tests.md) confirms the sibling. Both sites now carry the provenance, including the pre-existing instance #3182 flagged as leaned-on, and the readback destination now says a sensitive value is not in pluginConfigs at all. Finally, the workflow header's retained security rationale was false against the pinned reusable: it claims the body is read "from the event payload only", but the reusable live-refetches through the GitHub API and falls back to the payload only on failure. That sentence is the stated basis for a zizmor dangerous-triggers suppression, so it is corrected rather than left. The no-issue marker is a case-insensitive regex, not a literal, and is now described as one. Also: #3136 does not mention cite-versus-fetch, decidability, or the named file, so routing the open question there was deferral wearing resolution's clothes; the text now says no ticket owns it. The "strongest form" singular is dropped — three of the five are byte-identical. The second undecidability axis is marked weaker than it looks, since three architecture skill sites load that file during execution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 36s —— View job Code review complete ✅
Ran the Findings (posted inline)Both are the same defect class this PR exists to fix — a claim reaching past what backs it — freshly introduced by commit
Verified claims (no issue found)
Not independently verified
|
|
Claude finished @kyle-sexton's task in 1m 8s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." Per the last automated review's tracking comment, the last security-reviewed head was
This comment-only change updates a description of existing trust-boundary behavior — it does not alter the trigger, permissions, checkout behavior, or pinned reusable itself, so there's no new security-relevant surface to review even though the topic (a |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Third fresh-context verification returned FAIL with four blocking findings. Three were introduced by the commit answering the second FAIL. The pattern across three rounds is that each added factual claim became the next round's defect, so this round removes claims rather than sharpening them. "The first three are byte-identical" was false and trivially checkable: 4899, 7924, and 26531 bytes, diverging at line 1. It shipped in the same sentence as "named rather than counted so the claim can be checked", which is the sharpest form the defect has taken. Removed; the fail-closed characterisation, which does hold for all three, stays. The five-file enumeration selected on exactly the cite-versus-fetch distinction that the paragraph twelve lines above calls not cleanly separable, and it was incomplete under its own reading — write-for-agents and skill-authoring both sit inside it. Replaced with three clear instances and an explicit refusal to enumerate, naming the four near-boundary files as near-boundary. A statement that declares a line undecidable cannot also publish the list that line induces. The claim that claude-config's audit skill "reads code.claude.com that way and conforms" was scoped to that read: the same file also cites a publisher-owned convention doc through its curl route, which is the undecided question, not a settled one. The storage hedge over-corrected. Seam 1 tags the location [SPEC] -- project and local pluginConfigs entries are ignored since Claude Code 2.1.207 -- so downgrading it to "observed only at --scope local" put one fact under two incompatible epistemic labels across five sites, and made the prescribed readback able to produce the false failure the philosophy forbids. Both paired sites now cite the spec. The Reintegration citation still described Fresh-consumer step 3 in the wording this PR had already replaced, 60 lines away in the same commit. The workflow header's replacement claim was false in behavior: the reusable's "Load current PR body" step has no token in step env, so the gh api call fails silently and the payload fallback is what runs. Rather than describe the mechanism a third time, the sentence is dropped. What remains is what verification confirmed against the whole reusable and is what the zizmor suppression actually rests on -- it reads only PR metadata and never checks out or executes head code. The no-issue marker is quoted exactly now, word boundaries included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 21s —— View job Code review complete ✅
Ran the Finding (posted inline)
Verified claims (no issue found)
Not independently verifiedThe workflow header's underlying claim that the pinned |
|
Claude finished @kyle-sexton's task in 43s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." Per the last automated review's tracking comment, the last security-reviewed head was
No injection surfaces, authorization/access-control changes, secret/token handling, or GitHub Actions permission/trigger changes are present in the commit since the last security review. Per the security-review skill's skip gate, no further findings are reported. |
Review caught that `testing/skills/audit/SKILL.md` was listed as near-boundary while clearing the exact bar used to justify the three named as clear: `:87` "Read the producer contract before the first write" and `:90` "If it cannot be fetched, do not write". Both halves, same as the persist-findings trio. The near-boundary group now states what makes it near-boundary — those three direct the agent at a publisher-owned document without a fail-closed guard — rather than asserting the classification and leaving the reader to infer the criterion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…artifacts Fourth verification pass. Its first blocking finding was already fixed by 520a591; the rest are addressed here. Both paired storage sites warranted a write-location rule with seam 1's read-precedence clause ("project and local entries are ignored since 2.1.207"). Ignoring-on-read is compatible with writing there, so that clause does not entail the rule. Seam 1's first clause -- non-sensitive values store in user settings -- does, and is what both sites now cite. The rule and the outcome are unchanged; only the warrant was wrong, which is the shape #3182 item 3 was filed as blocking on. Three artifacts of removing claims, each the mirror of over-claiming: - "Enforcement reaches a strict subset of the whole" lost its antecedent when the paragraph it followed was restructured; it names the target again and starts its own paragraph. - The plugin.json parenthetical had drifted to the tail of the undecidability paragraph, where it reads as a non-sequitur. Returned to the token-rule paragraph it comments on. - The workflow header carried "not any claim about where the body is read from", a negation pointing at a sentence deleted in the same commit. A fresh reader cannot resolve it, so it is gone, and the retained rationale now states the three properties verification actually established: no checkout step, `pull-requests: read` + `actions: read` only, no head-code execution. One newly added clause over-claimed: a sensitive value "reads back from secure credential storage" asserts a route no smoke test covers, and Test B records a sensitive value as unreachable from a skill entirely. It now says only what smoke-test A establishes -- absent from settings, so the readback does not apply. Also corrected in the workflow comment: the no-issue marker is not matched "anywhere in the body" (fenced and indented code, inline spans, and HTML comments are stripped first), and the runner is pinned by this caller rather than inherited from the reusable's default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 2s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." Per the last automated review's tracking comment, the last security-reviewed head was
No injection surfaces, authorization/access-control changes, secret/token handling, or GitHub Actions permission/trigger changes are present in the commits since the last security review. Per the security-review skill's skip gate, no further findings are reported. |
Fifth verification pass returned FAIL with five blocking findings, the fifth consecutive round to do so. Reviewing where the findings came from: items 2 and 3 of #3182 have been clean since round three, and essentially every finding from round two onward landed in material this PR added beyond the ask -- an enumeration of nonconforming instances, a paragraph grounding the prohibition against the tree, a rewritten security rationale, and a sensitive-value carve-out. Each elaboration was written to close the previous round's finding and opened one or two of its own. So this round removes the elaborations rather than repairing them again. The org-agnosticism fix is now one paragraph that does what item 1 asked: scope the prohibition to publisher-owned targets, and state plainly that the cite-versus-fetch question for those targets is unsettled, with topic-docs.md as the open case. Gone: the four/three partition of live instances (whose classification was falsified twice), the "what ownership buys" elaboration (which asserted both readings of its own scope), and the claim that no enumeration is offered (made one paragraph after offering one). The workflow header returns to main's text with one correction -- the gate requires four sections, not a closing keyword plus `## Related`. That was the only part #3182 raised. The security-rationale sentence is false against the pinned reusable, which live-refetches rather than reading the payload, but it is pre-existing and out of this issue's scope; it gets its own issue rather than a fourth rewrite here. Both storage sites now carry identical wording, including the sensitive carve-out that had been added to one and not the other -- an asymmetry that would have sent a reader reconfiguring a sensitive option at project scope to look in user settings, find nothing, and report a failed write. The carve-out states the limit ("cannot be verified this way") rather than asserting a readback route no test covers. The provenance citation drops smoke-test C, which explicitly disclaims covering a sensitive option, in favour of seam 1, which documents both halves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 17s —— View job Security review — skipped (no security-relevant surface)
Skip gate triggered: condition 2 — "Change has no security-relevant surface after reading the diff." Per the last automated review's tracking comment, the last security-reviewed head was
One thing surfaced worth flagging for the correctness lane rather than security: this commit reverts the header's runner description from the prior round's more precise "the runner is pinned below rather than inherited from the reusable's default" back to "runs on the reusable's hosted default runner" — which is the same wording already on No injection surfaces, authorization/access-control changes, secret/token handling, or GitHub Actions permission/trigger changes are present in the commit since the last security review. Per the security-review skill's skip gate, no further findings are reported. |
Sixth verification pass. Two findings, both in the one paragraph this round had to write rather than cut; both repairs are deletions. "No ticket owns that question" is a tracker-wide negative asserted without a tracker search, and the tracker contradicts it. #432 carries an accepted operator ruling on this exact question -- bundle the contract, demote the raw URL to a pinned provenance link -- and generalises it across the eight implementer plugins sharing that binding. #410 and #531 carried it too. `scripts/skill-portability-tokens.txt:139-152` stages a lint class for the same distinction and records that splitting it "needs #432's bundle-vs-pin ruling first". All three tickets are closed and the remediation is undone, so a narrow "no open ticket" reading might have survived -- but that is not what the sentence said, and a reader would conclude the disposition is unowned when a ruling covers it. "and says nothing about third-party ones either way" contradicts the clause it modifies: "fetching one does not forfeit the citation" is itself a statement about third-party targets. The preceding clause already scopes the prohibition, so the hedge only undercut the sentence that un-breaks the shipped `code.claude.com` read. The #3136 disclaimer goes with the first sentence. The paragraph still says the question is unsettled and still names the open case, which is what item 1 asked for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d reusable (#3209) Closes #3184 ## Summary The header comment in `.github/workflows/pr-issue-linkage.yml` justified its `pull_request_target` trigger — and the `zizmor: ignore[dangerous-triggers]` suppression that points at it — with a sentence that does not describe the pinned reusable: "the reusable reads PR body metadata from the event payload only and runs no head code." Only the second half is true. The same block also misattributed the runner to "the reusable's hosted default" and described the no-issue opt-out as a literal string. This PR rewrites the header so every claim matches the reusable's actual code at the pinned SHA (`7107b34`, v0.14.2). ## Fix Comment text only; no functional YAML changes. Three corrections: - **Body-read mechanism.** The reusable's `Load current PR body` step live-refetches the body via `gh api` and uses the event payload only as a fallback when that call fails. Re-verified at the pinned SHA: no `GH_TOKEN`/`GITHUB_TOKEN` reaches that step (its `env:` carries only `PR_NUMBER` and `FALLBACK_BODY`, and neither the job nor the workflow wires a token), so the re-fetch fails auth every time and the payload fallback is the path that actually runs. The header now states this — payload-only in effect, as a side effect of the failed re-fetch, not by design — and rests the `pull_request_target` safety rationale on what actually makes the trigger safe: no head-branch code ever executes (the reusable checks out nothing, holds `pull-requests: read` / `actions: read` only, and passes the body through `env:`/`GITHUB_ENV` rather than splicing it into script text). - **Runner.** "Runs on the reusable's hosted default runner" replaced with the truth: the `with:` block pins `runner: ubuntu-24.04` explicitly, which coincides with the reusable's default. - **No-issue marker.** Described as what it is: a case-insensitive regex matching the phrase "no linked issue" or "no related issue" (`/\bno (?:linked|related) issue\b/i`), not a literal string, with both body scans running against a body whose fenced code blocks and 4-space/tab-indented lines are blanked, inline code spans masked, and HTML-commented text discarded. Per the issue's scope note, wiring a token into the reusable so the re-fetch succeeds is out of scope — that lives in `melodic-software/ci-workflows` and would be a behavior change, not a comment fix. ## Verification - Every claim in the new header was verified directly against the reusable's source fetched at the exact pinned SHA `7107b34832a7b6db5d08d3b132621c599fbe5e50`: the `gh api` re-fetch with `FALLBACK_BODY` fallback, the absence of any token in the step/job/workflow environment, the absence of any checkout step, the read-only `permissions:` block, the `env:`/`GITHUB_ENV` body path with random heredoc delimiter, the `runner` input default, the `NO_ISSUE_MARKER` regex, and the code/comment masking in `stripRenderedHtmlComments` (including the `/^(?: {4}|\t)/` indented-code branch). - `actionlint` passes on the edited file; `zizmor` passes with the `dangerous-triggers` suppression still honored (`No findings to report. (1 ignored, 1 suppressed)`). - `git diff -U0` filtered to non-comment lines is empty: `on:`, `permissions:`, the `uses:` pin, and the `with:` values are byte-identical to `main`. - An independent fresh-context reviewer re-verified each header claim against the pinned reusable source with the author rationale withheld: PASS on every criterion, plus two advisory phrasing findings (the "indented code blocks" wording and a dangling "Public repo:" lead-in), both applied in the second commit with linters re-run green. - This change is comment-only; no test can meaningfully cover it, so no test is claimed. ## Related - #3183 — corrected the gate-description half of this comment block; this PR corrects the half it deliberately left alone. - #3182 — parent issue that first flagged the block. - This PR does not touch `.github/workflows/ci.yml`; it cannot collide with the concurrent `docs_only` scope-resolution work there (#3159). --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Closes #3182
Summary
Post-merge verification of #3139 (merged as
ef4d53959) found three defects in the prose it shipped, each the same class that PR existed to correct: a claim or rule reaching past what backs it. They were filed rather than quietly patched because the content was already onmain.This fixes those three plus the smaller items #3182 lists, in 46 added lines across three files.
Fix
1. The fetch prohibition's stated cause did not entail its stated rule.
PLUGIN-PHILOSOPHY.mdsanctioned "a documentation URL", then condemned any skill instructed to fetch it as having "made the publisher a runtime dependency". Fetchingcode.claude.comcreates no such dependency, and a practice already shipping in the tree was condemned by it.The prohibition now turns on the target's owner and reaches publisher-owned targets only. For those targets, distinguishing an instruction to fetch from a citation offered for a reader is genuinely hard, and the statement says so rather than implying it has been settled —
plugins/architecture/reference/topic-docs.mdis named as the open case, and no ticket owns it (#3136 is enforcement-site consolidation, not this).2. The
evidence-bearingbullet was unsatisfiable as worded. It required setup to report "the effective value it observed", while the same section pins effective value to running-session behaviour and directs verification to a fresh session. A same-session run can only observe the stored value. One word:effective→stored.3. A narrowing presented as a faithful clarification. The hook-plugin eval skip stated its rationale as "no model-facing skill at all" where the prior text said "no model-invoked skill". Neither works: a
setupskill setsdisable-model-invocation: true, so either phrasing is satisfied by a plugin that ships one — admitting as skips exactly the plugins the rest of the rule excludes. The defect was stating the condition in terms of invocation mode at all. It now reads "no skill carrying a judgment-bearing contract", the test the warrant rule two sentences above already uses. Outcome unchanged: 19 hook plugins ship a setup skill, all 19 carry setup evals.Smaller items. Both paired reconfiguration sites in
MIGRATION-PLAYBOOK.mdnow name the readback location and agree in substance, including the sensitive-value limit — an asymmetry between them would have sent a reader reconfiguring a sensitive option at project scope to look in user settings, find nothing, and report a failed write, which is the false failurePLUGIN-PHILOSOPHY.mdexists to prevent. Their provenance cites seam 1, which documents both halves, rather than smoke-test C, which explicitly disclaims covering a sensitive option. The "step 3 above" cross-reference — which pointed from inside Reintegration's step 1 at Reintegration's own step 3, about verify-before-retiring — is replaced by a direct citation of seam 1. Thegithub.test.shsweep's wider/narrower axes are named, and "both steps of the same job" is corrected to "each running in its own step". The workflow header's gate description is corrected: the pinned reusable requires four sections, not a closing keyword plus## Related.Verification
Local gates at the final commit:
markdownlint-cli20 issues;check-contract-clause-coverage.pyexit 0;lychee --offline0 errors across 103 unique links;zizmorno findings. The workflow change is comment-only, confirmed by diff.Six fresh-context verification passes, each given the bounded criteria plus an unbounded criterion instructing it to hunt for claims reaching past their evidence anywhere in the touched paragraphs. All six returned FAIL, and each round's fixes introduced at least one new instance of the defect being repaired.
The sixth pass found two, both in the single paragraph this PR had to write rather than cut, and both repairs were deletions: a tracker-wide "no ticket owns that question" that the tracker contradicts (#432 carries an accepted ruling on it, and
scripts/skill-portability-tokens.txtstages a lint class blocked on that ruling), and a hedge that denied the statement its own preceding clause had just made. Every deletion from the prior round verified clean against the tree.Reviewing where the findings came from settled the approach. Items 2 and 3 were clean from round three onward; essentially every finding from round two on landed in material added beyond what #3182 asked for — an enumeration of nonconforming instances, a paragraph grounding the prohibition against the tree, a rewritten security rationale, a sensitive-value carve-out. Each was written to close the previous round's finding and opened one or two of its own. The final revision deletes those elaborations rather than repairing them again, which is why the diff is 46 lines rather than the 2,926 it peaked at.
Twenty-five instances of the defect class were found across the five rounds. One was caught by the author re-reading their own writing, four by the review bots, and the rest by fresh-context verification. None by self-review.
Two things generalise. A verifier is bounded by its criteria, so a defect nobody names survives any number of green passes — every round's findings came from the unbounded criterion, not the checklist. And under-claiming is not the safe direction: round four's findings were mostly repairs to what round three's removals broke. Both directions are the same failure to say exactly what the evidence supports.
One pre-existing defect is deliberately left alone and filed as #3184: the workflow header's security rationale ("reads PR body metadata from the event payload only") is false against the pinned reusable, which live-refetches. It is outside #3182's scope, the
zizmorsuppression it backs is independently sound, and three separate rewrites of that comment block each introduced a new inaccuracy.Related
origin/mainatef4d53959.🤖 Generated with Claude Code