diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47f12d1..dce1683 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: ci # pst-2sb: "render preview images", "unit tests (vitest)", and # "e2e tests (playwright)" are required status checks on main, and -# PRs merge via auto-merge — a required check that never reports +# PRs merge via the city merge-green-prs controller order — a required check that never reports # hangs the merge forever. So this workflow must run on EVERY PR: do # not add a workflow-level `paths:` filter here. Cheap skipping is # handled inside the render job (paths-filter step), which still diff --git a/.github/workflows/param-sweep.yml b/.github/workflows/param-sweep.yml index 6bd6a02..199201c 100644 --- a/.github/workflows/param-sweep.yml +++ b/.github/workflows/param-sweep.yml @@ -20,7 +20,7 @@ # Always-reporting (pst-2sb): the pull_request trigger deliberately # has NO paths filter. "wasm param sweep" (the gate job below) is a # required status check, and a required check that never reports -# hangs auto-merge forever. Instead of workflow-level path gating, +# hangs the controller merge forever. Instead of workflow-level path gating, # every PR runs the cheap `select` job (~20s) and the script's skip # mode makes sweep-irrelevant PRs pass in seconds. The push trigger # keeps its paths filter — main pushes aren't gated by required diff --git a/.github/workflows/pr-automerge.yml b/.github/workflows/pr-automerge.yml deleted file mode 100644 index f36749a..0000000 --- a/.github/workflows/pr-automerge.yml +++ /dev/null @@ -1,50 +0,0 @@ -# pst-2sb: arm squash auto-merge on every PR targeting main the -# moment it opens. With branch protection requiring all four CI -# checks (unit, e2e, render, sweep gate) in strict up-to-date mode, -# "armed" means the PR merges itself when CI goes green — no human -# or agent merge step. -# -# Token choice matters: the merge that auto-merge eventually performs -# is attributed to the actor who ENABLED it. If that actor is -# github-actions[bot] (GITHUB_TOKEN), the resulting push to main does -# not trigger workflows — no full-sweep coverage run, no render -# canonicalization, and no pr-autoupdate cascade. So prefer the -# THUMBNAIL_PUSH_TOKEN PAT and warn loudly on fallback. -# -# Fork PRs: secrets (including THUMBNAIL_PUSH_TOKEN) are not exposed -# to pull_request runs from forks and GITHUB_TOKEN there is -# read-only, so this job fails soft — fork PRs keep the manual merge -# path. Everything in this pilot works from same-repo branches. -name: pr-automerge - -on: - pull_request: - types: [opened, reopened, ready_for_review] - branches: [main] - -permissions: - contents: write - pull-requests: write - -jobs: - arm: - name: enable auto-merge - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Enable squash auto-merge - if: ${{ !github.event.pull_request.draft }} - env: - GH_TOKEN: ${{ secrets.THUMBNAIL_PUSH_TOKEN || secrets.GITHUB_TOKEN }} - HAS_PAT: ${{ secrets.THUMBNAIL_PUSH_TOKEN != '' }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - if [ "$HAS_PAT" != "true" ]; then - echo "::warning::THUMBNAIL_PUSH_TOKEN not set — arming auto-merge with GITHUB_TOKEN; the eventual merge push to main will NOT trigger main's workflows (full sweep, render canonicalization, pr-autoupdate)." - fi - # If every required check is somehow already green at open - # time, --auto errors with "clean status"; fall through to - # an immediate merge, which branch protection still gates - # server-side. - gh pr merge --auto --squash "$PR_NUMBER" -R "$GITHUB_REPOSITORY" \ - || gh pr merge --squash "$PR_NUMBER" -R "$GITHUB_REPOSITORY" diff --git a/.github/workflows/pr-autoupdate.yml b/.github/workflows/pr-autoupdate.yml index 1487436..5ff87c6 100644 --- a/.github/workflows/pr-autoupdate.yml +++ b/.github/workflows/pr-autoupdate.yml @@ -1,11 +1,11 @@ # pst-2sb: strict up-to-date branch protection strands every open PR -# whenever main advances (seen on PRs #11, #13, #21) — armed -# auto-merge then waits forever on an "out of date" branch. On each +# whenever main advances (seen on PRs #11, #13, #21) — the +# controller merge then waits forever on an "out of date" branch. On each # push to main, update every open PR branch via the update-branch -# API so their checks re-run against the new base and auto-merge can +# API so their checks re-run against the new base and the controller merge can # complete. # -# Cascade (intentional, accepted): with N armed PRs open, each merge +# Cascade (intentional, accepted): with N open PRs, each merge # to main re-updates the remaining N-1 and re-runs their CI, so # merges serialize at roughly one per CI wall-clock. That's the cost # of strict mode + full autonomy; fine at this repo's PR volume. diff --git a/.gitignore b/.gitignore index 67c341e..6d71997 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,7 @@ playwright/.cache/ # Gas Town (added by gt) .claude/ +.gc/ .opencode/ .logs/ state.json diff --git a/AGENTS.md b/AGENTS.md index adad2cc..123c84d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,3 +125,15 @@ rm -rf directory # not: rm -r directory ssh -o BatchMode=yes ... # fail instead of prompting apt-get -y ... # auto-confirm ``` + +## PR & merge workflow — the codex gate + +Open a PR from your `gc-pilot/` branch; never push `main`. Do +**not** click merge and do **not** enable GitHub auto-merge — a controller +order squash-merges once **every reported check** is green. That includes +**`codex-review`**, a real pre-merge gate set by an independent cross-model +reviewer (not a GitHub workflow). If it fails, the reviewer posts file:line +findings and you (or a fix bead) address them **on the same branch** and +push — the push triggers a fresh review of the new SHA. Your job ends when +the PR is open and CI is green. Full mechanism: +[docs/ci.md](docs/ci.md#merge-workflow-codex-gate--controller-merge). diff --git a/docs/ci.md b/docs/ci.md index 289d094..dcfef07 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -1,26 +1,26 @@ # Continuous integration -CI lives in four workflows: +CI lives in three workflows: - [`.github/workflows/ci.yml`](../.github/workflows/ci.yml) — the `render`, `unit`, and `e2e` jobs (pushes to `main` + PRs). - [`.github/workflows/param-sweep.yml`](../.github/workflows/param-sweep.yml) — the wasm param-sweep connectivity guard (pushes to `main` + PRs). -- [`.github/workflows/pr-automerge.yml`](../.github/workflows/pr-automerge.yml) - — arms squash auto-merge on every PR the moment it opens. - [`.github/workflows/pr-autoupdate.yml`](../.github/workflows/pr-autoupdate.yml) — on every push to `main`, updates all open PR branches so strict up-to-date mode can't strand them. -## The PR lifecycle: merges are fully autonomous (pst-2sb) +## The PR lifecycle: merges are fully autonomous (pst-2sb → codex gate, 2026-07-28) -PRs against `main` run CI and merge entirely on their own — there is -no human or agent merge step: +PRs against `main` run CI, get a cross-model code review, and merge +entirely on their own — there is no human merge step: -1. **PR opened** (or reopened / marked ready) → `pr-automerge.yml` - enables squash auto-merge on it. -2. **Checks run.** All four required contexts report a conclusion on - *every* PR — that's what makes auto-merge safe to arm blindly: +1. **PR opened** (branch `gc-pilot/*`) → the city's + `codex-review-gate-stuff` controller order slings a review of the + head SHA to the `stuff-codex-reviewer` agent, which sets the + `codex-review` commit status (see **Merge workflow** below). +2. **Checks run.** All CI contexts report a conclusion on *every* + PR — that's what lets the controller merge without human input: - `unit tests (vitest)` and `e2e tests (playwright)` always run. - `render preview images` always runs; a paths-filter step inside the job skips the heavy render/export work when nothing @@ -33,23 +33,23 @@ no human or agent merge step: mode marks the PR out of date → `pr-autoupdate.yml` updates every open PR branch via the update-branch API, re-running their checks against the new base. -4. **All four checks green + branch up to date** → GitHub merges the - PR itself (squash). +4. **All five checks green + branch up to date** → the city's + `merge-green-prs` controller order merges the PR (squash, + delete branch) on its next 3-minute pass. Consequences worth knowing: -- **The merge cascade is intentional.** With N armed PRs open, each +- **The merge cascade is intentional.** With N open PRs, each merge re-updates the other N−1 and re-runs their CI, so merges serialize at roughly one per CI wall-clock. Accepted cost of strict mode + autonomy. -- **Token attribution matters.** Auto-merge's eventual merge is - attributed to whoever *enabled* it, and pushes/merges made with - `GITHUB_TOKEN` don't trigger workflows. Both new workflows - therefore authenticate with the `THUMBNAIL_PUSH_TOKEN` PAT (and - warn on fallback): with `GITHUB_TOKEN` the merge wouldn't fire - `main`'s full sweep, render canonicalization, or the next - auto-update round, and branch updates would strand PR runs in - `action_required` (pst-dm9). +- **Token attribution matters.** Pushes/merges made with + `GITHUB_TOKEN` don't trigger workflows. `pr-autoupdate.yml` + therefore authenticates with the `THUMBNAIL_PUSH_TOKEN` PAT (and + warns on fallback): with `GITHUB_TOKEN`, branch updates would + strand PR runs in `action_required` (pst-dm9). The controller + merge runs under the operator's `gh` auth, so merges to `main` + fire the full sweep / render canonicalization normally. - **update-branch 422s can be transient.** Right after `main` moves, GitHub briefly reports PR mergeability as unknown and the update-branch API 422s even for genuinely-behind PRs (seen on the @@ -63,19 +63,21 @@ Consequences worth knowing: fire `pr-autoupdate.yml`. PRs stranded by such a bot-only main move get picked up on the next real push, or run the workflow manually (`workflow_dispatch`). -- **Fork PRs are excluded**: fork `pull_request` runs get no secrets - and a read-only `GITHUB_TOKEN`, so arming fails soft and those PRs - keep the manual merge path. +- **Fork PRs are excluded**: the controller only merges `gc-pilot/*` + branches, so fork PRs keep the manual merge path. - **Never make a check required unless it reports on every PR.** A required context that sometimes doesn't run (e.g. behind a - workflow-level `paths:` filter) hangs auto-merge forever. That's - why neither `ci.yml` nor `param-sweep.yml` path-filters its + workflow-level `paths:` filter) hangs the controller merge forever + (it waits for every reported and required check). That's why + neither `ci.yml` nor `param-sweep.yml` path-filters its `pull_request` trigger — cheap skipping happens *inside* jobs that - still report. + still report. (`codex-review` is the exception that proves the + rule: it always reports because the gate order slings a review for + every `gc-pilot/*` head SHA.) Required status checks on `main` (strict mode, set via the branch protection API): `unit tests (vitest)`, `e2e tests (playwright)`, -`render preview images`, `wasm param sweep`. +`render preview images`, `wasm param sweep`, `codex-review`. ## Job / trigger matrix @@ -86,8 +88,8 @@ protection API): `unit tests (vitest)`, `e2e tests (playwright)`, | `e2e` (Playwright, needs `unit`) | Always | Always | `ci.yml` | | `sweep` shards (wasm param sweep) | Selective — only the models the PR touched, sharded across parallel jobs (see below); skipped entirely on sweep-irrelevant PRs | Full sweep whenever sweep-relevant paths changed (the coverage guard) | `param-sweep.yml` | | `gate` (`wasm param sweep`, the required context) | Always reports — reduces shard results to one conclusion, passes immediately in skip mode | Same | `param-sweep.yml` | -| `arm` (enable auto-merge) | On open / reopen / ready-for-review | — | `pr-automerge.yml` | | `update` (update open PR branches) | — | Always (+ manual dispatch) | `pr-autoupdate.yml` | +| `codex-review` (required context) | Set by the `stuff-codex-reviewer` agent via the statuses API — not a workflow job | — | city `codex-review-gate-stuff` order | The `unit` and `e2e` jobs run on every PR regardless; both are cheap enough that paths-filtering isn't worth the complexity. @@ -274,7 +276,7 @@ outcomes: `models/**`, `libs/README.md`, `lib/wasm/**`, `package-lock.json`, and the workflow file itself. The `pull_request` trigger deliberately has **no** paths filter (pst-2sb): `wasm param sweep` is a required status check, and a -required check that never reports hangs auto-merge — so every PR +required check that never reports hangs the merge — so every PR runs the ~20-second `select` job, and the script's skip mode makes sweep-irrelevant PRs pass in seconds via the `gate` job instead of not reporting at all. @@ -427,3 +429,49 @@ ship as tracked files. `scripts/vendor-libs.sh` runs as the npm - **Sweep failure on a case listed in known-failures.ts** — the entry was removed or the label changed; re-check the tracking bead before re-registering. + +## Merge workflow (codex gate + controller merge) + +**Nobody hand-merges, and nobody enables GitHub native auto-merge.** Workers +open a PR from branch `gc-pilot/`, never push `main`, and their job +ends when the PR is open and CI is green. A city controller order +(`merge-green-prs`, 3-min cooldown, no LLM) squash-merges each open +`gc-pilot/* → main` PR once **every reported check** is green — it requires +both `gh pr checks --required` and `gh pr checks` (all reported contexts) to +pass, then independently verifies each branch-protection required context has +a genuine success on the head SHA before running +`gh pr merge --squash --delete-branch`. It never uses `--admin`. + +`main` has **five required status checks**: `unit tests (vitest)`, +`e2e tests (playwright)`, `render preview images`, `wasm param sweep`, and +**`codex-review`** (added 2026-07-28, matching underware-planner). The fifth +is not a GitHub workflow — it is set by an independent cross-model review +agent (`stuff-codex-reviewer`): + +- **`state=success`** when the PR's findings are non-blocking (style nits, + pre-existing issues) — recorded as bead notes / follow-ups, never blocking. +- **`state=failure`** only for defects **this PR introduces or worsens**: + correctness, broken invariants, geometry/watertightness regressions, + contract violations, security. + +A second controller order (`codex-review-gate-stuff`, 3-min cooldown) drives +it: for each open `gc-pilot/*` PR it slings a review bead to the reviewer when +the head SHA has no `codex-review` status. On failure it first **nudges the +PR's live author session** to fix on the same branch (no bead); it only +dispatches a fix bead when there is no live author or the nudge goes +unanswered past the grace window (~30 min). After 3 dispatched fix beads on +one PR it escalates to `human` and stops. A push to the branch resets the +cycle (new head SHA → fresh review). + +Settled context the reviewer will not relitigate: the render/invariants +pipeline conventions in `AGENTS.md`, the vendored lib pins and patches +(`libs/README.md`, BOSL2 `456fcd8`), the WASM-preview-only split, and the +MIT / CC BY-NC-SA licensing split. + +> Historical note: `pr-automerge.yml` (pst-2sb, 2026-07-13) used to arm GitHub +> native auto-merge on every PR. It was removed 2026-07-28 when the codex gate +> landed — native auto-merge satisfies only *required* checks and would bypass +> the controller's stricter all-reported-checks + verified-contexts condition, +> leaving two competing merge paths. The controller order is the only merge +> path now. (`pr-autoupdate.yml` stays — it only keeps branches current with +> `main`, it never merges.) diff --git a/docs/imported-model-opengrid-playbook.md b/docs/imported-model-opengrid-playbook.md index 80c0b17..851ef03 100644 --- a/docs/imported-model-opengrid-playbook.md +++ b/docs/imported-model-opengrid-playbook.md @@ -206,9 +206,10 @@ npx playwright test tests/e2e/stl-download.spec.ts # download route - Branch `gc-pilot/` from latest `origin/main`. - PR to `main`; never push `main` directly. -- Auto-merge lands the PR on green with no human merge step, and - open PR branches are auto-updated when `main` moves — see - [docs/ci.md](ci.md) for the full PR lifecycle. +- The `merge-green-prs` controller order lands the PR once every + reported check — including the `codex-review` gate — is green, with + no human merge step; open PR branches are auto-updated when `main` + moves — see [docs/ci.md](ci.md) for the full PR lifecycle. - Record the PR URL on the bead; expect operator corrections as follow-up beads rather than review comments (that's how this whole series ran).