Skip to content

fix(scripts): backfill the em-dash allowlist with already-clean surfaces - #3670

Merged
kyle-sexton merged 2 commits into
mainfrom
cursor/fix-3342-em-dash-allowlist-backfill-df93
Sep 2, 2026
Merged

fix(scripts): backfill the em-dash allowlist with already-clean surfaces#3670
kyle-sexton merged 2 commits into
mainfrom
cursor/fix-3342-em-dash-allowlist-backfill-df93

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 2, 2026 •
edited by cursor Bot
Loading

Copy link
Copy Markdown
Contributor

No linked issue

Summary

Grow scripts/em-dash-purged-paths.txt with plugin README and SKILL surfaces that already pass the one-rule em-dash gate from #3669. Does not close #3342: frontmatter description: lines and plugin.json descriptions remain out of this slice.

Fix

  • Expand the allowlist from 21 to 86 declared paths (105 scanned files after the review correction).
  • Add full README+SKILL coverage for plugins whose every skills/*/SKILL.md is byte-clean of U+2014.
  • Add README-only entries for every other plugin except the five READMEs that still contain a dash (ai-slop, disk-hygiene, evals, guardrails, mcp-tools).
  • Drop the five SKILL globs that still contain a visible em dash the detector's per-line inline-code strip misses across a wrapped span (bugs, claude-memory, context7, discovery, testing). Their READMEs stay.

Verification

  • scripts/check-purged-em-dashes.sh --check → 86 declared paths, 105 files scanned, no em dashes.
  • scripts/affected-tests.sh --run → scripts/check-purged-em-dashes.test.sh 23/23.

Related

Refs #3342
Refs #3669

Open in Web Open in Cursor 

@cursoragent ksextonmelodic
fix(scripts): backfill the em-dash allowlist with already-clean surfaces
a2493d8 
The one-rule gate makes a 123-file check cheap. Add every plugin
README that already passes, plus SKILL.md globs for the eleven
plugins whose skills are clean too. Dirty description: fields stay
off the list for the later frontmatter pass.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 2, 2026 15:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026 •
edited
Loading

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-02T15:36:51.276579Z a2493d8 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 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Claude finished the code review

  • Gather context (PR diff, CLAUDE.md/AGENTS.md, review skill instructions)
  • Verify the skip gate does not apply (PR is open, not draft, and the allowlist entries are the actual review surface)
  • Review scripts/em-dash-purged-paths.txt changes for correctness
  • Independently verify the added paths actually pass the em-dash check
  • Post findings

Summary

Reviewed the 90 new allowlist entries added to scripts/em-dash-purged-paths.txt (72 lines, +91 total declared paths / 123 files per the PR description). I couldn't execute scripts/check-purged-em-dashes.sh directly in this lane (only gh pr * subcommands are available to me), so I verified by reading check-purged-em-dashes.sh / detect.sh and independently grepping the newly-listed files for the em-dash character (—), then manually checking every match against fence/ignore-marker/inline-code-span context.

Most of the ~28 README files and 7 SKILL.md files that raw-grep for — are genuinely clean: the character only appears inside ``-bounded generated-options blocks (the sync-plugin-options-docs.py table) or inside fenced/inline code. Spot-checked several of these (`plugins/ai-briefing`, `plugins/autonomy`, `plugins/actionlint`, `plugins/testing/skills/plan`, `plugins/discovery/skills/research-deep`, `plugins/context7/skills/lookup`, `plugins/claude-memory/skills/stateless`, `plugins/bugs/skills/write`, `plugins/bugs/skills/scan`) — all correctly exempt. All plugin directories named in the new entries exist with the expected `README.md`/`skills//SKILL.md`, so no stale-glob risk. The 11 plugins getting the `skills//SKILL.md` glob match the PR's stated count.

One real finding, posted inline on scripts/em-dash-purged-paths.txt:62: the new plugins/bugs/skills/*/SKILL.md entry pulls in plugins/bugs/skills/setup/SKILL.md, which still renders an em dash in prose at lines 56-57. It only reads as "clean" to check-purged-em-dashes.sh because detect.sh's inline-code stripper works line-by-line and accidentally pairs an unrelated trailing backtick with the next code span's opening backtick, swallowing the em dash along with it — not a real code span, just a per-line parsing artifact. Details and a suggested fix are in the inline comment.

Everything else in the diff (comment updates, ordering, no duplicate/typo'd plugin names) looks correct. I did not independently re-run scripts/check-purged-em-dashes.sh --check or affected-tests.sh myself (blocked by this lane's tool allowlist), so the PR's own "91 declared paths, 123 files scanned" and "23/23" test results are author-claimed and unverified by me.

@claude

claude Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Security review: no security-relevant surface

  • Gather context (PR diff, changed files)
  • Apply skip gate / assess security-relevant surface
  • Review changed file(s) for security issues
  • Post final findings

