Repository navigation
ci: publish to PyPI from the static-IP GitHub-hosted runner - #400
Conversation
OneAsterisk
left a comment
There was a problem hiding this comment.
Claude review, round 1
Verdict: Approved — a one-line runner move that does what the description says; the two notes below are non-blocking.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved.
PR description vs code
| Claim | Status | Where |
|---|---|---|
PyPI Publish moves from Blacksmith to runpod-4vCPU-ubuntu-2404 |
✅ | release-please.yml:52 |
| The label is the org's GitHub-hosted runner with static, allow-listed IPs | ✅ GitHub-hosted: the probe log shows runner group runpod-4vcpu-ubuntu-2404 and image ubuntu-24.04 from actions/runner-images. |
job 124146750304 |
| Trusted publishing is built for GitHub-hosted runners; Blacksmith counts as self-hosted | README, Non-goals | |
RUNNER_4VCPU_UBUNTU_2404 isn't shared with this repo |
✅ repos/runpod/flash/actions/organization-variables returns 0 variables |
API |
| A probe confirmed the runner accepts jobs from runpod/flash, then was removed | ✅ Run 41080143161's sibling runner-probe on 204c67f succeeded on that runner group; 2507107 deletes runner-probe.yml; head has only ci.yml, e2e.yml, release-please.yml |
run 41080143737 |
This doesn't fix the current Release failure (workflows: write not granted) |
✅ The latest main run fails at the token step with exactly The permissions requested are not granted to this installation |
run 40991928237 |
| An org owner must grant Workflows: Read and write, then rerun Release | ||
| 1.21.1 needs runpod-workers/flash#112 released first, because of #390 | ✅ #390's warning names runpod-workers/flash#113 (merged 2026-10-27, on main since v1.6.0); #112 is the 1.6.1 release PR whose tag would include it. |
Findings
Critical
None.
High
None.
Medium
None.
Low
- Literal runner label instead of the org variable —
release-please.yml:52. The org's own tooling treats a written-out label as an error: runpod/spinnaker-platform'scheck-workflow-runners.shrejects anyruns-onthat is not${{ vars.RUNNER_4VCPU_UBUNTU_2404 }}("named by an organization variable, so the label changes in one place"), and runpod/KaOS and runpod/internal-workflows use the variable. If the runner group is renamed, the next release's publish job waits for a runner that never arrives. The body already explains why the literal is needed today. The fix is the same ask as the app permission: have the org admin shareRUNNER_4VCPU_UBUNTU_2404with runpod/flash, then switch to the variable. Fine to merge as-is and follow up.
Nits
release-please.yml:52"trusted publishing needs that" overclaims; pypa says it is expected to work on self-hosted runners and GitHub-hosted is the supported path. Suggested wording inline.
Simplification opportunities
None; one-line change.
Verified clean
- Workflow-level
id-token: write(line 11) still applies to the job, and PyPI's publisher match (repository,release-please.yml, environmentpypi-production) does not depend on the runner, so the OIDC exchange is unaffected. actions/setup-python,astral-sh/setup-uvwithenable-cache, andpypa/gh-action-pypi-publishare all in their supported configuration on a GitHub-hosted runner.Build Packageis skipped byguard, not failed:compute_should_buildonly fires for packaging paths (ci.yml:118-126), and this PR touches none. Every other check passes at 2507107.- Both commits use
ci:, hidden from the changelog; the repo squash-merges. - The other three jobs stay on Blacksmith, which already works under the org IP allow list (#397), so nothing else changes.
- Merging adds one more failing Release run on main until the app permission is granted; the body says so.
Questions for the author
- Can the org-admin request for the release app's Workflows permission also include sharing
RUNNER_4VCPU_UBUNTU_2404with runpod/flash? That closes the Low above. - runpod-workers/flash#112 still describes
mainas of 2026-10-23 and omits #113. Is that repo's release automation also stalled? The tag would still include 95de97c9, but the changelog would not.
Reviewed by Claude (claude-fable-5-1) via the pr-review skill. Tokens: 290 in / 31,674 out / 653,971 cache read / 95,402 cache write = 781,337. Estimated cost: $3.66 at $10 / $50 / $0.25 / $20 per MTok (input / output / cache read / 1-hour cache write), as of posting. Round 1 of this PR. This is automated assistance; a human owns the final call.
Co-authored-by: Ben Papp <[email protected]>
OneAsterisk
left a comment
There was a problem hiding this comment.
Claude review, round 2
Verdict: Approved — the only change since round 1 is the comment wording on line 52, applied from the round 1 suggestion; nothing new to raise.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved. Round 1's approval at 2507107 stands; this round re-approves at ccbb51a.
Status of prior findings
| # | Finding | Severity | Status | Evidence |
|---|---|---|---|---|
| R1-1 | Literal runner label instead of ${{ vars.RUNNER_4VCPU_UBUNTU_2404 }} |
Low | Not addressed (blocked on org admin, not the author) | release-please.yml:52 still literal at ccbb51a; organization-variables for runpod/flash still returns 0 |
| R1-2 | Comment claimed trusted publishing needs a GitHub-hosted runner | Nit | Fixed | ccbb51a applied the suggestion verbatim; thread resolved |
Existing feedback
- No human or bot comments on this PR; the two threads are round 1's own, answered above.
PR description vs code
Re-checked the claims that could have moved since round 1; the rest are unchanged and still hold.
| Claim | Status | Where |
|---|---|---|
PyPI Publish runs on runpod-4vCPU-ubuntu-2404 |
✅ | release-please.yml:52 |
| The probe job was removed | ✅ head has only ci.yml, e2e.yml, release-please.yml |
.github/workflows/ |
RUNNER_4VCPU_UBUNTU_2404 isn't shared with this repo |
✅ still 0 org variables visible | API |
| This doesn't fix the current Release failure | ✅ the newest Release run on main (40991928237, 02:28Z today) still fails at the token step; nothing newer has run | run 40991928237 |
Findings
None new. The diff since round 1 is one comment string on one line.
Simplification opportunities
None.
Verified clean
- The
runs-onlabel itself is byte-identical to the one the probe exercised, so the round 1 probe result still applies. - The new comment is valid YAML (space before
#, no quotes to break). - ccbb51a's subject (
Update .github/workflows/release-please.yml) has no conventional-commit type, so release-please ignores it in the squash body; nothing leaks into the changelog. - CI at ccbb51a: every check passes (run 41087170098).
Build Packagestays skipped byguard(should_buildonly fires for packaging paths);pre-check,quality-gates, andnixrun in full mode on every PR regardless, asci.yml:205,227,324gate them.
Questions for the author
- Same two from round 1, still open: can the org-admin ask bundle sharing
RUNNER_4VCPU_UBUNTU_2404with the Workflows permission? And is runpod-workers/flash#112's changelog going to pick up #113 before it ships?
Reviewed by Claude (claude-fable-5-1) via the pr-review skill. Tokens: 358 in / 11,206 out / 803,627 cache read / 62,849 cache write = 878,040. Estimated cost: $2.02 at $10 / $50 / $0.25 / $20 per MTok (input / output / cache read / 1-hour cache write), as of posting. Round 2 of this PR. This is automated assistance; a human owns the final call.
Moves the
PyPI Publishjob inrelease-please.ymlfrom Blacksmith torunpod-4vCPU-ubuntu-2404, the org's GitHub-hosted runner with static, allow-listed IPs. PyPI trusted publishing is built for GitHub-hosted runners, and Blacksmith counts as self-hosted. The label is literal because the org variableRUNNER_4VCPU_UBUNTU_2404isn't shared with this repo. A probe job on this branch confirmed the runner accepts jobs from runpod/flash. The probe was then removed.This doesn't fix the current Release failure. Since #399 the token step asks for
workflows: write, and the release app (RELEASE_APP_ID) isn't granted it:The permissions requested are not granted to this installation. An org owner has to add Workflows: Read and write to the app and accept it on the installation. Then rerun Release on main. That publishes 1.21.1, which also needs runpod-workers/flash#112 released first, because of #390.