fix(standards): stop passing a stray empty argument when scope is unset - #167
Merged
Merged
Conversation
`"${scope_args[@]-}"` expands an EMPTY array to one empty-string argument
rather than to nothing. run-standards.sh then hits that empty string in its
catch-all `*)` branch, reports `unknown argument: ` and exits 2.
The effect is that standards-check fails on any repo that requests no
`changed-since` narrowing — which is every repo running the check in
whole-repo hygiene mode. The failure surfaced on nightowlstudiollc/reliquarist
#93-96 after a rebase, but it is not specific to those PRs or to Dependabot.
Plain `"${scope_args[@]}"` expands to nothing when the array is empty and is
safe under `set -u` on bash 4.4+; runners are bash 5.x. Both forms behave
identically when the array is non-empty, so narrowing is unaffected.
Note that `--skip ""` is NOT the cause: run-standards.sh accepts an empty
`--skip` value without complaint. Verified by running the real runner with
both expansion forms — the old form produces one `unknown argument` hit, the
new form produces zero.
The regression test asserts on argument assembly rather than lint output, so
it does not depend on linters being installed, and it carries a paired
assertion that the known-bad form still reproduces — without which the first
assertion could silently become vacuous.
Advances #116 context: standards-check is now the sole required check on four
repos, so this failure blocks all merges there until fixed.
Claude-Session: https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o
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
1 participant
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.
The bug
"${scope_args[@]-}"expands an empty array to one empty-string argument, not to nothing.run-standards.shhits that empty string in its catch-all*)branch and exits 2:So
standards-checkfails on any repo that requests nochanged-sincenarrowing — that is, every repo running in whole-repo hygiene mode.Why it matters now
standards-check / run-standards-checkis the sole required check ondev-env,claude-config,dotfilesandgithub-workflowsafter today's protection change. While this is broken, those repos cannot merge anything.Surfaced on
nightowlstudiollc/reliquarist#93-96, which went from green to failing after a rebase onto currentmain. Not specific to those PRs, and not a Dependabot problem.What is not the cause
--skip ""is fine.run-standards.shparses--skip) skip="$2"and accepts an empty value without complaint — verified by running the real runner directly. The stray argument from the array expansion is the whole story.The fix
Plain
"${scope_args[@]}", which expands to nothing when empty and is safe underset -uon bash 4.4+ (runners are bash 5.x). Both forms are identical when the array is non-empty, so narrowing behaviour is unchanged.The file already solved this exact problem for
--changed-sinceat lines 235-240 — omit the flag rather than pass it empty. This applies the same discipline at the call site.Verification
Ran the real runner with both expansion forms, counting
unknown argumenthits:"${scope_args[@]-}"(old)"${scope_args[@]}"(new)The test
Asserts on argument assembly, not lint output, so it does not depend on linters being installed.
It carries a paired assertion that the known-bad form still reproduces. Without that, a future refactor could make the first assertion vacuous and the suite would stay green over a reintroduced bug. My first attempt at this test passed against the broken code — it is included in the form that actually fails against it.
17 passed, 0 failed. shellcheck-S infoclean, no disable directives.https://claude.ai/code/session_01ESsw699T54JHARkQXrdL3o