mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
lefarcenandGitHub
60d187b79f
* feat(ci): auto-merge the rolling plugin-preview manifest PR on green CI The post-merge bake opens ONE rolling PR (chore/plugin-previews) whenever a baked preview changes (spec slice 1). Its diff is pure generated data (data/plugin-previews/manifest.json; the clips already live on R2), yet someone still had to merge it by hand — the last manual step the pipeline rework left behind, and exactly the kind of bot PR that used to pile up. Two coupled changes: 1. bake-plugin-previews.yml now authors the rolling branch + PR with the release-bot App token instead of GITHUB_TOKEN. A GITHUB_TOKEN-authored push does not trigger workflows, so the manifest PR never ran `Validate workspace` (main's required check) and could not clear the merge queue at all — even manually. (PREVIEW_BAKE_TOKEN, the old fallback, was never configured.) 2. bake-plugin-previews-automerge.yml reacts to ci.yml completing for the rolling branch and, for a non-draft same-repo PR authored by the release bot with only bot commits (pristine) at the exact SHA CI passed, approves it as github-actions[bot] (main requires one review; an author cannot approve its own PR) and enqueues it with `gh pr merge --auto --squash --match-head-commit`. CI failure pings Feishu instead. Modeled on backport-automerge.yml, adapted for main: main has a merge queue, so the merge is GitHub-native --auto (enqueue) rather than a direct squash, and pristine checks the git committer email (the bot email maps to no GitHub user, so .committer.login is null). * fix(review): GitHub-backed pristine provenance + correct Feishu sign secret Address two blocking review findings on the auto-merge path: 1. pristine could be satisfied by spoofed commit metadata. The bake commit was authored with a plaintext `git config user.email "bot@open-design.ai"`, and the reactor trusted that raw committer email — any collaborator who can push to chore/plugin-previews could set the same email and fake pristine=true, landing unreviewed commits on main. Commit AS github-actions[bot] (its canonical noreply email) and gate on GitHub's resolved `.committer.login`, the same GitHub-backed field backport-automerge.yml trusts. 2. the failure Feishu ping signed with secrets.FEISHU_WEBHOOK_SECRET, but every other FEISHU_RELEASE_WEBHOOK caller (notify-release-feishu, notify-daily-feishu, backport-automerge) signs with FEISHU_RELEASE_SIGN_SECRET; the wrong secret resolves empty and the signed release webhook rejects the unsigned request. Use FEISHU_RELEASE_SIGN_SECRET and assert the pairing in the topology test. * test(ci): lock the auto-merge conclusion gates + bake producer invariants Address nettee's non-blocking coverage follow-up so a regression can't pass the topology test green: - Assert the approve and enqueue steps gate on `workflow_run.conclusion == 'success'`, and the Feishu step on `== 'failure'`, so dropping a conclusion predicate fails the test. - Add a companion `[P2]` block over bake-plugin-previews.yml asserting the three producer invariants the downstream pristine/auto-merge path depends on: the create-github-app-token mint, the App-token push URL, and the github-actions[bot] commit identity. Regressing the producer back to a GITHUB_TOKEN push or a different commit identity now fails here. * fix(review): bind the Feishu failure alert to the run's exact SHA A stale failed CI run can finish after the force-pushed rolling branch has advanced to a newer (green) head. The Feishu step only gated on author/cross + conclusion==failure, so it would page about a SHA that is no longer the PR head. Add the same steps.pr.outputs.head_oid == workflow_run.head_sha guard the approve/enqueue steps use, and lock it in the topology test. * fix(review): prove bot provenance with GitHub signatures, not git headers nettee correctly escalated: .committer.login is still a spoofable git-header identity. A collaborator who can push to chore/plugin-previews can mint a local commit with the bot's noreply email, GitHub then reports .committer.login == github-actions[bot], and pristine=true passes — landing unreviewed human changes on main. Switch to non-spoofable provenance: - bake-plugin-previews.yml now creates the manifest commit through the Git Data API (blob -> tree -> commit -> ref) with the App token, instead of a local `git commit` + push. A commit created through the API is signed by GitHub (verification.verified=true) and committed by the App bot (committer.type=Bot). Building the tree on the live main HEAD also rebases the rolling branch onto the latest main each run. - the reactor's pristine gate now requires EVERY commit to be both verification.verified==true AND committer.type==Bot. A human's locally pushed commit is unsigned (verified=false) even with the bot email; a human signing with their own key is committer.type=User. Neither can be both, so the pair is forge-proof. - topology test updated to lock the API-commit producer path (and forbid a regression to a local push) and the verified+Bot pristine check. * fix(review): parent the manifest commit on the rendered revision, not live main The previous change based the Git Data API commit on live main HEAD, but $NEW is rendered from the job's checkout at start. The bake can run ~90min and main can advance (unrelated merges) meanwhile, so basing the commit on live main would publish a manifest from an older checkout on top of newer commits — the rolling PR would look current while carrying stale preview data and could overwrite newer plugin-preview changes. Parent on `git rev-parse HEAD` (the exact revision that produced $NEW) instead, so GitHub correctly shows the PR as behind once main moves and the next bake re-renders on the newer HEAD. Topology test locks the rendered-revision base and forbids regressing to a live-main base. * fix(review): require the rolling PR's rendered base to equal main before merge main's merge queue does not require an up-to-date branch, so a bake rendered against an older main could be re-tested on the latest main and squash-merged with a stale manifest.json — overwriting a newer manifest that landed meanwhile. head_oid only proves the PR head is the SHA CI validated, not that it is current. Add a base-freshness gate: the rolling commit's first parent is the revision the manifest was rendered against, so require it to equal current main HEAD before approving/enqueuing. A behind PR is skipped and refreshed by the next bake (the nightly re-renders on the newer tip even if no plugin changed). Locked in the topology test. * fix(review): page Feishu on timed_out bakes too, not just failure workflow_run has terminal RED states beyond 'failure'. ci.yml defines many timeout-minutes jobs, so a timed-out run on the rolling manifest PR would block auto-merge with no Feishu page. Broaden the alert predicate to failure OR timed_out, and include the conclusion in the message. Deliberately exclude 'cancelled': a force-push of the rolling branch routinely cancels the prior in-flight run, so alerting on it would page on every normal refresh. Test locks the failure+timed_out set and the cancelled exclusion. * fix(review): gate auto-merge on a manifest-only diff, not commit signatures nettee is right that the Git Data API does not sign commits — I verified it empirically: a blob/tree/commit created via POST /git/commits comes back verification.verified=false / reason=unsigned (and the Contents API is unsigned too for non-web callers). So the previous verified==true && committer.type==Bot pristine gate would never pass and the rolling PR could never auto-merge. There is no per-commit field that is both bot-set and unforgeable without a signature, so stop trying to prove "the bot authored every commit" and instead bound what auto-merge can land: require the PR's changed files to be EXACTLY data/plugin-previews/manifest.json. A human with push access can still push to the rolling branch, but any commit that touches a code file (or anything but the manifest) makes the file set != the single manifest path, so pristine=false and the PR falls back to human review. The worst a forged manifest-only change can do is point a gallery preview at a bad video URL — generated data the next bake overwrites — never unreviewed code on main. With signing no longer relied upon, revert the producer from the Git Data API dance back to a plain App-token git push (still parented on the checked-out HEAD per the base-freshness fix, still pushed with the App token so CI runs). Tests updated to lock the manifest-only file gate and the simplified producer. * test(ci): update secretless-bake assertion for the App-token producer Merge main (#4691 landed the read-only pre-merge bake + its topology test). That test asserted the post-merge workflow contains PREVIEW_BAKE_TOKEN as the write-capable credential; this PR replaced that never-configured fallback with the release-bot App, so assert secrets.RELEASE_BOT_APP_ID (and that PREVIEW_BAKE_TOKEN is gone) instead.