From 1fa1305ee58677cf096bc78e33100d92a2416203 Mon Sep 17 00:00:00 2001 From: Jack Shelton Date: Mon, 24 Aug 2026 21:11:58 -0500 Subject: [PATCH 1/4] fix(cli): resolve astro add through yarn run and harden git/panic paths (#294) --- .../src/add-flow/command.ts | 6 +- libs/create-qwikdev-astro/src/app.ts | 55 +++++++++++++-- libs/create-qwikdev-astro/src/utils.ts | 68 +++++++++++++++++++ 3 files changed, 119 insertions(+), 10 deletions(-) diff --git a/libs/create-qwikdev-astro/src/add-flow/command.ts b/libs/create-qwikdev-astro/src/add-flow/command.ts index ec4f9248..fdfd1629 100644 --- a/libs/create-qwikdev-astro/src/add-flow/command.ts +++ b/libs/create-qwikdev-astro/src/add-flow/command.ts @@ -5,7 +5,7 @@ import pkg from "../../package.json" with { type: "json" }; import { type Definition as BaseDefinition, Program } from "../core.js"; import { assertPmResult, - npmSpec, + execLocalBin, resolveAbsoluteDir, stripJsonComments } from "../utils.js"; @@ -117,7 +117,7 @@ export class AddCommand extends Program { this.warn("No astro.config file found — running astro add directly."); if (!input.dryRun) { assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.absDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.absDir }), "astro add @qwik.dev/astro" ); } else { @@ -250,7 +250,7 @@ export class AddCommand extends Program { } assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.absDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.absDir }), "astro add @qwik.dev/astro" ); } diff --git a/libs/create-qwikdev-astro/src/app.ts b/libs/create-qwikdev-astro/src/app.ts index bd98d0a7..7e7a68da 100644 --- a/libs/create-qwikdev-astro/src/app.ts +++ b/libs/create-qwikdev-astro/src/app.ts @@ -1,3 +1,4 @@ +import { execFileSync } from "node:child_process"; import fs, { cpSync } from "node:fs"; import path from "node:path"; import { copySync, ensureDirSync, pathExistsSync } from "fs-extra/esm"; @@ -18,9 +19,10 @@ import { __dirname, assertPmResult, clearDir, + describeError, + execLocalBin, getPackageJson, notEmptyDir, - npmSpec, replacePackageJsonRunCommand, resolveAbsoluteDir, resolveRelativeDir, @@ -70,6 +72,39 @@ export type Input = Required> & { packageName: string; }; +const GIT_FALLBACK_NAME = "QwikDev Astro"; +const GIT_FALLBACK_EMAIL = "create-qwikdev-astro@users.noreply.github.com"; + +/** + * Extra `git` arguments seeding a committer identity, but ONLY when git has + * none of its own. + * + * `git commit` hard-fails with "Please tell me who you are" on any machine + * where neither config nor the environment supplies an identity — a fresh CI + * runner, a clean container — which turns the optional git step of a scaffold + * into a failed run. + * + * The probe is not optional: `git -c` outranks every config file, so passing + * these unconditionally would stamp this placeholder over the real + * `user.name` / `user.email` of every user who has git set up properly. + * `git var GIT_COMMITTER_IDENT` answers exactly the question that matters — + * "could git author a commit right now?" — including identities git derives + * on its own, and exits non-zero when it could not. + */ +function gitIdentityArgs(cwd: string): string[] { + try { + execFileSync("git", ["var", "GIT_COMMITTER_IDENT"], { cwd, stdio: "pipe" }); + return []; + } catch { + return [ + "-c", + `user.name=${GIT_FALLBACK_NAME}`, + "-c", + `user.email=${GIT_FALLBACK_EMAIL}` + ]; + } +} + export function defineDefinition(definition: UserDefinition): Definition { return { ...defaultDefinition, ...definition }; } @@ -433,7 +468,7 @@ export class Application extends Program { ); } else { assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.outDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.outDir }), "astro add @qwik.dev/astro" ); } @@ -454,7 +489,7 @@ export class Application extends Program { ); } else { assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.outDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.outDir }), "astro add @qwik.dev/astro" ); } @@ -468,7 +503,7 @@ export class Application extends Program { ); } else { assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.outDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.outDir }), "astro add @qwik.dev/astro" ); } @@ -481,7 +516,7 @@ export class Application extends Program { // No config file found — just run astro add directly if (!input.dryRun) { assertPmResult( - await pm.x(npmSpec("astro add @qwik.dev/astro"), { cwd: input.outDir }), + await execLocalBin("astro add @qwik.dev/astro", { cwd: input.outDir }), "astro add @qwik.dev/astro" ); } @@ -494,7 +529,7 @@ export class Application extends Program { this.copyTemplate(input); } } catch (e: any) { - this.panic(`${e.message ?? e}: . Please try it manually.`); + this.panic(`${describeError(e)} Please try it manually.`); } } @@ -733,6 +768,7 @@ export class Application extends Program { await $( "git", [ + ...gitIdentityArgs(outDir), "commit", "-m", `${addChanges ? "➕ Add @qwik.dev/astro" : "Initial commit 🎉"}` @@ -748,7 +784,12 @@ export class Application extends Program { s.stop(`${addChanges ? "Changes added to Git ✨" : "Git initialized 🎲"}`); } catch (e) { s.stop(`Git failed to ${addChanges ? "add new changes" : "initialize"}`); - if (!initialized) { + // Branch on `addChanges`, not `initialized`: in add mode against a + // directory with no .git yet, `initialized` is false but the step is + // still "add new changes", so keying off `initialized` reported + // "Git failed to initialize" for a failure that had nothing to do + // with init — and handed the user the wrong recovery command. + if (!addChanges) { this.error( "Git failed to initialize. You can do this manually by running: git init" ); diff --git a/libs/create-qwikdev-astro/src/utils.ts b/libs/create-qwikdev-astro/src/utils.ts index 28a5b590..6f1a365d 100644 --- a/libs/create-qwikdev-astro/src/utils.ts +++ b/libs/create-qwikdev-astro/src/utils.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path, { join, resolve, relative, basename } from "node:path"; import { fileURLToPath } from "node:url"; import { copySync, ensureDirSync, pathExistsSync } from "fs-extra/esm"; +import type { ProcessOptions, ProcessResult } from "panam/executor"; import pm from "panam/pm"; /** @@ -32,6 +33,73 @@ export function npmSpec(command: string): string { return pm.isDeno() ? `npm:${command}` : command; } +/** + * Run a CLI that lives in the project's local `node_modules/.bin` (currently + * only `astro add …`) through the active package manager. + * + * WHY yarn is special-cased: panam maps `pm.x()` onto `yarn exec` for yarn + * (`PackageManager#x` delegates to `#exec`, which emits `exec` for pnpm/yarn). + * On Windows, `yarn exec ` cannot resolve a locally installed binary: + * npm-style installs write BOTH an extension-less sh wrapper (`.bin/astro`) and + * a cmd shim (`.bin/astro.cmd`), and yarn v1's exec only ever looks for the + * extension-less file — which cmd.exe cannot execute. Every `astro add` call + * therefore failed on Windows + yarn, `assertPmResult` threw, and the CLI + * panicked (issue #294). + * + * `yarn run …` goes through yarn's own PATH setup, which prepends + * `node_modules/.bin` and lets the OS pick the `.cmd` shim, so it resolves the + * same binary on every platform. + * + * Every other package manager keeps the exact `pm.x(npmSpec(command))` path it + * has always used, including the deno `npm:` prefix applied by `npmSpec`. + */ +export function execLocalBin( + command: string, + options?: ProcessOptions +): Promise { + return pm.isYarn() ? pm.run(command, options) : pm.x(npmSpec(command), options); +} + +/** + * Render an unknown thrown value as one diagnostic sentence, keeping any + * `stderr` / `stdout` / `cause` detail the error happens to carry. + * + * WHY: the panic template used to be `${e.message ?? e}: .`, which threw away + * every detail a failed subprocess reported — the CI logs for issue #294 read + * literally "failed: .", which says something broke but not which command or + * why. Surface whatever the error actually knows instead. + */ +export function describeError(error: unknown): string { + const detail = (value: unknown): string => { + if (value === undefined || value === null) return ""; + if (value instanceof Error) return value.message.trim(); + if (typeof value === "string") return value.trim(); + return String(value).trim(); + }; + + const parts = [detail(error) || "Unknown error"]; + + if (typeof error === "object" && error !== null) { + const source = error as { stderr?: unknown; stdout?: unknown; cause?: unknown }; + + for (const [label, value] of [ + ["stderr", source.stderr], + ["stdout", source.stdout], + ["cause", source.cause] + ] as const) { + const text = detail(value); + // Skip empty streams and detail already contained in the message itself. + if (text && !parts.some((part) => part.includes(text))) { + parts.push(`${label}: ${text}`); + } + } + } + + const message = parts.join(" | "); + + return /[.!?:]$/.test(message) ? message : `${message}.`; +} + export const __filename = getModuleFilename(); export const __dirname = path.dirname(__filename); From cafa0d13a35f2d44e1a12bd2e3d7fb492cbad924 Mon Sep 17 00:00:00 2001 From: Jack Shelton Date: Mon, 24 Aug 2026 21:08:37 -0500 Subject: [PATCH 2/4] test: make cli specs windows-path-safe (#294) --- libs/create-qwikdev-astro/tests/jsx-strategy.spec.ts | 4 ++-- libs/create-qwikdev-astro/tests/smoke-pack.spec.ts | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libs/create-qwikdev-astro/tests/jsx-strategy.spec.ts b/libs/create-qwikdev-astro/tests/jsx-strategy.spec.ts index 24ae7f96..6b27c920 100644 --- a/libs/create-qwikdev-astro/tests/jsx-strategy.spec.ts +++ b/libs/create-qwikdev-astro/tests/jsx-strategy.spec.ts @@ -1,6 +1,6 @@ import { mkdtemp, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, sep } from "node:path"; import type { Assert } from "@japa/assert"; import { test } from "@japa/runner"; import { determineJsxStrategy } from "../src/add-flow/jsx-strategy.js"; @@ -65,7 +65,7 @@ test.group("scaffoldQwikComponent", () => { try { const strategy = determineJsxStrategy("primary"); const outPath = await scaffoldQwikComponent(tmpDir, strategy); - assert.include(outPath, "src/components/qwik"); + assert.include(outPath.split(sep).join("/"), "src/components/qwik"); await readFile(outPath, "utf-8"); } finally { await rm(tmpDir, { recursive: true }); diff --git a/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts b/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts index 5a34aa2f..ef2dc448 100644 --- a/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts +++ b/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts @@ -1,6 +1,8 @@ import { execSync } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync } from "node:fs"; -import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { isAbsolute, join } from "node:path"; +import { pathToFileURL } from "node:url"; import type { Assert } from "@japa/assert"; import { test } from "@japa/runner"; @@ -10,7 +12,7 @@ declare module "@japa/runner/core" { } } -const root = "/tmp/qwik-astro-smoke-pack"; +const root = join(tmpdir(), "qwik-astro-smoke-pack"); const pkgDir = join(import.meta.dirname!, ".."); const installDir = join(root, "install-test"); const pkgRoot = join(installDir, "node_modules", "@qwik.dev", "create-astro"); @@ -51,7 +53,7 @@ function ensureBuiltPackage() { throw new Error("npm pack produced no tarball name on stdout"); } - tarball = lastLine.startsWith("/") ? lastLine : join(root, lastLine); + tarball = isAbsolute(lastLine) ? lastLine : join(root, lastLine); // Install tarball into a clean directory mkdirSync(installDir, { recursive: true }); @@ -120,7 +122,9 @@ test.group("built-package smoke test", (group) => { // Dynamically import from the *installed* package — this exercises // the exact __dirname → ../stubs/templates/qwik-component/Counter.tsx // resolution path that previously broke due to a packaging regression. - const scaffoldModule = await import(join(distDir, scaffoldChunk!)); + const scaffoldModule = await import( + pathToFileURL(join(distDir, scaffoldChunk!)).href + ); // Find the scaffoldQwikComponent export (minified name varies). // It's the only async function in the chunk (takes projectDir, strategy, dryRun). From 84e8eb480f420e69a61b1d2e82a5a3189e96db15 Mon Sep 17 00:00:00 2001 From: Jack Shelton Date: Mon, 24 Aug 2026 21:15:28 -0500 Subject: [PATCH 3/4] ci: restore windows-latest to the cli matrix (#294) --- .github/workflows/cli.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index 24570d48..cdb10195 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -15,8 +15,7 @@ jobs: strategy: fail-fast: false matrix: - # windows-latest removed: CLI has multiple unresolved Windows gaps — see the tracked issue; re-add when fixed. - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] experimental: [false] runtime: [node, deno, bun] pm: ["", pnpm, yarn] From 84a296c16e694c4b3ecbe179ab4d9ffac7ce8f10 Mon Sep 17 00:00:00 2001 From: Jack Shelton Date: Mon, 24 Aug 2026 21:31:36 -0500 Subject: [PATCH 4/4] test: tolerate locked temp files in smoke-pack teardown on windows (#294) --- .../tests/smoke-pack.spec.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts b/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts index ef2dc448..e0dc0cac 100644 --- a/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts +++ b/libs/create-qwikdev-astro/tests/smoke-pack.spec.ts @@ -20,6 +20,26 @@ const cliPath = join(pkgRoot, "dist", "cli.mjs"); let tarball = ""; +/** + * Remove the temp root, tolerating files the OS refuses to unlink. + * + * WHY: this suite dynamically `import()`s an ESM chunk from inside `root`. + * Windows keeps a lock on a loaded module's file for the lifetime of the + * process, and a module cannot be unloaded once imported — so by the time + * teardown runs, that file is guaranteed to be locked and `rmSync` throws + * EPERM. A leftover file in the OS temp dir on an ephemeral CI runner is not + * a test failure, so warn once and keep going rather than turning a green + * suite into a non-zero exit. + */ +function removeRootBestEffort() { + try { + rmSync(root, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.warn(`smoke-pack: could not fully remove ${root} (${message}) — ignoring`); + } +} + /** * Build, pack, and install the tarball once before all tests. * Runs eagerly at import time so the group.setup teardown is simple. @@ -27,7 +47,7 @@ let tarball = ""; function ensureBuiltPackage() { if (tarball) return; - rmSync(root, { recursive: true, force: true }); + removeRootBestEffort(); mkdirSync(root, { recursive: true }); // Build @@ -68,7 +88,7 @@ function ensureBuiltPackage() { test.group("built-package smoke test", (group) => { group.setup(() => { ensureBuiltPackage(); - return () => rmSync(root, { recursive: true, force: true }); + return () => removeRootBestEffort(); }); group.each.timeout(60_000);