fix(install.sh): the installer could never upgrade an existing install#151
Merged
Conversation
`curl -fsSL openboot.dev/install.sh | bash` is the documented install command, and there stdin is the pipe carrying the script — not the user's keyboard. Both prompts read from it anyway. The consequence was not cosmetic. The already-installed branch asked "Reinstall? (y/N)" and defaulted to No; via curl|bash the `read` consumed the script's own next bytes as the answer, which never matched ^[Yy]$. So it printed "Using existing installation", exec'd the old binary, and every existing user stayed on their old version no matter how many times they ran the installer. Releases went out that nobody could receive. The symptom — `openboot version` reporting yesterday's build right after a successful-looking install — pointed nowhere near the cause. (It also ate a byte of the script, so the following line ran mangled.) Changes: - prompts go through a new ask_tty helper: reads /dev/tty, and falls back to an explicit default when there is no terminal (CI, piped shells). - the already-installed branch no longer asks. Someone running the installer wants the current release; "reinstall?" defaulting to No was hostile even when the read worked. - `brew update` before upgrading. Homebrew only refreshes a tap every HOMEBREW_AUTO_UPDATE_SECS (24h default), so a release published inside that window is invisible and `upgrade` reports success while leaving the old binary in place — the second, independent way to get a stale install. This is not theoretical either: the report that prompted this landed 23.5h after the previous release. - the resolved version is always printed. Running the installer and silently getting yesterday's binary is exactly what this path failed at; printing it makes that impossible to miss. An archtest pins the rule so a bare `read` can't come back. It is a static check because the behavioural gap is wider than one script: curl-bash-smoke is gated `if: github.event_name != 'pull_request'` and only drives the mock-server path, so scripts/install.sh's brew branch has no test at all. Noted in HARNESS.md as the next control to add.
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.
What does this PR do?
Stops
scripts/install.shprompting on stdin, and makes the already-installed path actually update.Why?
Found while trying to verify v0.66.0 in a VM:
openboot versionkept reporting 0.65.0 after a successful-lookingcurl -fsSL openboot.dev/install.sh | bash.curl … | bashmeans stdin is the pipe carrying the script, not the user's keyboard. Both prompts read from it anyway. Demonstrated:So the already-installed branch asked
Reinstall? (y/N), never received the answer, took the No default, printed "Using existing installation", andexec'd the old binary. Every existing user was pinned to whatever version they first installed, no matter how many times they ran the installer. Releases were going out that nobody could receive — and the symptom pointed nowhere near the cause.There's a second, independent staleness path: Homebrew only refreshes a tap every
HOMEBREW_AUTO_UPDATE_SECS(24h by default), so a release published inside that window is invisible andbrew upgradereports success while leaving the old binary in place. The report that prompted this landed 23.5h after the previous release.Changes
ask_ttyhelper — reads/dev/tty, falls back to an explicit default when there's no terminal (CI, piped shells)brew updatebefore upgrading, so a fresh release isn't hidden by the tap's auto-update windowTesting
go vet ./...passesinternal/archtest/installsh_test.gopins the no-bare-readrule; verified it fails (with file:line) when the oldread -pis restored, and passes on the fixbash -nclean; verified under a real pipe thatask_ttyreturns its default without consuming the scriptNotes for reviewer
The rule is enforced statically because the behavioural gap is wider than this one script:
curl-bash-smokeis gatedif: github.event_name != 'pull_request'— it never ran on any of the recent PRs (they all show it as skipping) — and it only drives the mock-server path, soscripts/install.sh's brew branch has no behavioural test at all. Upgrade-over-existing-install is the case to add, and un-gating the job on PRs is worth a decision. Recorded in HARNESS.md rather than fixed here to keep this PR to the bug.