Skip to content

Commit aa73f47

Browse files
janechuCopilot
andcommitted
fix(ci): address second round of PR review feedback
- download-github-releases.mjs: fail fast with a clear message when `GH_TOKEN` is missing in non-`--check-only` mode (where `gh release download` would otherwise produce a generic auth error mid-loop). - download-github-releases.mjs: clear `publish_artifacts_npm/`, `publish_artifacts_crates/`, and `publish_artifacts_meta/` at the start of non-check mode (in addition to the per-iteration `publish_artifacts_stage/` cleanup) so stale `.tgz`/`.crate` files from a partially-failed previous run cannot be reused. - download-github-releases.mjs: refuse to mark a tag as processed when its release contained no `.tgz` or `.crate` assets. Without this the Azure pipeline would push the `deployed/<tag>` marker for a release that never actually published anything, so the problem would never be retried. - download-github-releases.mjs + create-github-releases.mjs: narrow the `catch (error)` payload with `error instanceof Error ? error.message : String(error)` so we do not log `undefined` (or crash trying to read `.message`) if a non-`Error` value is thrown. - create-github-releases.mjs: fail fast with a clear message when `GH_TOKEN` is missing in non-`--check-only` mode (the script needs it to call `gh release create`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bfbf8f6 commit aa73f47

2 files changed

Lines changed: 42 additions & 5 deletions

File tree

build/scripts/create-github-releases.mjs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,15 @@ if (CHECK_ONLY || missing.length === 0) {
176176
process.exit(0);
177177
}
178178

179+
// `--check-only` only enumerates missing releases via local git state,
180+
// but creating releases requires `gh release create`, which needs a
181+
// token. Fail fast here so the workflow surfaces a clear error rather
182+
// than a generic `gh` auth failure mid-loop.
183+
if (!process.env.GH_TOKEN) {
184+
console.error("GH_TOKEN must be set so the `gh` CLI can create GitHub releases.");
185+
process.exit(1);
186+
}
187+
179188
mkdirSync(NPM_DIR, { recursive: true });
180189
mkdirSync(CRATES_DIR, { recursive: true });
181190

@@ -268,7 +277,8 @@ for (const { name, version, location, crateName, cargoTomlPath } of missing) {
268277
created += 1;
269278
} catch (error) {
270279
hasErrors = true;
271-
console.error(`Failed to release ${name}@${version}:`, error.message);
280+
const message = error instanceof Error ? error.message : String(error);
281+
console.error(`Failed to release ${name}@${version}: ${message}`);
272282
}
273283
}
274284

build/scripts/download-github-releases.mjs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ if (!repo) {
6262
process.exit(1);
6363
}
6464

65+
// `--check-only` works against the local git tag list and never invokes
66+
// `gh`, so it does not need a token. In non-check mode we call
67+
// `gh release download`, which fails with a generic auth error if
68+
// `GH_TOKEN` is missing; fail fast here with a clearer message.
69+
if (!CHECK_ONLY && !process.env.GH_TOKEN) {
70+
console.error("GH_TOKEN must be set so the `gh` CLI can download release assets.");
71+
process.exit(1);
72+
}
73+
6574
function run(file, args, opts = {}) {
6675
return execFileSync(file, args, { encoding: "utf8", ...opts });
6776
}
@@ -110,9 +119,13 @@ if (CHECK_ONLY || undeployed.length === 0) {
110119
process.exit(0);
111120
}
112121

113-
mkdirSync(NPM_DIR, { recursive: true });
114-
mkdirSync(CRATES_DIR, { recursive: true });
115-
mkdirSync(META_DIR, { recursive: true });
122+
// Clear the publish artifact directories before recreating them so a
123+
// re-run (locally or after a partially-failed Azure attempt) cannot
124+
// pick up stale `.tgz` / `.crate` files from a previous invocation.
125+
for (const dir of [NPM_DIR, CRATES_DIR, META_DIR]) {
126+
rmSync(dir, { recursive: true, force: true });
127+
mkdirSync(dir, { recursive: true });
128+
}
116129

117130
let hasErrors = false;
118131
const processed = [];
@@ -131,22 +144,36 @@ for (const tag of undeployed) {
131144
{ stdio: "inherit" },
132145
);
133146

147+
let foundAssets = 0;
134148
for (const file of readdirSync(STAGE_DIR)) {
135149
const src = join(STAGE_DIR, file);
136150
if (file.endsWith(".tgz")) {
137151
renameSync(src, join(NPM_DIR, file));
152+
foundAssets += 1;
138153
} else if (file.endsWith(".crate")) {
139154
renameSync(src, join(CRATES_DIR, file));
155+
foundAssets += 1;
140156
} else {
141157
console.warn(` Ignoring unknown asset type: ${file}`);
142158
unlinkSync(src);
143159
}
144160
}
145161

162+
// Refuse to mark a tag as processed if its release had no
163+
// recognised publish assets. Otherwise the Azure pipeline would
164+
// push the `deployed/<tag>` marker, and the (presumably broken)
165+
// release would never be retried.
166+
if (foundAssets === 0) {
167+
throw new Error(
168+
`Release ${tag} contained no .tgz or .crate assets; refusing to mark as deployed.`,
169+
);
170+
}
171+
146172
processed.push(tag);
147173
} catch (error) {
148174
hasErrors = true;
149-
console.error(`Failed to download release ${tag}:`, error.message);
175+
const message = error instanceof Error ? error.message : String(error);
176+
console.error(`Failed to download release ${tag}: ${message}`);
150177
}
151178
}
152179

0 commit comments

Comments
 (0)