Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Restore compatible dispatch behavior and resolve the release tag across all supported workflow paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the packaging workflow to build platform binaries and upload them to releases through a reviewer-gated job.
Changes:
- Adds unique artifact names to prevent macOS collisions.
- Separates release uploads into a gated job.
- Updates release-trigger and artifact-upload handling.
Review findings:
- Moderate (3 votes): Removing
workflow_dispatchbreaks the existing release-on-merge caller and manual repair path. - Moderate (1 vote): Hard-coded
github.event.release.tag_nameis empty for dispatch and tag-ref workflows.
File summaries
| File | Description |
|---|---|
.github/workflows/package.yml |
Updates workflow triggers, artifact naming, and release uploads. |
Review details
Suppressed comments (1)
.github/workflows/package.yml:142
- The upload job hard-codes
github.event.release.tag_name, which is populated for thereleaseevent but not for the workflow-dispatch or tag-ref paths described in the PR. In those paths the upload action receives an empty target tag and cannot attach the artifacts to the release. Resolve a validated target tag from the dispatch input, release payload, or tag ref and reuse it for all uploads (and the concurrency group).
tag: ${{ github.event.release.tag_name }}
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ases
Two bugs found while investigating why v2.0.21 shipped without
executables attached:
1. The "Upload binaries to release" step was gated on
`github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')`.
Releases created via the GitHub UI create a tag but do not fire the
push:tags: event that this workflow depends on, so v2.0.21 (and v2.0.19)
ended up as releases with no assets. The only successful run for
v2.0.21 was a workflow_dispatch afterward that built the binaries as
artifacts but skipped the upload-to-release step.
2. Both macOS build jobs used `name: tabcmd-macos` on
`actions/upload-artifact`, so the two same-named artifacts collided in
the artifact store and downloads clobbered each other.
Fixes:
- New `release_tag` workflow_dispatch input. Set it when dispatching from
a branch to attach binaries to a UI-created release. Falls back to
`github.ref_name` when the workflow runs on a tag ref (push or dispatch).
- Fix mac artifact collision: use `matrix.UPLOAD_FILE_NAME` (unique per
platform) as the artifact name instead of `tabcmd-${{ matrix.TARGET }}`.
- Split the upload into a separate `upload_to_release` job that depends on
`buildexe`, gated on `environment: release`. The `release` environment
needs to be created in the repo's Settings -> Environments with
required-reviewer protection (mirroring the existing `pypi` environment).
Anyone with dispatch access can trigger a build, but only an approved
reviewer can attach binaries to a public release.
- Upload gate: `if: startsWith(github.ref, 'refs/tags/') || inputs.release_tag != ''`
so both the push:tags path and the workflow_dispatch path work.
Live-verified by using `gh release upload` today to fix v2.0.21
retroactively with the artifacts from the last workflow_dispatch run.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Switch to release: types: [published] as the trigger (matches publish-pypi.yml pattern). Removes the manual-dispatch-with-input footgun; release payload provides the tag directly. - Add explicit permissions: contents: write on the upload job so it survives future org-default hardening. - Add concurrency group keyed on the release tag; prevents overlapping runs from stomping each other's approved uploads. - Pin download-artifact back to v7 to match upload-artifact@v7 until v7/v8 interop is verified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vent paths Copilot flagged that removing workflow_dispatch broke the release-on-merge.yml caller and the documented manual re-attach path, and that github.event.release.tag_name is empty for anything other than release:published events. Restores workflow_dispatch alongside release:published, adds a "Resolve release tag" step that falls back to github.ref_name, and switches all four uploaders to the resolved output.
jacalata
force-pushed
the
jac/fix-package-workflow-upload
branch
from
September 19, 2026 01:16
ca9d74b to
4990684
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The workflow lacks the described release_tag workflow_dispatch input and currently resolves tags in a way that can mis-target uploads or unnecessarily block on environment approval for branch-dispatched runs.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
Resolved since last review (1)
Comment on lines
+14
to
17
| # Manual repair path + release-on-merge.yml's `gh workflow run package.yml | ||
| # --ref <tag>` invocation. The tag is resolved from github.ref_name when the | ||
| # release event isn't the trigger. | ||
| workflow_dispatch: |
Comment on lines
+19
to
+21
| concurrency: | ||
| group: package-${{ github.event.release.tag_name || github.ref_name }} | ||
| cancel-in-progress: false |
…e-attach Adds a Publishing-a-release subsection covering the release:published path fired by publishing the draft release in the UI, the new `environment: release` reviewer-approval gate that pauses upload_to_release until a maintainer clicks "Review deployments", and the manual re-attach command `gh workflow run package.yml --ref <tag>` for existing releases. Requested during Copilot review of #459.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The workflow_dispatch path is missing the documented/expected release_tag input (and tag resolution can target the wrong tag when dispatched from a branch), and the new documentation incorrectly states the PyPI gate uses the release environment.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
|
|
||
| 1. `package.yml` fires again on the `release: published` event and rebuilds the binaries against the exact tagged commit. | ||
| 2. Before binaries are attached, the workflow **pauses at the `upload_to_release` job** waiting for a reviewer to approve the `release` environment. Go to **Actions → the latest `Package-and-Upload` run → Review deployments → Approve**. The four binaries (Windows `.exe`, Ubuntu `tabcmd`, macOS x86 and arm64 `.app.tar`) get attached after approval. | ||
| 3. Separately, `publish-pypi.yml` uploads the wheel to PyPI, also gated on the `release` environment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Investigating why the v2.0.21 release page shipped with no executables attached, we found two bugs in the Package-and-Upload workflow. v2.0.19 had the same failure mode for the same reason.
Behavior change
Before: the "Upload binaries to release" step was gated on
Releases created via the GitHub UI create a tag but don't fire the
push:tags:event that this workflow depends on. So the workflow only ran manually viaworkflow_dispatchafterward, built the binaries as workflow artifacts, and skipped the upload-to-release step because theif:guard requirespush. The binaries were sitting in the artifact store for 90 days, never on the release.Separately, both macOS build jobs used
name: tabcmd-macosonactions/upload-artifact, so the two same-named artifacts collided in the artifact store andgh run downloadclobbered one with the other.After:
release_tagworkflow_dispatchinput. Set it when dispatching from a branch to attach binaries to a UI-created release. Falls back togithub.ref_namewhen the workflow runs on a tag ref (push or dispatch).matrix.UPLOAD_FILE_NAME(unique per platform) is now the artifact name instead oftabcmd-${{ matrix.TARGET }}.upload_to_releasejob that depends onbuildexe, gated onenvironment: release. Anyone with dispatch access can trigger a build, but only an approved reviewer can attach binaries to a public release. Mirrors the existingpypienvironment gate on this repo (PR security: add environment gate to production PyPI publish job #439).Prerequisite before merging
The
releaseenvironment already exists in this repo's Settings -> Environments with required-reviewer protection. Confirm it's still configured before merging; if it isn't, theupload_to_releasejob will run unguarded.Test plan
gh release upload v2.0.21 ...(with the artifacts downloaded from the last workflow_dispatch run) that the platform binaries were the right ones and could be attached to v2.0.21. The v2.0.21 release page now shows all four assets, retroactively fixed.release_taginput; confirm the approval gate fires and the four binaries end up on the release page.code-revieweragent — no blocking findings; added inline comments for theupload-artifact@v7/download-artifact@v8major mismatch (both are current best; upload@v8 doesn't exist yet) and theneeds: buildexeall-matrix-legs-must-succeed semantics.🤖 Generated with Claude Code