Skip to content

Commit bfbf8f6

Browse files
janechuCopilot
andcommitted
fix(ci): address PR review feedback
- download-github-releases.mjs: tighten release tag regex to `_v\d+\.\d+\.\d+` (require a patch component) and explicitly exclude `deployed/<tag>` marker tags before applying it, so markers aren't re-treated as undeployed releases. - download-github-releases.mjs: clear `publish_artifacts_stage/` before each iteration and delete unknown-type files after warning, so leftovers from a previous tag don't trigger repeated warnings or incorrect moves on subsequent tags. - azure-pipelines-cd.yml: make the Mark-deployed step idempotent by checking for the marker tag via `git rev-parse --verify` before creating + pushing. Previously, a re-run after partial success would have aborted with `set -e` on the first pre-existing tag. - .github/workflows/README.md: update the release-job description to reflect the atomic `gh release create --target <sha>` flow (the earlier docs still claimed the workflow pushed an annotated tag before creating the release). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 49d9f9f commit bfbf8f6

3 files changed

Lines changed: 28 additions & 3 deletions

File tree

.github/workflows/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Nightly publishing is split into two coordinated jobs so that npm credentials ne
1717

1818
- **`cd-github-releases.yml`** (GitHub Actions) triggers on **`push` to `main`** and on `workflow_dispatch`. It does **not** bump versions or push source changes to `main` — version bumps land on `main` through ordinary human-authored pull requests (for example, by running `npm run bump` locally and opening a PR). The workflow has two jobs:
1919
1. **`detect`** — checks out `main` with `fetch-depth: 0` and runs [`build/scripts/create-github-releases.mjs --check-only`](../../build/scripts/create-github-releases.mjs). The script walks the workspaces tree (no `npm ci` required), computes `${name}_v${version}` for every non-private workspace, and emits `hasMissingReleases=true` if any of those git tags do not yet exist.
20-
2. **`release`** runs only when missing releases exist. Installs Node, the Rust toolchain (for `cargo package`), and the npm workspace dependencies, builds the repo, then runs the script in default mode. For every missing release the script: packs the npm tarball into `publish_artifacts_npm/`, packs the paired Rust crate (if `crates/<crate-name>/Cargo.toml` exists) into `publish_artifacts_crates/`, pushes an annotated `${name}_v${version}` git tag, and creates the GitHub release with both assets attached (via `gh release create --verify-tag`). The script errors if a paired crate's version does not match the npm package's version — but this is purely a safety net: the [`postbump` hook in `beachball.config.js`](../../beachball.config.js) rewrites the crate's `Cargo.toml` (and the matching entry in `Cargo.lock`) automatically whenever `npm run bump` bumps the paired npm package, so the two stay in sync from the same commit.
20+
2. **`release`** runs only when missing releases exist. Installs Node, the Rust toolchain (for `cargo package`), and the npm workspace dependencies, builds the repo, then runs the script in default mode. For every missing release the script: packs the npm tarball into `publish_artifacts_npm/`, packs the paired Rust crate (if `crates/<crate-name>/Cargo.toml` exists) into `publish_artifacts_crates/`, and creates the GitHub release with both assets attached via `gh release create --target <sha>`. The `gh` CLI creates the git tag atomically with the release, so "tag exists" and "release exists" are always the same fact — a failed release is safely retried on the next workflow run, with no orphan tag stranded behind. The script errors if a paired crate's version does not match the npm package's version — but this is purely a safety net: the [`postbump` hook in `beachball.config.js`](../../beachball.config.js) rewrites the crate's `Cargo.toml` (and the matching entry in `Cargo.lock`) automatically whenever `npm run bump` bumps the paired npm package, so the two stay in sync from the same commit.
2121
- **`azure-pipelines-cd.yml`** (Azure Pipelines) runs every night at **1am PST (`0 9 * * *` UTC)** with `always: true` so it still runs on no-op nights (it is checking external GitHub state, not repo commits). It is split into two stages so the heavy publish work is skipped on no-op nights:
2222
1. **`Check`** — runs [`build/scripts/download-github-releases.mjs --check-only`](../../build/scripts/download-github-releases.mjs). The script lists every git tag matching the beachball-style `${name}_v${version}` pattern that does **not** also have a `deployed/<tag>` counterpart and emits the comma-separated list via Azure Pipelines `setvariable` output variables (`needsDeployment`, `undeployedTags`). No network calls to npm or crates.io.
2323
2. **`Package`** — depends on `Check` and runs only when `needsDeployment == 'true'`. Downloads every undeployed release's assets via `gh release download`, sorts them into `publish_artifacts_npm/` (`.tgz`) and `publish_artifacts_crates/` (`.crate`), hands off to `FAST.Release.PipelineTemplate.yml@fastPipelines` for the actual `npm publish` / `cargo publish`, and on success pushes a `deployed/<tag>` git marker tag for each release that was just published. The next nightly run will see those markers and skip the corresponding releases.

