Add browser destination artifact publish workflow - #3787
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a manually-triggered GitHub Actions workflow to build browser destination bundles for a selected environment and upload the built dist/web/ output as an artifact for later download (e.g., by a backend that uploads to S3).
Changes:
- Introduces a new
workflow_dispatchworkflow withenvironmentandbranchinputs. - Builds the destinations manifest and then builds browser bundles using the existing package scripts.
- Uploads
packages/browser-destinations/dist/web/as a short-lived artifact.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use git rev-parse HEAD for artifact SHA instead of github.sha (was wrong for non-main branches) - Add job-level environment to enforce GitHub Environment protection rules - Add concurrency group to prevent simultaneous builds for the same environment - Add permissions: contents: read for least-privilege token - Add NPM_TOKEN and registry-url for authenticated yarn install - Use yarn nx instead of bare nx to avoid fragile transitive resolution - Set if-no-files-found: error on artifact upload to catch empty dist - Increase retention-days to 30 for production deploy artifacts - Reduce timeout to 20 minutes to match existing browser bundle CI job Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e backend service Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use github.event.inputs.* to match other workflow_dispatch workflows - Set retention-days to 7 to match PR description Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: semgrep-code-segmentio-2[bot] <240576539+semgrep-code-segmentio-2[bot]@users.noreply.github.com>
…flow The uses: key on the Use Node.js step was indented 6 spaces instead of 8, producing "Invalid workflow file" YAML syntax errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…convention - Use ubuntu-latest-large runner label like all other workflows - Pin setup-node to the same SHA (v4) used across ci.yml and others Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
b87db00 to
0dd8f15
Compare
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
.github/workflows/publish.yml:40
- This diff only compares
HEAD^..HEAD, which misses changes when the push contains multiple commits (e.g., browser-destinations changed in an earlier commit but not the finalHEADcommit). For push-triggered workflows, diff against the full pushed range (e.g.,${{ github.event.before }}..${{ github.sha }}) so the gate reflects all changes included in the push.
run: |
if git diff --quiet HEAD^ HEAD -- packages/browser-destinations/; then
echo "changed=false" >> "$GITHUB_OUTPUT"
else
echo "changed=true" >> "$GITHUB_OUTPUT"
fi
.github/workflows/publish.yml:20
- The comparison to
== trueis redundant in GitHub Actions expressions. You can simplify toif: startsWith(github.event.head_commit.message, 'Publish')for clarity.
if: startsWith(github.event.head_commit.message, 'Publish') == true
.github/workflows/publish.yml:48
ubuntu-latest-largeis not a standard GitHub-hosted runner label. If this repository isn’t configured with a self-hosted runner (or a custom runner label) namedubuntu-latest-large, this job will never start. Consider usingubuntu-latestor a verified runner label used elsewhere in the repo.
runs-on: ubuntu-latest-large
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/publish.yml:46
git log ... HEAD^will fail on an orphan branch / first commit (no parent). Since Actions bash steps typically run with-e, this would fail the job (and block downstream jobs). Consider making the parent lookup resilient (e.g., guard for a missing parent and treat it asno previous publish, or make thegit logcommand non-fatal) so publishing doesn’t break in these scenarios.
previous_publish=''
while IFS=$'\t' read -r commit subject; do
if [[ "$subject" == Publish* ]]; then
previous_publish="$commit"
break
fi
done < <(git log --first-parent --format='%H%x09%s' HEAD^)
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Suppressed comments (4)
.github/workflows/publish-browser-destinations.yml:62
- Artifact lookup only fetches the first 100 artifacts. If the repository produces more than 100 artifacts within the retention window, the target artifact could be on a later page, causing unnecessary rebuilds. Consider using
gh api --paginate(and filtering viajq) so the search is correct regardless of artifact volume.
if artifacts=$(gh api --method GET \
"$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/actions/artifacts" \
-f name="$artifact_name" \
-f per_page=100) && \
artifact_count=$(jq --arg name "$artifact_name" \
'[.artifacts[] | select(.name == $name and .expired == false)] | length' \
<<< "$artifacts") && \
[ "$artifact_count" -gt 0 ]; then
.github/workflows/publish-browser-destinations.yml:9
- PR description states
mainandhotfix/**runbuild-web, but this workflow only triggers onmainandstaging. Either update the workflowon.push.branchesto includehotfix/**(if intended), or adjust the PR description to match the actual triggers.
push:
branches:
- main
- staging
.github/workflows/publish-browser-destinations.yml:31
- The environment-to-suffix mapping (
staging→stage, elseproduction) is duplicated in two places. To reduce drift risk, consider setting a single job/workflow-level env (or an output from the detect job) and reusing it for both detection and artifact naming.
ARTIFACT_ENVIRONMENT: ${{ github.ref_name == 'staging' && 'stage' || 'production' }}
.github/workflows/publish-browser-destinations.yml:114
- The environment-to-suffix mapping (
staging→stage, elseproduction) is duplicated in two places. To reduce drift risk, consider setting a single job/workflow-level env (or an output from the detect job) and reusing it for both detection and artifact naming.
name: browser-destinations-${{ github.ref_name == 'staging' && 'stage' || 'production' }}-${{ github.sha }}
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
| name: browser-destinations-${{ github.ref_name == 'staging' && 'stage' || 'production' }}-${{ github.sha }} | ||
| path: packages/browser-destinations/dist/web/ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
.github/workflows/publish-browser-destinations.yml:13
- The
== truecomparison is redundant in GitHub Actions expressions and makes the condition noisier. Preferif: startsWith(github.event.head_commit.message, 'Publish')for clarity.
if: startsWith(github.event.head_commit.message, 'Publish') == true
.github/workflows/publish-browser-destinations.yml:62
- The
actions/artifactsREST endpoint doesn’t support filtering bynamevia query string, so-f name=...is misleading and may be ignored by the API. Since you already filter by name injq, remove the unsupported query param (or add a brief comment explaining it’s intentionally client-side filtered) to avoid future confusion.
if artifacts=$(gh api --method GET \
"$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/actions/artifacts" \
-f name="$artifact_name" \
-f per_page=100) && \
artifact_count=$(jq --arg name "$artifact_name" \
'[.artifacts[] | select(.name == $name and .expired == false)] | length' \
<<< "$artifacts") && \
[ "$artifact_count" -gt 0 ]; then
.github/workflows/publish-browser-destinations.yml:62
- This only requests the first page (
per_page=100) of artifacts. If the repo accumulates more than 100 artifacts, an older (but still non-expired) matching artifact might not be returned, causing unnecessary rebuilds. Consider usinggh api --paginateand aggregating results (or stop early once a match is found) so the skip logic remains correct as artifact count grows.
if artifacts=$(gh api --method GET \
"$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/actions/artifacts" \
-f name="$artifact_name" \
-f per_page=100) && \
artifact_count=$(jq --arg name "$artifact_name" \
'[.artifacts[] | select(.name == $name and .expired == false)] | length' \
<<< "$artifacts") && \
[ "$artifact_count" -gt 0 ]; then
| HUSKY: 0 | ||
| NX_DISABLE_DB: true | ||
| if: needs.detect-browser-destination-changes.outputs.changed == 'true' | ||
| runs-on: ubuntu-latest-large |
Goal
Add a short-term browser destination publishing workflow that runs independently and concurrently with npm package publishing on
mainandstaging.Review Guide
publish-browser-destinations.ymllistens for pushes tomainandstaging, then runs only when the pushed commit message starts withPublish. The existingpublish.ymlis unchanged, so both workflows start independently on the same release push.Publishcommit on first-parent history and watches browser destinations plus their bundled runtime/shared packages. Additions, renames, deletions, multi-commit releases, and shared-runtime changes are covered.mainbuilds production bundles;stagingbuilds stage bundles.github.sha. Artifacts are namedbrowser-destinations-{environment}-{sha}, belong to a workflow run with the samehead_sha, fail on empty output, and are retained for 7 days. Consumers should fetch by workflow run ID and verifyhead_sha.NPM_TOKEN.stagingat commit83a2bd4a4; its non-Publishinstallation run was skipped as intended.