diff --git a/.github/required-checks.txt b/.github/required-checks.txt index 3128ff2..535ee75 100644 --- a/.github/required-checks.txt +++ b/.github/required-checks.txt @@ -10,3 +10,4 @@ lint unit (L1) +vm-e2e diff --git a/.github/workflows/auto-release.yml b/.github/workflows/auto-release.yml index f04e89e..ed6dea7 100644 --- a/.github/workflows/auto-release.yml +++ b/.github/workflows/auto-release.yml @@ -173,16 +173,16 @@ jobs: **Why:** ${REASON} Because this release includes \`feat:\` changes, the sensor did - **not** auto-tag. Run the destructive e2e suite locally, then - cut the release manually: + **not** auto-tag. Confirm the merge and post-merge checks, then cut + the release manually: - [ ] L4 CI (\`vm-e2e-spike.yml\`) is green on the latest commit on \`main\` - [ ] sanity-check the curl|bash smoke and cli-compat results in the most recent test.yml run on main - [ ] \`git tag -a ${NEW_TAG} -m "..."\` and \`git push origin ${NEW_TAG}\` - [ ] close this issue - L4 CI is not yet a hard merge gate, but \`feat:\` changes carry - more risk than \`fix:\` patches — verify it before tagging. + L4 CI is a required merge check; verify the latest \`main\` run too + because \`feat:\` changes carry more risk than \`fix:\` patches. EOF ) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 37330ce..5e77e54 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -65,6 +65,7 @@ jobs: run: make test-unit vm-e2e: + name: release vm-e2e (L4) needs: gate-tests runs-on: macos-14 timeout-minutes: 60 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3ad8652..997eaca 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ Tests are split across four tiers. Which one runs where: | **L1 Unit + Integration + Contract** | Pure-Go logic with faked `Runner` *plus* real `brew` / `git` / `npm` against temp dirs and real `httptest` servers | `make test-unit` (~75s) | Every push (pre-push hook); CI on push/PR | | **L2 Contract schema** | JSON schema validation against [openboot-contract](https://github.com/openbootdotdev/openboot-contract) | (runs in CI only) | CI on push/PR | | **L3 E2E binary** | Compiled binary driven by scripts; `-tags=e2e` | `make test-e2e` | CI on release | -| **L4 VM e2e** | Full destructive suite (`-tags="e2e,vm"`). Installs real packages, modifies `~/.zshrc`, writes `defaults`. Each run requires a clean macOS host (Apple Silicon). | CI only — `vm-e2e-spike.yml` on `macos-14` | **CI** — GitHub Actions `macos-14` runner, two parallel jobs. No local target. | +| **L4 VM e2e** | Full destructive suite (`-tags="e2e,vm"`). Installs real packages, modifies `~/.zshrc`, writes `defaults`. Each run requires a clean macOS host (Apple Silicon). | CI only — `vm-e2e-spike.yml` on `macos-14` | Every PR as the required `vm-e2e` check. No local target. | Rules of thumb: diff --git a/docs/HARNESS.md b/docs/HARNESS.md index 8caa48c..4b84bfb 100644 --- a/docs/HARNESS.md +++ b/docs/HARNESS.md @@ -51,9 +51,10 @@ Three regulation categories: | 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` | -| Behav. | L4 VM e2e (`vm`) — full destructive suite on a clean macOS host | every PR | `.github/workflows/vm-e2e-spike.yml` (macos-14 runner, two parallel jobs) | +| Behav. | L4 VM e2e (`vm`) — required full destructive suite on a clean macOS host | every PR | `.github/workflows/vm-e2e-spike.yml` (`vm-e2e` required check on a macos-14 runner) | +| Behav. | `install.sh` upgrade over an existing install — tap refresh, upgrade, reinstall fallback, and resolved-version reporting with fake Homebrew/OpenBoot commands | L1 | `test/integration/install_script_test.go` | | Behav. | Install-wizard TUI on a real pty — L3: launch/quit smoke + full keyboard choreography (stops before confirm, installs nothing); L4: same key sequence through a real install via `expect(1)`, asserting brew/git system state | L3 at release, L4 every PR | `test/e2e/install_wizard_e2e_test.go`, `test/e2e/install_wizard_vm_test.go` | -| Behav. | curl\|bash smoke (install.sh + mock server) | every PR | `.github/workflows/test.yml` `curl-bash-smoke` job | +| Behav. | curl\|bash smoke (install.sh + mock server) | push to main / dispatch | `.github/workflows/test.yml` `curl-bash-smoke` job | | Behav. | Auto-release sensor — patch fast lane (`fix:`-only) auto-tags + dispatches `release.yml`; feat threshold opens a `release-ready` issue (check L4 CI green, then tag manually) | push to `main` | `.github/workflows/auto-release.yml` | | Behav. | Release notes — Conventional Commits since previous tag, grouped by type (Features / Bug Fixes / etc) + Full Changelog link, appended to the install-instructions template | tag push or `workflow_dispatch` | `.github/workflows/release.yml` (`Write release notes` step) | | Behav. | Old-CLI compat (previous release × current mock server) | every PR | `.github/workflows/test.yml` `cli-compat` job | @@ -76,7 +77,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. | +| "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." | The `installsh` archtest blocks stdin prompts, and `TestIntegration_InstallScript_ExistingInstallUpgrade` drives the real script through piped stdin with fake commands to pin tap refresh, upgrade, reinstall fallback, and version reporting. | | "Agent missed an AGENTS.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 AGENTS.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. | @@ -119,9 +120,8 @@ it survives doc rot. - **L4 runs on GitHub Actions, not a self-hosted runner.** `macos-14` runners are Apple Silicon VMs — each job gets a fresh clean macOS environment, which is exactly what L4 needs. Tart is no longer required. - The L4 workflow (`vm-e2e-spike.yml`) is not yet a hard merge gate (not in - `required-checks.txt`); it runs on every PR. Promoting it to a required - check is the next step once the workflow has proven stable. + The L4 workflow (`vm-e2e-spike.yml`) runs on every PR as the required + `vm-e2e` merge check. ## How agents should think about this file diff --git a/docs/MERGE_POLICY.md b/docs/MERGE_POLICY.md index 3e09096..71e2fcb 100644 --- a/docs/MERGE_POLICY.md +++ b/docs/MERGE_POLICY.md @@ -21,6 +21,7 @@ Runs on every PR. Must pass before merge. |---|---|---| | `lint` | Test | Catches gofmt / gosec / staticcheck issues that block release builds. | | `unit (L1)` | Test | Unit + integration + contract: faked-runner Go tests *and* real `brew` / `git` / `npm` against temp dirs. Includes `internal/archtest` fitness rules. | +| `vm-e2e` | vm-e2e-spike | Exercises the destructive install paths and TUI choreography on a fresh Apple Silicon macOS VM. | ### Post-merge (runs on push to `main`) @@ -38,7 +39,6 @@ the auto-release sensor can tag. Runs on `workflow_dispatch` and | Check | Status | Reason | |---|---|---| -| `macos e2e (L4)` | runs only on tag pushes / manual dispatch | Slow + destructive; runs at release time, not per PR. | | Harness drift sensors (`govulncheck`, `deadcode`, `mod-tidy diff`, `archtest stale baseline`) | `continue-on-error: true` | Informational by design. Failures surface as annotations and, on `main`, open tracking issues via `drift-to-issue.yml`. | | `codecov/patch` | informational | Coverage threshold is a guideline, not a gate. Hard coverage gates push toward test-shaped code without raising actual quality. | @@ -56,7 +56,7 @@ the auto-release sensor can tag. Runs on `workflow_dispatch` and add it to this list. Promote a check to required by editing this doc and updating branch protection in the same PR. -## Why these two +## Why these three Each required check covers a class of regression that has shipped to users in past commits: @@ -65,6 +65,9 @@ users in past commits: - `unit (L1)` is the broadest behaviour check — covers both faked-runner unit logic and real-subprocess integration drift (brew flag changes, `git` exit-code shifts between macOS versions). +- `vm-e2e` (L4) covers the destructive and terminal-dependent paths that + cannot safely run inside L1, including real Homebrew installs and the + install-wizard choreography on a fresh macOS VM. The three heavier checks (`contract schema (L2)`, `curl|bash smoke`, `old-cli compat`) still run on every merge to `main` — they just don't diff --git a/test/integration/install_script_test.go b/test/integration/install_script_test.go new file mode 100644 index 0000000..397d3af --- /dev/null +++ b/test/integration/install_script_test.go @@ -0,0 +1,115 @@ +package integration + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIntegration_InstallScript_ExistingInstallUpgrade(t *testing.T) { + script, err := os.ReadFile(filepath.Join("..", "..", "scripts", "install.sh")) + require.NoError(t, err) + + tests := []struct { + name string + upgradeExit string + wantCalls []string + }{ + { + name: "upgrades after refreshing the tap", + upgradeExit: "0", + wantCalls: []string{ + "brew list openboot", + "brew update", + "brew upgrade openbootdotdev/tap/openboot", + "openboot version", + "openboot install --help", + }, + }, + { + name: "reinstalls when upgrade fails", + upgradeExit: "1", + wantCalls: []string{ + "brew list openboot", + "brew update", + "brew upgrade openbootdotdev/tap/openboot", + "brew reinstall openbootdotdev/tap/openboot", + "openboot version", + "openboot install --help", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + fakeBin := filepath.Join(tmpDir, "bin") + require.NoError(t, os.Mkdir(fakeBin, 0o755)) + + callLog := filepath.Join(tmpDir, "calls.log") + writeFakeCommand(t, fakeBin, "uname", `#!/bin/bash +case "${1:-}" in + -s) echo Darwin ;; + -m) echo arm64 ;; + *) exit 90 ;; +esac +`) + writeFakeCommand(t, fakeBin, "xcode-select", "#!/bin/bash\nexit 0\n") + writeFakeCommand(t, fakeBin, "brew", `#!/bin/bash +printf 'brew %s\n' "$*" >> "$CALL_LOG" +case "${1:-}" in + list) + [[ "${2:-}" == "openboot" ]] || exit 91 + exit 0 + ;; + update) exit 0 ;; + upgrade) exit "${BREW_UPGRADE_EXIT:-0}" ;; + reinstall) exit 0 ;; + *) exit 92 ;; +esac +`) + writeFakeCommand(t, fakeBin, "openboot", `#!/bin/bash +printf 'openboot %s\n' "$*" >> "$CALL_LOG" +case "${1:-}" in + version) echo "openboot version v-test" ;; + install) exit 0 ;; + *) exit 93 ;; +esac +`) + + // Feed the script through bash's stdin to exercise the documented + // `curl | bash` execution mode as well as the already-installed path. + cmd := exec.Command("/bin/bash", "-s", "--", "--help") + cmd.Stdin = bytes.NewReader(script) + cmd.Env = []string{ + "PATH=" + fakeBin + ":/usr/bin:/bin", + "HOME=" + filepath.Join(tmpDir, "home"), + "CALL_LOG=" + callLog, + "BREW_UPGRADE_EXIT=" + tt.upgradeExit, + } + + output, err := cmd.CombinedOutput() + require.NoError(t, err, "install.sh output:\n%s", output) + + calls, err := os.ReadFile(callLog) + require.NoError(t, err) + assert.Equal(t, tt.wantCalls, strings.Split(strings.TrimSpace(string(calls)), "\n")) + assert.Contains(t, string(output), "OpenBoot is already installed — updating...") + assert.Contains(t, string(output), "✓ OpenBoot updated!") + assert.Contains(t, string(output), "openboot version v-test") + assert.NotContains(t, string(output), "Reinstall?") + assert.NotContains(t, string(output), "Installing OpenBoot via Homebrew...") + }) + } +} + +func writeFakeCommand(t *testing.T, dir, name, contents string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(contents), 0o755)) +}