Skip to content

feat(auto-merge): require ui-visual and design-review success for UI PRs before arming (warren-dbef) - #1360

Merged
jayminwest merged 3 commits into
ui-visual/golden-approval-4780from
ui-visual/require-checks-dbef
Oct 8, 2026
Merged

jayminwest merged 3 commits into
ui-visual/golden-approval-4780from
ui-visual/require-checks-dbef

Conversation

@jayminwest

Copy link
Copy Markdown
Owner

Stacked on #1356 (warren-4780). Article IX: needs a human merge.
This PR changes .github/workflows/auto-merge.yml, so the Article IX gate refuses auto-merge, and it carries no-automerge as well. It targets ui-visual/golden-approval-4780 so the diff stays reviewable. Retarget it to main after #1356 merges.
The branch also merges current main (it needs #1359's design-review workflow). Until #1356's branch picks up main, "Files changed" includes #1359's files. The commit feat(auto-merge): ... holds this change alone.

What

Seed warren-dbef (plan pl-10db step 16). A PR whose diff touches src/ui/** or scripts/ui-visual/** now arms auto-merge only after the ui-visual and design-review check runs both report success on its exact head sha. Every other PR behaves as before.

  • scripts/ui-visual/required-checks.ts: new self-contained gate in the style of baseline-approval.ts. The workflow runs it from the base branch tip with the same fail-closed wrapper: only hit=false from a gate that exited 0 permits arming.
    • Only check runs count. Commit statuses are ignored. A check run must come from the GitHub Actions app (app.slug == github-actions).
    • A ui-visual run counts only when its check suite belongs to a pull_request run of .github/workflows/ui-visual.yml on the same head. That excludes a second workflow with a job of the same name, and the update_goldens dispatch, which reports success without comparing.
    • Among the runs that count, the newest one decides.
    • The refusal names each check and its state, for example ui-visual concluded failure; design-review is queued on <sha>. It goes to the log, a notice annotation, and the job summary.
  • Timing (auto-merge.yml): the workflow now also runs on workflow_run of UI design review (completed). One completes after every ui-visual run, success or failure.
    • A new targets job holds the existing admission if:. On pull_request it emits this PR. On workflow_run it runs the sweep (required-checks.ts targets): open, non-draft, unarmed PRs by the owner or an AUTO_MERGE_BOT_LOGIN login, without no-automerge, whose head has both checks green. All of it is read from the API, never from the payload.
    • enable-auto-merge is now a matrix over that list and judges every PR with all three gates (Article IX, baseline approval, UI checks) from scratch.
    • The first run after a push refuses while the checks are pending, and disarms any earlier arm. The disarm step now covers both UI gates. --match-head-commit and the head-moved guard from feat(ui-visual): human-only approval for golden baseline changes; auto-merge refuses without it (warren-4780) #1356 are kept.
    • Rejected alternatives: a check_run trigger (a GITHUB_TOKEN-written check does not start workflows), polling (holds a runner for the length of a review), and re-running the first run (it replays stale labels).
  • baseline-approval.ts adds required-checks.ts to GATE_PATHS, so a change to the new gate always needs a human merge.
  • Docs: docs/design/ui-visual-gate.md gets a new "The merge gate" section, the dbef scope row moves to shipped, and the names and limits are updated. AGENTS.md "Protected paths" gains one sentence pointing there. The scripts/ui-visual/README.md is updated too.

Validation

Limits (documented in the design record)

  • A workflow on a same-repo branch with checks: write can create a design-review check as GitHub Actions. That check is written through the Checks API, so its check suite cannot prove its origin. Follow-up suggested: protect .github/workflows/ with a human-merge rule.
  • UI PRs into a base other than main never get both checks, so they never auto-merge.
  • Sweep runs report on main's Actions page, not on the PR.

…and reports a design-review check (warren-a694) (#1359)

.github/workflows/ui-design-review.yml runs the ui-design-review skill
through claude-code-action (pinned by commit) after each ui-visual run,
validates the verdict with scripts/design-review/verdict.ts, and reports
the result as the `design-review` check run on the PR head plus one sticky
comment keyed by <!-- design-review -->.

- gate (pull_request_target): every PR head into main gets the check at
  once, success "Skipped: no src/ui changes" or queued, so the check
  warren-dbef makes required never hangs.
- prepare (workflow_run): trusts the artifact's PR number only when that
  PR's head is the run's head sha, maps changed files to manifest pages,
  stages only in-scope screenshots and the src/ui diff.
- evaluate: contents: read token, scoped tools, no web tools, fixed
  prompt with no PR text, model/turn/budget caps from repo variables.
- report: re-validates in a fresh checkout and fails closed on a missing
  or invalid verdict.

Logic lives in pure, unit-tested modules under scripts/design-review/.
@jayminwest jayminwest added the no-automerge Human-gated PR: auto-merge workflow must never enable auto-merge label Oct 8, 2026
@jayminwest
jayminwest merged commit d03b7e6 into ui-visual/golden-approval-4780 Oct 8, 2026
5 checks passed
@jayminwest
jayminwest deleted the ui-visual/require-checks-dbef branch October 8, 2026 23:30
jayminwest added a commit that referenced this pull request Oct 9, 2026
…o-merge refuses without it (warren-4780) (#1356)

* feat(ui-visual): human-only approval for golden baseline changes; auto-merge refuses without it (warren-4780)

auto-merge.yml gains a UI baseline approval check next to Article IX. A PR
that changes scripts/ui-visual/__golden__/ or the comparison machinery arms
only when a human approver applied ui-baseline-approved to the exact baseline
content the head carries. The gate reads the labeler from the issue events
API (never label presence), the head the approver saw from the repository
activity API, and compares baseline object ids. It runs the base branch's
copy of scripts/ui-visual/baseline-approval.ts, fails closed, and disarms on
refusal. The arm step now also refuses to arm a head the gates did not judge.

Adds the label, CODEOWNERS routing, and docs/design/ui-visual-gate.md.

* feat(auto-merge): require ui-visual and design-review success for UI PRs before arming (warren-dbef) (#1360)

* docs(ui): AGENTS.md UI conventions section and a UI definition of done (warren-598d) (#1361)

* feat(ui-visual): design-review workflow runs the evaluator on UI PRs and reports a design-review check (warren-a694) (#1359)

.github/workflows/ui-design-review.yml runs the ui-design-review skill
through claude-code-action (pinned by commit) after each ui-visual run,
validates the verdict with scripts/design-review/verdict.ts, and reports
the result as the `design-review` check run on the PR head plus one sticky
comment keyed by <!-- design-review -->.

- gate (pull_request_target): every PR head into main gets the check at
  once, success "Skipped: no src/ui changes" or queued, so the check
  warren-dbef makes required never hangs.
- prepare (workflow_run): trusts the artifact's PR number only when that
  PR's head is the run's head sha, maps changed files to manifest pages,
  stages only in-scope screenshots and the src/ui diff.
- evaluate: contents: read token, scoped tools, no web tools, fixed
  prompt with no PR text, model/turn/budget caps from repo variables.
- report: re-validates in a fresh checkout and fails closed on a missing
  or invalid verdict.

Logic lives in pure, unit-tested modules under scripts/design-review/.

* feat(auto-merge): require ui-visual and design-review success for UI PRs before arming (warren-dbef)

* docs(ui): AGENTS.md UI conventions section and a UI definition of done (warren-598d)

Adds a UI conventions section to AGENTS.md that pairs each rule with the
check that holds it: the Tailwind arbitrary-value ratchet, the GritQL
primitive and inline-style bans, check:ui-visual, the golden baseline
approval label, and the design-review rubric. Folds the UI merge-gate
paragraph out of Protected paths into it and moves the repro-first
section next to it. The PR template gains a collapsed UI definition of
done, and src/ui/README.md links both UI design records.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-automerge Human-gated PR: auto-merge workflow must never enable auto-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant