[AndreasKaratzas]

• build.sh: records the stack pair for /ci rocm cache under rocm-rocm-stack-built only when the build pushed it, not when it reused an existing tag
• run_ci_command.py: /ci rocm cache copies from the latest build of the PR that recorded it, at any commit, matched by the build's pull request rather than its github-pr-number metadata, and names that commit in its reply
• test_rocm_build.py, test_run_ci_command.py

Follow-up to #65276, which caches the stack image a PR's CI records. A PR build reuses rocm/vllm-dev:pr<N>-stack-rocm-<key> if that tag exists. The key is a hash of public files and every PR's CI can push to rocm/vllm-dev, so another PR's CI could plant an image under that tag before this PR's CI runs; the PR's CI would record the planted digest, and /ci rocm cache would copy it into rocm/vllm-ci. The lookup could also be fooled by github-pr-number, which any job of any build can set. Now only a stack the PR's own build pushed can be cached; when CI reused one instead, the bot refuses and points to /ci rocm nocache. This does not cover the PR's own code, which a maintainer still vouches for, or the rocm/vllm-ci push credentials on amd-cpu, which PR builds can reach and need a separate fix. test_run_ci_command.py (67) and test_rocm_build.py (44) pass, and both fail if either check is reverted.

AI assistance (Claude Code) was used to draft this change. Every line was reviewed by the submitter.

[mergify]

Documentation preview: https://vllm--65957.org.readthedocs.build/en/65957/

[AndreasKaratzas]

/ci run

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[github-actions]

✅ Triggered Buildkite CI #100328 for commit 966b36e53afa.

[mawong-amd]

The built-only marker and use of Buildkite’s authoritative PR association are useful improvements. I have three focused requests:

1. Preserve recipe compatibility when selecting older commits.
The current check compares main against source["commit"], but does not compare that source against pr["head"]["sha"]. A stack built with main’s recipe can therefore remain eligible after the current PR head changes build.sh or the bake recipe. That doesn’t match the refusal message directing a changed recipe to /ci rocm nocache.

Please retain the main/source check, but also establish the intended historical-source/current-head compatibility rule. For example, if current-head recipe identity is required:

• An older build remains eligible after later commits that keep the recipe unchanged.
• A later head that changes the recipe follows the documented refusal path.

If validation is intentionally only about the selected artifact’s recipe, please make that explicit in the policy/message so current-head changes aren’t handled inconsistently.

2. Make candidate discovery complete.
list_builds fetches only the first 100 results, so a long-lived PR’s valid older source may disappear behind newer runs. The source query also tries only head.label or head.ref, while handle_cancel_ci supports both branch-name forms.

Please paginate using the existing links.next continuation pattern used by list_failed_jobs, query both branch forms, and deduplicate overlapping results.

Two focused tests should cover this general lookup contract:

• A self-built artifact on a second page remains discoverable.
• An artifact found only by the bare-ref query is discovered after the owner-qualified query returns nothing.

3. Run the existing unit suite in CI.
It passed locally with 67 tests and mocked subcases, but no configured job invokes it. Please add a small PR-triggered, path-scoped unit job, without registry access or privileged CI credentials:

uv run --no-project --python 3.12 \
  .github/workflows/scripts/test_run_ci_command.py

The broader trusted-worker, credential, and producer-provenance context should stay separated as in my review of #64131. Those are CI control-plane concerns; this PR need not expand to solve them. The new marker is useful bookkeeping, not by itself producer authentication.

[AndreasKaratzas]

thanks, all three are in db12044.

