fix: avoid breaking standalone downloads during release pipeline by joehan · Pull Request #11029 · firebase/firebase-tools

joehan · 2026-09-03T20:14:51Z

Description

Refactors the release pipeline to prevent download breakages from firebase.tools during release runs:

Publish new version to npm under a staging tag (--tag staging) without moving the latest dist-tag prematurely.
Build standalone artifacts from the new version in the firepit-builder pipeline and attach them to the draft release created by publish.sh.
Validate that the draft release contains all 4 expected artifacts (firebase-tools-instant-win.exe, firebase-tools-linux, firebase-tools-macos, firebase-tools-win.exe) with uploaded states and non-zero sizes before publishing the release.
Publish the GitHub release and move the npm latest dist-tag to the new version, cleaning up the temporary staging tag.
Add verification step in cloudbuild.yaml.

Scenarios Tested

Ran bash -n scripts/publish.sh syntax check.
Ran node -c scripts/firepit-builder/pipeline.js syntax check.
Checked scripts/publish/cloudbuild.yaml YAML parsing and prettier formatting.
Ran eslint on scripts/firepit-builder/pipeline.js and verified clean linting with npm run lint:changed-files (0 errors).
Validated hub release show -f "%as" formatting and parsing against live release tag.

Sample Commands

./scripts/publish/run.sh patch

### Description Refactors the release pipeline to prevent download breakages from firebase.tools during release runs: 1. Publish new version to npm under a staging tag (`--tag staging`) without moving the `latest` dist-tag prematurely. 2. Build standalone artifacts from the new version in the firepit-builder pipeline and attach them to the draft release created by `publish.sh`. 3. Validate that the draft release contains all 4 expected artifacts (`firebase-tools-instant-win.exe`, `firebase-tools-linux`, `firebase-tools-macos`, `firebase-tools-win.exe`) with uploaded states and non-zero sizes before publishing the release. 4. Publish the GitHub release and move the npm `latest` dist-tag to the new version, cleaning up the temporary staging tag. 5. Add verification step in `cloudbuild.yaml` and support for `publish.sh move-latest`. ### Scenarios Tested - Ran `bash -n scripts/publish.sh` syntax check. - Ran `node -c scripts/firepit-builder/pipeline.js` syntax check. - Checked `scripts/publish/cloudbuild.yaml` YAML parsing and prettier formatting. - Ran eslint on `scripts/firepit-builder/pipeline.js` and verified clean linting with `npm run lint:changed-files`. ### Sample Commands `./scripts/publish/run.sh patch` `./scripts/publish.sh move-latest`

gemini-code-assist

Code Review

This pull request updates the release pipeline and scripts to automate the publication of standalone artifacts, verify artifact health via the GitHub API, and manage moving the npm latest tag. The review feedback identifies several critical issues: an incorrect format placeholder (%as instead of %an) in hub release show that would cause validation to fail, a silent bypass in the GitHub API health check if an expected artifact is missing, a potential crash in publish.sh if package.json is missing when running jq, and redundant conditions in publish.sh for the move-latest and tag-latest options.

gemini-code-assist · 2026-09-03T20:16:45Z

+  if [[ -z "$TARGET_VERSION" ]]; then
+    TARGET_VERSION=$(jq -r ".version" package.json)
+  fi


If package.json does not exist in the current working directory, running jq directly will fail with a non-zero exit code. Since set -e is active at the top of the script, this will cause the script to crash immediately.

Add a check to ensure package.json exists before running jq.

Suggested change

if [[ -z "$TARGET_VERSION" ]]; then

TARGET_VERSION=$(jq -r ".version" package.json)

fi

if [[ -z "$TARGET_VERSION" && -f package.json ]]; then

TARGET_VERSION=$(jq -r ".version" package.json)

fi

gemini-code-assist · 2026-09-03T20:16:45Z

    exit 1
  fi
-elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major") ]]; then
+elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major" || $VERSION == "move-latest" || $VERSION == "tag-latest") ]]; then


The check for move-latest and tag-latest in this condition is redundant. If VERSION is either of those values, it is already handled in the elif block on line 22, which terminates the script with exit 0. Thus, these conditions are unreachable here.

Suggested change

elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major" || $VERSION == "move-latest" || $VERSION == "tag-latest") ]]; then

elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major") ]]; then

joehan · 2026-09-03T20:29:47Z

/joe-review

joehan

⚠️ Disclaimer: This review was generated by an automated review agent based on the firebase-tools-pr-review guidelines. Please verify all findings before acting on them.

Code Review Summary: `firebase/firebase-tools` (PR #11029)

🟢 Strengths & LGTM Aspects

Targeted & High-Impact Solution: Decoupling the npm package publish under a temporary staging tag from the release of standalone binaries and final latest dist-tag update directly resolves the download 404 race condition for firebase.tools.
Pre-publication Health Check: Validating that all 4 standalone artifacts exist, have non-zero file sizes, and have been confirmed uploaded prior to publishing the GitHub release and tagging latest is an excellent safeguard against botched releases.
Fail-Safe Fallbacks: Adding the post-publish verification step in scripts/publish/cloudbuild.yaml and CLI support for publish.sh move-latest provides robust recovery options if a step terminates mid-pipeline.

🔴 Overview of Findings & Action Items

🔴 Blocking: Missing Artifact Check in GitHub API Health Verification (scripts/firepit-builder/pipeline.js:180): If an asset is completely missing from matchedRelease.assets, matchedRelease.assets.find(...) returns undefined, bypassing the assertion checks entirely without throwing.
🔴 Blocking: Asset Name Parsing in Hub Query (scripts/firepit-builder/pipeline.js:143-156): hub release show -f "%as" outputs <DownloadURL>\t<Label> per asset rather than file basenames, which will cause attachedAssets.includes(expectedFile) to fail validation unless parsed.
🟡 Nit / Robustness: Ensure package.json Exists in publish.sh (scripts/publish.sh:28): Guard the fallback jq extraction with [[ -f package.json ]] before execution.
🟡 Nit: Prune Redundant move-latest Check (scripts/publish.sh:40): The earlier elif branch handles move-latest and terminates via exit 0, making the condition in line 40 dead code.
🟡 Nit: Quote Interpolations in Shell Commands (scripts/firepit-builder/pipeline.js:125, 203, 209): Quote interpolated tags and version variables in shell execution calls.

joehan · 2026-09-03T20:33:43Z

+    .filter(Boolean);
+  echo(`Found attached assets:\n${attachedAssets.map((a) => "  - " + a).join("\n")}`);
+
+  const missing = publishedFiles.filter((f) => !attachedAssets.includes(f));


🔴 [Pipeline Verification] Hub Release Show Asset Formatting Mismatch

Rationale:
In GitHub's hub CLI (commands/release.go), the format placeholder %as expands to strings formatted as "<DownloadURL>\t<Label>":

assets[i] = fmt.Sprintf("%s\t%s", asset.DownloadURL, asset.Label)

Consequently, showResult.stdout.split("\n") will contain download URLs (e.g. https://github.com/.../releases/download/v.../firebase-tools-linux\t). Matching directly via attachedAssets.includes(f) will fail because attachedAssets entries are full URLs / tab-separated strings, causing missing to trigger a false-positive validation failure and abort the pipeline.

Suggested Fix:
Extract the basename from each URL before checking inclusion:

const attachedAssets = showResult.stdout .split("\n") .map((s) => s.trim().split(/[\t\s]/)[0]) .filter(Boolean) .map((url) => path.basename(url));

joehan · 2026-09-03T20:33:43Z

+          if (matchedRelease && Array.isArray(matchedRelease.assets)) {
+            for (const expectedFile of publishedFiles) {
+              const asset = matchedRelease.assets.find((a) => a.name === expectedFile);
+              if (asset) {


🔴 [Error Handling & Verification Safety] Silent Bypass on Missing Artifacts in API Check

Rationale:
In the GitHub API asset verification loop:

for (const expectedFile of publishedFiles) { const asset = matchedRelease.assets.find((a) => a.name === expectedFile); if (asset) { // validations... } }

If an expected asset was never attached or failed to upload entirely, asset is undefined, silently bypassing both the asset.state !== "uploaded" check and the asset.size <= 0 check.

Suggested Fix:
Explicitly throw when asset is missing:

Suggested change

if (asset) {

const asset = matchedRelease.assets.find((a) => a.name === expectedFile);

if (!asset) {

throw new Error(`Artifact ${expectedFile} is missing from GitHub release assets.`);

}

if (asset.state !== "uploaded") {

throw new Error(

`Artifact ${expectedFile} state is "${asset.state}", expected "uploaded".`,

);

}

if (typeof asset.size === "number" && asset.size <= 0) {

throw new Error(`Artifact ${expectedFile} has invalid size: ${asset.size}`);

}

joehan · 2026-09-03T20:33:44Z

+    TARGET_VERSION=$(cat /workspace/version_number.txt)
+  fi
+  if [[ -z "$TARGET_VERSION" ]]; then
+    TARGET_VERSION=$(jq -r ".version" package.json)


🟡 Nit: Guard package.json file existence before invoking jq

Rationale:
If publish.sh is invoked from a directory where package.json is not present (or in an unexpected working directory), jq will fail with a non-zero exit code. With set -e enabled at line 2, this will terminate the script without an actionable error message.

Suggested Fix:

Suggested change

TARGET_VERSION=$(jq -r ".version" package.json)

if [[ -z "$TARGET_VERSION" && -f package.json ]]; then

TARGET_VERSION=$(jq -r ".version" package.json)

fi

joehan · 2026-09-03T20:33:44Z

    exit 1
  fi
-elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major") ]]; then
+elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major" || $VERSION == "move-latest" || $VERSION == "tag-latest") ]]; then


🟡 Nit: Redundant condition in version check

Rationale:
The cases move-latest and tag-latest are already handled in the elif [[ $VERSION == "move-latest" || $VERSION == "tag-latest" ]] block at line 22, which terminates the script via exit 0. As a result, checking move-latest and tag-latest again in this negative condition is redundant.

Suggested Fix:

Suggested change

elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major" || $VERSION == "move-latest" || $VERSION == "tag-latest") ]]; then

elif [[ ! ($VERSION == "patch" || $VERSION == "minor" || $VERSION == "major") ]]; then

joehan · 2026-09-03T20:33:44Z

+            echo "Updating npm latest tag to $${VERSION_NUM}..."
+            npm dist-tag add "firebase-tools@$${VERSION_NUM}" latest --registry https://wombat-dressing-room.appspot.com
+            npm dist-tag rm firebase-tools staging --registry https://wombat-dressing-room.appspot.com || true
+          fi


🟡 Nit: Defensive handling when removing staging tag

Rationale:
Running npm dist-tag rm firebase-tools staging ... || true gracefully handles when the staging tag was already removed in pipeline.js. If npm dist-tag rm prints error noise to stderr when the tag is absent, consider suppressing stderr or checking tag existence:

npm dist-tag rm firebase-tools staging --registry https://wombat-dressing-room.appspot.com 2>/dev/null || true

…dation ### Description Addresses code review comments on the release pipeline: - Parse asset basenames from `hub release show -f "%as"` output to prevent false-positive validation failures. - Throw an explicit error if an expected artifact is missing in the GitHub API release health check. - Safely check for `package.json` before running `jq` in `publish.sh`. - Prune redundant `move-latest` and `tag-latest` conditions in `publish.sh`. - Quote shell command interpolations in `pipeline.js`. - Suppress stderr when removing the staging npm dist-tag in `publish.sh` and `cloudbuild.yaml`. ### Scenarios Tested - Ran `bash -n scripts/publish.sh` - Ran `node -c scripts/firepit-builder/pipeline.js` - Ran `npm run lint:changed-files` (0 errors) - Ran prettier formatting check

joehan · 2026-09-03T21:40:14Z

Thanks for the reviews! Addressed all review comments in commit ebca2ae:

Asset Name Parsing in Hub Query: Extracted basenames from hub release show -f "%as" tab-delimited download URLs to prevent false-positive validation mismatches.
GitHub API Artifact Validation: Added an explicit check throwing an error if an expected asset is missing from the GitHub release, and rethrow artifact validation failures to fail the release.
Shell Command Quoting: Quoted interpolated release tags and version strings in pipeline.js.
Package.json Existence Guard: Added [[ -f package.json ]] check before running jq in publish.sh, with clear error messaging if target version cannot be resolved.
Redundant Condition Pruned: Removed redundant move-latest and tag-latest checks from publish.sh.
Staging Tag Removal: Added 2>/dev/null || true to suppress noise when removing the staging npm dist-tag in publish.sh and cloudbuild.yaml.
Ran linter and formatting checks clean.

joehan · 2026-09-03T22:00:47Z

+    "--registry",
+    registry,
+  );
+  shelljs.config.fatal = false;


Add a comment about intentionally not erroring out if we can't remove the staging tag - this is weird behavior otherwise

### Description Removes the unused `move-latest` and `tag-latest` options from `scripts/publish.sh` to keep the script and PR minimal and focused on the core release workflow. ### Scenarios Tested - Ran `bash -n scripts/publish.sh` - Ran `npm run lint:changed-files` (0 errors)

gemini-code-assist Bot reviewed Sep 3, 2026

View reviewed changes

joehan commented Sep 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: avoid breaking standalone downloads during release pipeline - #11029

fix: avoid breaking standalone downloads during release pipeline#11029
joehan wants to merge 3 commits into
mainfrom
jh/safe-release-artifacts

joehan commented Sep 3, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Sep 3, 2026

Uh oh!

gemini-code-assist Bot Sep 3, 2026

Uh oh!

joehan commented Sep 3, 2026

Uh oh!

joehan left a comment

Uh oh!

joehan Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Uh oh!

joehan commented Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	elif [[ ! ($VERSION == "patch" \|\| $VERSION == "minor" \|\| $VERSION == "major" \|\| $VERSION == "move-latest" \|\| $VERSION == "tag-latest") ]]; then
	elif [[ ! ($VERSION == "patch" \|\| $VERSION == "minor" \|\| $VERSION == "major") ]]; then

-              if (asset) {
+              const asset = matchedRelease.assets.find((a) => a.name === expectedFile);
+              if (!asset) {
+                throw new Error(`Artifact ${expectedFile} is missing from GitHub release assets.`);
+              }
+              if (asset.state !== "uploaded") {
+                throw new Error(
+                  `Artifact ${expectedFile} state is "${asset.state}", expected "uploaded".`,
+                );
+              }
+              if (typeof asset.size === "number" && asset.size <= 0) {
+                throw new Error(`Artifact ${expectedFile} has invalid size: ${asset.size}`);
+              }

Uh oh!

Conversation

joehan commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Scenarios Tested

Sample Commands

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan commented Sep 3, 2026

Uh oh!

joehan left a comment

Choose a reason for hiding this comment

Code Review Summary: firebase/firebase-tools (PR #11029)

🟢 Strengths & LGTM Aspects

🔴 Overview of Findings & Action Items

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

joehan commented Sep 3, 2026

Uh oh!

joehan Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joehan commented Sep 3, 2026 •

edited

Loading

Code Review Summary: `firebase/firebase-tools` (PR #11029)