fix: increment build number for preview releases - #10946
Open
chkuang-g wants to merge 3 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the scripts/publish.sh script to dynamically determine and bump preview versions by querying the npm registry for existing versions with the same prefix. Feedback was provided to improve the robustness of the version parsing logic, specifically pointing out that dots in the prefix could act as wildcards in sed and that non-numeric suffixes could cause arithmetic syntax errors.
### Description When running `./scripts/publish/run.sh preview <branch>` multiple times, the version generated by `npm version prerelease` was always `X.Y.Z-<branch>.0` because the temporary clone checked out by Cloud Build always has the base version in `package.json`, and the bumped version is not pushed back to the branch. Consequently, subsequent publishes of the preview package failed with a `403 Forbidden` error because overwriting an already published package version on the registry is not allowed. This change updates `scripts/publish.sh` to get the latest 6-character commit SHA and append it to the preview version (e.g. `15.27.1-chkuang-connect-to-a1b2c3`), ensuring a unique version for each commit without requiring registry checks. ### Scenarios Tested - Tested version generation in local mock environment. - Verified `npm run lint:changed-files` passes. ### Sample Commands `./scripts/publish/run.sh preview chkuang/connect-to`
chkuang-g
force-pushed
the
chkuang/fix-preview-publish
branch
from
August 18, 2026 00:18
87a296a to
8aa8558
Compare
### Description Reverts the preview versioning logic back to using an incremental counter queried from the registry. This is more robust against build retries on the same commit and ensures proper chronological SemVer comparison. ### Scenarios Tested - Tested version logic locally with mock registry outputs. - Verified lint checks pass. ### Sample Commands `./scripts/publish/run.sh preview chkuang/connect-to`
### Description
Improves the robustness of the preview build suffix calculation by:
1. Using a safer `sed` expression `s/^.*\.//` to strip the prefix instead of literal regular expression interpolation of `${PREFIX}` (which contains dots).
2. Filtering out any non-numeric suffixes or empty outputs with `grep -E '^[0-9]+$'` to ensure only integer suffixes are considered.
3. Supplying a default fallback value of `0` in case no numeric suffixes are found, preventing syntax errors in the arithmetic expansion.
### Scenarios Tested
- Verified version incrementing logic with mock outputs.
### Sample Commands
`./scripts/publish/run.sh preview chkuang/connect-to`
Member
|
/joe-review |
joehan
approved these changes
Aug 18, 2026
joehan
left a comment
joehan
left a comment
Member
There was a problem hiding this comment.
⚠️ Disclaimer: This draft review was generated by an experimental AI review agent. Please verify all findings before acting on them.
Code Review Summary: firebase/firebase-tools
🟢 Strengths & LGTM Aspects
- Dynamic Versioning: The script dynamically queries the NPM registry to find existing preview versions and increments the suffix, preventing 403 publish conflicts.
- Robust Execution: Added
|| trueon registry queries to degrade gracefully if the query fails, and sanitized the branch name to keep version strings valid. - Clean Registry Integration: Correctly uses the staging/preview registry (
wombat-dressing-room) for checks.
🔴 Overview of Findings
- 1 Nit: Recommendation to use
jq --arginstead of shell string interpolation in thejqfilter to follow best practices for safety and readability.
| INITIAL_VERSION=$(jq -r ".version" package.json) | ||
| PREFIX=${INITIAL_VERSION%.*} | ||
| echo "Checking registry for existing preview versions with prefix ${PREFIX}..." | ||
| MATCHING_VERSIONS=$(npm view firebase-tools versions --registry https://wombat-dressing-room.appspot.com --json | jq -r 'if type == "array" then .[] else . end | select(startswith("'"$PREFIX"'."))' || true) |
Member
There was a problem hiding this comment.
🟡 Nit: Use jq --arg instead of shell interpolation
Rationale: Using shell interpolation inside jq filter strings can lead to escaping issues if variables contain special characters. Passing variables via jq --arg is safer and cleaner.
Suggested Fix:
MATCHING_VERSIONS=$(npm view firebase-tools versions --registry https://wombat-dressing-room.appspot.com --json | jq -r --arg prefix "$PREFIX" 'if type == "array" then .[] else . end | select(startswith($prefix + "."))' || true)
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
3 participants
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.
Description
When running
./scripts/publish/run.sh preview <branch>multiple times, the version generated bynpm version prereleasewas alwaysX.Y.Z-<branch>.0because the temporary clone checked out by Cloud Build always has the base version inpackage.json, and the bumped version is not pushed back to the branch. Consequently, subsequent publishes of the preview package failed with a403 Forbiddenerror because overwriting an already published package version on the registry is not allowed.This change updates
scripts/publish.shto check the registry for existing preview versions with the same prefix and increment the suffix dynamically.Scenarios Tested
npm run lint:changed-filespasses.Sample Commands
./scripts/publish/run.sh preview $BRANCH