1. the head's build.sh/bake now has to match main too, and the stack inputs at the commit that built the image have to match the head's, so a later commit that changes the image waits for its own build. since merging main reuses the stack rather than rebuilding it, the refusal now says to use /ci rocm nocache when main changed the recipe after the build.
2. both branch forms, all pages (capped), only builds since the PR opened. the builds list pages through the Link header / page param rather than links.next (that's the jobs endpoint), so list_builds takes page now.
3. added ci-command-tests.yml: pull_request only, path-scoped, read-only, runs that uv run command as is.

[mawong-amd]

The follow-up addresses my three concerns:

• Current-head compatibility: the selected build’s stack inputs must match the current PR head, and both the source and current-head recipes must match main.
• Complete candidate lookup: both owner-qualified and bare-ref branches are searched with bounded pagination and an explicit limit error. Excluding the base branch avoids unrelated main builds.
• CI coverage: the new pull_request-scoped, path-filtered, read-only workflow runs the existing unit suite without privileged CI credentials.

I independently extracted the bare PR head and ran 71 passing tests. The GitHub workflow passed 72 because it checked out the merge ref, which includes one unrelated upstream test for pipeline branch-filter handling.

The built-stack metadata remains useful bookkeeping rather than independent producer authentication. The broader trusted-worker and artifact-provenance discussion remains appropriately separated, as in my review of #64131.

[AndreasKaratzas]

/ci run

[github-actions]

✅ Triggered Buildkite CI #100669 for commit 33e85fcfdc73.

ℹ️ This PR is 110 commits behind upstream main. Merged upstream main into this branch as 33e85fcfdc73 before starting CI. Pull the branch before pushing more changes.

[AndreasKaratzas]

/ci retry

[khluu]

Kevin's review agent: approve for CI changes only

[github-actions]

✅ The previous CI build is still running: https://buildkite.com/vllm/ci/builds/100669

[peymanr]

PR Pundit review

(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)

#		Finding	Where
1	🟠	Stack from a non-passing build can be cached to rocm/vllm-ci: this PR writes cache key before tests run	.buildkite/scripts/rocm/build.sh:281-283
2	🟠	/ci rocm cache permanently refuses a PR's stack after a transient meta-set failure: this PR records the key only on the post-push build path	.buildkite/scripts/rocm/build.sh:282-283
3	🟠	Cache command makes up to 400 Buildkite list calls and can fail with 'confused' on busy shared branches: this PR replaced commit-filtered lookup with branch-wide paging	.github/workflows/scripts/run_ci_command.py:1270-1279

1. 🟠 Should fix: Stack from a non-passing build can be cached to rocm/vllm-ci: this PR writes cache key before tests run

.buildkite/scripts/rocm/build.sh:281-283

handle_rocm_cache picks the newest build of this PR carrying rocm-rocm-stack-built without checking its state. This PR now writes that key inside ensure() as soon as the stack bake pushes, before ensure runtime/ensure test-deps. So a running, cancelled or later-failed build can be the cache source. The base also never checked state, and its keys were written before any tests ran, so only the runtime/test-deps window is new. The copied stack is a fully pushed, digest-pinned pair and a maintainer must issue the command; requiring state == "passed" would make the 'once it passes' guidance enforced.

Who is affected: Maintainers who run /ci rocm cache on a PR whose latest stack-building CI build has not passed, or whose runtime/test-deps step failed after the stack push.

Checked against the code by an independent pass.

2. 🟠 Should fix: /ci rocm cache permanently refuses a PR's stack after a transient meta-set failure: this PR records the key only on the post-push build path

.buildkite/scripts/rocm/build.sh:282-283

If meta set rocm-$STACK-stack-built fails after the bake pushed the stack pair, ensure() returns 75. Base mode then exits 75 under set -e, and the step's automatic retry runs. On the retry, ensure finds the PR's own $REPO:pr<N>-stack-… tag and takes the 'Reusing' path, which records no rocm-rocm-stack-built. /ci rocm cache then refuses with 'no CI build of this PR built the ROCm base image itself' until the stack inputs change and a new stack is built. Consider retrying the meta set in place, or recording the key before returning 75.

Who is affected: PRs that change the ROCm stack inputs and hit a transient Buildkite metadata failure in the base step right after the stack bake.

Checked against the code by an independent pass.

3. 🟠 Should fix: Cache command makes up to 400 Buildkite list calls and can fail with 'confused' on busy shared branches: this PR replaced commit-filtered lookup with branch-wide paging

.github/workflows/scripts/run_ci_command.py:1270-1279

handle_rocm_cache now finds the source build by paging through every build of the head label and the bare head ref, on both Buildkite clients. The only server-side bound is created_from=pr["created_at"], and there is no early stop once a match is found. When the bare ref is a busy shared branch, e.g. a fork PR from owner:main into a release branch, the main builds since the PR opened are listed too. Past 10,000 such builds, iter_builds raises ApiError, and run() answers it with only a 'confused' reaction and no comment. Builds are listed newest first, so stopping at the first match (or skipping refs that are another repo's branch) would bound this.

Who is affected: Maintainers running /ci rocm cache on a long-lived fork PR whose head branch is named after a busy upstream branch (e.g. main) but which targets a different base branch.

Checked against the code by an independent pass.

_{Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at 411e56a2; findings checked and posted by @peymanr; points already raised on this PR are left out.}

[AndreasKaratzas]

/ci run

[github-actions]

✅ Triggered Buildkite CI #100707 for commit 2a86f63d42a3.

ℹ️ This PR is 10 commits behind upstream main. Merged upstream main into this branch as 2a86f63d42a3 before starting CI. Pull the branch before pushing more changes.

[AndreasKaratzas]

/ci run

[github-actions]

✅ Triggered Buildkite CI #100735 for commit 79658db15f33.

[peymanr]

PR Pundit review

(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)

#		Finding	Where
1	🟠	Another PR's planted cache layers can be promoted into trusted rocm/vllm-ci: per-PR cache-from reads untrusted cache	.buildkite/scripts/rocm/build.sh:280-283

1. 🟠 Should fix: Another PR's planted cache layers can be promoted into trusted rocm/vllm-ci: per-PR cache-from reads untrusted cache

.buildkite/scripts/rocm/build.sh:280-283

rocm-rocm-stack-built proves this PR's build pushed the stack, but not that the layers came only from this PR. In PR builds, the stack bake's cache-from (cfrom(["stack"]) in docker-bake-rocm.hcl) also reads $REPO:cache-pr<N>-rocm-stack in rocm/vllm-dev, which build.sh itself says any PR can write. Another PR can therefore plant cache layers that this build pushes and /ci rocm cache then copies into rocm/vllm-ci. Consider making the stack bake read only the trusted cache, without its own CACHE_TO cache, when its result may be promoted. Overwrites by later jobs of the same build are not a separate gap: those jobs run the PR's own code, which already controls the stack.

Origin: pre-existing

Who is affected: Maintainers running /ci rocm cache on a PR whose ROCm stack was built in rocm/vllm-dev, while another, malicious PR wrote that PR's per-PR cache tag.

Checked against the code by an independent pass.

_{Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at 79658db1; findings checked and posted by @peymanr; points already raised on this PR are left out.}