Skip to content

Commit ef2ee44

Browse files
gustavoliraclaude
andcommitted
chore: apply local review findings — dep bumps, workflow dedup/cache, small cleanups
- bump esbuild 0.24.2 -> 0.28.1 (clears GHSA-67mh-4wv8-2f99 advisory range) and typescript 6.0.2 -> 6.0.3; engines.yarn ">=4" to match packageManager yarn@4 - workflow: single source of truth for the default plugin ref (env only, input default removed); corepack before setup-node + yarn cache keyed on smoke-tests-native/yarn.lock - native-smoke: shared BackendStartResult type; writeErrorReport helper also covers usage errors (results.json now written on exit 2 too); drop unused env param from run(); single-pass KNOWN_FAILURES partition - loader: normalize "./dist/…" package mains; type-guard filter instead of "as string[]"; hoist repeated role check Verified on Node 24: tsc clean, esbuild build ok, end-to-end smoke run status: pass (backend loaded 1/1, startTestBackend booted), usage-error path writes results.json (status: error) and exits 2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ee02d01 commit ef2ee44

5 files changed

Lines changed: 181 additions & 160 deletions

File tree

.github/workflows/native-smoke.yaml

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ on:
2323
inputs:
2424
plugin_ref:
2525
description: >-
26-
OCI package ref to validate (oci://…:tag!name). Defaults to a known-good
27-
pure-backend plugin. Catalog-extending modules are not supported yet.
26+
OCI package ref to validate (oci://…:tag!name). Leave empty to use the
27+
known-good pure-backend default (env.DEFAULT_PLUGIN_REF below).
28+
Catalog-extending modules are not supported yet.
2829
type: string
2930
required: false
30-
default: "oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/roadiehq-scaffolder-backend-module-http-request:bs_1.49.4__5.6.0!roadiehq-scaffolder-backend-module-http-request"
3131

3232
concurrency:
3333
group: ${{ github.workflow }}-${{ github.event.number || github.ref }}
@@ -38,7 +38,8 @@ permissions:
3838
packages: read
3939

4040
env:
41-
# Default plugin validated on pull_request; overridable via the dispatch input.
41+
# Single source of truth for the plugin validated on pull_request and when the
42+
# dispatch input is left empty.
4243
DEFAULT_PLUGIN_REF: "oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/roadiehq-scaffolder-backend-module-http-request:bs_1.49.4__5.6.0!roadiehq-scaffolder-backend-module-http-request"
4344

4445
jobs:
@@ -54,13 +55,16 @@ jobs:
5455
with:
5556
persist-credentials: false
5657

58+
- name: Enable Corepack
59+
# Before setup-node so its yarn-cache detection resolves Yarn 4 via corepack.
60+
run: corepack enable
61+
5762
- name: Setup Node.js
5863
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
5964
with:
6065
node-version: 24
61-
62-
- name: Enable Corepack
63-
run: corepack enable
66+
cache: yarn
67+
cache-dependency-path: smoke-tests-native/yarn.lock
6468

6569
- name: Install skopeo
6670
# The install-dynamic-plugins CLI shells out to skopeo to pull OCI plugins.

smoke-tests-native/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"description": "Docker-free, in-process smoke harness for RHDH dynamic backend plugins: install via the published install-dynamic-plugins CLI, then boot with startTestBackend — no container, no cluster.",
77
"engines": {
88
"node": ">=24",
9-
"yarn": ">=3"
9+
"yarn": ">=4"
1010
},
1111
"packageManager": "yarn@4.12.0",
1212
"scripts": {
@@ -23,7 +23,7 @@
2323
"devDependencies": {
2424
"@red-hat-developer-hub/cli-module-install-dynamic-plugins": "0.3.0",
2525
"@types/node": "24.13.2",
26-
"esbuild": "0.24.2",
27-
"typescript": "6.0.2"
26+
"esbuild": "0.28.1",
27+
"typescript": "6.0.3"
2828
}
2929
}

smoke-tests-native/src/loader.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,16 @@ export function discoverPlugins(root: string): PluginManifest {
6767
continue;
6868
}
6969
const role: string = pkg.backstage?.role ?? "";
70+
const isFrontend = role.includes("frontend");
7071
const item: PluginEntry = {
7172
name: pkg.name ?? entry.name,
7273
version: pkg.version ?? "0.0.0",
7374
dirName: entry.name,
7475
path: dir,
75-
role: role.includes("frontend") ? "frontend" : "backend",
76+
role: isFrontend ? "frontend" : "backend",
7677
};
7778

78-
if (role.includes("frontend")) frontend.push(item);
79+
if (isFrontend) frontend.push(item);
7980
else if (role.includes("backend")) backend.push(item);
8081
// dirs without a backstage role aren't plugins — skip
8182
}
@@ -90,12 +91,14 @@ function resolveEntryPoint(pluginPath: string): string {
9091
throw new Error(`package.json not found in ${pluginPath}`);
9192
}
9293
const pkg = JSON.parse(readFileSync(pkgPath, "utf8"));
94+
// Normalize "./dist/…" → "dist/…" so an explicit main is not silently excluded.
95+
const main: string | undefined = pkg.main?.replace(/^\.\//, "");
9396
const candidates = [
9497
"dist/index.cjs.js",
9598
"dist/index.esm.js",
9699
"dist/index.js",
97-
pkg.main?.startsWith("dist/") ? pkg.main : undefined,
98-
].filter(Boolean) as string[];
100+
main?.startsWith("dist/") ? main : undefined,
101+
].filter((c): c is string => Boolean(c));
99102

100103
for (const candidate of candidates) {
101104
const full = join(pluginPath, candidate);

smoke-tests-native/src/native-smoke.ts

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ const CLI_BIN = join(
7373
const coreFeatures = [scaffolderPlugin];
7474

7575
type Status = "pass" | "fail-load" | "fail-start" | "fail-bundle" | "error";
76+
type BackendStartResult = { ok: boolean; skipped?: boolean; error?: string };
7677
type Report = {
7778
cliVersion: string;
7879
backend: {
@@ -81,19 +82,34 @@ type Report = {
8182
skipped: string[];
8283
errors: PluginError[];
8384
};
84-
backendStart: { ok: boolean; skipped?: boolean; error?: string };
85+
backendStart: BackendStartResult;
8586
frontend: { total: number; valid: number; errors: PluginError[] };
8687
status: Status;
8788
};
8889

8990
// execFileSync (args array, no shell) so workspace names / OCI refs can never be
9091
// interpolated into a shell command as this grows beyond a single fixed plugin.
91-
function run(file: string, args: string[], env?: NodeJS.ProcessEnv): string {
92-
return execFileSync(file, args, {
93-
encoding: "utf-8",
94-
stdio: "pipe",
95-
env: { ...process.env, ...env },
96-
}).trim();
92+
function run(file: string, args: string[]): string {
93+
return execFileSync(file, args, { encoding: "utf-8", stdio: "pipe" }).trim();
94+
}
95+
96+
// Any failure — bad args, install CLI crash, boot error before the report is built —
97+
// still produces a results.json (status: error), so a consumer never reads a stale
98+
// "pass" or finds no report at all.
99+
async function writeErrorReport(
100+
out: string,
101+
cliVersion: string,
102+
message: string,
103+
): Promise<void> {
104+
const report: Report = {
105+
cliVersion,
106+
backend: { total: 0, loaded: 0, skipped: [], errors: [] },
107+
backendStart: { ok: false, error: message },
108+
frontend: { total: 0, valid: 0, errors: [] },
109+
status: "error",
110+
};
111+
await writeFile(out, JSON.stringify(report, null, 2));
112+
console.error(`▶ report → ${out} (status: error)\n${message}`);
97113
}
98114

99115
function computeStatus(
@@ -122,9 +138,7 @@ async function extractPlugins(root: string, dynamicPlugins: string): Promise<voi
122138
}
123139

124140
// Boot the loaded backend features in-process to confirm they integrate.
125-
async function startBackend(
126-
loaded: LoadedPlugin[],
127-
): Promise<{ ok: boolean; skipped?: boolean; error?: string }> {
141+
async function startBackend(loaded: LoadedPlugin[]): Promise<BackendStartResult> {
128142
// No backend plugins (e.g. a frontend-only workspace) — boot wasn't attempted, not a
129143
// failure. Flag it so results.json doesn't read like the backend crashed.
130144
if (loaded.length === 0) return { ok: true, skipped: true };
@@ -172,11 +186,11 @@ async function main(): Promise<number> {
172186
const dynamicPlugins = values["dynamic-plugins"];
173187

174188
if (!dynamicPlugins) {
175-
console.error("Provide --dynamic-plugins <dynamic-plugins.yaml>.");
189+
await writeErrorReport(out, "unknown", "Provide --dynamic-plugins <dynamic-plugins.yaml>.");
176190
return 2;
177191
}
178192
if (!existsSync(dynamicPlugins)) {
179-
console.error(`dynamic-plugins file not found: ${dynamicPlugins}`);
193+
await writeErrorReport(out, "unknown", `dynamic-plugins file not found: ${dynamicPlugins}`);
180194
return 2;
181195
}
182196

@@ -204,18 +218,18 @@ async function main(): Promise<number> {
204218
// Let extracted plugins (under a temp dir) resolve their @backstage/* peers here.
205219
patchModuleResolution(HARNESS_NODE_MODULES);
206220

207-
const skipped = manifest.backend
208-
.filter((p) => KNOWN_FAILURES.has(p.dirName))
209-
.map((p) => p.dirName);
221+
// Partition in one pass so `skipped` and `backendPlugins` stay complementary.
222+
const skipped: string[] = [];
223+
const backendPlugins: PluginEntry[] = [];
224+
for (const p of manifest.backend) {
225+
if (KNOWN_FAILURES.has(p.dirName)) skipped.push(p.dirName);
226+
else backendPlugins.push(p);
227+
}
210228
if (skipped.length > 0) {
211229
console.warn(
212230
`⚠ skipped ${skipped.length} known-failure backend plugin(s): ${skipped.join(", ")}`,
213231
);
214232
}
215-
216-
const backendPlugins = manifest.backend.filter(
217-
(p) => !KNOWN_FAILURES.has(p.dirName),
218-
);
219233
const { loaded, errors: loadErrors } = loadBackendPlugins(backendPlugins);
220234
const start = await startBackend(loaded);
221235
const frontend = validateFrontends(manifest.frontend);
@@ -258,18 +272,8 @@ async function main(): Promise<number> {
258272
);
259273
return report.status === "pass" ? 0 : 1;
260274
} catch (err) {
261-
// Any failure before the report is built (e.g. the install CLI failing on a bad
262-
// OCI ref) still writes a results.json, so a consumer never reads a stale "pass".
263-
const message = err instanceof Error ? err.message : String(err);
264-
const report: Report = {
265-
cliVersion,
266-
backend: { total: 0, loaded: 0, skipped: [], errors: [] },
267-
backendStart: { ok: false, error: message },
268-
frontend: { total: 0, valid: 0, errors: [] },
269-
status: "error",
270-
};
271-
await writeFile(out, JSON.stringify(report, null, 2));
272-
console.error(`▶ report → ${out} (status: error)\n${message}`);
275+
// e.g. the install CLI failing on a bad OCI ref — see writeErrorReport.
276+
await writeErrorReport(out, cliVersion, err instanceof Error ? err.message : String(err));
273277
return 1;
274278
} finally {
275279
if (tempDir) await rm(tempDir, { recursive: true, force: true });

0 commit comments

Comments
 (0)