From 6916b71d08a14c5b4b99c8023ea289c77c5658e6 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 13 Aug 2026 20:07:32 -0600 Subject: [PATCH 1/5] fix(lint): lint-skill.sh recognizes `read` as a variable binding, not just VAR= Check 1 (cross-fence variable usage) only tracked VAR=value assignments, so a var re-bound via `read`/`read -r VAR1 VAR2` in a later block looked identical to a stale reference leaking from an earlier block. This false- flagged fixer/SKILL.md's own I4 integrity check, where a loop-local $COUNT (bound via `read`) collides in name only with the unrelated batch-size $COUNT set in Phase 0. Closes #2344 --- .../skills/create-skill/scripts/lint-skill.sh | 32 ++++- .../unit/lint-skill-read-binding-2344.test.ts | 131 ++++++++++++++++++ 2 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 tests/unit/lint-skill-read-binding-2344.test.ts diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 8bc6847db..9741a0bef 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -52,6 +52,15 @@ awk ' # Collect variable assignments per block and build reassignment lookup (O(1) per check) declare -A VAR_BLOCK declare -A REASSIGNED +register_var() { + local var="$1" bnum="$2" + if [ -z "${VAR_BLOCK[$var]+x}" ]; then + VAR_BLOCK["$var"]="$bnum" + else + # Track re-assignments in later blocks for O(1) lookup + REASSIGNED["${var}:${bnum}"]=1 + fi +} while IFS=$'\t' read -r bnum line; do # Skip comment lines — they document context but don't register variable assignments [[ "$line" =~ ^[[:space:]]*# ]] && continue @@ -59,13 +68,24 @@ while IFS=$'\t' read -r bnum line; do # Use while-read instead of for-in-$() to avoid empty-string iteration when grep matches nothing while IFS= read -r var; do [ -z "$var" ] && continue - if [ -z "${VAR_BLOCK[$var]+x}" ]; then - VAR_BLOCK["$var"]="$bnum" - else - # Track re-assignments in later blocks for O(1) lookup - REASSIGNED["${var}:${bnum}"]=1 - fi + register_var "$var" "$bnum" done < <(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+\+?=' | sed -E 's/\+?=$//') + # `read`/`read -r VAR1 VAR2` binds variables just like VAR=, but with no '=' + # after the name — e.g. `while IFS=$'\t' read -r F COUNT LINE; do`. Without + # this, a var re-derived via `read` in a later block (a legitimate, fresh, + # block-local binding) looks identical to a stale reference to an + # earlier block's same-named variable, producing a false Pattern-1 error + # (issue #2344 — fixer/SKILL.md's own I4 integrity check does exactly this + # with a loop-local $COUNT that collides in name only with the unrelated + # batch-size $COUNT set up in Phase 0). + if [[ "$line" =~ (^|[^A-Za-z0-9_])read([[:space:]].*)?$ ]]; then + read_args="${BASH_REMATCH[2]}" + read_args="${read_args%%;*}" + while IFS= read -r var; do + [ -z "$var" ] && continue + register_var "$var" "$bnum" + done < <(echo "$read_args" | grep -oE '\b[A-Z][A-Z0-9_]+\b') + fi done < "$BLOCKS_FILE" # Check for references in later blocks without file persistence diff --git a/tests/unit/lint-skill-read-binding-2344.test.ts b/tests/unit/lint-skill-read-binding-2344.test.ts new file mode 100644 index 000000000..e850f097a --- /dev/null +++ b/tests/unit/lint-skill-read-binding-2344.test.ts @@ -0,0 +1,131 @@ +/** + * Regression test for issue #2344: `lint-skill.sh`'s cross-fence variable + * check (Pattern 1) only recognised `VAR=value` as a variable assignment. + * `read`/`read -r VAR1 VAR2 ...` binds variables just as validly, but with + * no `=` after the name — e.g. `while IFS=$'\t' read -r F COUNT LINE; do`. + * + * `fixer/SKILL.md`'s own I4 integrity check does exactly this: a loop-local + * `$COUNT` (the per-line expected occurrence count, bound via `read`) that + * collides in name only with the unrelated batch-size `$COUNT` set up in + * Phase 0. Since each ```bash fence is a separate bash invocation, there is + * no real collision at runtime — but the linter didn't know the later block + * had its own fresh `$COUNT` via `read`, so it flagged the block's own local + * variable as if it were a stale reference leaking from Phase 0. + */ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(__dirname, '..', '..'); +const LINT_SCRIPT = path.join( + REPO_ROOT, + '.claude', + 'skills', + 'create-skill', + 'scripts', + 'lint-skill.sh', +); + +// lint-skill.sh requires bash 4+ (associative arrays). macOS ships bash 3.2 +// as the `bash` on PATH, including on GitHub Actions' macos-latest runner — +// resolve a real bash 4+ explicitly rather than assuming plain "bash" works. +function resolveBash4(): string | null { + const candidates = ['/opt/homebrew/bin/bash', '/usr/local/bin/bash', 'bash']; + for (const candidate of candidates) { + try { + const version = execFileSync(candidate, ['--version'], { encoding: 'utf8' }); + const match = version.match(/version (\d+)\./); + if (match && Number(match[1]) >= 4) return candidate; + } catch { + // candidate not on PATH — try the next one + } + } + return null; +} + +const BASH4 = resolveBash4(); + +const FRONTMATTER = `--- +name: lint-test-skill +description: test +argument-hint: none +allowed-tools: Bash +--- + +## Phase 0 + +pre-flight +`; + +// Unlike the mktemp test's single-block helper, this bug is inherently +// cross-block: it only reproduces across two or more separate \`\`\`bash +// fences (each its own bash invocation), so the fixture builder takes a +// list of blocks rather than one. +function runLint(bashBlocks: string[]): { stdout: string; ranSuccessfully: boolean } { + const blocksMarkdown = bashBlocks.map((b) => `\`\`\`bash\n${b}\n\`\`\`\n`).join('\n'); + const content = `${FRONTMATTER}\n${blocksMarkdown}\n## Rules\n\nrules here\n\n## Examples\n\nexample here\n\n**Exit condition:** done\n`; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'lint-skill-read-')); + const skillPath = path.join(dir, 'SKILL.md'); + fs.writeFileSync(skillPath, content); + try { + const result = execFileSync(BASH4!, [LINT_SCRIPT, skillPath], { encoding: 'utf8' }); + return { stdout: result, ranSuccessfully: true }; + } catch (err) { + const e = err as { stdout?: string }; + const stdout = e.stdout ?? ''; + return { stdout, ranSuccessfully: /lint-skill: \d+ error/.test(stdout) }; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +describe.skipIf(BASH4 === null)( + 'lint-skill.sh recognises `read` as a variable binding, not just VAR= (#2344)', + () => { + it('does not flag a var re-bound via `read` and referenced in the same later block', () => { + const { stdout, ranSuccessfully } = runLint([ + 'COUNT=5\nprintf \'%s\\n\' "$COUNT" > .codegraph/count', + [ + "while IFS=$'\\t' read -r F COUNT LINE; do", + ' if [ "$LINE" != "" ] && [ 0 -lt "$COUNT" ]; then', + ' echo "seen $COUNT for $F"', + ' fi', + 'done < .codegraph/data.tsv', + ].join('\n'), + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).not.toContain('Cross-fence variable: $COUNT'); + }); + + it('still flags a genuine cross-fence leak (no read-rebinding in the referencing block)', () => { + const { stdout, ranSuccessfully } = runLint(['FOO=hello', 'echo "leak: $FOO"']); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); + + it('still flags a reference in a block whose own `read` binds a different variable', () => { + const { stdout, ranSuccessfully } = runLint([ + 'FOO=hello', + 'read -r OTHER < .codegraph/other\necho "leak: $FOO"', + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); + }, +); + +describe.skipIf(BASH4 === null)('.claude/skills/fixer/SKILL.md itself (#2344 repro)', () => { + it('passes lint-skill.sh with no Cross-fence $COUNT error', () => { + const realSkillPath = path.join(REPO_ROOT, '.claude', 'skills', 'fixer', 'SKILL.md'); + const stdout = execFileSync(BASH4!, [LINT_SCRIPT, realSkillPath], { encoding: 'utf8' }); + expect(stdout).not.toContain('Cross-fence variable: $COUNT'); + }); +}); From 227c8db5514c7473733b144491868c209bf01130 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 13 Aug 2026 20:17:47 -0600 Subject: [PATCH 2/5] fix(lint): restrict read-binding scan to actual destination operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile review on PR #2490: the read-binding scan treated every uppercase token after `read` as a bound destination, so a quoted prompt string (-p "..."), a here-string/redirection operand, or a value-taking flag's own argument (-t/-u/-d/-n/-N) could contain an unrelated uppercase word and get misread as a binding — silently suppressing a genuine cross-fence reference to that word elsewhere in the block. Now strips redirections, quoted arguments, and value-taking flags' own arguments before extracting destination names, and excludes any `$`-prefixed token (a reference, not a binding). Filed #2491 for a separate, pre-existing false-negative in the reference-checking side (a blanket "read " substring exemption) that predates this PR and is out of scope here. --- .../skills/create-skill/scripts/lint-skill.sh | 14 +++++++++- .../unit/lint-skill-read-binding-2344.test.ts | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 9741a0bef..0017d8f09 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -80,11 +80,23 @@ while IFS=$'\t' read -r bnum line; do # batch-size $COUNT set up in Phase 0). if [[ "$line" =~ (^|[^A-Za-z0-9_])read([[:space:]].*)?$ ]]; then read_args="${BASH_REMATCH[2]}" + # Only the destination variable *names* on a `read` line are real + # bindings — strip everything else first so none of it is misread as + # one (Greptile review on PR #2490/#2344): + # - a trailing command on the same line (`; do`) + # - the input source (`< file`, `<<< "$X"` here-strings) + # - quoted option arguments (`-p "prompt text"`, which may contain + # arbitrary uppercase words that aren't destinations at all) + # - a value-taking flag's own argument (`-t 5`, `-u FD`, `-d ':'`) + # - a `$`-prefixed token, which REFERENCES a var rather than binding it read_args="${read_args%%;*}" + read_args="${read_args%%<*}" + read_args=$(printf '%s' "$read_args" | sed -E 's/"[^"]*"//g' | sed -E "s/'[^']*'//g") + read_args=$(printf '%s' "$read_args" | sed -E 's/-[ptnNdu][[:space:]]+[^[:space:]]+//g') while IFS= read -r var; do [ -z "$var" ] && continue register_var "$var" "$bnum" - done < <(echo "$read_args" | grep -oE '\b[A-Z][A-Z0-9_]+\b') + done < <(printf '%s' "$read_args" | grep -oE '(^|[^A-Za-z0-9_$])[A-Z][A-Z0-9_]+' | sed -E 's/^[^A-Za-z0-9_]//') fi done < "$BLOCKS_FILE" diff --git a/tests/unit/lint-skill-read-binding-2344.test.ts b/tests/unit/lint-skill-read-binding-2344.test.ts index e850f097a..30c4cc6f3 100644 --- a/tests/unit/lint-skill-read-binding-2344.test.ts +++ b/tests/unit/lint-skill-read-binding-2344.test.ts @@ -119,6 +119,32 @@ describe.skipIf(BASH4 === null)( 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', ); }); + + // Greptile review on PR #2490: the read-binding scan must not treat every + // uppercase token after `read` as a destination — a prompt string, or a + // value-taking flag's own argument, can contain an unrelated uppercase + // word that is not actually being bound. + it('still flags a leak past a `read -p` prompt string containing an unrelated uppercase word', () => { + const { stdout, ranSuccessfully } = runLint([ + 'FOO=hello', + 'read -p "Enter FOO value: " BAR\necho "leak: $FOO"', + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); + + it('still flags a leak past a value-taking flag argument that is an uppercase word', () => { + const { stdout, ranSuccessfully } = runLint([ + 'FOO=hello', + 'read -u FOO BAR\necho "leak: $FOO"', + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); }, ); From c0a2df8361dafaa42dfb0349ccf5b90fd4b161d8 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 13 Aug 2026 20:24:25 -0600 Subject: [PATCH 3/5] fix(lint): strip -i (readline initial text) from read-binding scan too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile round 2 on PR #2490: -i takes a value argument just like -p/-t/-u/-d/-n/-N, but was missing from the value-taking-flag strip list, so an uppercase -i argument (readline initial text) could still be misread as a bound destination. -a is deliberately left out of the strip list — its argument is the array actually being read into, a genuine destination, not a value. --- .../skills/create-skill/scripts/lint-skill.sh | 6 ++++-- .../unit/lint-skill-read-binding-2344.test.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 0017d8f09..75cd6ac0c 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -87,12 +87,14 @@ while IFS=$'\t' read -r bnum line; do # - the input source (`< file`, `<<< "$X"` here-strings) # - quoted option arguments (`-p "prompt text"`, which may contain # arbitrary uppercase words that aren't destinations at all) - # - a value-taking flag's own argument (`-t 5`, `-u FD`, `-d ':'`) + # - a value-taking flag's own argument (`-t 5`, `-u FD`, `-d ':'`, + # `-i "initial text"`) — every read flag EXCEPT `-a` (whose argument + # is itself a genuine destination: the array read into) # - a `$`-prefixed token, which REFERENCES a var rather than binding it read_args="${read_args%%;*}" read_args="${read_args%%<*}" read_args=$(printf '%s' "$read_args" | sed -E 's/"[^"]*"//g' | sed -E "s/'[^']*'//g") - read_args=$(printf '%s' "$read_args" | sed -E 's/-[ptnNdu][[:space:]]+[^[:space:]]+//g') + read_args=$(printf '%s' "$read_args" | sed -E 's/-[ptnNdui][[:space:]]+[^[:space:]]+//g') while IFS= read -r var; do [ -z "$var" ] && continue register_var "$var" "$bnum" diff --git a/tests/unit/lint-skill-read-binding-2344.test.ts b/tests/unit/lint-skill-read-binding-2344.test.ts index 30c4cc6f3..1a08a3452 100644 --- a/tests/unit/lint-skill-read-binding-2344.test.ts +++ b/tests/unit/lint-skill-read-binding-2344.test.ts @@ -145,6 +145,25 @@ describe.skipIf(BASH4 === null)( 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', ); }); + + // Greptile round 2 on PR #2490: `-i` (readline initial text) is a + // value-taking flag too, and was missing from the first fix's flag list. + it('still flags a leak past a `read -i` initial-text argument that is an uppercase word', () => { + const { stdout, ranSuccessfully } = runLint([ + 'FOO=hello', + 'read -e -i FOO BAR\necho "leak: $FOO"', + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); + + it('does not flag `-a` (its argument is a genuine array destination, not a value)', () => { + const { stdout, ranSuccessfully } = runLint(['read -a ARR <<< "one two three"']); + expect(ranSuccessfully).toBe(true); + expect(stdout).not.toContain('Cross-fence variable: $ARR'); + }); }, ); From 48e5dc7e9c60549f680c42253cba3f05aa6ca081 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 13 Aug 2026 20:30:49 -0600 Subject: [PATCH 4/5] fix(lint): handle combined short-flag clusters ending in a value-taking flag Greptile round 3 on PR #2490: the strip regex only matched a standalone single-letter flag (-i FOO), not a combined cluster like -ei FOO where -e takes no argument but the trailing -i does. Matches any -[letters] cluster whose LAST letter is a value-taking flag, so its following token is stripped as that flag's value rather than extracted as a destination. -a stays excluded even combined (-ra ARR still keeps ARR as the genuine array destination). --- .../skills/create-skill/scripts/lint-skill.sh | 9 +++++--- .../unit/lint-skill-read-binding-2344.test.ts | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 75cd6ac0c..374b96dcb 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -88,13 +88,16 @@ while IFS=$'\t' read -r bnum line; do # - quoted option arguments (`-p "prompt text"`, which may contain # arbitrary uppercase words that aren't destinations at all) # - a value-taking flag's own argument (`-t 5`, `-u FD`, `-d ':'`, - # `-i "initial text"`) — every read flag EXCEPT `-a` (whose argument - # is itself a genuine destination: the array read into) + # `-i "initial text"`, and combined short forms like `-ei FOO`, + # where `-e` takes no argument but the trailing `-i` does — every + # read flag EXCEPT `-a` (whose argument is itself a genuine + # destination: the array read into), so the strip only fires when + # the cluster's LAST letter is one of the other value-taking flags # - a `$`-prefixed token, which REFERENCES a var rather than binding it read_args="${read_args%%;*}" read_args="${read_args%%<*}" read_args=$(printf '%s' "$read_args" | sed -E 's/"[^"]*"//g' | sed -E "s/'[^']*'//g") - read_args=$(printf '%s' "$read_args" | sed -E 's/-[ptnNdui][[:space:]]+[^[:space:]]+//g') + read_args=$(printf '%s' "$read_args" | sed -E 's/-[a-zA-Z]*[ptnNdui][[:space:]]+[^[:space:]]+//g') while IFS= read -r var; do [ -z "$var" ] && continue register_var "$var" "$bnum" diff --git a/tests/unit/lint-skill-read-binding-2344.test.ts b/tests/unit/lint-skill-read-binding-2344.test.ts index 1a08a3452..64357e773 100644 --- a/tests/unit/lint-skill-read-binding-2344.test.ts +++ b/tests/unit/lint-skill-read-binding-2344.test.ts @@ -164,6 +164,27 @@ describe.skipIf(BASH4 === null)( expect(ranSuccessfully).toBe(true); expect(stdout).not.toContain('Cross-fence variable: $ARR'); }); + + // Greptile round 3 on PR #2490: a value-taking flag combined into a + // multi-letter cluster (e.g. `-ei`, where `-e` takes no argument but + // the trailing `-i` does) wasn't recognized — only a standalone + // single-letter flag was. + it('still flags a leak past a combined short-flag cluster ending in a value-taking flag', () => { + const { stdout, ranSuccessfully } = runLint([ + 'FOO=hello', + 'read -ei FOO BAR\necho "leak: $FOO"', + ]); + expect(ranSuccessfully).toBe(true); + expect(stdout).toContain( + 'Cross-fence variable: $FOO assigned in bash block 1, referenced in block 2', + ); + }); + + it('does not flag a combined cluster ending in `-a` (still a genuine array destination)', () => { + const { stdout, ranSuccessfully } = runLint(['read -ra ARR <<< "one two three"']); + expect(ranSuccessfully).toBe(true); + expect(stdout).not.toContain('Cross-fence variable: $ARR'); + }); }, ); From 072890da8a4db7adc0c25b301dc846ea5ac90856 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 13 Aug 2026 21:02:59 -0600 Subject: [PATCH 5/5] fix(lint): replace per-line subprocess pipeline with pure bash builtins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows CI failure on PR #2490: a new test now runs lint-skill.sh directly against the real fixer/SKILL.md (by far the largest SKILL.md in the repo, ~850 non-comment bash-block lines) and timed out at the default 30s testTimeout. Check 1's assignment-collection loop forked an echo|grep|sed pipeline unconditionally for every such line — a few seconds on macOS/Linux, but Windows process creation is ~100x slower per this file's own header comment, so a file this size was already at a latent performance cliff pre-existing this PR; a direct per-file test is what first tripped it. Replaces the pipeline with a pure-bash extraction loop ([[ =~ ]] + parameter expansion), matching the file's stated design (every OTHER check already avoids subprocess forking for this reason). Cuts wall time against fixer/SKILL.md from ~4.0s to ~0.95s on macOS. Verified byte-identical error/warning counts across every SKILL.md in the repo before and after. Also bumps the new real-file test's own timeout to 120s as a safety margin, since Windows fork overhead is inherently harder to predict precisely than to bound generously. --- .../skills/create-skill/scripts/lint-skill.sh | 21 ++++++++++++++----- .../unit/lint-skill-read-binding-2344.test.ts | 9 +++++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 374b96dcb..9474b6e8c 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -61,15 +61,26 @@ register_var() { REASSIGNED["${var}:${bnum}"]=1 fi } +# Finds every UPPER_CASE_VAR=/VAR+= assignment on a line using pure bash +# builtins ([[ =~ ]] + parameter expansion) rather than forking echo|grep|sed +# per line. This loop runs once per non-comment bash-block line — for a large +# SKILL.md (fixer/SKILL.md is ~850 such lines) a per-line subprocess pipeline +# is a few seconds on macOS/Linux but can blow well past CI's default test +# timeout on Windows, where process creation is ~100x slower (see this +# file's own performance note above; discovered via PR #2490/#2344 once a +# test exercised lint-skill.sh against this specific large file directly). +collect_assignments() { + local s="$1" bnum="$2" + while [[ "$s" =~ (^|[^A-Za-z0-9_])([A-Z][A-Z0-9_]+)\+?= ]]; do + register_var "${BASH_REMATCH[2]}" "$bnum" + s="${s#*"${BASH_REMATCH[0]}"}" + done +} while IFS=$'\t' read -r bnum line; do # Skip comment lines — they document context but don't register variable assignments [[ "$line" =~ ^[[:space:]]*# ]] && continue # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) - # Use while-read instead of for-in-$() to avoid empty-string iteration when grep matches nothing - while IFS= read -r var; do - [ -z "$var" ] && continue - register_var "$var" "$bnum" - done < <(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+\+?=' | sed -E 's/\+?=$//') + collect_assignments "$line" "$bnum" # `read`/`read -r VAR1 VAR2` binds variables just like VAR=, but with no '=' # after the name — e.g. `while IFS=$'\t' read -r F COUNT LINE; do`. Without # this, a var re-derived via `read` in a later block (a legitimate, fresh, diff --git a/tests/unit/lint-skill-read-binding-2344.test.ts b/tests/unit/lint-skill-read-binding-2344.test.ts index 64357e773..744eb8faa 100644 --- a/tests/unit/lint-skill-read-binding-2344.test.ts +++ b/tests/unit/lint-skill-read-binding-2344.test.ts @@ -189,7 +189,14 @@ describe.skipIf(BASH4 === null)( ); describe.skipIf(BASH4 === null)('.claude/skills/fixer/SKILL.md itself (#2344 repro)', () => { - it('passes lint-skill.sh with no Cross-fence $COUNT error', () => { + // fixer/SKILL.md is by far the largest SKILL.md in the repo (~1600 lines, + // ~1000 non-comment bash-block lines), and Check 1 forks a handful of + // subprocesses per line. That's a few seconds on macOS/Linux but can blow + // past the 30s default testTimeout on Windows CI, where process creation + // is ~100x slower (see this script's own header comment) — this is the + // first test to run lint-skill.sh against this specific file directly, so + // it's the first to hit that pre-existing characteristic head-on. + it('passes lint-skill.sh with no Cross-fence $COUNT error', { timeout: 120_000 }, () => { const realSkillPath = path.join(REPO_ROOT, '.claude', 'skills', 'fixer', 'SKILL.md'); const stdout = execFileSync(BASH4!, [LINT_SCRIPT, realSkillPath], { encoding: 'utf8' }); expect(stdout).not.toContain('Cross-fence variable: $COUNT');