fix(docs-hygiene): narrow audit-noise's negation shape to imperatives (0.21.1) - #3202
Conversation
… (0.21.1)
The shape shipped in 0.21.0 fired 1053 times on an 85-file sample of this
repo's own tracked markdown — 12.4 per file, 99% of every finding the skill
produced. The eight older shapes produced 10 between them. At that rate it
swamps the human report on every run and would flood the apply relay under
--persist-findings.
Two scope gates bring it to 31 on the same files, with every other shape's
count byte-identical (3/2/2/1/1/1 before and after), so the whole delta is
this shape:
- imperative only: the cue must open the line, after list, blockquote and
emphasis markers. "Prompt the positive" is a rule about instructions, so
descriptive prose was never in scope, and a mid-sentence cue is already the
paired form ("Prefer X; never Y")
- the line must close its own sentence: this repo hard-wraps prose and the
pairing rule is per sentence, so a continuation cannot be shown to lack a
positive on the next line; the same test excludes a table row
Both narrowings are #3180's, established there against a 1140-file corpus
sweep while that PR was open against the same issue. This adopts the
calibration rather than discarding it.
Accepted cost, pinned by an assertion so a future widening cannot pass
silently: a subject-led instruction no longer selects.
Closes #3201
|
Claude finished @kyle-sexton's task in 2m 50s —— View job Code review complete
Scope: No blocking findings. I traced both new gates against the fixtures and the existing call site rather than trusting the author's stated numbers:
One non-blocking observation, not filed inline since it's speculative rather than a concrete gap: Verification note: this review lane's Bash access is scoped to |
|
Claude finished @kyle-sexton's task in 1m 4s —— View job Security review — no issues found
Skip gate result: stopping early — this PR has no security-relevant surface. All six changed files are documentation and a local, offline bash text classifier:
This tool runs locally against a repo's own tracked markdown (developer-supplied input, not attacker-controlled/untrusted external input), performs no code execution of matched content, and touches no auth, secrets handling, network I/O, or GitHub Actions workflow config. No CRITICAL/IMPORTANT/SUGGESTION security findings to report. |
|
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: 6c99cd1a93
ℹ️ 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".
A naive gsub double-escaped a pipe the source had already escaped: `a \| b` became `a \\| b`, which GFM reads as a literal backslash followed by a LIVE delimiter, splitting the row so the fix action misreads it. This repo writes literal `\|` in its own tables, so the case is real rather than theoretical. Already-escaped pipes are parked on a sentinel and restored single-escaped. Also identified in #3180, which notes the sibling producers carry the same latent defect in their own copies of esc() — out of scope here. Refs #3201
|
Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Claude finished @kyle-sexton's task in 47s —— View job Security review — no issues found
Skip gate result: stopping early — the commit added since the last review has no security-relevant surface. Per the bot's tracking comment, the last security-reviewed head was
This is a bug fix to a local text-formatting routine (correctness of markdown table escaping), not a change to any auth, secrets, network, or Actions-workflow surface. No CRITICAL/IMPORTANT/SUGGESTION security findings to report for this commit or the PR overall. |
|
Claude finished @kyle-sexton's task in 3m 35s —— View job Code review complete
FindingsPosted 2 inline comments on
Everything else in the diff (the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…e lint Three review findings on #3202, each reproduced before being fixed: - shell-portability-lint FAILED on `\>` — a GNU word-boundary operator elsewhere in the toolchain. The lead pattern now lives in a variable with `>` unescaped, which also stops the unquoted `>` confusing the [[ ]] parser - the imperative gate was per LINE, so a line opening imperatively carried a later DESCRIPTIVE sentence into a finding — exactly the prose the gate exists to exclude, re-entering behind a compliant opener. Now per sentence - the lead stripper missed `1)` ordered items and task-list checkboxes, both ordinary ways this repo writes a directive; missing them withheld silently The per-sentence gate moves the measured count on the 85-file sample from 31 to 69, because 38 genuine imperative prohibitions sit as a LATER sentence on their line and were being withheld. Sampling those additions found real findings plus a residual false-positive class: a positive supplied as a bare imperative after a separator ("Never confirm X — delegate to Y") is not recognised, because pairing matches a fixed marker list. #3180 solves that with a closed function-word stoplist; filed as #3204 rather than guessed at here. Refs #3204
Review round addressed —
|
| Finding | Disposition |
|---|---|
shell-portability-lint failed on \> — a GNU word-boundary operator elsewhere in the toolchain |
Fixed. The lead pattern now lives in a variable with > unescaped, which also stops the unquoted > reading as a redirection to the [[ ]] parser |
| Codex: the imperative gate was per LINE, not per sentence | Fixed, and it was the important one. Do not use markdown; instead use HTML. Older versions do not support SVG. opens imperatively, passes a line-level gate, and the loop then reports the second sentence — exactly the descriptive prose the gate exists to exclude, re-entering behind a compliant opener |
Codex: 1) ordered items not recognised |
Fixed, and task-list checkboxes (- [ ] …, - [x] …) with it — the reviewer flagged that as a speculative aside and it was real. Both are ordinary ways this repo writes a directive, so missing them withheld silently |
The number moved: 31 → 69, and why that is the right direction
The per-sentence gate raises the measured count on the same 85-file sample. That is not a regression — 38 genuine imperative prohibitions sit as a later sentence on their line and the line-level gate was withholding them. Headline is still 1053 → 69 against 0.21.0, with every other shape's count byte-identical.
I sampled those 38 additions and read the source lines rather than trusting the total. They are mixed:
SKILL.md:126— "…and stop. Never auto-file, never auto-edit." → genuineSKILL.md:145— "Do not carry one surface's verdict to the other." → genuineSKILL.md:142— "Never confirm a load-bearing deletion — delegate to a fresh subagent" → false positive. The positive alternative is present; pairing is matched against a fixed marker list (instead,rather than,prefer…) which does not recognise a bare imperative after a separator.
That residual class is #3204, to be fixed with #3180's closed function-word stoplist — testing what a clause is not generalises where a marker list only covers the phrasings its author thought of. It existed at 31 too; the per-sentence gate exposes more of it. I filed it rather than guessing at the stoplist here, having already misjudged this heuristic's breadth once in #3194.
On the verification note
Both review lanes flagged that they could not execute the suites. Worth recording that the static traces were accurate: the code-review lane's read of the gate ordering, the strip_line_lead marker consumption, and the doc/version consistency all match what running them shows. Its task-list-checkbox aside — offered as speculative — was a real gap and is fixed here.
Gates now: detect.test.sh 133, emit-findings.test.sh 35, all docs-hygiene suites, portability, typos/shellcheck/shfmt/markdownlint, crosswalk, changelog parity, check-skill 0 errors.
Generated by Claude Code
|
Claude finished @kyle-sexton's task in 2m 53s —— View job Code review complete
Scope: this pass focused on the new commit Fixes verified correct by trace:
One finding, posted inline on
Everything else in the commit ( |
|
Claude finished @kyle-sexton's task in 48s —— View job Security review — no issues found
Skip gate result: stopping early — the commit added since the last security review has no security-relevant surface. Per the bot's tracking comment, the last security-reviewed head was
This remains a local, offline bash text classifier operating on a repo's own tracked markdown (developer-supplied input, not attacker-controlled), performing no code execution of matched content, and touching no auth, secrets handling, network I/O, or GitHub Actions workflow config. No CRITICAL/IMPORTANT/SUGGESTION security findings to report for this commit or the PR overall. |
…pe rule An earlier edit inserted the corrected clause ahead of the old wrapped line rather than replacing it, so the marker list read twice — once with task-list-checkbox and once without. The CHANGELOG bullet was edited as a clean replacement and already reads correctly; this matches it.
|
Claude finished @kyle-sexton's task in 1m 14s —— View job Code review complete
Scope: the only commit added since this lane's last review ( No findings. This commit fixes exactly the duplication a prior pass on this PR flagged inline on I did not re-review the rest of the PR's diff ( |
|
Claude finished @kyle-sexton's task in 39s —— View job Security review — no issues found
Skip gate result: stopping early — the commit added since the last security review has no security-relevant surface. Per the bot's tracking comment, the last security-reviewed head was
No other files changed in this commit. This remains a local, offline bash text classifier operating on a repo's own tracked markdown; nothing in this push touches auth, secrets handling, network I/O, or GitHub Actions workflow config. No CRITICAL/IMPORTANT/SUGGESTION security findings to report for this commit or the PR overall. |
…resolve repo-root spellings (0.3.6, 0.39.3) Both sibling emit-findings.sh copies still carried the two defects #3202 fixed only in docs-hygiene. A naive gsub double-escapes a pipe the source already escaped, which GFM reads as a live delimiter. esc() now parks already-escaped pipes on a sentinel first. git rev-parse --show-toplevel can spell the same directory differently than the caller pwd (Git Bash). ai-slop failed open (Location stayed absolute, nothing reported it). claude-config failed closed (in-repo hits were declined as outside-repo-root). Root resolution now prefers the caller pwd minus git --show-prefix, with git's two spellings as fallbacks. Shared code was considered and declined: plugins are portable and there is no existing cross-plugin emit-findings library. Closes #3203 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…resolve repo-root spellings (0.3.6, 0.39.3) Both sibling emit-findings.sh copies still carried the two defects #3202 fixed only in docs-hygiene. A naive gsub double-escapes a pipe the source already escaped, which GFM reads as a live delimiter. esc() now parks already-escaped pipes on a sentinel first. git rev-parse --show-toplevel can spell the same directory differently than the caller pwd (Git Bash). ai-slop failed open (Location stayed absolute, nothing reported it). claude-config failed closed (in-repo hits were declined as outside-repo-root). Root resolution now prefers the caller pwd minus git --show-prefix, with git's two spellings as fallbacks. Shared code was considered and declined: plugins are portable and there is no existing cross-plugin emit-findings library. Closes #3203 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…resolve repo-root spellings (0.3.6, 0.39.4) (#3242) ## Summary #3202 fixed two latent defects in `docs-hygiene:audit-noise` `emit-findings.sh` only. Each producer carries its own copy, so the same defects were still live in `ai-slop` and `claude-config:audit-instructions`. **Defect 1 — `esc()` is not idempotent.** A naive `gsub(/\|/, "\\|")` turns `a \| b` into `a \\| b`. GFM reads that as a literal backslash followed by a live delimiter, so the row splits and the fix action misreads it. This repo writes literal `\|` in its own tables. **Defect 2 — repo-root spelling mismatch.** `git rev-parse --show-toplevel` can answer `C:/Users/…` while the caller is at `/tmp/…` (Git Bash). Consequences differed per producer and were confirmed individually: - `ai-slop` **failed open** — Location stayed absolute and nothing reported it, because an absolute path is still a well-formed cell. - `claude-config` **failed closed** — a path it could not prove was under the root was declined as `outside-repo-root`, so an in-repo finding was silently missed. ## Fix `esc()` now parks an already-escaped pipe on a sentinel first, matching #3202. Root resolution prefers the caller `pwd` minus git `--show-prefix`, with git's two spellings as fallbacks, matching the #3180 form that #3202 did not actually land. Shared code was considered and declined: plugins are portable and there is no existing cross-plugin emit-findings library. The three copies now agree on the same two helpers instead. ai-slop 0.3.6 (0.3.5 is #3179), claude-config 0.39.3. **docs-hygiene residual (not fixed here):** #3202 shipped the idempotent `esc()` but not the three-spelling root. That producer still resolves root via `git rev-parse --show-toplevel` only. ## Verification - `plugins/ai-slop/skills/audit/scripts/detect.test.sh` — 98/98 (idempotent esc + symlink spelling mismatch) - `plugins/claude-config/skills/audit-instructions/scripts/emit-findings.test.sh` — 81/81 (same two cases; fail-closed fence still declines a true out-of-repo path) - `scripts/check-changelog-parity.sh --check-bump origin/main` — pass - markdownlint on both changelogs — 0 issues ## Related Closes #3203 Refs #3180, #3202 --------- Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
Closes #3201
Summary
The
negationshape shipped in 0.21.0 (#3194) selects far too broadly to be usable. This adopts the two scope gates from #3180, which had already established and corpus-calibrated them while open against the same issue.Fix
The measurement, on an 85-file sample of this repo's own tracked markdown (
plugins/docs-hygiene+plugins/source-control, minusevals/fixturesandCHANGELOG.md):negationghost-refticket-pr-residuecitationscope-metaplan-referenceenum-list12.4 negation findings per file, 99% of everything the skill produced — the eight older shapes produced 10 between them. Every other shape's count is byte-identical before and after, so the entire delta is this shape.
Two gates, both load-bearing:
write-for-agents"Prompt the positive" is a rule about instructions, so descriptive prose (Older versions do not support this flag.) was never in its scope. A mid-sentence cue is excluded by construction and that is the right result: a correctly paired sentence puts the cue after its positive (Prefer X; never Y), so the test declines exactly what is already compliant.|.Accepted cost, stated rather than hidden: a subject-led instruction (
The agent must not emit a bare summary.) no longer selects. Pinned by an assertion so a future widening cannot pass silently.Verification
detect.test.shemit-findings.test.shdocs-hygienesuitescheck-skill.sh(source checkout)check-detector-findings-crosswalk.shcheck-changelog-parity.sh(--check,--check-bump,--check-order)New regression tests cover both gates in both directions: descriptive prose, an already-paired mid-sentence cue and a table row do not select; plain, list, bolded and blockquoted imperatives all still do; a hard-wrapped continuation does not select on its own. The abbreviation fixture from #3194 was rebuilt so it exercises the splitter inside the new scope rather than passing on a line the gates now decline.
Related
Why the collision happened
#3194 was produced in a session where the
work-itemsseam claim was unavailable (gh2.45 against a required ≥ 2.94, plus an invalid token), so no assignee/lease was acquired. The in-flight-PR check also used a keyword search rather than the closing-keyword linkage the skill specifies, which is what would have surfaced #3180. Both are recorded in #3201.Generated by Claude Code