garrytan / garrytan/gstack

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

Open
#1,667 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
133k
Forks
19.9k
Avg merge
18h 46m
Merged PRs (30d)
26

Description

## Problem

`bun test` validates many properties of SKILL.md files — unresolved placeholders, token budgets, preamble freshness, hardcoded branch names, AskUserQuestion format — but never checks whether the **bash code blocks inside them are syntactically valid**.

This means an entire class of breakage can ship silently: corrupted shell redirects, bad quoting, broken pipes, and malformed heredocs all pass `bun test` and only blow up at runtime when a user invokes the skill.

### Motivating case: PR #1654

[PR #1654](https://github.com/garrytan/gstack/pull/1654) systematically changed `2>/dev/null` → `2/dev/null` (removed the `>` redirection operator) in **12+ places** throughout `autoplan/SKILL.md`. Every one of those lines would fail at runtime. `bun test` passed. The only guard was human review.

This is not a one-off — any automated tool, refactor, or merge conflict resolution that corrupts bash syntax inside a code block will go undetected until a user hits it.

## Proposed solution

Add a `bash -n` (syntax-check-only, no execution) pass over every ` ```bash ``` ` block in every generated SKILL.md file as part of `bun test`.

### Implementation sketch

```typescript
// test/skill-bash-syntax.test.ts
// Extract all ```bash ... ``` blocks from every SKILL.md,
// preprocess template placeholders, then run `bash -n` on each.

function extractBashBlocks(content: string): { block: string; lineNum: number }[]

function preprocessForSyntaxCheck(block: string): string {
// Replace with valid bash variable references
// Replace {{RESOLVER}} template tokens (these are .tmpl artifacts, not bash)
// Substitute common gstack-specific tokens
}

// For each skill's SKILL.md:
// for each bash block:
// const result = spawnSync('bash', ['-n'], { input: preprocessed })
// expect(result.status).toBe(0) // fail with file + line on error
```

### What it catches
- Missing `>` in `2>/dev/null` (the PR #1654 bug)
- Unclosed quotes or heredocs
- Invalid redirection syntax
- Broken pipe chains (`cmd |` with nothing after)
- Mismatched `if/fi`, `for/done`, `case/esac`

### What it explicitly does NOT check
- Correctness of commands (not `bash -e`, just `bash -n`)
- Runtime behaviour
- Blocks in `.tmpl` files (only generated `.md` files, since those are what ships)
- Non-bash blocks (` ```python `, ` ```json `, ` ```markdown `)

## Scope

- New test file: `test/skill-bash-syntax.test.ts`
- Tier: `gate` (free, fast — `bash -n` on 50+ skills takes < 2s total)
- No new dependencies — `bash` is already assumed present

## Expected outcome

Running `bun test` after PR #1654's changes would produce:

```
(fail) bash syntax: autoplan/SKILL.md block at line 43 [1.2ms]
bash: line 1: 2/dev/null: No such file or directory
+ 11 more failures
```

instead of silently passing.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.