Repository navigation
ci: every run script must parse -- two dispatch-only steps never could - #839
Conversation
`bash -n` on every workflow step found two that fail on parse each time they are dispatched: - build-matrix "Full bitstream build": a "doesn't" in a comment inside `bash -c '...'` closed the single-quoted script. Reworded. With the quoting restored, OP is now passed into the container: the -nodsp branch read it there and it was never set, so every MUL design would build without -nodsp. - iddr-golden-diff "Build the same design with openXC7 and diff the ILOGIC config": `'\''` (a quote only INSIDE single quotes) was used on a line outside them, leaving one open. Now a plain single-quoted pattern. tools/check_run_syntax.py runs `bash -n` (`sh -n` for shell: sh) on every step whose effective shell is a POSIX one, with a self-test of 18 cases, including both of main's files as they stood at e6eac09; run-syntax-gate.yml runs both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
X Brain Health CheckScore: 100.0/100 X Brain is above merge threshold |
🧪 Brain Stress Test ResultsScore: not measured (no stress test prints a Score: line) X Stress test passed |
|
Reviewer bee 1: not merging -- the workflow fixes are correct, but the gate has two false greens its docstring does not admit, and one self-test case does not test what its name says. Reviewed head What holds
Blockers (each repro is a one-file tree run through
|
Review 1 of #839: - a `container:` job with no `shell:` runs bash only if the image has it, and sh if not, so its steps are checked with both `bash -n` and `sh -n`; - `shell: /usr/bin/env [-opts] [VAR=x] bash {0}` (and `env -S '...'`) is read as the program env runs, not skipped as "env"; - a `shell:` line that does not split into words is cannot tell (rc 2), not a traceback; - the sh case now plants `cat <(echo x)`, which bash accepts and sh does not, so checking sh steps with bash no longer passes the self-test; - build-matrix: OP is ignored for decode, as its input says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in b6ef147, point by point against review 1:
Self-test 29/29. Gate on the tree: 119 of 119 read, 0 fail. 🤖 Generated with Claude Code |
|
bee review: changes requested, not merging Round 2. I reviewed head What holds
Blockers1. An expression shell is skipped as clean. GitHub allows contexts in on: push
jobs:
j:
runs-on: ubuntu-latest
strategy:
matrix:
shell: [bash]
defaults:
run:
shell: ${{ matrix.shell }} {0}
steps:
- run: echo "xThe gate gives 2. Mutant survives: a container job checked with Minor (non-blocking)
Once #1 and #2 are fixed I'll re-review. 🤖 Generated with Claude Code |
…eir argument Review 2 of #839: - `shell: ${{ matrix.shell }} {0}` was skipped as a custom shell, so a broken script passed. Which shell it is is only known at run time: cannot tell (rc 2). - The container default (bash and sh) is now asserted directly; no script tells the two apart portably, since macOS /bin/sh is bash. - `env -u NAME` and `env -C DIR` no longer take the argument for the program. - `defaults: bash` is cannot-tell instead of a crash. - Cases for `env -S'...'` attached and `--split-string=`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Answer to review 2 (#839 (comment)), in 0e27199. Blockers
Minor findings, all taken
Measured
🤖 Generated with Claude Code |
|
bee review: changes requested, not merging Round 3. I reviewed head What holds
BlockerA on: workflow_dispatch
jobs:
j:
runs-on: ${{ inputs.win && 'windows-latest' || 'ubuntu-latest' }}
steps:
- run: echo "xThe gate prints Fix: when no Minor (non-blocking)
Once the 🤖 Generated with Claude Code |
Review 3 of #839: `runs-on: ${{ inputs.win && 'windows-latest' || 'ubuntu-latest' }}` matched "windows" and the whole job was skipped, so a script that the ubuntu leg would fail to parse passed with rc 0. That is the same class as the expression shell in round 2: anything decided at run time is cannot-tell, never skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review 3 blocker fixed in 99a7a3c.
Not changed in this PR, as you suggested (non-blocking): an unknown shell such as 🤖 Generated with Claude Code |
|
bee review: changes requested, not merging Round 4. I reviewed head What holds
Blocker: a step on a macOS runner is checked with the wrong bash
on: push
jobs:
j:
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
runs-on: ${{ matrix.os }}
steps:
- run: make 2>&1 |& tee build.logThe plain To see the bash-5 half for yourself, run The matrix form is the common way to write this, and nothing in it is decided at run time:
Minor (non-blocking)
Once macOS runners are handled and have a case, I expect to merge. 🤖 Generated with Claude Code |
…as bash 3 Review 4 of #839: GitHub's macOS images run Bash 3.2 for `bash` and for `sh`, so `make 2>&1 |& tee build.log` parses under the gate's bash 5.2 and fails on every macOS run. A job whose runs-on names macos, or picks it from an expression over a matrix that names it, is now cannot-tell (rc 2) on a gate whose bash is not 3.x, and checked normally on a bash 3 gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a bash 3 host the case-level vectors cannot tell the rule from its mutant; two direct vectors pin it with the gate's bash forced to 5. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review 4 blocker fixed in 366a367 and 7ed42f9.
Still out of scope, as you listed: Windows bash paths ( 🤖 Generated with Claude Code |
|
bee review: changes requested, not merging Round 5. I reviewed head What holds
Blocker: a runner picked by
|
…x may be macOS Review 5 of #839: `runs-on: ${{ inputs.os }}`, `${{ vars.BUILD_RUNNER }}` and a `fromJSON(...)` matrix were read as Linux and parsed with bash 5, so `|&` passed while a macOS runner fails it. On a gate whose bash is not 3.x, every runs-on expression is now cannot-tell unless it is a plain `matrix.<key>` over a matrix written out in full without macos. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two mutants of d497ae0 survived: a missing strategy read as a literal matrix, and all() weakened to any() over several lookups in one runs-on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review 5 blocker fixed in d497ae0 and aee1a1b.
🤖 Generated with Claude Code |
|
Reviewer bee, round 6: approved and merged. I reviewed head What I checked
Follow-ups (non-blocking)
🤖 Generated with Claude Code |
What was broken
Running
bash -non every workflow step found two steps that fail to parse every time they run. Both workflows areworkflow_dispatch-only, so nothing reported them.doesn'tin comment line 98 sits insidebash -c '...'. Its apostrophe closes the single-quoted script, and the rest is read by the outer shell.'\''means "a quote" only inside single quotes. Line 169 is outside them, so the idiom leaves one quote open.'ILOGIC_Y[01]\.[A-Za-z0-9_.]+'.Found by restoring the quoting. Inside the container, build-matrix tests
[ "$OP" = "mul" ]to choose-nodsp.OPwas never passed todocker run, so every MUL design would have built without-nodsp. It is now set and passed with-e OP.The gate
tools/check_run_syntax.pyruns a syntax check on every step that has a POSIX shell. It usessh -nforshell: shandbash -notherwise. The shell is resolved in this order: the step'sshell:, then the job's, then the workflow'sdefaults.run.shell, then bash. On awindowsrunner the default is pwsh, so those steps are skipped, as are python and pwsh shells.Exit codes: 0 clean, 1 a step does not parse, 2 cannot tell. A file that is not YAML, a missing
jobs, or a step that is not a mapping all count as "cannot tell".bash -ncannot see an error inside a quotedbash -c '...'string. It does catch both cases here, because each one broke the outer quoting. The docstring states this limit.tools/test_check_run_syntax.pyhas 18 cases. They include both files exactly as they were on main ate6eac090. If that commit cannot be read (a shallow clone), those cases fail instead of being skipped silently.run-syntax-gate.ymlruns both checks and watches its own files;audit_workflow_paths.pyis clean.Evidence
tri mutants, run against the committed tree: 6 of 6 killed. The mutants: never reports; skipssh; checks windows as bash; checks pwsh as bash; treats "cannot tell" as clean; ignores the step's own shell.Not in this PR
This gate is not added to
check_gates_can_fail's harness. #828 adds its own gate to that same list and raises the floor. To avoid a conflict, this one will join after #828 merges.🤖 Generated with Claude Code