Say which shell is reading start.sh and stop.sh, instead of exiting 1 with nothing printed - #567
Merged
Merged
Conversation
…othing printed `sh scripts/start.sh` produced exit 1 and not one character of output. No line number, no failing command, no message — the whole of what somebody had to go on was the number, which reads as "this script is broken" rather than "run it the other way". Two things were behind it. The shebang says bash and `sh` overrides it. On macOS `sh` IS bash, run in POSIX mode, where a failure these scripts survive under bash is fatal instead. So the run ended at the first setting read out of `.env`, before any output existed to say so. That setting reads its key with a `grep` pipeline, and `grep` finding nothing is an exit status of 1 that `pipefail` makes the pipeline's. `set -e` then ended the script on its way to the fallback sitting on the next line — the fallback that is the entire reason `setting` takes a second argument. `.env.example` does not list `APP_PORT`, so every `.env` copied from it is missing exactly the first key the script reads. Both scripts now source `scripts/require-bash.sh` before they set any options, which names the shell it found and prints the command to retype. It is POSIX syntax throughout, because the shell it is warning about may not be bash at all — `set -o pipefail` is itself a bashism and a syntax error in dash, which is `sh` on most Linux distributions. `SHELLOPTS` carrying `posix` is what separates bash-as-sh from bash; a `BASH_VERSION` check alone cannot see that case, and that case is every Mac. `stop.sh` had both defects identically and is fixed with the same two changes. The refusal has a test that was watched failing. The absent-key test passes either way and says so in its own docblock: under bash the fallback already won, and with the refusal in place there is no longer an invocation that can watch it fail — so what it pins is the contract rather than the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mxmzb
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 15, 2026 19:27
davidmckayv
approved these changes
Sep 15, 2026
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
Reviewed commit 97a4978 against the local code at 554f2b2. No actionable findings. Applied the patch to a temporary copy of the local scripts: all 4 tests passed, both scripts correctly rejected sh, bash --posix, and dash, and Bash syntax checks passed. Validation used simulated processes; the live desktop stack was not restarted.
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
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.
sh scripts/start.shproduced exit 1 and not one character of output. No line number, no failing command, no message — the whole of what somebody had to go on was the number, which reads as "this script is broken" rather than "run it the other way". It cost an afternoon before it was traced.What was happening
Two things, one on top of the other.
The shebang says bash and
shoverrides it. On macOSshis bash, run in POSIX mode, where a failure these scripts survive under bash is fatal instead. So the run ended at the first setting read out of.env, before any output existed to say so.That setting could not tolerate an absent key.
settingreads its key with agreppipeline, andgrepfinding nothing is an exit status of 1 thatpipefailmakes the pipeline's.set -ethen ended the script on its way to the fallback sitting on the very next line — the fallback that is the entire reasonsettingtakes a second argument..env.exampledoes not listAPP_PORT, so every.envcopied from it is missing exactly the first key the script reads.Neither half is visible on its own. Under bash the failed status does not escape the command substitution, so the fallback wins and everything works; the trap only fires under
sh, where it fires in silence.What this changes
Both scripts source
scripts/require-bash.shbefore they set any options. It names the shell it found and prints the command to retype:It is POSIX syntax throughout, because the shell it is warning about may not be bash at all —
set -o pipefailis itself a bashism and a syntax error in dash, which isshon most Linux distributions, so the refusal has to be written before it and withoutlocal,[[or${!name}.SHELLOPTScarryingposixis what separates bash-as-sh from bash; aBASH_VERSIONcheck alone cannot see that case, and that case is every Mac.And
settingtakes|| true, so the second argument is the value an absent key gets by construction rather than by a subtlety of whereset -eapplies.stop.shhad both defects identically —sh scripts/stop.shwas the same silent exit 1 — and gets the same two changes.Tests
The refusal has a test that was watched failing against the unfixed script:
shexits non-zero, names bash, and names the command to run, on stderr.The absent-key test passes either way, and its docblock says so rather than dressing it up. Under bash the fallback already won, and with the refusal in place there is no longer an invocation that can watch it fail — what it pins is the contract, that the second argument is what an absent key takes and no property of the reading shell may decide otherwise.
Both run through the existing harness in
start-restart-guard.test.ts, which fakes.envand the binaries, so nothing here starts a stack.Verified
bash scripts/start.shandbash scripts/stop.shboth run the real local stack through a full stop-and-start cycle, ending with the app answering 200 on 3010.shanddashboth refuse with their own correct sentence, naming the right script in each case.bun test scripts/25/25, typecheck clean across all four packages, biome format and lint clean.Nothing in the repo, the docs or the workflows invokes either script without
bash.