Skip to content

fix(ai-slop): clear the post-purge full-repo audit findings (#4606) - #5586

Merged
kyle-sexton merged 30 commits into
mainfrom
fix/4606-post-purge-audit-findings
Sep 30, 2026
Merged

kyle-sexton merged 30 commits into
mainfrom
fix/4606-post-purge-audit-findings

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #4606

Summary

The full-repo ai-slop detector reports zero findings. The rule half landed in #5380; this PR clears the rest of criterion 1 by following the owner's 2026-09-30 decision on the issue. The four non-em-dash findings are fixed. Em dashes are purged in docs/adr, docs/conventions (including the standards contract and its two plugin copies in plugins/planning and plugins/review), docs/out-of-scope and plugins/attribution. Six first-party docs/upstream ledgers and the frozen .claude/unhobble evidence memos are allowed in .claude/ai-slop.json for now, each with its reason written in _comment. Three decisions are left to the owner (see "Owner veto").

Fix

  • Fixed the four non-em-dash findings: the knowledge-cutoff wording in docs/conventions/loop-lane/README.md, the filler "in order to" in plugins/discovery/skills/explore/SKILL.md and reference/workflow.md, and the same filler in plugins/rate-limit-guard/CHANGELOG.md.
  • Purged em dashes from docs/adr (ADR 0001 onward, one commit per ADR range), rewriting each with the punctuation the sentence needs. In docs/out-of-scope, shared-surface-instruction-governance.md was the only record with an em dash, and it is purged.
  • Purged plugins/attribution: skills/audit/context/persist-findings.md, and the restated-fact golden cases c12, c14 and c16, rewritten on the same lines so every expected span is unchanged. The c24 source is a verbatim upstream excerpt, so its passage sits inside an ai-slop-ignore region instead of being rewritten. This is the repo's existing rule in the header of scripts/em-dash-purged-paths.txt: what a file quotes from upstream "stays byte-exact inside ai-slop-ignore-marked regions". It is not a config exemption.
  • Followed the owner's step 3 for the standards contract. docs/conventions/standards/README.md has its 31 em-dash lines rewritten, the contract is standards-contract: 1.0.1 with a docs/conventions/standards/CHANGELOG.md entry, and both plugin copies are regenerated with scripts/sync-standards-contract.sh. Planning is 0.52.1 and review is 0.34.3. The contract and its copies are declared in scripts/em-dash-purged-paths.txt, so check-purged-em-dashes.sh holds them.
  • Made audit-native-overlap generate append an ai-slop-ignore comment to a native description: evidence line that carries an em dash, because the native description is quoted verbatim in docs/native-surfaces.md (the same byte-exact-quote rule as above). Authored evidence lines are never marked. The view is regenerated (two lines change), claude-ops is 0.77.2, and test_overlap.py has a test for the marker. docs/native-surfaces.md needs no config exemption, so the exemption an earlier version of this PR carried is gone.
  • Declared docs/adr/*.md and docs/out-of-scope/*.md in scripts/em-dash-purged-paths.txt and removed docs/adr/** from that file's "absent on purpose" list, so the purged records stay purged (see "Owner veto", item 2).
  • em_dash_allowed_paths in .claude/ai-slop.json holds seven entries: the six docs/upstream/*.md ledgers that still carry em dashes, each named by file, and .claude/unhobble/*/evidence/**. _comment states the reasons (see "Owner veto", item 1). scripts/em-dash-purged-paths.txt now says these entries defer a purge of first-party prose, so the two files agree. docs/upstream/claude-code-mods (declared purged) and every other docs/upstream file are not covered and stay at zero tolerance.
  • Plugin versions, all above origin/main: attribution 0.8.1, discovery 0.25.18, planning 0.52.1, review 0.34.3, claude-ops 0.77.2, each with a changelog entry. The rate-limit-guard changelog wording fix carries no version change.

The contract change has this consumer effect: a consuming repository's standards index at standards-contract: 1.0.0 is older than the bundled 1.0.1 contract. Skills report "index at v1.0.0, contract at v1.0.1: re-run setup to migrate" and route best-effort, and setup offers the guided migration. The step-3 purge is the reason for the bump. It also reverses the non-purge that scripts/em-dash-purged-paths.txt used to document for the contract (see "Owner veto", item 3).

Owner veto

  1. Seven entries in em_dash_allowed_paths: six docs/upstream ledgers and .claude/unhobble/*/evidence/**. They reverse the config's zero-tolerance wording. The issue's step 4 recommended (b) on the premise that docs/upstream holds upstream snapshots. By the repository's own record it does not: scripts/em-dash-purged-paths.txt says "docs/upstream/ IS NOT A VENDOR TREE" and calls these per-source ledgers first-party prose, and the files agree (claude-code.md is "What this marketplace decided about its own components"; aihero-course.md lists decided lanes). So the docs/upstream entries defer a purge of first-party prose; they do not exempt vendored text.
    • The six entries cover 304 em dashes: aihero-shipping-course.md 93, cursor-pstack.md 67, aihero-course.md 53, mattpocock-skills.md 47, mattpocock-skills-v12-map.md 37, humanlayer-skills.md 7. Each is one file, so docs/upstream/claude-code-mods and every other file there stay at zero tolerance, and an entry is deleted in the change that purges its file.
    • The .claude/unhobble/*/evidence/** entry covers five research memos and decision verdicts (119 em dashes), the frozen record of one experiment run. The live stumbles.md and the README are not exempt.
    • If the owner rejects the docs/upstream entries, the 304 rewrites follow as per-file PRs and those six entries are dropped. If the owner rejects the .claude/unhobble entry, 119 rewrites of the frozen memos follow and that entry is dropped.
  2. docs/adr/*.md and docs/out-of-scope/*.md in scripts/em-dash-purged-paths.txt. The file's own convention is to add a surface in the PR that purges it, so this PR does. It also turns the one-time purge of each directory into a standing CI gate (check-purged-em-dashes.sh): once this merges, an ADR or an out-of-scope record with an em dash fails CI, and any open PR that adds one goes red. The alternative is to drop either line (for docs/adr/**, also restoring its "absent on purpose" wording), which leaves those records purged but not enforced.
  3. The standards-contract bump to 1.0.1. The owner's step 3 ordered the contract purged, and changing the contract text forces a version bump. The bump makes every consuming repository's standards index (at 1.0.0) older than the bundled contract, so each gets the "re-run setup" prompt and best-effort routing until it does. It also reverses the non-purge that scripts/em-dash-purged-paths.txt documented on purpose, because the contract and its two plugin copies in plugins/planning and plugins/review now sit on that list. The alternative is to revert the contract, its changelog entry, the two copies and the planning and review bumps (0.51.1 and 0.34.3), restore the non-purge wording, and add em_dash_allowed_paths entries for the contract and its two copies so the full-repo detector run stays at zero.

Verification

Run from the worktree at head 9697ca0e2, which includes origin/main at 27206c96e.

  • bash plugins/ai-slop/skills/audit/scripts/detect.sh, every Summary line:

    Summary rule=ai-slop/audit/rule-em-dash findings=0 declined=334 declined_marker=299 declined_quote=0 declined_config=35 disabled=0
    Summary rule=ai-slop/audit/rule-emoji-formatting findings=0 declined=24 declined_marker=0 declined_quote=0 declined_config=24 disabled=1
    Summary rule=ai-slop/audit/rule-curly-artifacts findings=0 declined=24 declined_marker=0 declined_quote=0 declined_config=24 disabled=1
    Summary rule=ai-slop/audit/rule-significance-inflation findings=0 declined=29 declined_marker=0 declined_quote=5 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-negative-parallelism findings=0 declined=39 declined_marker=2 declined_quote=13 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-challenges-conclusion findings=0 declined=30 declined_marker=0 declined_quote=6 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-knowledge-cutoff-disclaimer findings=0 declined=36 declined_marker=5 declined_quote=7 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-llm-citation-artifacts findings=0 declined=24 declined_marker=0 declined_quote=0 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-utm-params findings=0 declined=24 declined_marker=0 declined_quote=0 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-chatbot-artifacts findings=0 declined=30 declined_marker=0 declined_quote=6 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-filler-phrases findings=0 declined=47 declined_marker=2 declined_quote=21 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-stacked-hedging findings=0 declined=30 declined_marker=0 declined_quote=6 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-model-era-phrases findings=0 declined=31 declined_marker=0 declined_quote=7 declined_config=24 disabled=0
    Summary rule=ai-slop/audit/rule-ai-vocabulary findings=0 declined=87 declined_marker=2 declined_quote=56 declined_config=29 disabled=0
    Summary rule=ai-slop/audit/rule-copulative-avoidance findings=0 declined=48 declined_marker=0 declined_quote=24 declined_config=24 disabled=0
    Summary total: 0 findings across 1902 files scanned (24 files declined)
    
  • scripts/check-changelog-parity.sh --check, --check-order, --check-bump origin/main and --check-preserved origin/main all exit 0.

  • scripts/sync-standards-contract.sh --check: All 2 plugin copies match docs/conventions/standards/README.md. --check-bump origin/main: Contract changed vs origin/main with frontmatter, changelog, and every carrying plugin bumped.

  • scripts/check-purged-em-dashes.sh: check-purged-em-dashes: 391 declared paths, 1382 files scanned, no em dashes. Its test scripts/check-purged-em-dashes.test.sh passes (24 of 24).

  • scripts/check-adr-numbers.sh --check exits 0. scripts/validate-plugins.sh: All plugin manifests and the catalog validated.

  • overlap.py generate --check: docs/native-surfaces.md is in sync with docs/native-surfaces/records.json. scripts/run-ruff.sh check and format --check pass on overlap.py and test_overlap.py.

  • markdownlint-cli2 over the 49 changed markdown files: Summary: 0 issues in 0 files.

  • scripts/affected-tests.sh --run --jobs 4 over the diff, run at this head. Every selected suite passed except three that this worktree cannot run: scripts/check-html-assets.test.sh and scripts/check-script-contract.test.sh (htmlhint is not installed here) and scripts/hook-census.test.sh (no strace). The runner does not execute 23 selected Python and Node suites; I ran 22 of them directly at this head and they pass, and the 23rd, plugins/session-flow/scripts/tests/test_save_point.py, needs pytest, which is not installed here. CI runs check-html-assets.test.sh and hook-census.test.sh in test-linux legs 2 and 0; I did not find check-script-contract.test.sh or test_save_point.py in the job steps, so those two are unrun by me. The three failures are the missing tool, not an assertion. The ai-slop detect.test.sh (242 cases), cross-check.test.sh (24 cases), overlap.test.sh and test_overlap.py (179 tests) were also run directly at this head and pass.

  • CI on head 9697ca0e2 (run 36770676069): lint, lint-2, hook-utils, test-linux (0) to (3) and ci-status pass. The PR is mergeable and its merge state is clean.

Related

#5269, #5380, #3988, #4288, #5288

🤖 Generated with Claude Code

kyle-sexton and others added 28 commits September 30, 2026 01:33
…ream snapshots (#4606)

Rewrite the four live filler and knowledge-cutoff sentences instead of
marking them, and add em_dash_allowed_paths for docs/upstream and
.claude/unhobble with the reason in _comment.

Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… surfaces (#4606)

Rewrite the standards contract and its two generated plugin copies, the
out-of-scope records and the attribution persist-findings note with the
punctuation each sentence needs. Declare the purged paths in the ratchet
list and exempt the generated native-surfaces view, which quotes native
descriptions verbatim.

Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #4606

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Patch bumps and changelog entries for attribution, discovery, planning,
rate-limit-guard and review, whose files lost em dashes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…udit-findings

Conflict in plugins/planning/CHANGELOG.md resolved to main's side. The
standards contract, its two plugin copies, and the planning and review
manifests and changelogs take main's version, so the contract keeps its
em dashes and no contract or plugin bump is needed for it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ect changelog entries (#4606)

The standards contract carries a semver, and changing it needs a
contract version bump, a contract CHANGELOG entry, and a bump of both
carrying plugins. The bump makes every consumer index at the current
version mismatch until setup is re-run. Instead of paying that for a
punctuation pass, the contract README and its two plugin copies stay as
on main, are listed in em_dash_allowed_paths with the reason, and the
comments in scripts/em-dash-purged-paths.txt that explain the exclusion
are restored.

- planning and review carry no change, so neither is bumped.
- rate-limit-guard changed only its changelog wording, so the 0.9.1 bump
  and entry are dropped and the wording edit stays.
- The discovery 0.25.14 entry now names the real change, the filler
  "in order to".
- docs/out-of-scope/*.md gets its own comment in the purged-paths list
  instead of sitting under the convention documents comment.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…udit-findings

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…es, mark the native quote (#4606)

Follow the owner's step 3 for planning, review and docs/conventions: purge the
31 em dashes in the standards contract, bump it to 1.0.1 with a changelog entry,
regenerate both plugin copies, and bump planning and review. Declare the contract
and its copies in em-dash-purged-paths.txt and drop their em_dash_allowed_paths
entries.

Fix the seven em dashes main's attribution golden fixtures added without moving
any line, and have the native-overlap view generator mark an evidence line that
quotes a native description with an ai-slop-ignore comment, so
docs/native-surfaces.md needs no config exemption.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…udit-findings

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…are docs/adr as purged (#4606)

The native-overlap view generator now adds the ai-slop-ignore comment to a
`native description:` evidence line only, so an authored evidence line with an
em dash is still reported. docs/adr is now in em-dash-purged-paths.txt so the
purged decision records stay purged, and the list's header no longer names
docs/adr as unpurged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…udit-findings

Main carries planning 0.51.0; the standards-contract purge entry moves to 0.51.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd narrow its scope (#4606)

The em_dash_allowed_paths reason called docs/upstream snapshots of upstream
material this repository does not author. The repository's own record says the
reverse: the per-source ledgers are first-party prose, and the tree is not a
vendor tree. The glob also covered docs/upstream/claude-code-mods, which
scripts/em-dash-purged-paths.txt declares purged.

- Name the six ledgers that still carry em dashes instead of docs/upstream/**,
  and describe them as unpurged campaign backlog, each entry deleted when its
  file is purged.
- Narrow .claude/unhobble/** to .claude/unhobble/*/evidence/** so the live
  ledger and README stay at zero tolerance.
- Reword the docs/upstream paragraph in em-dash-purged-paths.txt so it agrees
  with the new entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… by a separate reason (#4606)

The _comment lead-in still said no authored file is exempt by volume and that
both groups are allowed for a reason other than volume. The docs/upstream
entries defer a purge of first-party prose, so only the unhobble evidence rests
on a reason other than purge cost.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…udit-findings

Renumber claude-ops to 0.77.1 above main's 0.77.0 and discovery to 0.25.18
above main's 0.25.17.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…udit-findings

# Conflicts:
#	plugins/claude-ops/CHANGELOG.md
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 19:47
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T19:54:55.691855Z e247449 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 1s —— View job


Security review

  • Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md conventions)
  • Check skip-gate conditions
  • Read changed files for security-relevant surface
  • Check instruction-exception register for any deleted/narrowed rules
  • Review overlap.py change for injection/logic risk
  • Post findings

No security issues found.

This PR is a documentation/config punctuation-purge campaign (em dashes) plus one small script change. Reviewed under /review:security-review criteria (logic flaws, authz/access gaps, injection surfaces, secret handling, and the instruction-surface-deletion lens):

  • 56 of 58 changed files are prose-only em-dash rewrites in ADRs, conventions docs, and CHANGELOGs/plugin.json version bumps. No semantic, behavioral, or control-flow content changed — confirmed by diffing a sample (e.g. docs/conventions/standards/README.md) against origin/main.
  • .claude/ai-slop.json adds 7 entries to em_dash_allowed_paths, narrowing a style-linter's zero-tolerance rule. I checked this against the instruction exception register's Gate 0 protected classes (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority). An em-dash/prose-style lint exemption doesn't match any protected class — it's not a security or safety guardrail, it's a punctuation convention, and the PR body documents each exemption's rationale explicitly under "Owner veto." No finding.
  • scripts/em-dash-purged-paths.txt changes actually tighten enforcement (adds docs/adr/*.md, docs/out-of-scope/*.md, and the standards contract to the CI-gated purged-path list), the opposite direction of a guardrail removal.
  • plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py adds a regex (fullmatch, anchored) that conditionally appends a static ai-slop-ignore HTML-comment marker to a generated markdown line. The line is already written verbatim to the doc pre-existing this diff; no new write path, no command execution, no new untrusted-input handling. Covered by a new unit test in test_overlap.py. No exploitable path introduced.
  • No auth, secrets, credential, injection (command/SQL/path/template), or GitHub Actions workflow changes are present in this diff.

Actions/CI hardening findings, if any existed, are out of scope for this lane (zizmor's advisory lane) — not applicable here since no workflow files changed.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 38s —— View job


/review:code-review findings for PR #5586

  • Gather context (PR body, diff, repo instructions)
  • Run /review:code-review per custom instructions
  • Review the logic changes (overlap.py/test_overlap.py, .claude/ai-slop.json, scripts/em-dash-purged-paths.txt, standards-contract sync) and spot-check the mechanical em-dash rewrites
  • Post summary review comment

No blocking findings. This PR is overwhelmingly a mechanical, well-scoped punctuation pass (em dashes → the punctuation each sentence needs) across ADRs, conventions, and a handful of plugin surfaces, plus two small, well-tested feature additions. Specifics I checked directly rather than taking on faith:

  • plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py:203-206,1058-1062 — the new VERBATIM_LINE_RE/VERBATIM_MARKER logic that appends an ai-slop-ignore comment to a verbatim native description: evidence line. The regex, its fullmatch anchoring, and the tag-prefix handling ([N] ) are correct, and test_overlap.py's new test_only_a_native_description_with_an_em_dash_is_marked_verbatim exercises the tagged/untagged/no-dash/non-description cases. The regenerated docs/native-surfaces.md view matches what the code would produce.
  • docs/conventions/standards/README.md vs plugins/planning/reference/standards-contract.md and plugins/review/reference/standards-contract.md — diffed all three directly; the two plugin copies are byte-identical to the source contract, confirming the sync claim.
  • The three edited golden fixtures in plugins/attribution/skills/audit/evals/fixtures/golden/ (c12, c14, c16) — checked each expected.json's line-numbered span against the edited case.md; every em-dash→punctuation substitution stayed on its original line, so no span shifted. c24's source.md only gained ai-slop-ignore-start/-end markers around the verbatim upstream quote; case.md is untouched, so the hard-negative eval is unaffected.
  • Spot-checked TOC anchor links in the largest ADR diffs (0002, 0004, 0005, 0011, 0016) whose headings changed from em dash to colon/semicolon — GitHub's slug algorithm drops both, and the updated anchors in each Contents list match the corresponding heading text.
  • scripts/em-dash-purged-paths.txt changes (declaring docs/adr/*.md, docs/out-of-scope/*.md, and the full docs/conventions/standards/*.md/plugins/{planning,review}/reference/*.md globs) read consistently with the new state described in "Owner veto" items 2-3.

One non-blocking note, not anchored to a line since it's prose, not code: the PR body's Verification section says "claude-ops is 0.77.1", but the committed plugins/claude-ops/.claude-plugin/plugin.json and its CHANGELOG.md entry are 0.77.2 (correctly, since origin/main already carries 0.77.1 from #5614). Looks like the body text wasn't updated after a rebase forced the extra bump — worth a one-line fix before merge so history doesn't have to be cross-referenced to find the real version. See plugin.json#L4 and CHANGELOG.md#L6.

I was not able to execute the repo's shell/Python verification scripts in this environment (bash scripts/check-purged-em-dashes.sh, the ai-slop detect.sh, sync-standards-contract.sh, etc. all required approval I couldn't grant here), so I did not re-run them; the PR body's own Verification section and cited CI run (36765413601, all green) are the author's claim, not something I independently reproduced. My review above is based on direct inspection of the diff and the referenced files rather than re-running those scripts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e24744927a

ℹ️ 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".

Comment thread plugins/claude-ops/skills/audit-native-overlap/scripts/overlap.py Outdated
kyle-sexton and others added 2 commits September 30, 2026 16:06
…udit-findings

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ption

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 3s —— View job


Security review

  • Gather context (PR diff, changed files, instruction-exception register)
  • Check skip gate (PR open; diff has instruction-surface-relevant surface, so not skipped)
  • Review all 58 changed files for security-relevant surface
  • Check the .claude/ai-slop.json narrowing against the instruction-exception register's Gate 0 protected classes
  • Review the new overlap.py commit (multiline verbatim-marker fix) for logic/injection risk
  • Post findings

No security issues found.

This head (9697ca0) adds one commit since the last review (e247449): fix(claude-ops): mark every em-dash line of a multiline native description. It's the fix for the Codex P2 finding on this PR (marker was only applied to the last physical line of a multiline verbatim evidence item). The rest of the diff (56 prose-only em-dash rewrites + version bumps) is unchanged from the prior review and re-confirmed against origin/main.

  • overlap.py#L1058-L1065 now splits a matched verbatim evidence item on \n and appends the static ai-slop-ignore HTML-comment marker (VERBATIM_MARKER, a fixed string, not derived from input) to each physical line containing an em dash, rather than only the item as a whole. The VERBATIM_LINE_RE fullmatch (with re.DOTALL) still gates which items qualify, so an authored (non-native description:) line is never marked. No new write path, no command execution, and the source data (row["evidence"]) is maintainer-curated JSON checked into the repo, not runtime-untrusted input, so this isn't a new injection surface. Covered by the new test_every_em_dash_line_of_a_multiline_native_description_is_marked in test_overlap.py.
  • .claude/ai-slop.json still adds 7 entries to em_dash_allowed_paths, narrowing the em-dash lint's zero-tolerance default. Checked directly against docs/conventions/instruction-exception-register/README.md's six Gate 0 classes (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority): a punctuation-style lint exemption matches none of them, and the PR body documents each exemption's rationale under "Owner veto." No finding.
  • scripts/em-dash-purged-paths.txt changes still tighten CI enforcement (adds docs/adr/*.md, docs/out-of-scope/*.md, and the standards contract to the gated purged-path list), the opposite of a guardrail removal.
  • No auth, secrets, credential, injection (command/SQL/path/template), or GitHub Actions workflow changes anywhere in this diff. Actions hardening (zizmor's lane) doesn't apply since no workflow files changed.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 46s —— View job


/review:code-review findings for PR #5586 (head 9697ca0)

  • Gather context (PR body, diff, repo conventions)
  • Invoke /review:code-review and follow its instructions for this PR
  • Post findings as inline PR review comments, summary here

1 non-blocking nit, no blocking findings.

This head (9697ca0) already includes the follow-up fix for the earlier Codex P2 comment on
overlap.py ("Mark each physical line of multiline descriptions"): commit 9697ca0 ("fix(claude-ops): mark every em-dash line of a multiline native description")
rewrote the render_block loop to split each evidence item on \n and append VERBATIM_MARKER
to every physical line that contains an em dash, not just the last one — see
overlap.py#L1060-L1066.
A new test, test_every_em_dash_line_of_a_multiline_native_description_is_marked
(test_overlap.py#L342-L351),
exercises exactly the multiline-description case the Codex comment raised — I traced this back
through inventory.py's _js_unescape (plugins/claude-ops/skills/inventory/scripts/inventory.py:597-609),
which does decode an escaped \n in extracted native source into a real newline character, so the
scenario Codex flagged is real and the fix is the right shape for it. Codex's finding is resolved
as of this head.

Things I directly checked and confirmed match the PR body's claims:

  • plugins/planning/reference/standards-contract.md and plugins/review/reference/standards-contract.md are byte-for-byte identical to docs/conventions/standards/README.md (diff against all three).
  • The four non-em-dash findings (docs/conventions/loop-lane/README.md, the two discovery:explore files, plugins/rate-limit-guard/CHANGELOG.md) are clean meaning-preserving rewrites.
  • The golden fixture edits (c12, c14, c16) keep every expected.json span's line numbers unchanged; spot-checked c16's span (lines 4-6) against its diff.
  • c24's source.md gets ai-slop-ignore-start/-end markers around the verbatim upstream quote rather than a rewrite, consistent with the byte-exact-quote rule in scripts/em-dash-purged-paths.txt's header.
  • scripts/em-dash-purged-paths.txt's new docs/adr/*.md, docs/out-of-scope/*.md, and docs/conventions/standards/*.md/plugins/{planning,review}/reference/*.md declarations are internally consistent with the new state described in "Owner veto" items 2-3.
  • attribution (0.8.1), discovery (0.25.18), and review (0.34.3) plugin versions match the PR body's Verification claims.

One non-blocking finding, posted inline on plugins/planning/.claude-plugin/plugin.json:4:
the committed version is 0.52.1, but the PR body's Verification section still says "Planning is
0.51.1." origin/main already carried 0.52.0 from #5470 by the time this branch's own bump
landed, so the body text is stale — the same rebase-drift pattern an earlier review already caught
for claude-ops (0.77.1 → 0.77.2, still unfixed in the body too). Neither affects any shipped
file; both are one-line fixes to the PR description.

I could not execute the repo's verification scripts in this environment (detect.sh,
check-purged-em-dashes.sh, sync-standards-contract.sh, etc. all required approval I couldn't
grant here), so the PR body's Verification section and its cited CI run remain the author's claim,
not something I independently reproduced.
· branch fix/4606-post-purge-audit-findings

Comment thread plugins/planning/.claude-plugin/plugin.json
@kyle-sexton
kyle-sexton merged commit 819c7e1 into main Sep 30, 2026
29 checks passed
@kyle-sexton
kyle-sexton deleted the fix/4606-post-purge-audit-findings branch September 30, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai-slop: vendored-docs rule line and post-purge config tightening (follow-up from #2891)

1 participant