diff --git a/docs/HARNESS.md b/docs/HARNESS.md index 1bf392a..00d240e 100644 --- a/docs/HARNESS.md +++ b/docs/HARNESS.md @@ -47,6 +47,7 @@ Three regulation categories: | Arch. | `no-os-getenv-home` | L1 | `internal/archtest/envhome_test.go` | | Arch. | `dryrun` — destructive ops must check `DryRun` | L1 | `internal/archtest/dryrun_test.go` | | Arch. | `no-raw-fmtprint` — UI output via `ui.*` helpers, not raw `fmt.Print*` | L1 | `internal/archtest/fmtprint_test.go` | +| Arch. | `install.sh` must not prompt on stdin — under `curl \| bash` stdin is the script | L1 | `internal/archtest/installsh_test.go` | | Behav. | L1 unit + integration + contract (faked runners *and* real brew/git/npm in temp dirs) | pre-push, CI | `make test-unit` | | Behav. | L2 contract schema (against openboot-contract repo) | CI | `.github/workflows/test.yml` `contract` job | | Behav. | L3 e2e binary | release | `make test-e2e` | @@ -74,6 +75,7 @@ When you observe a recurring issue, decide where to encode the fix: | "Agent doesn't know about preset X." | Update `internal/config/data/presets.yaml`. Source of truth, not docs. | | "Agent introduced a new lint failure that golangci-lint should have caught." | Enable the relevant linter in `.golangci.yml`. | | "Agent broke a behaviour that has no test." | Write the test at the right tier — L1 covers both faked-runner units in `internal//` and real-subprocess integration in `test/integration/`. | +| "A shipped release reached nobody: `install.sh`'s already-installed branch prompted on stdin, which under `curl \| bash` is the script itself, so it always took the don't-upgrade default." | Already handled by the `installsh` archtest. The wider lesson the sensor does *not* cover: `curl-bash-smoke` is gated `if: github.event_name != 'pull_request'` and only exercises the mock-server path, so the real `scripts/install.sh` brew branch has no behavioural test. Upgrade-over-existing-install is the case to add. | | "Agent missed a CLAUDE.md rule we keep restating." | Make it a hard or soft archtest rule (a docs rule that doesn't fail is a docs rule that drifts). | | "Agent did something safe but suboptimal." | Add to CLAUDE.md "Project-specific conventions" and consider whether it's encodable. | | "Agent guessed at an API contract." | Update `openboot-contract` repo + fixtures; CI already runs schema validation. | diff --git a/internal/archtest/installsh_test.go b/internal/archtest/installsh_test.go new file mode 100644 index 0000000..8261027 --- /dev/null +++ b/internal/archtest/installsh_test.go @@ -0,0 +1,72 @@ +package archtest + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// bareReadRe matches a `read` prompt that takes no explicit input redirect. +// `read ... 0 { + t.Errorf("install.sh prompts on stdin, which under `curl | bash` is the script itself:\n %s\n\n"+ + "Fix: ask via the ask_tty helper (reads /dev/tty, falls back to a default with no terminal).", + strings.Join(bad, "\n ")) + } +} + +func itoa(n int) string { + if n == 0 { + return "0" + } + var b []byte + for n > 0 { + b = append([]byte{byte('0' + n%10)}, b...) + n /= 10 + } + return string(b) +} diff --git a/scripts/install.sh b/scripts/install.sh index 3ae119a..30b4b5b 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -7,6 +7,27 @@ BINARY_NAME="openboot" TAP_NAME="openbootdotdev/tap" DRY_RUN="${OPENBOOT_DRY_RUN:-false}" +# This script's headline use is `curl -fsSL openboot.dev/install.sh | bash`, +# where stdin is the pipe carrying the script itself — NOT the user's keyboard. +# A bare `read` there consumes the script's own next bytes as the answer, which +# both mangles the script and silently takes a branch the user never chose. +# Every prompt must therefore read from the controlling terminal, and must have +# a safe answer for when there isn't one (piped into a CI job, no tty). +has_tty() { [[ -e /dev/tty ]] && (: >/dev/tty) 2>/dev/null; } + +# ask_tty [read-opts...] — echoes the reply, or +# when no terminal is available to ask. +ask_tty() { + local prompt="$1" default="$2"; shift 2 + if ! has_tty; then + echo "$default" + return + fi + local reply="" + read -r "$@" -p "$prompt" reply /dev/null; then return 0 @@ -18,7 +39,7 @@ install_xcode_clt() { echo "Xcode Command Line Tools need to be installed." echo "A dialog will appear - please click 'Install' and enter your password." echo "" - read -p "Press Enter to launch installer..." -r + ask_tty "Press Enter to launch installer..." "" >/dev/null echo "" xcode-select --install 2>/dev/null || true @@ -161,29 +182,36 @@ main() { fi if brew list openboot &>/dev/null 2>&1; then - echo "OpenBoot is already installed via Homebrew." + echo "OpenBoot is already installed — updating..." echo "" - read -p "Reinstall? (y/N) " -n 1 -r - echo - - if [[ $REPLY =~ ^[Yy]$ ]]; then - echo "Reinstalling OpenBoot..." + + # Refresh the tap before upgrading. Homebrew only auto-updates it every + # HOMEBREW_AUTO_UPDATE_SECS (24h by default), so a release published + # since the last refresh is invisible to `upgrade`, which then reports + # success while leaving the old binary in place. + brew update >/dev/null 2>&1 || true + if ! brew upgrade ${TAP_NAME}/openboot 2>/dev/null; then brew reinstall ${TAP_NAME}/openboot - echo "" - echo "✓ OpenBoot reinstalled!" - else - echo "Using existing installation." fi + + echo "" + echo "✓ OpenBoot updated!" else echo "Installing OpenBoot via Homebrew..." echo "" - + brew install ${TAP_NAME}/openboot - + echo "" echo "✓ OpenBoot installed!" fi + # Always state the version we ended up on. Running the installer and + # silently getting yesterday's binary is the failure this whole path is + # guarding against; printing it makes that impossible to miss. + hash -r 2>/dev/null || true + echo " $(openboot version 2>/dev/null || echo 'version unavailable')" + echo "" echo "Starting OpenBoot setup..." echo ""