From 4b6df061caff2c0c08ab7a870c5601ea8255daba Mon Sep 17 00:00:00 2001 From: fullstackjam Date: Sun, 2 Aug 2026 10:57:53 +0800 Subject: [PATCH 1/2] docs: realign branch-protection sources of truth with live protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #161 made `contract schema (L2)`, `curl|bash smoke`, and `old-cli compat` run on pull_request and added them to branch protection on main, but left the in-repo files describing the previous three-check world. Live protection requires six contexts; the repo claimed three. The maintainer confirmed the six-check state is intended, so the in-repo files catch up to protection rather than the reverse. Live protection is unchanged by this commit. - .github/required-checks.txt: add the three missing contexts, so the file matches `.required_status_checks.contexts` exactly. - docs/MERGE_POLICY.md: drop the pre-merge/post-merge split, which no longer describes anything real — every job in test.yml fires on push, pull_request, and both dispatch events with no job-level `if:`, and vm-e2e on push and pull_request. "Why these three" becomes "Why these six". Adds the external-state trade-off these required checks carry, and notes the drift sensor's blind spot: it compares the file against workflow job names only, never against live protection, which is how this drift went unnoticed. - docs/HARNESS.md: the curl|bash smoke row said "push to main / dispatch" and the L2 contract row said "CI"; both now say "every PR". --- .github/required-checks.txt | 3 ++ docs/HARNESS.md | 4 +-- docs/MERGE_POLICY.md | 60 +++++++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/.github/required-checks.txt b/.github/required-checks.txt index 535ee75..5d2ad3b 100644 --- a/.github/required-checks.txt +++ b/.github/required-checks.txt @@ -10,4 +10,7 @@ lint unit (L1) +contract schema (L2) +curl|bash smoke +old-cli compat vm-e2e diff --git a/docs/HARNESS.md b/docs/HARNESS.md index 4b84bfb..b2b2850 100644 --- a/docs/HARNESS.md +++ b/docs/HARNESS.md @@ -49,12 +49,12 @@ Three regulation categories: | Arch. | `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. | L2 contract schema (against openboot-contract repo) | every PR | `.github/workflows/test.yml` `contract` job | | Behav. | L3 e2e binary | release | `make test-e2e` | | 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) | push to main / dispatch | `.github/workflows/test.yml` `curl-bash-smoke` job | +| Behav. | curl\|bash smoke (install.sh + mock server) | every PR | `.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 | diff --git a/docs/MERGE_POLICY.md b/docs/MERGE_POLICY.md index 71e2fcb..3ac0e64 100644 --- a/docs/MERGE_POLICY.md +++ b/docs/MERGE_POLICY.md @@ -9,32 +9,22 @@ branch protection rules configured at: If the two ever drift, **GitHub is authoritative** — file a PR against this doc to bring it back in sync. -## CI stages +## Required checks (block merge) -CI is split into two stages to keep PR feedback fast. - -### Pre-merge (required — blocks merge) - -Runs on every PR. Must pass before merge. +All six run on every PR and again on push to `main`. There is no +post-merge-only tier: every job in `.github/workflows/test.yml` fires on +`push`, `pull_request`, `repository_dispatch`, and `workflow_dispatch` +alike, and `vm-e2e` on `push` and `pull_request`. | Check | Workflow | Why required | |---|---|---| | `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. | +| `contract schema (L2)` | Test | Validates remote-config / snapshot JSON against the `openboot-contract` schemas, and asserts the CLI decoders consume the canonical fixtures losslessly. | +| `curl\|bash smoke` | Test | Builds binary + starts mock server. Confirms `scripts/install.sh` still bootstraps the CLI. | +| `old-cli compat` | Test | Runs the previous release binary against the current mock server. Catches server-side changes that would break already-shipped CLIs. | | `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`) - -Does not block merge. Catches regressions on the merged state before -the auto-release sensor can tag. Runs on `workflow_dispatch` and -`repository_dispatch` too. - -| Check | Workflow | Why post-merge | -|---|---|---| -| `contract schema (L2)` | Test | Clones external repo + pip install — too slow for every PR. Validates remote-config / snapshot JSON against the `openboot-contract` schemas. | -| `curl\|bash smoke` | Test | Builds binary + starts mock server — too slow for every PR. Confirms `scripts/install.sh` still bootstraps the CLI. | -| `old-cli compat` | Test | Downloads previous release from GitHub — too slow and network-dependent for every PR. Catches server-side changes that would break already-shipped CLIs. | - ### Not required (and why) | Check | Status | Reason | @@ -56,7 +46,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 three +## Why these six Each required check covers a class of regression that has shipped to users in past commits: @@ -65,14 +55,33 @@ 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). +- `contract schema (L2)` catches CLI ↔ server wire drift. Tolerant + decoders like `UnmarshalRemoteConfigFlexible` will silently repair, + move, or drop fields; only a canonical-fixture comparison notices. +- `curl|bash smoke` catches breakage in the one install path every new + user takes, which no Go test exercises end to end. +- `old-cli compat` catches server-side changes that break CLIs already + on users' machines — the one regression class the current binary's + own tests structurally cannot see. - `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 -block PRs, because they're too slow or network-dependent to require on -every push to a feature branch. +### The cost of requiring the network-dependent three + +`old-cli compat` downloads the previous GitHub release, and +`contract schema (L2)` checks out `openboot-contract@main`. As *required* +checks they make every merge depend on external state: a GitHub API +blip, a yanked release asset, or a red `openboot-contract` main blocks +all PRs, including ones that touch neither. That is the deliberate +trade — the two regression classes they cover (shipped-CLI breakage and +silent wire drift) are invisible to every other check, and a merge that +introduces one is discovered by users rather than by CI. + +If external flakiness starts blocking unrelated work, the escape hatch +is the documented bypass under *Operating principles*, not quietly +dropping the contexts from protection while this doc still lists them — +that is the exact drift this file exists to prevent. ## How to change this policy @@ -82,6 +91,11 @@ The required-checks list has an in-repo source of truth: [`.github/workflows/harness.yml`](../.github/workflows/harness.yml) fails on PRs that desync it from the workflow `name:` values. +That sensor only compares the file against workflow job names — it has +no visibility into live branch protection, so a context added or removed +in the GitHub UI alone drifts silently. Step 3 below is the only thing +that catches it. + 1. Open a PR that edits this file **and** `.github/required-checks.txt` with the proposed change. 2. In the same PR, update live branch protection via the GitHub UI **or** From 98c0050758777e1089851d9822464d8d383be0ff Mon Sep 17 00:00:00 2001 From: fullstackjam Date: Sun, 2 Aug 2026 11:16:58 +0800 Subject: [PATCH 2/2] docs: correct CI claims found by review fact-check A fact-check of the previous commit against the actual workflows found three wrong claims in the new text. All verified by reading the files. - `vm-e2e` does not run on push to `main`. vm-e2e-spike.yml scopes its push trigger to the `test/vm-e2e-speed` spike branch, so the claim "all six run on every PR and again on push to main" was false. It is PR-only, now stated as such. - `curl|bash smoke` never executes `scripts/install.sh`. The job curls `localhost:18888/testuser/test-config/install`, which mock-server.py serves as a synthetic stub that execs the freshly built binary in dry-run mode; no workflow references scripts/install.sh at all. The real installer is covered in L1 by install_script_test.go, which pipes it through `/bin/bash -s`. Both the table row (pre-existing error) and the new "no Go test exercises it end to end" bullet were wrong. - The network-dependency section said "three" but named two, and claimed a GitHub API blip or yanked asset blocks all PRs. `old-cli compat` ends its lookup with `|| true` and gates every later step on a non-empty version, so it passes green having tested nothing. Rewritten to name the real asymmetry: L2 blocks, cli-compat fails open. Also corrects "fails on PRs" for the drift sensor, which is continue-on-error, and notes its second blind spot: it only checks that listed checks have jobs, never that required contexts are listed. --- docs/HARNESS.md | 2 +- docs/MERGE_POLICY.md | 68 +++++++++++++++++++++++++++----------------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/docs/HARNESS.md b/docs/HARNESS.md index b2b2850..2438cd5 100644 --- a/docs/HARNESS.md +++ b/docs/HARNESS.md @@ -54,7 +54,7 @@ Three regulation categories: | 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 — mock-served install script piped into `bash`, driving the built binary through a config install (**not** `scripts/install.sh`; that's the L1 row above) | every PR | `.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 | diff --git a/docs/MERGE_POLICY.md b/docs/MERGE_POLICY.md index 3ac0e64..d848d4d 100644 --- a/docs/MERGE_POLICY.md +++ b/docs/MERGE_POLICY.md @@ -11,17 +11,21 @@ this doc to bring it back in sync. ## Required checks (block merge) -All six run on every PR and again on push to `main`. There is no -post-merge-only tier: every job in `.github/workflows/test.yml` fires on -`push`, `pull_request`, `repository_dispatch`, and `workflow_dispatch` -alike, and `vm-e2e` on `push` and `pull_request`. +All six run on every PR. There is no post-merge-only tier: every job in +`.github/workflows/test.yml` fires on `push` (to `main`/`master`), +`pull_request`, `repository_dispatch` (`contract-updated`), and +`workflow_dispatch` alike, with no job-level `if:` gating. + +`vm-e2e` is the exception in the other direction — it is **PR-only**. Its +workflow's `push` trigger is scoped to the `test/vm-e2e-speed` spike +branch, so it does not run on `main` at all. | Check | Workflow | Why required | |---|---|---| | `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. | | `contract schema (L2)` | Test | Validates remote-config / snapshot JSON against the `openboot-contract` schemas, and asserts the CLI decoders consume the canonical fixtures losslessly. | -| `curl\|bash smoke` | Test | Builds binary + starts mock server. Confirms `scripts/install.sh` still bootstraps the CLI. | +| `curl\|bash smoke` | Test | Builds the binary, starts `scripts/mock-server.py`, and pipes a served install script into `bash`, driving `openboot install -s -u ` end to end against a mock API. Despite the name it does **not** exercise `scripts/install.sh` — that is covered in L1 by `test/integration/install_script_test.go`. | | `old-cli compat` | Test | Runs the previous release binary against the current mock server. Catches server-side changes that would break already-shipped CLIs. | | `vm-e2e` | vm-e2e-spike | Exercises the destructive install paths and TUI choreography on a fresh Apple Silicon macOS VM. | @@ -58,8 +62,9 @@ users in past commits: - `contract schema (L2)` catches CLI ↔ server wire drift. Tolerant decoders like `UnmarshalRemoteConfigFlexible` will silently repair, move, or drop fields; only a canonical-fixture comparison notices. -- `curl|bash smoke` catches breakage in the one install path every new - user takes, which no Go test exercises end to end. +- `curl|bash smoke` is the only check that drives a real built binary + through a config install against a live HTTP API, catching wiring + breakage that faked-runner tests structurally cannot reach. - `old-cli compat` catches server-side changes that break CLIs already on users' machines — the one regression class the current binary's own tests structurally cannot see. @@ -67,19 +72,28 @@ users in past commits: cannot safely run inside L1, including real Homebrew installs and the install-wizard choreography on a fresh macOS VM. -### The cost of requiring the network-dependent three - -`old-cli compat` downloads the previous GitHub release, and -`contract schema (L2)` checks out `openboot-contract@main`. As *required* -checks they make every merge depend on external state: a GitHub API -blip, a yanked release asset, or a red `openboot-contract` main blocks -all PRs, including ones that touch neither. That is the deliberate -trade — the two regression classes they cover (shipped-CLI breakage and -silent wire drift) are invisible to every other check, and a merge that -introduces one is discovered by users rather than by CI. - -If external flakiness starts blocking unrelated work, the escape hatch -is the documented bypass under *Operating principles*, not quietly +### The cost of requiring the two network-dependent checks + +`contract schema (L2)` checks out `openboot-contract@main` and +`old-cli compat` downloads the previous GitHub release, so requiring +both ties every merge to state outside this repo. They fail in opposite +directions, and the difference is worth knowing before you trust either +badge: + +- `contract schema (L2)` **blocks**. A red or mid-edit `openboot-contract` + main stops every PR, including ones that touch neither the contract nor + the decoders. That is the deliberate trade: tolerant decoders like + `UnmarshalRemoteConfigFlexible` hide wire drift from every other check, + so the alternative to a blocked PR is a user bug report. +- `old-cli compat` **passes silently**. Its release lookup ends in + `|| true` and writes a possibly-empty `version=`, and every step after + it is gated on `steps.prev.outputs.version != ''`. A GitHub API blip, + or no stable release still carrying the arch asset, yields a green + check that ran no compat test at all. It cannot block a PR — but a + green tick is therefore not evidence that compat was verified. + +If external flakiness does start blocking unrelated work, the escape +hatch is the documented bypass under *Operating principles*, not quietly dropping the contexts from protection while this doc still lists them — that is the exact drift this file exists to prevent. @@ -89,12 +103,14 @@ The required-checks list has an in-repo source of truth: [`.github/required-checks.txt`](../.github/required-checks.txt). The `required-checks alignment (drift)` sensor in [`.github/workflows/harness.yml`](../.github/workflows/harness.yml) -fails on PRs that desync it from the workflow `name:` values. - -That sensor only compares the file against workflow job names — it has -no visibility into live branch protection, so a context added or removed -in the GitHub UI alone drifts silently. Step 3 below is the only thing -that catches it. +flags PRs that desync it from the workflow `name:` values. It is +`continue-on-error: true`, so it annotates rather than blocks. + +It also has two blind spots. It never reads live branch protection, so a +context added or removed in the GitHub UI alone drifts silently; and it +only checks one direction — every line in the file must map to a job, +but a required context missing from the file is not flagged. Step 3 +below is the only thing that catches either. 1. Open a PR that edits this file **and** `.github/required-checks.txt` with the proposed change.