diff --git a/apps/desktop/scripts/bundle-cli.mjs b/apps/desktop/scripts/bundle-cli.mjs index c3a619273..46eef4016 100644 --- a/apps/desktop/scripts/bundle-cli.mjs +++ b/apps/desktop/scripts/bundle-cli.mjs @@ -74,9 +74,14 @@ const srcBinary = join(serverDir, "bin", `${goos}-${goarch}`, binName); const destDir = join(repoRoot, "apps", "desktop", "resources", "bin"); const destBinary = join(destDir, binName); -function sh(cmd) { +// Hand git arguments straight to the binary (no shell). A match pattern like +// `v[0-9]*` must reach git as one literal argument; routing it through a shell +// string breaks on Windows, where cmd.exe keeps the POSIX single quotes and +// git matches no tag — degrading the bundled CLI's version to the +// 0.0.0-g fallback. +function git(...args) { try { - return execSync(cmd, { encoding: "utf-8" }).trim(); + return execFileSync("git", args, { encoding: "utf-8" }).trim(); } catch { return ""; } @@ -101,8 +106,10 @@ async function exists(p) { } if (hasGo()) { - const version = sh("git describe --tags --match 'v[0-9]*' --always --dirty") || "dev"; - const commit = sh("git rev-parse --short HEAD") || "unknown"; + const version = + git("describe", "--tags", "--match", "v[0-9]*", "--always", "--dirty") || + "dev"; + const commit = git("rev-parse", "--short", "HEAD") || "unknown"; const date = new Date().toISOString().replace(/\.\d+Z$/, "Z"); const ldflags = `-X main.version=${version} -X main.commit=${commit} -X main.date=${date}`; diff --git a/apps/desktop/scripts/package.mjs b/apps/desktop/scripts/package.mjs index aa1f5d953..0450ecf4f 100644 --- a/apps/desktop/scripts/package.mjs +++ b/apps/desktop/scripts/package.mjs @@ -22,10 +22,11 @@ // build, set `CSC_IDENTITY_AUTO_DISCOVERY=false` so electron-builder falls // back to an ad-hoc signature instead of requiring a Developer ID cert. // -// The `normalizeGitVersion` helper is exported so tests can cover the -// version-derivation logic without shelling out. +// The `normalizeGitVersion`, `deriveVersion`, and `DESCRIBE_ARGS` exports let +// tests cover version derivation both as a pure string transform and as the +// real `git describe` invocation against a throwaway repo. -import { execFileSync, spawnSync, execSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import { delimiter, dirname, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -71,9 +72,17 @@ const MAC_ALL_PLATFORM_TARGETS = [ { platform: "linux", arch: "arm64" }, ]; -function sh(cmd) { +// Run a git subcommand with its arguments handed straight to the binary, +// never through a shell. A match pattern like `v[0-9]*` must reach git as a +// single literal argument on every platform. Passing the whole command as a +// shell string (execSync) is unsafe on Windows: cmd.exe does not strip the +// POSIX single quotes around 'v[0-9]*', so git receives the quotes verbatim, +// matches no tag, and the version silently degrades to the 0.0.0-g +// fallback — which is exactly how a `0.0.0-…` Windows Desktop build once +// escaped to a GitHub Release. +function git(args, cwd) { try { - return execSync(cmd, { encoding: "utf-8" }).trim(); + return execFileSync("git", args, { encoding: "utf-8", cwd }).trim(); } catch { return ""; } @@ -126,10 +135,24 @@ export function normalizeGitVersion(raw) { return stripped; } -function deriveVersion() { - return normalizeGitVersion( - sh("git describe --tags --match 'v[0-9]*' --always --dirty"), - ); +// The exact argv handed to `git describe` for version derivation. Kept as a +// standalone array — never a shell command string — so the `v[0-9]*` match +// pattern reaches git as one literal argument regardless of platform (see the +// git() note above for why a shell string breaks on Windows). +export const DESCRIBE_ARGS = [ + "describe", + "--tags", + "--match", + "v[0-9]*", + "--always", + "--dirty", +]; + +// Exported (with an optional cwd) so tests can exercise the real describe +// invocation against a throwaway repo, not just normalizeGitVersion in +// isolation — the gap that let the Windows quoting regression through CI. +export function deriveVersion(cwd) { + return normalizeGitVersion(git(DESCRIBE_ARGS, cwd)); } function uniqueOrdered(values) { diff --git a/apps/desktop/scripts/package.test.mjs b/apps/desktop/scripts/package.test.mjs index 8f880370c..cc1c549cb 100644 --- a/apps/desktop/scripts/package.test.mjs +++ b/apps/desktop/scripts/package.test.mjs @@ -1,7 +1,12 @@ -import { delimiter, resolve } from "node:path"; -import { describe, it, expect } from "vitest"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, join, resolve } from "node:path"; +import { afterEach, describe, it, expect } from "vitest"; import { builderArgsForTarget, + deriveVersion, + DESCRIBE_ARGS, envWithLocalBins, normalizeGitVersion, parsePackageArgs, @@ -79,6 +84,75 @@ describe("normalizeGitVersion", () => { }); }); +describe("DESCRIBE_ARGS", () => { + it("passes the match pattern as one bare argv token, never a shell-quoted string", () => { + // The Windows regression this locks down: the pattern used to be embedded + // in a shell command string as `--match 'v[0-9]*'`. cmd.exe does not strip + // POSIX single quotes, so git received them literally and matched no tag, + // collapsing the Desktop version to the 0.0.0-g fallback. As a + // standalone argv element with no surrounding quotes the pattern is + // shell-independent. + expect(DESCRIBE_ARGS).toContain("v[0-9]*"); + for (const arg of DESCRIBE_ARGS) { + expect(arg).not.toContain("'"); + expect(arg).not.toContain('"'); + } + }); +}); + +describe("deriveVersion (real git describe)", () => { + // These exercise the actual `git describe` invocation — not just the + // normalizeGitVersion string transform — because the bug that shipped a + // `0.0.0-…` Windows Desktop build lived in HOW git was called, not in the + // string handling. package.mjs now runs git with an argv array (no shell), + // so the `v[0-9]*` match pattern reaches git as a literal argument + // identically on every platform. + const repos = []; + + function initRepo() { + const dir = mkdtempSync(join(tmpdir(), "multica-desktop-ver-")); + repos.push(dir); + const run = (...args) => + execFileSync("git", args, { cwd: dir, encoding: "utf-8" }); + run("init", "-q"); + run("config", "user.email", "test@multica.ai"); + run("config", "user.name", "test"); + run("config", "commit.gpgsign", "false"); + run("commit", "-q", "--allow-empty", "-m", "root"); + return { dir, run }; + } + + afterEach(() => { + while (repos.length) rmSync(repos.pop(), { recursive: true, force: true }); + }); + + it("resolves a clean semver tag to its bare version", () => { + const { dir, run } = initRepo(); + run("tag", "v1.4.2"); + expect(deriveVersion(dir)).toBe("1.4.2"); + }); + + it("selects the semver tag even when a nearer non-semver tag exists", () => { + // A release-train tag like `release_iteration/…` sitting closer to HEAD + // must not become the version. With the match pattern correctly reaching + // git, describe skips it and reports the real vX.Y.Z tag. If the pattern + // were mangled (e.g. quotes leaking through a shell) git would match + // nothing and the version would collapse to `0.0.0-…`. + const { dir, run } = initRepo(); + run("tag", "v1.4.2"); + run("commit", "-q", "--allow-empty", "-m", "sprint"); + run("tag", "release_iteration/Sprint_0705"); + const version = deriveVersion(dir); + expect(version).toMatch(/^1\.4\.2-1-g[0-9a-f]+$/); + expect(version).not.toMatch(/^0\.0\.0/); + }); + + it("falls back to 0.0.0-g when no semver tag is reachable", () => { + const { dir } = initRepo(); + expect(deriveVersion(dir)).toMatch(/^0\.0\.0-g[0-9a-f]+$/); + }); +}); + describe("stripLeadingSeparator", () => { it("removes the leading -- inserted by npm/pnpm", () => { expect(stripLeadingSeparator(["--", "--mac", "--arm64", "--publish", "always"])).toEqual([ diff --git a/apps/desktop/src/main/app-version.ts b/apps/desktop/src/main/app-version.ts index 73b4406c0..42139949c 100644 --- a/apps/desktop/src/main/app-version.ts +++ b/apps/desktop/src/main/app-version.ts @@ -1,5 +1,5 @@ import { app } from "electron"; -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; /** * Resolve the running app version. In packaged builds this is the value @@ -20,11 +20,18 @@ export function getAppVersion(): string { return app.getVersion(); } try { - const raw = execSync("git describe --tags --match 'v[0-9]*' --always --dirty", { - cwd: app.getAppPath(), - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); + // argv array, not a shell string: the `v[0-9]*` match pattern must reach + // git as one literal argument. A shell string breaks on Windows, where + // cmd.exe keeps the single quotes and git matches no tag. + const raw = execFileSync( + "git", + ["describe", "--tags", "--match", "v[0-9]*", "--always", "--dirty"], + { + cwd: app.getAppPath(), + encoding: "utf-8", + stdio: ["ignore", "pipe", "ignore"], + }, + ).trim(); if (!raw) return app.getVersion(); return raw.replace(/^v/, ""); } catch {