Skip to content

ci: regenerate generated manifests on Dependabot PRs - #245

Open
rominf wants to merge 1 commit into
mainfrom
ci/dependabot-regenerate-manifests
Open

ci: regenerate generated manifests on Dependabot PRs#245
rominf wants to merge 1 commit into
mainfrom
ci/dependabot-regenerate-manifests

Conversation

@rominf

@rominf rominf commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

MANIFEST.md's dependency table and THIRD_PARTY_NOTICES.txt are generated
from the dependency graph (cargo xtask manifest / cargo xtask tpn), and
ci.yml gates both against the committed copies. Dependabot cannot run a
generator, so every cargo bump it opens fails those two gates on arrival.
Right now 7 of the 10 open cargo PRs (#225, #227, #229, #230, #231, #232, #234)
fail on nothing else — they compile and lint clean. A permanently red dependency
queue teaches reviewers to stop reading red, which costs more than the noise
itself.

This adds a workflow that regenerates both files on Dependabot's PRs and commits
them onto the bump branch.

Why it is shaped this way

Two jobs, not one. generate runs the generators, which means it compiles
the bumped dependency graph — building xtask executes the new versions'
build.rs and proc macros. It is contents: read. commit holds
contents: write, runs no third-party code, and only unpacks an artifact and
calls the API. This is the split GitHub recommends over pull_request_target,
and it keeps untrusted code and a write token from ever coexisting.

createCommitOnBranch, not git push. Two properties this repo needs:
commits from that mutation are GitHub web-flow signed, so
verify-commits --require-verified (the blocking Commit signatures + sign-off
gate) accepts them, where a runner's git push would produce an unsigned commit
and fail. And expectedHeadOid makes the mutation fail rather than clobber if
Dependabot force-pushes the branch mid-run. The DCO half of that gate is covered
by a Signed-off-by trailer in the commit body.

Writes are bounded to MANIFEST.md and THIRD_PARTY_NOTICES.txt. The
artifact content originates in the untrusted job, so the path allowlist is what
keeps a hostile build script from reaching anything else — at worst it garbles
two files a human still reviews.

One manual step remains, by design

A commit made with GITHUB_TOKEN
does not start a new workflow run.
So after this commit lands on the branch, a maintainer clicks
"Approve workflows to run" in the merge box to re-run the gates against the
regenerated files. That click is the entire remaining cost, down from
regenerating two files by hand and pushing them.

Removing it needs a non-GITHUB_TOKEN identity — a GitHub App installation
token, with credentials stored as Dependabot secrets (Actions secrets are not
available on Dependabot-triggered runs). That is org-level setup rather than a
workflow change, so it is deliberately not in this PR. Happy to follow up if you
want it fully unattended.

Risk

Medium, and worth naming precisely: this grants contents: write to a workflow
reachable from a Dependabot PR, where today all of CI is read-only. It is
bounded to two generated paths on a dependabot/* branch, and the job holding
the token runs no third-party code — but it is a real escalation and should be
read as one, not waved through.

Test plan

Verified locally:

  • Both generators run clean against current main and are idempotent — a second
    cargo xtask manifest && cargo xtask tpn produces no diff, and both
    --check modes pass. cargo-about 0.9.1 (the version ci.yml pins) reproduces
    the committed notices byte-for-byte.
  • The commit job's script was extracted from the YAML and executed verbatim
    against the real files: the payload is valid JSON (~888 KB), and both files
    base64 round-trip byte-identically. The API validated every field and rejected
    only a deliberately-invalid expectedHeadOid, which is the expected
    no-clobber behavior.
  • verify-commits' sign-off detection accepts the trailer format used here
    (any line beginning signed-off-by:, per xtask/src/verify_commits.rs).
  • prek run --all-files --no-group local-tools and cargo test -p xtask
    (including the workflow-contract tests) pass.

Not verifiable before merge: the end-to-end path on a real Dependabot PR — the
workflow has to exist on the bump branch to run, which needs this merged and the
branch rebased. The seven manifest-only PRs above are the intended first
exercise, and I will report what actually happens on them, including whether the
approval banner appears as documented.

  • Not a bug fix; no tests/e2e-cucumber/expectations.toml xfail rows to narrow.

MANIFEST.md's dependency table and THIRD_PARTY_NOTICES.txt are generated
from the dependency graph, and ci.yml gates both against the committed
copies. Dependabot cannot run a generator, so every cargo bump it opens
fails those two gates on arrival -- 7 of the 10 currently open cargo PRs
fail on nothing else. A permanently red dependency queue teaches
reviewers to stop reading red, which costs more than the noise.

Regenerate both and commit them onto the bump branch. Two jobs, split so
that the one compiling the bumped graph (running its build.rs) stays
read-only, and the one holding contents: write runs no third-party code
and can only write the two generated paths.

Commits go through createCommitOnBranch rather than git push: the
mutation produces a GitHub-signed commit, which the blocking commit
signature gate requires, and expectedHeadOid makes it fail instead of
clobber if Dependabot force-pushes mid-run.

One manual step remains by design: a GITHUB_TOKEN commit does not start
a new workflow run, so a maintainer approves the re-run from the merge
box. Removing that needs a GitHub App identity, which is org-level setup
rather than a workflow change.

Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
@rominf
rominf requested a review from a team as a code owner August 13, 2026 06:00
@rominf

rominf commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

E2E tests (GPU) is red here and it is not this PR — this change only adds a
workflow file. The same scenario (serve-hf-checkpoint-inference, 46/53
passing, 6 expected xfails, 1 unexpected failure) is already failing on main:
run 31643703431, which predates this branch. The lane is continue-on-error, and
every blocking check is green.

Worth someone triaging separately — main's GPU lane has been red since before
this work, so it is currently red on every PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant