Skip to content

feat(test): bash syntax validation for all SKILL.md code blocks (#1667)#1722

Open
NikhileshNanduri wants to merge 2 commits into
garrytan:mainfrom
NikhileshNanduri:feat/1667-bash-syntax-validation
Open

feat(test): bash syntax validation for all SKILL.md code blocks (#1667)#1722
NikhileshNanduri wants to merge 2 commits into
garrytan:mainfrom
NikhileshNanduri:feat/1667-bash-syntax-validation

Conversation

@NikhileshNanduri
Copy link
Copy Markdown

Summary

Closes #1667.

  • Adds test/skill-bash-syntax.test.ts: gate-tier free test that runs bash -n on every ```bash ``` block in all 53 generated SKILL.md files (1327+ blocks total), catching syntax errors before they reach users
  • Fixes the first bug the test caught: codex/SKILL.md had a missing closing ``` fence on the resumed-session bash block, causing prose with backtick-quoted identifiers to be parsed as shell — bash -n reported "unexpected EOF while looking for matching `"
  • Angle-bracket placeholders like <branch-name> are preprocessed to PLACEHOLDER before validation to avoid false positives on intentional template tokens

Why it matters

bun test previously never validated bash syntax in skill files. A typo like 2/dev/null instead of 2>/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
  • Full free suite passes

@jbetala7
Copy link
Copy Markdown
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:

  1. the codex/SKILL.md.tmpl fence fix,
  2. the bash syntax validation test or equivalent,
  3. no VERSION/CHANGELOG/plan-pm-review/autoplan changes.

The important coverage to preserve is a regression that catches both the missing Codex fence and the 2>/dev/null -> 2/dev/null corruption pattern from #1654.

@NikhileshNanduri NikhileshNanduri force-pushed the feat/1667-bash-syntax-validation branch from 29a8588 to 5ca2c6f Compare May 27, 2026 14:52
NikhileshNanduri and others added 2 commits May 28, 2026 01:00
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>
@NikhileshNanduri NikhileshNanduri force-pushed the feat/1667-bash-syntax-validation branch from 5ca2c6f to 1754e15 Compare May 27, 2026 19:32
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.

feat: validate bash syntax in SKILL.md code blocks during bun test (prevent silent runtime breakage)

2 participants