Skip to content

fix: scan JSON and front-matter prose values for banned punctuation - #1334

Merged
vivek7405 merged 1 commit into
mainfrom
fix/prose-hook-json-values
Aug 8, 2026
Merged

fix: scan JSON and front-matter prose values for banned punctuation#1334
vivek7405 merged 1 commit into
mainfrom
fix/prose-hook-json-values

Conversation

@vivek7405

@vivek7405 vivek7405 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Closes #1269

The prose-punctuation hook gated its pause-hyphen and pause-semicolon rules on four line shapes (a comment line, a markdown heading, a blockquote, an HTML prose tag), so a JSON string value matched none of them and invariant 11 shipped straight through. Rules 2 and 3 now also scan a description, title, or displayName value, in JSON and in column-0 YAML front matter. The scope is the KEY, not the file and not the value, which is what keeps the rule off semver ranges, script commands, urls, paths and globs, since every one of those lives under a different key.

What changed

  • Rules 2 and 3 gain two contexts each, reusing each rule's existing character-class core byte for byte. Reusing it is load-bearing: the letter bound is what makes "1.2.3 - 2.3.4" unmatchable.
  • Rules 1 through 4 are SIGPIPE-safe. Every one was written as printf ... | grep -q, and grep -q exits on its first match, which closes the pipe under printf. Under set -o pipefail that SIGPIPE became the pipeline status, so the if was false and the rule silently skipped on any payload past the pipe buffer. Measured on this branch: 0 of 8 blocks at 200 KB before, 8 of 8 after. Every match now reads from a here-string. Rule 5 was already safe (it reads all of its input through a command substitution) and is untouched.
  • Both live violations are fixed: the repo root manifest description and packages/ui/packages/registry/package.json. The repo-wide punctuation cleanup that introduced one of them swapped an em-dash for a space-hyphen inside a JSON value, and nothing caught it, because the hook did not read JSON.
  • The three JSDoc @param {type} name - description separator lines the SIGPIPE fix now surfaces are rewritten. Those were the only three in the tree, so the repo convention was already not to use the separator.
  • All three copies of the hook are in step (repo, packages/cli/templates/, examples/blog/), and the two scaffold copies were derived from the repo copy by a script validated to reproduce the committed scaffold byte for byte from the committed repo copy.
  • Seven stale item 10 citations now read item 11.

Test plan

test/hooks/block-prose-punctuation.test.mjs, 32 tests, all passing. New coverage:

  • Blocked: a pause-hyphen under each of the three keys, a pause-semicolon, a deeply indented value, the pre-fix repo root string, column-0 front matter for description: and title:, and all four ORIGINAL contexts (the file had zero rule 2 / rule 3 coverage before).
  • Allowed, one candidate each: an npm version range, an engines range, a scripts command, a command path, a name, a config-block leaf, a main path, raw SQL under default, a compound word inside a scanned value, an ordinary English semicolon with no surrounding spaces, and an INDENTED YAML description: (which is what proves the column-0 anchor is real).
  • A 200 KB payload blocks on 5 consecutive runs for a markdown heading, a JSON value, and an em-dash. The loop is deliberate: the bug it covers is a race, so a single run could pass against the broken hook.
  • Repo drift guard: the four prose-key patterns are read out of the live hook and run over every tracked *.json and *.md. It reds against the unfixed tree at package.json:5 and packages/ui/packages/registry/package.json:6.
  • Copy drift guard: the two scaffold copies are byte-identical, carry all four new patterns, and no copy still pipes into grep -q.

Counterfactuals, each isolating one failure mode:

  1. Remove only the four new prose-key blocks. 6 tests red, every allowed case and every original-context case stays green.
  2. Put the printf ... | pipes back. The 200 KB test and both drift guards red, every small-payload test stays green. This is the one that proves the SIGPIPE fix is load-bearing rather than cosmetic.
  3. Revert only the scaffold mirror. The copy drift guard reds alone.