azure-pipelines-cd.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ extends:
122122
# Push a `deployed/<tag>` marker tag for each release that was just
123123
# published so that the next nightly Check stage sees it and skips
124124
# the release. Reads the list of tags written by the download script.
125+
# Idempotent: a marker tag that already exists locally (i.e. fetched
126+
# from origin via `fetchTags: true`) is left alone instead of failing.
125127
- script: |
126128
set -euo pipefail
127129
META_FILE=publish_artifacts_meta/undeployed-tags.txt
@@ -132,6 +134,10 @@ extends:
132134
while IFS= read -r tag; do
133135
[ -z "$tag" ] && continue
134136
DEPLOY_TAG="deployed/${tag}"
137+
if git rev-parse --verify --quiet "refs/tags/${DEPLOY_TAG}" >/dev/null; then
138+
echo "Already marked deployed: ${DEPLOY_TAG} (skipping)"
139+
continue
140+
fi
135141
echo "Marking deployed: ${DEPLOY_TAG}"
136142
git tag "${DEPLOY_TAG}"
137143
git push origin "${DEPLOY_TAG}"

build/scripts/download-github-releases.mjs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,14 @@
3939
*/
4040

4141
import { execFileSync } from "node:child_process";
42-
import { mkdirSync, readdirSync, renameSync, writeFileSync } from "node:fs";
42+
import {
43+
mkdirSync,
44+
readdirSync,
45+
renameSync,
46+
rmSync,
47+
unlinkSync,
48+
writeFileSync,
49+
} from "node:fs";
4350
import { join } from "node:path";
4451

4552
const NPM_DIR = "publish_artifacts_npm";
@@ -75,7 +82,14 @@ const allTags = listGitTags();
7582
const deployed = new Set(
7683
allTags.filter(t => t.startsWith("deployed/")).map(t => t.slice("deployed/".length)),
7784
);
78-
const releaseTags = allTags.filter(t => /_v\d+\.\d+/.test(t));
85+
// Match beachball's release tag format: `${name}_v${major}.${minor}.${patch}`,
86+
// where the version is the publishable portion (`1.0.0` or `1.0.0-alpha.3`).
87+
// `deployed/` marker tags are excluded explicitly so they aren't re-processed
88+
// as if they were releases.
89+
const RELEASE_TAG_RE = /_v\d+\.\d+\.\d+/;
90+
const releaseTags = allTags.filter(
91+
t => !t.startsWith("deployed/") && RELEASE_TAG_RE.test(t),
92+
);
7993
const undeployed = releaseTags.filter(t => !deployed.has(t)).sort();
8094

8195
console.log(`Release tags total: ${releaseTags.length}`);
@@ -106,6 +120,10 @@ const processed = [];
106120
for (const tag of undeployed) {
107121
try {
108122
console.log(`\nDownloading assets for ${tag}...`);
123+
// Clear the stage dir before each iteration so leftover unknown-type
124+
// files from a previous release are not reprocessed (or warned about
125+
// repeatedly).
126+
rmSync(STAGE_DIR, { recursive: true, force: true });
109127
mkdirSync(STAGE_DIR, { recursive: true });
110128
run(
111129
"gh",
@@ -121,6 +139,7 @@ for (const tag of undeployed) {
121139
renameSync(src, join(CRATES_DIR, file));
122140
} else {
123141
console.warn(` Ignoring unknown asset type: ${file}`);
142+
unlinkSync(src);
124143
}
125144
}
126145

0 commit comments

Comments
 (0)