Skip gate triggered: this PR only adds plain data entries to scripts/em-dash-purged-paths.txt — a text allowlist of relative path/glob patterns consumed by scripts/check-purged-em-dashes.sh (that script itself is unchanged). Every added line is a simple plugins/<name>/README.md or plugins/<name>/skills/*/SKILL.md glob: no path traversal, no absolute paths, no shell metacharacters, no code, and no CI workflow or permissions changes.

No security review findings. Nothing to report.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Last security-reviewed head: a2493d8af15794786406a8859afc39ce483c18a8. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@claude

claude Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

/review:security-review — PR #3670

  • Gather context (diff vs origin/main, PR body, skill instructions)
  • Apply skip gate
  • Post result

Skip gate hit: no security-relevant surface.

The only change is scripts/em-dash-purged-paths.txt, a data file listing already-clean paths for the existing scripts/check-purged-em-dashes.sh gate (diff). It adds 72 lines of plugin README/SKILL.md glob paths to the allowlist; no script logic, workflow permissions, or executable code is touched.

No injection surface, no auth/access-control change, no secrets handling, no Actions trigger/permission change, no supply-chain pin change. Nothing to report.

claude[bot]
claude Bot reviewed Sep 2, 2026
View reviewed changes
Comment thread scripts/em-dash-purged-paths.txt Outdated
@github-actions

github-actions Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@cursoragent ksextonmelodic
fix(scripts): drop SKILL globs that still contain a visible em dash
265c64d 
The detector strips inline code per line, so a wrapped span plus a
later `already installed` pair hid the dash in bugs:setup. Drop the
five SKILL globs that still contain U+2014 anywhere; keep their
READMEs. The campaign goal is no visible em dash, not detector-miss.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@claude

claude Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 4m 24s —— View job

Claude finished the code review

  • Gather context (PR diff vs 1fe9381, prior review comments, CLAUDE.md/AGENTS.md, /review:code-review skill instructions)
  • Apply skip gate (PR is open, not draft, real review surface, no prior code-review-lane pass on this head)
  • Confirm the previously-flagged issue (plugins/bugs/skills/*/SKILL.md catching a genuinely em-dashed SKILL.md) was addressed
  • Independently spot-check the new allowlist entries
  • Post findings

Summary

This PR only touches scripts/em-dash-purged-paths.txt, growing it from 21 to 86 declared entries (60 new README.md lines, 6 new skills/*/SKILL.md globs: github, improvement, instruction-placement, performance, playgrounds, visualization). Commit 265c64d dropped the 5 skills/*/SKILL.md globs (bugs, claude-memory, context7, discovery, testing) that an earlier review round on this PR found were pulling in a not-actually-clean SKILL.md (the plugins/bugs/skills/setup/SKILL.md wrapped-code-span case), so that finding is resolved.

I couldn't execute scripts/check-purged-em-dashes.sh in this session (not on this lane's tool allowlist), so I verified independently instead of trusting the PR's own "91 declared paths, 123 files scanned" / "23/23" claims (which are author-claimed and unverified by me, and are now stale numbers regardless — see below):

  • All 6 new skills/*/SKILL.md globs (github, improvement, instruction-placement, performance, playgrounds, visualization) resolve to SKILL.md files with zero raw em-dash occurrences — no code-fence/inline-span ambiguity possible, strictly clean.
  • Every new plugins/<name>/README.md directory exists (no stale/dead globs).
  • The 5 READMEs the PR says still fail (ai-slop, disk-hygiene, evals, guardrails, mcp-tools) are correctly absent from the list.
  • Spot-checked READMEs with raw em-dash hits (plugins/bugs/README.md, plugins/claude-memory/README.md) and confirmed every occurrence sits inside a fenced code block or an …-wrapped generated-options block, both legitimate exemptions per detect.sh's prose rules. plugins/context7, plugins/discovery, plugins/testing READMEs have no em dash at all.

Minor, non-blocking: the PR description's "91 entries / 123 files" and "SKILL.md globs for the eleven plugins" are the pre-fix-commit counts from a2493d8; after 265c64d dropped 5 globs, the actual current state is 86 entries and 6 new globs (verified above). Worth a quick edit to the PR body for accuracy, but it doesn't affect the shipped file.

No code-review findings to block on. The file's structure, comments, and ordering are consistent with the existing convention, and the campaign's own stricter-than-the-gate bar (README entries checked for zero raw em dashes, not just prose-em-dashes) holds for the samples I checked.
· branch cursor/fix-3342-em-dash-allowlist-backfill-df93

@kyle-sexton
kyle-sexton merged commit a3d3733 into main Sep 2, 2026
70 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/fix-3342-em-dash-allowlist-backfill-df93 branch September 2, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claude claude[bot] claude[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back-fill the em-dash purged-paths allowlist to the surfaces the de-slop campaign has already cleaned

2 participants

@kyle-sexton @cursoragent