From 4f073264b294861d89d3f0db41800f823b348d0b Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Sun, 16 Aug 2026 11:55:59 -0700 Subject: [PATCH] feat(demo): run real CI on the github rung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? The GitHub rung was described as the one where everything is real, and it was not. It took real pull requests from a real repository and pushed real commits back — but the build in the middle was the fake runner, passing instantly. `landed` there meant the queue had run its pipeline, not that anything had been built. That is a bad thing to be unclear about, because it is the one rung a reader is most likely to trust. Watching three pull requests merge in twenty seconds looks like a submit queue validating and landing them; nothing in the output says the validation was a stub. It was also the last thing separating the demo from the product. Speculation exists to test the *combination* — a batch on top of the in-flight changes assumed to land before it — and a fake runner exercises the trigger-and-poll loop while proving nothing about the combination it was built to check. The switch was already documented as an opt-in, three manual steps in a section further down. Nobody who has just watched the demo will go and do them, which meant the interesting half of the feature was reachable in principle and unused in practice. ### What? `demo/provider/github/profiles.yaml` selects the `githubactions` runner for `demo-queue` instead of inheriting the fake one. That is the whole change in behaviour: the queue now dispatches a `workflow_dispatch` per speculative batch, polls the resulting check, and decides merge or eject on the answer. The defaults are untouched, so a queue not listed still cannot reach GitHub by accident. `fake` and `git` are untouched: their builds stay instant, which is what keeps the first two rungs free and fast. The docs stop describing this as opt-in. "Using real CI" now covers what the *repository* must provide rather than what to switch on — a dispatchable workflow accepting the four `sq_*` inputs, and, the part that decides whether any of it means anything, a workflow that actually applies both input sets. A workflow that checks out the pull request and builds it is green about the wrong thing. Two consequences are stated where a reader meets them rather than buried: a land now takes as long as the workflow does, and every speculation path dispatched is a billable run — which makes the queue's build budget a spending control as much as a CI one. The ladder table gains a **Building it** column. It previously described where changes come from and what landing does, and said nothing about the build, which is how the fake runner stayed invisible in the summary a reader skims. The same paragraph now also says plainly that `fake` and `git` fake the *change provider* too, so `sq-files=` is how the analyzer sees paths there — true before this change and undocumented. Token permissions move Actions from "only if you switch the build runner" to required, and the classic-PAT line from "`repo`, plus `workflow` if…" to both. ## Test Plan - ✅ ran it: `PROVIDER=github make demo-requests COUNT=2` against a scratch repository opened pull requests 525 and 526 and landed both - ✅ the dispatches are real and are the queue's, confirmed from the Actions API rather than from the queue's own logs: two new `workflow_dispatch` runs at 18:53:26, both `success`, titled `SQ - demo-queue - speculative batch` — the workflow's own `run-name`, built from the `sq_queue` input the runner sent - ✅ before this change the same repository showed no `workflow_dispatch` run since 2026-08-10 and zero check-runs on a landed head, which is what established the gap in the first place - ✅ `make fmt`, `make lint` Not verified: cancellation. The runner cancels a build whose speculation path is abandoned, but the workflow used here finishes in seconds, so a run is almost always complete before there is anything to cancel — the same reason abandoned-path builds show up as post-terminal events in a fake run. It needs a slow workflow to exercise honestly. A note for anyone pointing this at their own repository: `ci.yml` and `behinddwalls/sq-demo` are marked in the file the same way `merge.yaml`'s remote is. A workflow triggered only by `pull_request` cannot be dispatched at all, and GitHub rejects the call rather than falling back. --- doc/howto/QUICKSTART.md | 39 +++++++------------ .../demo/provider/github/profiles.yaml | 30 ++++++++------ 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/doc/howto/QUICKSTART.md b/doc/howto/QUICKSTART.md index b4f04be9..19859165 100644 --- a/doc/howto/QUICKSTART.md +++ b/doc/howto/QUICKSTART.md @@ -4,14 +4,16 @@ Start the stack, put traffic through it, and watch changes land — beginning wi The stack always runs the same way. What changes is where the changes come from and what landing them does, chosen with `PROVIDER`: -| `PROVIDER` | A change is | Landing it | Needs | -|---|---|---|---| -| **`fake`** (default) | a URI, and nothing else | reports success without touching a repository | nothing | -| **`git`** | a branch in a bare repository on disk | a real fetch, cherry-pick and push | nothing | -| **`github`** | a real pull request | a real push to a real repository | a repository and a token | +| `PROVIDER` | A change is | Building it | Landing it | Needs | +|---|---|---|---|---| +| **`fake`** (default) | a URI, and nothing else | instant fake pass | reports success without touching a repository | nothing | +| **`git`** | a branch in a bare repository on disk | instant fake pass | a real fetch, cherry-pick and push | nothing | +| **`github`** | a real pull request | a real GitHub Actions run per batch | a real push to a real repository | a repository, a token, and CI minutes | They are a ladder, not alternatives: the same commands work on each rung, so you can start with the one that needs nothing and only pay for what you want to see next. Each is a directory of configuration under [`service/submitqueue/demo/provider/`](../../service/submitqueue/demo/provider) — the difference between rungs is two YAML files, not a code path. +The queue's own logic is real on every rung; what changes is how much of the world around it is. Two things are worth knowing before reading a `landed` as more than it is. On `fake` and `git` **the build is faked**, so `landed` means the pipeline ran, not that anything was tested. And on `fake` and `git` the change provider is faked too: it cannot read a repository to see what a change touched, so `make demo-requests` states the paths on the change URI itself (`sq-files=`) for the conflict analyzer to key on. A change submitted by hand on those rungs touches nothing as far as the analyzer can tell, and conflicts with nothing. + ## Start the stack ```bash @@ -252,9 +254,9 @@ For a **fine-grained** token, grant these repository permissions. Each is here b | Contents | Read and write | the git merger — clone, fetch, push to the target branch, and force-move each landed change's head branch | | Pull requests | Read | the change provider reads pull request metadata, and `land -pr` reads the head commit | | Pull requests | Read **and write** | only for `make demo-requests`, which opens pull requests | -| Actions | Read and write | only if you switch the build runner to GitHub Actions — dispatch a run, poll it, cancel it | +| Actions | Read and write | the build runner — dispatch a run per batch, poll it, cancel it | -A **classic** PAT needs `repo`, plus `workflow` if you use the GitHub Actions build runner. +A **classic** PAT needs `repo` and `workflow`. Two things people get caught by. Fine-grained tokens must have the repository explicitly selected under "Repository access" — org-owned repositories also need the org to have approved fine-grained tokens at all. And **Contents: Read and write is the one that cannot be reduced**: landing *is* pushing, so a read-only token fails at the last step, after everything else has appeared to work. @@ -299,7 +301,7 @@ Worth understanding *why* they show merged, because nothing called an API to clo `make demo-requests STACKED=true` submits a chain instead, each pull request targeting the previous one's branch. All of them land as one push to `main`, and all of them show as merged. -**Your CI does not run these builds.** Even here the build runner is fake, so a land takes seconds and costs no Actions minutes — GitHub supplies the change metadata and takes the push, and nothing else. That is worth knowing before reading `landed` as "CI passed on the combination", because it did not run. See [Using real CI](#using-real-ci) below. +**These builds are real.** This rung dispatches your workflow once per speculative batch and polls it to completion, so `landed` here means a build of that batch passed — not that a fake said so. It is the only rung where nothing is faked, and the only one that costs you Actions minutes. [Using real CI](#using-real-ci) below covers what the workflow has to accept and why testing the *combination* is the whole point. ### Land an existing pull request @@ -323,14 +325,12 @@ The order of `PRS` is the stack order. ### Using real CI -The demo keeps the build runner fake so a land finishes in seconds. Switching to real GitHub Actions takes three things. +This rung runs your workflow for real — `profiles.yaml` already selects the `githubactions` runner. Two things have to be true of the repository you point it at. -**1. The workflow must be dispatchable.** The runner triggers builds with `POST /actions/workflows/{id}/dispatches`, which only works if the workflow declares `workflow_dispatch`. A typical scratch-repo `ci.yml` triggered on `pull_request` alone cannot be dispatched at all — GitHub rejects it. Add the trigger and the inputs the runner sends: +**The workflow must be dispatchable, and must accept the `sq_*` inputs.** The runner triggers builds with `POST /actions/workflows/{id}/dispatches`, which only works if the workflow declares `workflow_dispatch`. A typical scratch-repo `ci.yml` triggered on `pull_request` alone cannot be dispatched at all — GitHub rejects the call: ```yaml on: - pull_request: - merge_group: workflow_dispatch: inputs: sq_head_uris: @@ -347,20 +347,11 @@ on: required: false ``` -**2. Point the runner at it.** Replace the `buildRunner` line for the queue in `profiles.yaml`: - -```yaml -buildRunner: - type: githubactions - owner: behinddwalls - repo: sq-demo - workflow: ci.yml # file name or numeric workflow id - ref: main # the branch the workflow definition is read from -``` +**And the workflow must apply both input sets**, which is the part that decides whether any of this means anything. A workflow that checks out the pull request and builds it tests *that change alone* — not what a submit queue is for. The point of speculation is the **combination**: `sq_base_uris` are the in-flight changes assumed to land first, `sq_head_uris` is the batch under test on top of them. A workflow that ignores them is green about the wrong thing, and the queue is only exercising its trigger-and-poll loop. -**3. Grant Actions: Read and write** on the token (see the permissions table above). +Two consequences worth planning for. A land now takes as long as the workflow does, so runs are minutes rather than seconds. And every speculation path dispatched is a run: the queue's [build budget](#put-traffic-through-it) is the cap on how many can be in flight at once, and on a metered account it is a spending control as much as a CI one. -One caveat worth understanding before you rely on the result. A workflow that only checks out the pull request tests *that change alone* — which is not what a submit queue is for. The point of speculation is to test the **combination**: `sq_base_uris` are the in-flight changes assumed to land first, and `sq_head_uris` is the batch under test on top of them. Until the workflow actually applies both, a green run says nothing about whether the batch lands cleanly, and the queue is only exercising its trigger-and-poll loop. +`pull_request` is deliberately absent above. With the queue dispatching a run per batch, leaving it on doubles the runs and tests something the queue already covers. ## Make a change fail diff --git a/service/submitqueue/demo/provider/github/profiles.yaml b/service/submitqueue/demo/provider/github/profiles.yaml index d210837d..5ef8c34c 100644 --- a/service/submitqueue/demo/provider/github/profiles.yaml +++ b/service/submitqueue/demo/provider/github/profiles.yaml @@ -25,17 +25,23 @@ queues: # detection. analyzer: {type: pathoverlap, by: directory} - # The build runner is inherited from the defaults above, so every build - # succeeds instantly. A land then completes in seconds, and the demo shows - # the queue and the merge rather than spending its time waiting on CI. + # Real CI. Each speculative batch is a workflow_dispatch, polled to + # completion, so `landed` here means a build of that batch passed rather + # than that a fake said so — this is the one rung where nothing is faked. # - # To run real CI instead, add the block below. It needs a workflow in the - # target repository that is triggerable by workflow_dispatch and accepts the - # sq_base_uris / sq_head_uris inputs, and a token with the `workflow` scope. + # The cost is honest too: a land takes as long as the workflow does, and + # every speculation path dispatched is Actions minutes spent. Set the build + # budget accordingly — see `speculator: {buildBudget: N}`. # - # buildRunner: - # type: githubactions - # owner: behinddwalls - # repo: sq-demo - # workflow: submitqueue-validate.yml - # ref: main + # ---- point these at your own scratch repo ---- + buildRunner: + type: githubactions + owner: behinddwalls + repo: sq-demo + # File name or numeric id. The workflow must declare workflow_dispatch and + # accept the four sq_* inputs; a workflow triggered only by pull_request + # cannot be dispatched at all and GitHub rejects the call. + workflow: ci.yml + # The branch the workflow definition is read from, not the code under test. + ref: main + # ----------------------------------------------