feat(test): bash syntax validation for all SKILL.md code blocks (#1667)#1722
Open
NikhileshNanduri wants to merge 2 commits into
Open
feat(test): bash syntax validation for all SKILL.md code blocks (#1667)#1722NikhileshNanduri wants to merge 2 commits into
NikhileshNanduri wants to merge 2 commits into
Conversation
Contributor
|
Thanks for tackling #1667. I think this PR currently collides with #1697 and is hard to land as-is. Live map evidence:
Recommended path: make #1697 the canonical PR for the bash-snippet validation gate, or rebase/narrow this PR down to only the #1667 pieces:
The important coverage to preserve is a regression that catches both the missing Codex fence and the |
29a8588 to
5ca2c6f
Compare
The bash block opened at the "For a resumed session" section was missing its closing ``` fence. Prose text with backtick-quoted identifiers (e.g. `SESSION_ID:<id>`) was being parsed as shell code, causing bash -n to report "unexpected EOF while looking for matching `". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds test/skill-bash-syntax.test.ts — a gate-tier free test that extracts all bash code blocks from all 53 generated SKILL.md files and runs bash -n on each. Covers 1327+ blocks in ~4s with no API calls. Fixes garrytan#1667. Angle-bracket placeholders like <branch-name> are preprocessed to PLACEHOLDER before validation so template markers don't produce false positives. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5ca2c6f to
1754e15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1667.
test/skill-bash-syntax.test.ts: gate-tier free test that runsbash -non every```bash ```block in all 53 generated SKILL.md files (1327+ blocks total), catching syntax errors before they reach userscodex/SKILL.mdhad a missing closing```fence on the resumed-session bash block, causing prose with backtick-quoted identifiers to be parsed as shell —bash -nreported "unexpected EOF while looking for matching `"<branch-name>are preprocessed toPLACEHOLDERbefore validation to avoid false positives on intentional template tokensWhy it matters
bun testpreviously never validated bash syntax in skill files. A typo like2/dev/nullinstead of2>/dev/null(the exact corruption in PR #1654) would pass all CI checks and silently break at runtime. Now it's a gate failure in ~4 seconds with zero API cost.Test plan
bun test test/skill-bash-syntax.test.ts— 54 tests pass (1 discovery + 53 skills)bun test test/skill-validation.test.ts test/gen-skill-docs.test.ts test/skill-bash-syntax.test.ts— 772 tests pass