The full Node suite is green apart from 5 assertions in packages/server/test/elision/differential-elision.test.js and test/bun/listener*, which are pre-existing in a linked worktree rather than a regression here: they pass in a checkout with a real install, and they still fail with this branch's diff reverted file by file to origin/main content. CI builds from the branch, so it is unaffected.

Doc surfaces

  • Updated: AGENTS.md invariant 11, whose "semicolons and colons stay fine inside code / TS / JSON / CSS" clause read as a blanket JSON exemption and is now wrong for the three prose keys. It now says JSON SYNTAX is code while a prose-key VALUE is prose.
  • Updated: the hook's own header comment, which documents its contexts.
  • N/A, docs site / marketing / README / .agents/skills/webjs/ / MCP / editor plugins: this is repo tooling, not framework behaviour, so there is no user-facing surface to sync. The scaffold and dogfood rule copies state invariant 11 generically with no JSON exemption, so they are already correct.
  • N/A, Bun parity: a shell hook has no runtime-sensitive surface. No browser or e2e layer applies either.
  • N/A, version bump: no published package surface changed. The two packages/ edits are a JSDoc separator and a manifest description.

@vivek7405 vivek7405 self-assigned this Aug 8, 2026

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Read the whole diff. Nothing to fix.

The key-scoped design is the right call. Scoping to description / title / displayName rather than to a file path is what keeps this off semver ranges, script commands, urls and globs, and it means the Bash payload is covered too, which a file_path gate could never do since a heredoc writing a manifest carries no path. Reusing each rule's existing character class byte for byte is the load-bearing part: the letter bound is the only reason "1.2.3 - 2.3.4" cannot match, and a loosened class would quietly admit every version range in the tree.

The SIGPIPE find is the more valuable half of this. Rules 1 through 4 were silently no-ops on any payload past the pipe buffer, so the gate was strongest on exactly the small edits that need it least. Worth remembering that grep -q behind a pipe under pipefail is a trap, not a style choice, and the comment at the top of the block says so.

The one thing I would keep an eye on is the column-0 anchor on the front-matter pattern. It is what separates document front matter from an indented workflow input, and it is a single character with no second line of defence, so if a surface ever emits front matter with leading whitespace the rule goes quiet rather than loud. That is the correct direction to fail for a hard-blocking gate, and the indented-YAML test pins the behaviour, so it stays as is.

@vivek7405
vivek7405 marked this pull request as ready for review August 8, 2026 12:57
The prose-punctuation hook gated its pause-hyphen and pause-semicolon rules
on four line shapes (a comment, a markdown heading, a blockquote, an HTML
prose tag), so a JSON string value matched none of them and invariant 11
shipped straight through. The repo root manifest description and the ui
registry description both carried a pause-hyphen because of it, and the
repo-wide punctuation cleanup that introduced one of them was not caught.

Rules 2 and 3 now also scan a description / title / displayName value, in
JSON and in column-0 YAML front matter. The scope is the KEY, not the file,
which is what keeps the rule off semver ranges, script commands, urls, paths
and globs, since every one of those lives under a different key.

Rules 1 through 4 also silently stopped enforcing on a payload past the pipe
buffer: grep -q exits on its first match, that closes the pipe under printf,
and under pipefail the SIGPIPE became the pipeline status, so the if was
false and the rule skipped. Measured 0 of 8 blocks at 200 KB before, 8 of 8
after. Every match now reads from a here-string.
@vivek7405
vivek7405 force-pushed the fix/prose-hook-json-values branch from c874ce4 to 1f309c0 Compare August 8, 2026 13:21
@vivek7405
vivek7405 merged commit c0917df into main Aug 8, 2026
10 checks passed
@vivek7405
vivek7405 deleted the fix/prose-hook-json-values branch August 8, 2026 13:22
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.

bug: prose punctuation hook misses JSON string values, so invariant 11 ships in package.json descriptions

1 participant