Skip to content

lint: stop --fix from destroying inline code and allowed-tools lists - #128

Merged
NagyVikt merged 1 commit into
mainfrom
fix/skill-linter-fixer-data-loss
Aug 7, 2026
Merged

lint: stop --fix from destroying inline code and allowed-tools lists#128
NagyVikt merged 1 commit into
mainfrom
fix/skill-linter-fixer-data-loss

Conversation

@NagyVikt

@NagyVikt NagyVikt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

cue lint-skill --fix corrupts the files it edits. Measured over the 453-skill library, one run destroyed 723 inline code spans across 178 files and folded block-sequence allowed-tools into a single garbage string.

R009 — the em-dash fixer eats adjacent code spans

stripCodeAndFrontmatter is length-preserving: it blanks inline code to spaces so offsets stay aligned with the original. The fixer used that mask to find em dashes (correct) and to walk backwards over surrounding whitespace (not correct). A blanked `code span` reads as whitespace, so the walk crossed it and the slice dropped it:

-- `get_latest_news` — Fetch the most recent news articles
+-, Fetch the most recent news articles

The tool name is simply gone. Worst single file lost 44 spans.

Fix: the search still uses the mask; the whitespace walk now runs on the original content.

R005 — allowed-tools block lists collapse into one string

Two defects compounding:

  1. It read the field with fmField, whose \s* crosses a newline. For a block sequence that returned the first bullet line, so - was parsed as a tool name → Bash(-:*).
  2. Its fix rewrote only the key line, orphaning the remaining - item lines beneath a scalar. YAML folds those into the scalar.

research/trendradar went from a 17-entry list to one ~600-character string:

allowed-tools: Bash(-:*), Bash(mcp__trendradar__get_latest_news:*)
  - mcp__trendradar__get_latest_news
  - mcp__trendradar__get_trending_topics

That is still valid YAML — which is why a syntax-level integrity check waved it through. Only a semantic comparison (list of 17 → str) catches it.

Fix: R005 now reads both YAML shapes, replaces the whole construct, and leaves mcp__* ids and top-level tools bare rather than wrapping them. Bare Bash is a tool name, not a CLI name — wrapping it produced the nonsense Bash(Bash:*).

Why this shipped

The existing suite tested em dashes inside fenced code blocks but never inline spans. This PR adds 12 regression tests built from the shapes that actually broke, including the real-world lines above.

Verification

Check Result
bun test src/lib/skill-linter.test.ts 91 pass / 0 fail
bun test (full) 3088 pass / 1 skip / 0 fail
--fix over 453 skills, inline spans lost 0 (was 723)
allowed-tools list → scalar 0
… frontmatter unparseable / name: / description: / fences altered 0 / 0 / 0 / 0
lint result unchanged in substance 45 errors → 0, warnings 745 → 406

Also confirmed no new Bash(Bash:*) is introduced: 47 pre-existing occurrences, unchanged.

The corpus-wide application of the repaired fixer is opencue/skills#21.

🤖 Generated with Claude Code

…ists

`cue lint-skill --fix` corrupted content. Measured over the 453-skill
library, one run destroyed 723 inline code spans across 178 files and folded
block-sequence allowed-tools into a single garbage string.

R009 (em dashes). stripCodeAndFrontmatter is length-preserving: it blanks
inline code to spaces so offsets stay aligned. The fixer used that mask to
FIND em dashes, correctly, and then also to walk backwards over surrounding
whitespace. A blanked `` `code span` `` reads as whitespace, so the walk
crossed it and the slice dropped it:

    - `get_latest_news` — Fetch the most recent news articles
    -, Fetch the most recent news articles

The tool name is simply gone. The search still uses the mask; the whitespace
walk now runs on the original content.

R005 (allowed-tools). Two defects. It read the field with fmField, whose
`\s*` crosses a newline, so a block sequence returned the first bullet line
and `-` was parsed as a tool name, yielding `Bash(-:*)`. Its fix then
rewrote only the key line, orphaning the remaining `  - item` lines under a
scalar, which YAML folds into that scalar. research/trendradar went from a
17-entry list to one 600-character string. It stayed VALID YAML, which is why
a syntax-level check waved it through -- only a semantic comparison catches
it.

R005 now reads both YAML shapes, replaces the whole construct, and leaves
`mcp__*` ids and top-level tools bare instead of wrapping them. Bare `Bash`
is a tool name, not a CLI name; wrapping produced `Bash(Bash:*)`.

12 regression tests, built from the shapes that actually broke. The existing
suite covered em dashes in fenced blocks but never inline spans, which is how
this shipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@NagyVikt
NagyVikt merged commit 61886e9 into main Aug 7, 2026
6 checks passed
NagyVikt added a commit that referenced this pull request Aug 7, 2026
…#130)

Two defects the review of #128 turned up in that PR, after it had merged.

Space-separated bare names regressed. The pre-#128 code split on /[,\s]+/;
readAllowedTools split on commas only, so `allowed-tools: nmap curl` produced
the single bogus `Bash(nmap curl:*)` instead of two entries. splitToolNames
restores the old behaviour but leaves a part containing `(` whole, so
`Bash(git diff:*)` stays one tool rather than being torn in half -- something
the original only avoided by early-returning on any wrapped value, which also
meant it fixed nothing in a mixed list.

fixedNames was computed and never read: the fix closure recomputes the same
map from a fresh parse. `bun run lint` flags it, and #128 passed CI only
because unused variables are a warning. Removed, and the wrap predicate that
had been written out three times is now isWellFormedTool + wrapTool.

4 more regression tests: space-separated names, a wrapper containing a space,
and flow-sequence brackets.

Neither defect reached opencue/skills#21: no skill carries a bare multi-word
allowed-tools value, and on its own base the repaired fixer reproduces that
PR byte-for-byte.

Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant