From b2e02ad717f8a9f4d7671ea8da896f3cd8252875 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 17:20:53 +0000 Subject: [PATCH 1/4] fix: make empty-chunk release checks fail closed Stop sharing main-process chunks by building a single Vite entry and letting ?modulePath emit the session server on its own. Teach the asar inspector to strip listPackage paths, refuse extract failures, and catch missing named exports. Align unsigned GitHub releases with the macOS pipeline, raise the release-job heap, and only mark a tag latest when it is actually newest. --- .github/workflows/ci.yml | 8 ++ .github/workflows/release-macos.yml | 5 +- README.md | 2 +- apps/desktop/electron.vite.config.ts | 21 ++-- scripts/inspect-packaged-js.mjs | 108 ++++++++++++++++-- scripts/publish-verified-release.mjs | 52 +++++++++ scripts/release-helpers.mjs | 65 +++++++++++ scripts/release-macos.mjs | 70 ++++++------ scripts/verify-macos-release.mjs | 69 ++++++----- .../apps-desktop/electron-vite-config.spec.ts | 10 +- tests/integration/inspect-packaged-js.spec.ts | 87 ++++++++++++++ tests/integration/release-helpers.spec.ts | 34 ++++++ 12 files changed, 443 insertions(+), 88 deletions(-) create mode 100644 scripts/publish-verified-release.mjs create mode 100644 scripts/release-helpers.mjs create mode 100644 tests/integration/release-helpers.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3430559..b6f3d11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,3 +61,11 @@ jobs: - name: Inspect packaged JavaScript graph run: bun ./scripts/inspect-packaged-js.mjs apps/desktop/out + + - name: Reject shared main-process chunks + run: | + if compgen -G "apps/desktop/out/main/chunks/*.js" > /dev/null; then + echo "Shared main-process chunks were emitted:" >&2 + ls -la apps/desktop/out/main/chunks >&2 + exit 1 + fi diff --git a/.github/workflows/release-macos.yml b/.github/workflows/release-macos.yml index 4f4f357..9ff6a33 100644 --- a/.github/workflows/release-macos.yml +++ b/.github/workflows/release-macos.yml @@ -33,9 +33,10 @@ jobs: bun run typecheck bun run test - - name: Build, sign, notarize, and verify + - name: Build, inspect, and optionally sign run: bun run release:mac env: + NODE_OPTIONS: --max-old-space-size=8192 CSC_LINK: ${{ secrets.MAC_CSC_LINK }} CSC_KEY_PASSWORD: ${{ secrets.MAC_CSC_KEY_PASSWORD }} APPLE_ID: ${{ secrets.APPLE_ID }} @@ -60,4 +61,4 @@ jobs: env: GH_TOKEN: ${{ github.token }} RELEASE_TAG: ${{ inputs.tag || github.ref_name }} - run: gh release edit "${RELEASE_TAG}" --draft=false --latest + run: bun ./scripts/publish-verified-release.mjs "${RELEASE_TAG}" diff --git a/README.md b/README.md index 96dc367..3de9242 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ xattr -dr com.apple.quarantine "/Applications/Pi Desktop.app" ``` -Release builds are unsigned (no paid Apple Developer ID). That command is what makes the download launch. +Release builds are unsigned unless Developer ID credentials are present in the release workflow. Unsigned downloads need that `xattr` command to launch. **From source** diff --git a/apps/desktop/electron.vite.config.ts b/apps/desktop/electron.vite.config.ts index b50cd5b..821216d 100644 --- a/apps/desktop/electron.vite.config.ts +++ b/apps/desktop/electron.vite.config.ts @@ -7,16 +7,20 @@ import type { Plugin } from "vite"; const mainEntry = fileURLToPath( new URL("./src/main/index.ts", import.meta.url), ); -const agentHostSessionServerEntry = fileURLToPath( - new URL("./src/main/agent-host-session-server-entry.ts", import.meta.url), -); function rejectEmptyMainChunks(): Plugin { return { name: "reject-empty-main-chunks", generateBundle(_options, bundle) { const empty = Object.values(bundle).filter( - (item) => item.type === "chunk" && item.code.trim().length === 0, + (item) => + (item.type === "chunk" && item.code.trim().length === 0) || + (item.type === "asset" && + typeof item.fileName === "string" && + /\.(?:[cm]?js)$/.test(item.fileName) && + (typeof item.source === "string" + ? item.source.trim().length === 0 + : Buffer.from(item.source).toString("utf8").trim().length === 0)), ); if (empty.length === 0) { return; @@ -53,15 +57,14 @@ export default defineConfig({ external: ["electron", "node-pty"], input: { index: mainEntry, - agentHostSessionServer: agentHostSessionServerEntry, }, treeshake: false, plugins: [rejectEmptyMainChunks()], output: { - // Fold empty/tiny shared chunks into their importers. A 0-byte - // shared chunk is the v0.9.3 launch crash (rolldown#6677). Infinity - // OOMs electron-vite on GitHub-hosted runners. - experimentalMinChunkSize: 512, + // Single main entry. The agent session server is bundled separately + // via `?modulePath`. Two Rollup inputs here created the shared + // empty chunk that crashed v0.9.3. + inlineDynamicImports: true, }, }, }, diff --git a/scripts/inspect-packaged-js.mjs b/scripts/inspect-packaged-js.mjs index 34143be..1605baa 100644 --- a/scripts/inspect-packaged-js.mjs +++ b/scripts/inspect-packaged-js.mjs @@ -5,12 +5,18 @@ import { fileURLToPath } from "node:url"; const JS_FILE_PATTERN = /\.(?:[cm]?js)$/; const RELATIVE_SPECIFIER_PATTERN = - /(?:import|export)\s+(?:[^'"\n;]*?\sfrom\s+)?["'](\.[^"']+)["']|import\s*\(\s*["'](\.[^"']+)["']\s*\)/g; + /(?:import|export)\s+["'](\.[^"']+)["']|(?:import|export)(?:\s+type)?(?:\s*\{[^}]*\}|\s+\*\s+as\s+[\w$]+|\s+\*|[\s\w*,]+)\s*from\s*["'](\.[^"']+)["']|import\s*\(\s*["'](\.[^"']+)["']\s*\)/g; +const NAMED_FROM_PATTERN = + /(?:import|export)\s*\{([^}]*)\}\s*from\s*["'](\.[^"']+)["']/g; function toPosixPath(filePath) { return filePath.split(path.sep).join("/"); } +export function asarEntryPath(listedPath) { + return toPosixPath(listedPath).replace(/^\/+/, ""); +} + function walkJsFiles(rootDir, currentRelativeDir = "") { const absoluteDir = path.join(rootDir, currentRelativeDir); if (!existsSync(absoluteDir)) { @@ -34,7 +40,7 @@ function walkJsFiles(rootDir, currentRelativeDir = "") { function extractRelativeSpecifiers(source) { const specifiers = []; for (const match of source.matchAll(RELATIVE_SPECIFIER_PATTERN)) { - const specifier = match[1] ?? match[2]; + const specifier = match[1] ?? match[2] ?? match[3]; if (specifier) { specifiers.push(specifier); } @@ -42,6 +48,54 @@ function extractRelativeSpecifiers(source) { return specifiers; } +function parseImportedNames(clause) { + return clause + .split(",") + .map((part) => part.trim()) + .filter((part) => part.length > 0 && part !== "type") + .map((part) => { + const normalized = part.replace(/^type\s+/, "").trim(); + const [exportedName] = normalized.split(/\s+as\s+/); + return exportedName?.trim(); + }) + .filter((name) => Boolean(name) && name !== "..."); +} + +function extractNamedImports(source) { + const namedImports = []; + for (const match of source.matchAll(NAMED_FROM_PATTERN)) { + const specifier = match[2]; + const names = parseImportedNames(match[1] ?? ""); + if (specifier && names.length > 0) { + namedImports.push({ specifier, names }); + } + } + return namedImports; +} + +function escapeRegExp(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +export function moduleExportsName(source, name) { + if (name === "default") { + return /\bexport\s+default\b/.test(source); + } + if (/\bexport\s+\*\s+from\b/.test(source)) { + return true; + } + const escaped = escapeRegExp(name); + return ( + new RegExp( + `\\bexport\\s+(?:async\\s+)?(?:function\\*?|class)\\s+${escaped}\\b`, + ).test(source) || + new RegExp(`\\bexport\\s+(?:const|let|var)\\s+${escaped}\\b`).test( + source, + ) || + new RegExp(`\\bexport\\s*\\{[^}]*\\b${escaped}\\b`).test(source) + ); +} + function isEmptyBytes(bytes) { return bytes.length === 0 || bytes.toString("utf8").trim().length === 0; } @@ -53,8 +107,9 @@ function resolveRelativeImport(fromRelativePath, specifier) { } /** - * Inspect a JavaScript module graph for empty files and dangling relative ESM - * imports. `files` is a map of posix-relative path → file bytes. + * Inspect a JavaScript module graph for empty files, dangling relative ESM + * imports, and named imports that the target module does not export. + * `files` is a map of posix-relative path → file bytes. */ export function inspectJavaScriptGraph(files) { const problems = []; @@ -96,6 +151,23 @@ export function inspectJavaScriptGraph(files) { }); } } + + for (const { specifier, names } of extractNamedImports(source)) { + const target = resolveRelativeImport(relativePath, specifier); + if (!target || !(target in files) || isEmptyBytes(files[target])) { + continue; + } + const targetSource = files[target].toString("utf8"); + for (const name of names) { + if (!moduleExportsName(targetSource, name)) { + problems.push({ + kind: "missing-export", + file: relativePath, + detail: `imports ${name} from ${specifier} but that export is missing`, + }); + } + } + } } return problems; @@ -106,6 +178,9 @@ export function inspectDirectory(rootDir) { for (const relativePath of walkJsFiles(rootDir)) { files[relativePath] = readFileSync(path.join(rootDir, relativePath)); } + if (Object.keys(files).length === 0) { + throw new Error(`No JavaScript files found under ${rootDir}`); + } return inspectJavaScriptGraph(files); } @@ -132,18 +207,29 @@ export function inspectAsar(asarPath, resolveFrom = process.cwd()) { const asar = loadAsarModule(resolveFrom); const listed = asar.listPackage(asarPath); const files = {}; + const extractErrors = []; for (const listedPath of listed) { - const relativePath = toPosixPath(listedPath).replace(/^\/+/, ""); + const relativePath = asarEntryPath(listedPath); if (!JS_FILE_PATTERN.test(relativePath)) { continue; } - // Native modules live beside the asar, not inside it. - if (relativePath.startsWith("node_modules/")) { - continue; - } try { - files[relativePath] = asar.extractFile(asarPath, listedPath); - } catch {} + files[relativePath] = asar.extractFile(asarPath, relativePath); + } catch (error) { + extractErrors.push( + `${relativePath}: ${error instanceof Error ? error.message : error}`, + ); + } + } + if (extractErrors.length > 0) { + throw new Error( + `Failed to read JavaScript from ${asarPath}:\n${extractErrors + .map((line) => `- ${line}`) + .join("\n")}`, + ); + } + if (Object.keys(files).length === 0) { + throw new Error(`No JavaScript files found in ${asarPath}`); } return inspectJavaScriptGraph(files); } diff --git a/scripts/publish-verified-release.mjs b/scripts/publish-verified-release.mjs new file mode 100644 index 0000000..6edb32e --- /dev/null +++ b/scripts/publish-verified-release.mjs @@ -0,0 +1,52 @@ +#!/usr/bin/env node +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { shouldMarkReleaseLatest } from "./release-helpers.mjs"; + +function run(command, args) { + const result = spawnSync(command, args, { encoding: "utf8" }); + if (result.status !== 0) { + throw new Error( + `${command} ${args.join(" ")} failed:\n${`${result.stdout ?? ""}${ + result.stderr ?? "" + }`.trim()}`, + ); + } + return result.stdout ?? ""; +} + +function publishVerifiedRelease(tag) { + if (!tag) { + throw new Error("Usage: publish-verified-release.mjs "); + } + + const listed = JSON.parse( + run("gh", [ + "release", + "list", + "--exclude-drafts", + "--limit", + "50", + "--json", + "tagName", + ]) || "[]", + ); + const publishedTags = listed.map((release) => release.tagName); + const args = ["release", "edit", tag, "--draft=false"]; + if (shouldMarkReleaseLatest(tag, publishedTags)) { + args.push("--latest"); + } + const result = spawnSync("gh", args, { stdio: "inherit" }); + if (result.status !== 0) { + process.exit(result.status ?? 1); + } +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + try { + publishVerifiedRelease(process.argv[2]); + } catch (error) { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + } +} diff --git a/scripts/release-helpers.mjs b/scripts/release-helpers.mjs new file mode 100644 index 0000000..86d9cb2 --- /dev/null +++ b/scripts/release-helpers.mjs @@ -0,0 +1,65 @@ +export const BUILD_HEAP_OPTION = "--max-old-space-size=8192"; + +export function withBuildHeap(env = process.env) { + const current = env.NODE_OPTIONS ?? ""; + if (/(?:^|\s)--max-old-space-size=/.test(current)) { + return { ...env }; + } + return { + ...env, + NODE_OPTIONS: [current, BUILD_HEAP_OPTION].filter(Boolean).join(" "), + }; +} + +export function hasNotarizationCredentials(env = process.env) { + const hasAppleIdCredentials = + Boolean(env.APPLE_ID) && + Boolean(env.APPLE_APP_SPECIFIC_PASSWORD) && + Boolean(env.APPLE_TEAM_ID); + const hasApiKeyCredentials = + Boolean(env.APPLE_API_KEY) && + Boolean(env.APPLE_API_KEY_ID) && + Boolean(env.APPLE_API_ISSUER); + const hasKeychainProfile = Boolean(env.APPLE_KEYCHAIN_PROFILE); + return hasAppleIdCredentials || hasApiKeyCredentials || hasKeychainProfile; +} + +export function isDeveloperIdSignature(signature) { + return ( + !signature.includes("Signature=adhoc") && + !signature.includes("TeamIdentifier=not set") && + signature.includes("Authority=Developer ID Application") + ); +} + +function parseVersionParts(tag) { + const normalized = String(tag).replace(/^v/, ""); + return normalized.split(/[.-]/).map((part) => { + const value = Number.parseInt(part, 10); + return Number.isNaN(value) ? part : value; + }); +} + +export function compareReleaseTags(left, right) { + const leftParts = parseVersionParts(left); + const rightParts = parseVersionParts(right); + const length = Math.max(leftParts.length, rightParts.length); + for (let index = 0; index < length; index += 1) { + const leftPart = leftParts[index] ?? 0; + const rightPart = rightParts[index] ?? 0; + if (leftPart === rightPart) { + continue; + } + if (typeof leftPart === "number" && typeof rightPart === "number") { + return leftPart - rightPart; + } + return String(leftPart).localeCompare(String(rightPart)); + } + return 0; +} + +export function shouldMarkReleaseLatest(tag, publishedTags) { + return publishedTags.every( + (publishedTag) => compareReleaseTags(tag, publishedTag) >= 0, + ); +} diff --git a/scripts/release-macos.mjs b/scripts/release-macos.mjs index 0271425..b0e07fa 100644 --- a/scripts/release-macos.mjs +++ b/scripts/release-macos.mjs @@ -2,6 +2,10 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { + hasNotarizationCredentials, + withBuildHeap, +} from "./release-helpers.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(scriptDir, ".."); @@ -11,7 +15,7 @@ function fail(message) { process.exit(1); } -function hasKeychainDeveloperId() { +export function hasKeychainDeveloperId() { const result = spawnSync( "security", ["find-identity", "-v", "-p", "codesigning"], @@ -22,42 +26,40 @@ function hasKeychainDeveloperId() { ); } -const hasSigningMaterial = - Boolean(process.env.CSC_LINK) || hasKeychainDeveloperId(); -if (!hasSigningMaterial) { - fail( - "Missing Developer ID Application signing identity. Set CSC_LINK and CSC_KEY_PASSWORD or install the certificate in the login keychain.", - ); +export function hasSigningMaterial(env = process.env) { + return Boolean(env.CSC_LINK) || hasKeychainDeveloperId(); } -const hasAppleIdCredentials = - Boolean(process.env.APPLE_ID) && - Boolean(process.env.APPLE_APP_SPECIFIC_PASSWORD) && - Boolean(process.env.APPLE_TEAM_ID); -const hasApiKeyCredentials = - Boolean(process.env.APPLE_API_KEY) && - Boolean(process.env.APPLE_API_KEY_ID) && - Boolean(process.env.APPLE_API_ISSUER); -const hasKeychainProfile = Boolean(process.env.APPLE_KEYCHAIN_PROFILE); - -if (!hasAppleIdCredentials && !hasApiKeyCredentials && !hasKeychainProfile) { - fail( - "Missing Apple notarization credentials. Configure APPLE_ID credentials, Notary API credentials, or APPLE_KEYCHAIN_PROFILE.", +function runRelease() { + const signing = hasSigningMaterial(); + if (signing && !hasNotarizationCredentials()) { + fail( + "Missing Apple notarization credentials. Configure APPLE_ID credentials, Notary API credentials, or APPLE_KEYCHAIN_PROFILE.", + ); + } + if (!signing) { + console.log( + "No Developer ID signing identity; building an unsigned macOS release.", + ); + } + + const build = spawnSync( + "bun", + ["run", "--filter", "@pi-desktop/desktop", "dist:mac"], + { cwd: repoRoot, stdio: "inherit", env: withBuildHeap(process.env) }, ); -} + if (build.status !== 0) { + process.exit(build.status ?? 1); + } -const build = spawnSync( - "bun", - ["run", "--filter", "@pi-desktop/desktop", "dist:mac"], - { cwd: repoRoot, stdio: "inherit", env: process.env }, -); -if (build.status !== 0) { - process.exit(build.status ?? 1); + const verify = spawnSync( + "node", + [path.join(scriptDir, "verify-macos-release.mjs")], + { cwd: repoRoot, stdio: "inherit", env: withBuildHeap(process.env) }, + ); + process.exit(verify.status ?? 1); } -const verify = spawnSync( - "node", - [path.join(scriptDir, "verify-macos-release.mjs")], - { cwd: repoRoot, stdio: "inherit", env: process.env }, -); -process.exit(verify.status ?? 1); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + runRelease(); +} diff --git a/scripts/verify-macos-release.mjs b/scripts/verify-macos-release.mjs index 828e043..cb9f65c 100644 --- a/scripts/verify-macos-release.mjs +++ b/scripts/verify-macos-release.mjs @@ -4,6 +4,7 @@ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { assertJavaScriptGraph, inspectAsar } from "./inspect-packaged-js.mjs"; +import { isDeveloperIdSignature } from "./release-helpers.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(scriptDir, ".."); @@ -39,38 +40,48 @@ function run(command, args, options = {}) { return output; } -for (const target of [appPath, dmgPath]) { - if (!existsSync(target)) { - throw new Error(`Missing release artifact: ${target}`); +function verifyRelease() { + for (const target of [appPath, dmgPath]) { + if (!existsSync(target)) { + throw new Error(`Missing release artifact: ${target}`); + } + } + + const asarPath = path.join(appPath, "Contents", "Resources", "app.asar"); + assertJavaScriptGraph(inspectAsar(asarPath, repoRoot), asarPath); + + const signature = run("codesign", ["-dv", "--verbose=4", appPath]); + if (!isDeveloperIdSignature(signature)) { + console.log( + `✅ Verified unsigned Pi Desktop ${version} artifacts and JavaScript graph.`, + ); + return; } -} -const asarPath = path.join(appPath, "Contents", "Resources", "app.asar"); -assertJavaScriptGraph(inspectAsar(asarPath, repoRoot), asarPath); + run("codesign", ["--verify", "--deep", "--strict", "--verbose=2", appPath]); + run("xcrun", ["stapler", "validate", appPath]); + run("xcrun", ["stapler", "validate", dmgPath]); + run("spctl", ["--assess", "--type", "execute", "--verbose=4", appPath]); + run("spctl", [ + "--assess", + "--type", + "open", + "--context", + "context:primary-signature", + "--verbose=4", + dmgPath, + ]); -run("codesign", ["--verify", "--deep", "--strict", "--verbose=2", appPath]); -const signature = run("codesign", ["-dv", "--verbose=4", appPath]); -if ( - signature.includes("Signature=adhoc") || - signature.includes("TeamIdentifier=not set") || - !signature.includes("Authority=Developer ID Application") -) { - throw new Error( - "Release app is not signed with a Developer ID Application certificate.", + console.log( + `✅ Verified signed and notarized Pi Desktop ${version} release.`, ); } -run("xcrun", ["stapler", "validate", appPath]); -run("xcrun", ["stapler", "validate", dmgPath]); -run("spctl", ["--assess", "--type", "execute", "--verbose=4", appPath]); -run("spctl", [ - "--assess", - "--type", - "open", - "--context", - "context:primary-signature", - "--verbose=4", - dmgPath, -]); - -console.log(`✅ Verified signed and notarized Pi Desktop ${version} release.`); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + try { + verifyRelease(); + } catch (error) { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + } +} diff --git a/tests/integration/apps-desktop/electron-vite-config.spec.ts b/tests/integration/apps-desktop/electron-vite-config.spec.ts index 031f124..5c39e03 100644 --- a/tests/integration/apps-desktop/electron-vite-config.spec.ts +++ b/tests/integration/apps-desktop/electron-vite-config.spec.ts @@ -21,12 +21,18 @@ describe("electron-vite config", () => { expect(config.renderer?.build?.outDir).toBe("out/renderer"); }); - it("refuses empty shared main-process chunks", () => { + it("builds the main process as a single entry without shared chunks", () => { const output = config.main?.build?.rollupOptions?.output; const outputOptions = Array.isArray(output) ? output[0] : output; const plugins = config.main?.build?.rollupOptions?.plugins ?? []; + const input = config.main?.build?.rollupOptions?.input; - expect(outputOptions?.experimentalMinChunkSize).toBe(512); + expect(outputOptions?.inlineDynamicImports).toBe(true); + expect(outputOptions?.experimentalMinChunkSize).toBeUndefined(); + expect(input).toEqual({ + index: expect.any(String), + }); + expect(input).not.toHaveProperty("agentHostSessionServer"); expect( plugins.some((plugin) => plugin?.name === "reject-empty-main-chunks"), ).toBe(true); diff --git a/tests/integration/inspect-packaged-js.spec.ts b/tests/integration/inspect-packaged-js.spec.ts index 18c722c..3e8748f 100644 --- a/tests/integration/inspect-packaged-js.spec.ts +++ b/tests/integration/inspect-packaged-js.spec.ts @@ -1,13 +1,18 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { + asarEntryPath, + inspectAsar, inspectDirectory, inspectJavaScriptGraph, + moduleExportsName, } from "../../scripts/inspect-packaged-js.mjs"; +const require = createRequire(import.meta.url); const tempRoots: string[] = []; function createFixtureRoot(): string { @@ -68,6 +73,29 @@ describe("inspectJavaScriptGraph", () => { }), ]); }); + + it("reports a multiline named import whose target does not export that name", () => { + const problems = inspectJavaScriptGraph({ + "index.js": Buffer.from('import {\n ot\n} from "./chunks/lib.js";\n'), + "chunks/lib.js": Buffer.from("export const other = 1;\n"), + }); + + expect(problems).toEqual([ + expect.objectContaining({ + kind: "missing-export", + file: "index.js", + detail: "imports ot from ./chunks/lib.js but that export is missing", + }), + ]); + }); +}); + +describe("moduleExportsName", () => { + it("accepts renamed and minified export forms", () => { + expect(moduleExportsName("export { foo as ot };\n", "ot")).toBe(true); + expect(moduleExportsName("export{ot};\n", "ot")).toBe(true); + expect(moduleExportsName("export const other = 1;\n", "ot")).toBe(false); + }); }); describe("inspectDirectory", () => { @@ -82,4 +110,63 @@ describe("inspectDirectory", () => { expect(inspectDirectory(rootDir)).toEqual([]); }); + + it("fails closed when the directory has no JavaScript", () => { + const rootDir = createFixtureRoot(); + writeFileSync(path.join(rootDir, "readme.txt"), "no js\n"); + + expect(() => inspectDirectory(rootDir)).toThrow(/No JavaScript files/); + }); +}); + +describe("inspectAsar", () => { + it("strips leading slashes from listed asar paths", () => { + expect(asarEntryPath("/out/main/index.js")).toBe("out/main/index.js"); + expect(asarEntryPath("out/main/index.js")).toBe("out/main/index.js"); + }); + + it("reads packed files and reports an empty imported chunk", async () => { + const asar = require("@electron/asar"); + const rootDir = createFixtureRoot(); + const appDir = path.join(rootDir, "app"); + const asarPath = path.join(rootDir, "app.asar"); + mkdirSync(path.join(appDir, "out/main/chunks"), { recursive: true }); + writeFileSync( + path.join(appDir, "out/main/index.js"), + 'import { ot } from "./chunks/lib-WNnjYEnG.js";\n', + ); + writeFileSync(path.join(appDir, "out/main/chunks/lib-WNnjYEnG.js"), ""); + + await asar.createPackage(appDir, asarPath); + + const problems = inspectAsar(asarPath, process.cwd()); + expect(problems).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "empty", + file: "out/main/chunks/lib-WNnjYEnG.js", + }), + expect.objectContaining({ + kind: "empty-import", + file: "out/main/index.js", + }), + ]), + ); + }); + + it("reads files even when listPackage prefixes paths with a slash", async () => { + const asar = require("@electron/asar"); + const rootDir = createFixtureRoot(); + const appDir = path.join(rootDir, "app"); + const asarPath = path.join(rootDir, "app.asar"); + mkdirSync(appDir, { recursive: true }); + writeFileSync(path.join(appDir, "index.js"), "export const ot = 1;\n"); + await asar.createPackage(appDir, asarPath); + + const listed = asar.listPackage(asarPath); + expect(listed.some((entry: string) => entry.startsWith("/"))).toBe(true); + + const problems = inspectAsar(asarPath, process.cwd()); + expect(problems).toEqual([]); + }); }); diff --git a/tests/integration/release-helpers.spec.ts b/tests/integration/release-helpers.spec.ts new file mode 100644 index 0000000..3d18cca --- /dev/null +++ b/tests/integration/release-helpers.spec.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; + +import { + compareReleaseTags, + isDeveloperIdSignature, + shouldMarkReleaseLatest, + withBuildHeap, +} from "../../scripts/release-helpers.mjs"; + +describe("release helpers", () => { + it("preserves an existing heap cap and otherwise sets 8GB", () => { + expect( + withBuildHeap({ NODE_OPTIONS: "--max-old-space-size=4096" }).NODE_OPTIONS, + ).toBe("--max-old-space-size=4096"); + expect(withBuildHeap({}).NODE_OPTIONS).toBe("--max-old-space-size=8192"); + }); + + it("detects Developer ID signatures and rejects adhoc builds", () => { + expect( + isDeveloperIdSignature( + "Authority=Developer ID Application: Example (TEAMID)\nTeamIdentifier=TEAMID\n", + ), + ).toBe(true); + expect( + isDeveloperIdSignature("Signature=adhoc\nTeamIdentifier=not set\n"), + ).toBe(false); + }); + + it("only marks a release latest when its tag is newest", () => { + expect(compareReleaseTags("v0.9.5", "v0.9.3")).toBeGreaterThan(0); + expect(shouldMarkReleaseLatest("v0.9.5", ["v0.9.3", "v0.9.5"])).toBe(true); + expect(shouldMarkReleaseLatest("v0.9.3", ["v0.9.5"])).toBe(false); + }); +}); From 34d9c291d821158d9f2338a0bd2816f8b3051593 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 01:55:28 +0000 Subject: [PATCH 2/4] fix: close remaining empty-chunk release review gaps Always pass --latest=true or --latest=false so publishing an older draft cannot steal GitHub latest. Follow export * when checking named exports, compare release tags as semver, pack the Vite out tree into asar in CI, and assert the modulePath session-server file is present and referenced. --- .github/workflows/ci.yml | 12 +- scripts/assert-main-build-layout.mjs | 84 +++++++++++++ scripts/inspect-packaged-js.mjs | 119 ++++++++++++++---- scripts/publish-verified-release.mjs | 24 ++-- scripts/release-helpers.mjs | 84 +++++++++++-- .../assert-main-build-layout.spec.ts | 76 +++++++++++ tests/integration/inspect-packaged-js.spec.ts | 46 +++++++ tests/integration/release-helpers.spec.ts | 23 ++++ 8 files changed, 418 insertions(+), 50 deletions(-) create mode 100644 scripts/assert-main-build-layout.mjs create mode 100644 tests/integration/assert-main-build-layout.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b6f3d11..3e0da70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,10 +62,8 @@ jobs: - name: Inspect packaged JavaScript graph run: bun ./scripts/inspect-packaged-js.mjs apps/desktop/out - - name: Reject shared main-process chunks - run: | - if compgen -G "apps/desktop/out/main/chunks/*.js" > /dev/null; then - echo "Shared main-process chunks were emitted:" >&2 - ls -la apps/desktop/out/main/chunks >&2 - exit 1 - fi + - name: Inspect Vite output packed as asar + run: bun ./scripts/inspect-packaged-js.mjs --pack-dir apps/desktop/out + + - name: Assert isolated main-process layout + run: bun ./scripts/assert-main-build-layout.mjs apps/desktop/out/main diff --git a/scripts/assert-main-build-layout.mjs b/scripts/assert-main-build-layout.mjs new file mode 100644 index 0000000..2358c2f --- /dev/null +++ b/scripts/assert-main-build-layout.mjs @@ -0,0 +1,84 @@ +#!/usr/bin/env node +import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const SESSION_SERVER_PATTERN = /^agent-host-session-server-entry-[^./]+\.js$/; + +export function assertMainProcessLayout(mainOutDir) { + const indexPath = path.join(mainOutDir, "index.js"); + if (!existsSync(indexPath)) { + throw new Error(`Missing main entry: ${indexPath}`); + } + const indexSource = readFileSync(indexPath, "utf8"); + if (indexSource.trim().length === 0) { + throw new Error(`Main entry is empty: ${indexPath}`); + } + + const chunksDir = path.join(mainOutDir, "chunks"); + if (existsSync(chunksDir) && statSync(chunksDir).isDirectory()) { + const sharedChunks = readdirSync(chunksDir).filter((name) => + /\.(?:[cm]?js)$/.test(name), + ); + if (sharedChunks.length > 0) { + throw new Error( + `Shared main-process chunks were emitted: ${sharedChunks.join(", ")}`, + ); + } + } + + const sessionFiles = readdirSync(mainOutDir).filter((name) => + SESSION_SERVER_PATTERN.test(name), + ); + if (sessionFiles.length === 0) { + throw new Error( + `Missing ?modulePath session-server bundle under ${mainOutDir}`, + ); + } + + const referenced = sessionFiles.filter((fileName) => { + const relative = `./${fileName}`; + return ( + indexSource.includes(JSON.stringify(relative)) || + indexSource.includes(relative) + ); + }); + if (referenced.length === 0) { + throw new Error( + `Main entry does not reference a session-server bundle (${sessionFiles.join(", ")})`, + ); + } + + for (const fileName of referenced) { + const sessionPath = path.join(mainOutDir, fileName); + if (readFileSync(sessionPath, "utf8").trim().length === 0) { + throw new Error(`Session-server bundle is empty: ${sessionPath}`); + } + } + + return { + indexPath, + sessionFiles: referenced, + }; +} + +function runCli(argv) { + const target = argv[0]; + if (!target) { + throw new Error("Usage: assert-main-build-layout.mjs "); + } + const resolved = path.resolve(target); + const result = assertMainProcessLayout(resolved); + console.log( + `Main process layout OK: ${result.indexPath} -> ${result.sessionFiles.join(", ")}`, + ); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + try { + runCli(process.argv.slice(2)); + } catch (error) { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + } +} diff --git a/scripts/inspect-packaged-js.mjs b/scripts/inspect-packaged-js.mjs index 1605baa..d6c15b3 100644 --- a/scripts/inspect-packaged-js.mjs +++ b/scripts/inspect-packaged-js.mjs @@ -1,5 +1,6 @@ -import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readdirSync, readFileSync } from "node:fs"; import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -48,17 +49,29 @@ function extractRelativeSpecifiers(source) { return specifiers; } -function parseImportedNames(clause) { +function parseSpecifierBindings(clause) { return clause .split(",") .map((part) => part.trim()) .filter((part) => part.length > 0 && part !== "type") .map((part) => { const normalized = part.replace(/^type\s+/, "").trim(); - const [exportedName] = normalized.split(/\s+as\s+/); - return exportedName?.trim(); + if (!normalized || normalized === "...") { + return null; + } + const pieces = normalized.split(/\s+as\s+/); + const local = pieces[0]?.trim(); + const exported = (pieces[1] ?? pieces[0])?.trim(); + if (!local || !exported) { + return null; + } + return { local, exported }; }) - .filter((name) => Boolean(name) && name !== "..."); + .filter(Boolean); +} + +function parseImportedNames(clause) { + return parseSpecifierBindings(clause).map((binding) => binding.local); } function extractNamedImports(source) { @@ -81,21 +94,58 @@ export function moduleExportsName(source, name) { if (name === "default") { return /\bexport\s+default\b/.test(source); } - if (/\bexport\s+\*\s+from\b/.test(source)) { - return true; - } const escaped = escapeRegExp(name); - return ( + if ( new RegExp( `\\bexport\\s+(?:async\\s+)?(?:function\\*?|class)\\s+${escaped}\\b`, ).test(source) || - new RegExp(`\\bexport\\s+(?:const|let|var)\\s+${escaped}\\b`).test( - source, - ) || - new RegExp(`\\bexport\\s*\\{[^}]*\\b${escaped}\\b`).test(source) + new RegExp(`\\bexport\\s+(?:const|let|var)\\s+${escaped}\\b`).test(source) + ) { + return true; + } + for (const match of source.matchAll(/\bexport\s*\{([^}]*)\}/g)) { + if ( + parseSpecifierBindings(match[1] ?? "").some( + (binding) => binding.exported === name, + ) + ) { + return true; + } + } + return Boolean( + new RegExp(`\\bexport\\s+\\*\\s+as\\s+${escaped}\\s+from\\b`).test(source), ); } +export function moduleHasExport( + files, + relativePath, + name, + visiting = new Set(), +) { + if (visiting.has(relativePath)) { + return false; + } + visiting.add(relativePath); + const bytes = files[relativePath]; + if (!bytes || isEmptyBytes(bytes)) { + return false; + } + const source = bytes.toString("utf8"); + if (moduleExportsName(source, name)) { + return true; + } + for (const match of source.matchAll( + /\bexport\s+\*\s+from\s*["'](\.[^"']+)["']/g, + )) { + const target = resolveRelativeImport(relativePath, match[1]); + if (target && moduleHasExport(files, target, name, visiting)) { + return true; + } + } + return false; +} + function isEmptyBytes(bytes) { return bytes.length === 0 || bytes.toString("utf8").trim().length === 0; } @@ -157,9 +207,8 @@ export function inspectJavaScriptGraph(files) { if (!target || !(target in files) || isEmptyBytes(files[target])) { continue; } - const targetSource = files[target].toString("utf8"); for (const name of names) { - if (!moduleExportsName(targetSource, name)) { + if (!moduleHasExport(files, target, name)) { problems.push({ kind: "missing-export", file: relativePath, @@ -249,10 +298,35 @@ export function assertJavaScriptGraph(problems, label) { ); } +export async function packDirectoryAndInspect( + rootDir, + resolveFrom = process.cwd(), +) { + const asar = loadAsarModule(resolveFrom); + const tempDir = mkdtempSync(path.join(tmpdir(), "pi-pack-asar-")); + const asarPath = path.join(tempDir, "app.asar"); + await asar.createPackage(path.resolve(rootDir), asarPath); + const problems = inspectAsar(asarPath, resolveFrom); + assertJavaScriptGraph(problems, asarPath); + console.log( + `JavaScript graph OK: ${asarPath} (packed from ${path.resolve(rootDir)})`, + ); + return problems; +} + function runCli(argv) { const target = argv[0]; + if (target === "--pack-dir") { + const directory = argv[1]; + if (!directory) { + throw new Error("Usage: inspect-packaged-js.mjs --pack-dir "); + } + return packDirectoryAndInspect(directory); + } if (!target) { - throw new Error("Usage: inspect-packaged-js.mjs "); + throw new Error( + "Usage: inspect-packaged-js.mjs | --pack-dir ", + ); } const resolved = path.resolve(target); const problems = resolved.endsWith(".asar") @@ -260,13 +334,14 @@ function runCli(argv) { : inspectDirectory(resolved); assertJavaScriptGraph(problems, resolved); console.log(`JavaScript graph OK: ${resolved}`); + return problems; } if (process.argv[1] === fileURLToPath(import.meta.url)) { - try { - runCli(process.argv.slice(2)); - } catch (error) { - console.error(error instanceof Error ? error.message : error); - process.exitCode = 1; - } + Promise.resolve() + .then(() => runCli(process.argv.slice(2))) + .catch((error) => { + console.error(error instanceof Error ? error.message : error); + process.exitCode = 1; + }); } diff --git a/scripts/publish-verified-release.mjs b/scripts/publish-verified-release.mjs index 6edb32e..bf58ca0 100644 --- a/scripts/publish-verified-release.mjs +++ b/scripts/publish-verified-release.mjs @@ -1,7 +1,9 @@ #!/usr/bin/env node import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; -import { shouldMarkReleaseLatest } from "./release-helpers.mjs"; +import { githubReleaseEditArgs } from "./release-helpers.mjs"; + +const RELEASE_LIST_LIMIT = "1000"; function run(command, args) { const result = spawnSync(command, args, { encoding: "utf8" }); @@ -15,27 +17,27 @@ function run(command, args) { return result.stdout ?? ""; } -function publishVerifiedRelease(tag) { - if (!tag) { - throw new Error("Usage: publish-verified-release.mjs "); - } - +export function listPublishedReleaseTags() { const listed = JSON.parse( run("gh", [ "release", "list", "--exclude-drafts", "--limit", - "50", + RELEASE_LIST_LIMIT, "--json", "tagName", ]) || "[]", ); - const publishedTags = listed.map((release) => release.tagName); - const args = ["release", "edit", tag, "--draft=false"]; - if (shouldMarkReleaseLatest(tag, publishedTags)) { - args.push("--latest"); + return listed.map((release) => release.tagName); +} + +export function publishVerifiedRelease(tag) { + if (!tag) { + throw new Error("Usage: publish-verified-release.mjs "); } + + const args = githubReleaseEditArgs(tag, listPublishedReleaseTags()); const result = spawnSync("gh", args, { stdio: "inherit" }); if (result.status !== 0) { process.exit(result.status ?? 1); diff --git a/scripts/release-helpers.mjs b/scripts/release-helpers.mjs index 86d9cb2..6c0b8b3 100644 --- a/scripts/release-helpers.mjs +++ b/scripts/release-helpers.mjs @@ -32,34 +32,98 @@ export function isDeveloperIdSignature(signature) { ); } -function parseVersionParts(tag) { - const normalized = String(tag).replace(/^v/, ""); - return normalized.split(/[.-]/).map((part) => { +function parsePrereleaseIdentifiers(prerelease) { + return prerelease.split(".").map((part) => { const value = Number.parseInt(part, 10); - return Number.isNaN(value) ? part : value; + return Number.isNaN(value) || String(value) !== part ? part : value; }); } -export function compareReleaseTags(left, right) { - const leftParts = parseVersionParts(left); - const rightParts = parseVersionParts(right); - const length = Math.max(leftParts.length, rightParts.length); +export function parseReleaseTag(tag) { + const normalized = String(tag).replace(/^v/, ""); + const match = normalized.match( + /^(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?(?:\+.*)?$/, + ); + if (!match) { + return null; + } + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: Number(match[3]), + prerelease: match[4] ? parsePrereleaseIdentifiers(match[4]) : null, + }; +} + +function comparePrerelease(left, right) { + const length = Math.max(left.length, right.length); for (let index = 0; index < length; index += 1) { - const leftPart = leftParts[index] ?? 0; - const rightPart = rightParts[index] ?? 0; + const leftPart = left[index]; + const rightPart = right[index]; + if (leftPart === undefined) { + return -1; + } + if (rightPart === undefined) { + return 1; + } if (leftPart === rightPart) { continue; } if (typeof leftPart === "number" && typeof rightPart === "number") { return leftPart - rightPart; } + if (typeof leftPart === "number") { + return -1; + } + if (typeof rightPart === "number") { + return 1; + } return String(leftPart).localeCompare(String(rightPart)); } return 0; } +export function compareReleaseTags(left, right) { + const leftVersion = parseReleaseTag(left); + const rightVersion = parseReleaseTag(right); + if (!leftVersion || !rightVersion) { + return String(left).localeCompare(String(right)); + } + if (leftVersion.major !== rightVersion.major) { + return leftVersion.major - rightVersion.major; + } + if (leftVersion.minor !== rightVersion.minor) { + return leftVersion.minor - rightVersion.minor; + } + if (leftVersion.patch !== rightVersion.patch) { + return leftVersion.patch - rightVersion.patch; + } + if (!leftVersion.prerelease && !rightVersion.prerelease) { + return 0; + } + if (!leftVersion.prerelease) { + return 1; + } + if (!rightVersion.prerelease) { + return -1; + } + return comparePrerelease(leftVersion.prerelease, rightVersion.prerelease); +} + export function shouldMarkReleaseLatest(tag, publishedTags) { return publishedTags.every( (publishedTag) => compareReleaseTags(tag, publishedTag) >= 0, ); } + +export function githubReleaseEditArgs(tag, publishedTags) { + return [ + "release", + "edit", + tag, + "--draft=false", + shouldMarkReleaseLatest(tag, publishedTags) + ? "--latest=true" + : "--latest=false", + ]; +} diff --git a/tests/integration/assert-main-build-layout.spec.ts b/tests/integration/assert-main-build-layout.spec.ts new file mode 100644 index 0000000..875f51b --- /dev/null +++ b/tests/integration/assert-main-build-layout.spec.ts @@ -0,0 +1,76 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; + +import { assertMainProcessLayout } from "../../scripts/assert-main-build-layout.mjs"; + +const tempRoots: string[] = []; + +function createFixtureRoot(): string { + const fixtureRoot = mkdtempSync(path.join(tmpdir(), "pi-main-layout-")); + tempRoots.push(fixtureRoot); + return fixtureRoot; +} + +afterEach(() => { + for (const rootDir of tempRoots.splice(0)) { + rmSync(rootDir, { recursive: true, force: true }); + } +}); + +describe("assertMainProcessLayout", () => { + it("accepts an isolated index that references the modulePath session server", () => { + const mainOutDir = createFixtureRoot(); + writeFileSync( + path.join(mainOutDir, "index.js"), + 'import { join } from "path";\nexport default join(import.meta.dirname, "./agent-host-session-server-entry-abc123.js");\n', + ); + writeFileSync( + path.join(mainOutDir, "agent-host-session-server-entry-abc123.js"), + "export const start = 1;\n", + ); + + expect(assertMainProcessLayout(mainOutDir)).toEqual({ + indexPath: path.join(mainOutDir, "index.js"), + sessionFiles: ["agent-host-session-server-entry-abc123.js"], + }); + }); + + it("rejects a missing session-server reference", () => { + const mainOutDir = createFixtureRoot(); + writeFileSync( + path.join(mainOutDir, "index.js"), + "export const main = 1;\n", + ); + writeFileSync( + path.join(mainOutDir, "agent-host-session-server-entry-abc123.js"), + "export const start = 1;\n", + ); + + expect(() => assertMainProcessLayout(mainOutDir)).toThrow( + /does not reference a session-server bundle/, + ); + }); + + it("rejects shared chunks", () => { + const mainOutDir = createFixtureRoot(); + mkdirSync(path.join(mainOutDir, "chunks")); + writeFileSync( + path.join(mainOutDir, "index.js"), + 'import { join } from "path";\nexport default join(import.meta.dirname, "./agent-host-session-server-entry-abc123.js");\n', + ); + writeFileSync( + path.join(mainOutDir, "agent-host-session-server-entry-abc123.js"), + "export const start = 1;\n", + ); + writeFileSync( + path.join(mainOutDir, "chunks", "lib-WNnjYEnG.js"), + "export const ot = 1;\n", + ); + + expect(() => assertMainProcessLayout(mainOutDir)).toThrow( + /Shared main-process chunks/, + ); + }); +}); diff --git a/tests/integration/inspect-packaged-js.spec.ts b/tests/integration/inspect-packaged-js.spec.ts index 3e8748f..d1543bc 100644 --- a/tests/integration/inspect-packaged-js.spec.ts +++ b/tests/integration/inspect-packaged-js.spec.ts @@ -10,6 +10,7 @@ import { inspectDirectory, inspectJavaScriptGraph, moduleExportsName, + packDirectoryAndInspect, } from "../../scripts/inspect-packaged-js.mjs"; const require = createRequire(import.meta.url); @@ -88,6 +89,27 @@ describe("inspectJavaScriptGraph", () => { }), ]); }); + + it("follows export * from instead of treating star re-exports as universal", () => { + const missing = inspectJavaScriptGraph({ + "index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "barrel.js": Buffer.from('export * from "./lib.js";\n'), + "lib.js": Buffer.from("export const other = 1;\n"), + }); + expect(missing).toEqual([ + expect.objectContaining({ + kind: "missing-export", + file: "index.js", + }), + ]); + + const present = inspectJavaScriptGraph({ + "index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "barrel.js": Buffer.from('export * from "./lib.js";\n'), + "lib.js": Buffer.from("export const ot = 1;\n"), + }); + expect(present).toEqual([]); + }); }); describe("moduleExportsName", () => { @@ -96,6 +118,15 @@ describe("moduleExportsName", () => { expect(moduleExportsName("export{ot};\n", "ot")).toBe(true); expect(moduleExportsName("export const other = 1;\n", "ot")).toBe(false); }); + + it("does not treat the local name of `export { ot as n }` as exported", () => { + expect(moduleExportsName("export { ot as n };\n", "ot")).toBe(false); + expect(moduleExportsName("export { ot as n };\n", "n")).toBe(true); + }); + + it("does not treat export * from as exporting every name", () => { + expect(moduleExportsName('export * from "./lib.js";\n', "ot")).toBe(false); + }); }); describe("inspectDirectory", () => { @@ -169,4 +200,19 @@ describe("inspectAsar", () => { const problems = inspectAsar(asarPath, process.cwd()); expect(problems).toEqual([]); }); + + it("packs a directory to asar and inspects the archive", async () => { + const rootDir = createFixtureRoot(); + mkdirSync(path.join(rootDir, "out/main"), { recursive: true }); + writeFileSync( + path.join(rootDir, "out/main/index.js"), + 'import { ot } from "./lib.js";\nexport { ot };\n', + ); + writeFileSync( + path.join(rootDir, "out/main/lib.js"), + "export const ot = 1;\n", + ); + + await expect(packDirectoryAndInspect(rootDir)).resolves.toEqual([]); + }); }); diff --git a/tests/integration/release-helpers.spec.ts b/tests/integration/release-helpers.spec.ts index 3d18cca..c918b0f 100644 --- a/tests/integration/release-helpers.spec.ts +++ b/tests/integration/release-helpers.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { compareReleaseTags, + githubReleaseEditArgs, isDeveloperIdSignature, shouldMarkReleaseLatest, withBuildHeap, @@ -31,4 +32,26 @@ describe("release helpers", () => { expect(shouldMarkReleaseLatest("v0.9.5", ["v0.9.3", "v0.9.5"])).toBe(true); expect(shouldMarkReleaseLatest("v0.9.3", ["v0.9.5"])).toBe(false); }); + + it("ranks a release above a matching prerelease", () => { + expect(compareReleaseTags("v1.0.0", "v1.0.0-beta.1")).toBeGreaterThan(0); + expect(shouldMarkReleaseLatest("v1.0.0-beta.1", ["v1.0.0"])).toBe(false); + }); + + it("explicitly sets --latest=false so draft publish cannot steal latest", () => { + expect(githubReleaseEditArgs("v0.9.5", ["v0.9.3"])).toEqual([ + "release", + "edit", + "v0.9.5", + "--draft=false", + "--latest=true", + ]); + expect(githubReleaseEditArgs("v0.9.3", ["v0.9.5"])).toEqual([ + "release", + "edit", + "v0.9.3", + "--draft=false", + "--latest=false", + ]); + }); }); From 2d994606872039b0bee6872b43b915251126c68e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 17:46:32 +0000 Subject: [PATCH 3/4] fix: close leftover inspect, CI pack, and latest-tag gaps Follow named `export { local as exported } from` (including minified forms) instead of treating brace re-exports as local. Limit named-export checks to main-process files and skip asar `node_modules`. Pack a real unsigned electron-builder dir in CI so afterPack inspects the shipped asar. Paginate GitHub releases, never mark prereleases latest, and treat a failed `codesign -dv` as unsigned. --- .github/workflows/ci.yml | 13 +- apps/desktop/package.json | 1 + scripts/inspect-packaged-js.mjs | 64 +++++- scripts/publish-verified-release.mjs | 28 +-- scripts/release-helpers.mjs | 18 +- scripts/verify-macos-release.mjs | 10 +- tests/integration/inspect-packaged-js.spec.ts | 196 ++++++++++++++++-- .../publish-verified-release.spec.ts | 51 +++++ tests/integration/release-helpers.spec.ts | 36 ++++ 9 files changed, 372 insertions(+), 45 deletions(-) create mode 100644 tests/integration/publish-verified-release.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e0da70..73d0f9b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: verify: name: Lint, typecheck, test runs-on: macos-latest - timeout-minutes: 30 + timeout-minutes: 45 steps: - name: Check out repository @@ -59,11 +59,14 @@ jobs: env: NODE_OPTIONS: --max-old-space-size=8192 - - name: Inspect packaged JavaScript graph + - name: Inspect Vite JavaScript graph run: bun ./scripts/inspect-packaged-js.mjs apps/desktop/out - - name: Inspect Vite output packed as asar - run: bun ./scripts/inspect-packaged-js.mjs --pack-dir apps/desktop/out - - name: Assert isolated main-process layout run: bun ./scripts/assert-main-build-layout.mjs apps/desktop/out/main + + - name: Pack unsigned macOS dir and inspect asar + run: bun run --filter @pi-desktop/desktop pack:mac:dir + env: + NODE_OPTIONS: --max-old-space-size=8192 + CSC_IDENTITY_AUTO_DISCOVERY: "false" diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 673c695..f14e760 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -15,6 +15,7 @@ "build": "electron-vite build", "dev": "electron-vite dev", "dist:mac": "bun run build && bun ../../scripts/stage-desktop-release.mjs && electron-builder --projectDir . --config ../../electron-builder.yml --mac dmg zip --arm64", + "pack:mac:dir": "CSC_IDENTITY_AUTO_DISCOVERY=false bun ../../scripts/stage-desktop-release.mjs && CSC_IDENTITY_AUTO_DISCOVERY=false electron-builder --projectDir . --config ../../electron-builder.yml --mac dir --arm64", "postinstall": "electron-rebuild -f -w node-pty", "typecheck": "tsc --project tsconfig.json --noEmit" }, diff --git a/scripts/inspect-packaged-js.mjs b/scripts/inspect-packaged-js.mjs index d6c15b3..e5b8aaf 100644 --- a/scripts/inspect-packaged-js.mjs +++ b/scripts/inspect-packaged-js.mjs @@ -9,6 +9,10 @@ const RELATIVE_SPECIFIER_PATTERN = /(?:import|export)\s+["'](\.[^"']+)["']|(?:import|export)(?:\s+type)?(?:\s*\{[^}]*\}|\s+\*\s+as\s+[\w$]+|\s+\*|[\s\w*,]+)\s*from\s*["'](\.[^"']+)["']|import\s*\(\s*["'](\.[^"']+)["']\s*\)/g; const NAMED_FROM_PATTERN = /(?:import|export)\s*\{([^}]*)\}\s*from\s*["'](\.[^"']+)["']/g; +const LOCAL_EXPORT_BRACE_PATTERN = /\bexport\s*\{([^}]*)\}(\s*from\b)?/g; +const STAR_REEXPORT_PATTERN = /\bexport\s*\*\s*from\s*["'](\.[^"']+)["']/g; +const NAMED_REEXPORT_PATTERN = + /\bexport\s*\{([^}]*)\}\s*from\s*["'](\.[^"']+)["']/g; function toPosixPath(filePath) { return filePath.split(path.sep).join("/"); @@ -18,6 +22,18 @@ export function asarEntryPath(listedPath) { return toPosixPath(listedPath).replace(/^\/+/, ""); } +export function isNodeModulesPath(relativePath) { + return toPosixPath(relativePath).split("/").includes("node_modules"); +} + +export function isMainProcessJs(relativePath) { + return /(^|\/)main\//.test(toPosixPath(relativePath)); +} + +function shouldInspectJsPath(relativePath) { + return JS_FILE_PATTERN.test(relativePath) && !isNodeModulesPath(relativePath); +} + function walkJsFiles(rootDir, currentRelativeDir = "") { const absoluteDir = path.join(rootDir, currentRelativeDir); if (!existsSync(absoluteDir)) { @@ -26,6 +42,9 @@ function walkJsFiles(rootDir, currentRelativeDir = "") { const files = []; for (const entry of readdirSync(absoluteDir, { withFileTypes: true })) { + if (entry.name === "node_modules") { + continue; + } const relativePath = toPosixPath(path.join(currentRelativeDir, entry.name)); if (entry.isDirectory()) { files.push(...walkJsFiles(rootDir, relativePath)); @@ -103,7 +122,10 @@ export function moduleExportsName(source, name) { ) { return true; } - for (const match of source.matchAll(/\bexport\s*\{([^}]*)\}/g)) { + for (const match of source.matchAll(LOCAL_EXPORT_BRACE_PATTERN)) { + if (match[2]) { + continue; + } if ( parseSpecifierBindings(match[1] ?? "").some( (binding) => binding.exported === name, @@ -113,7 +135,7 @@ export function moduleExportsName(source, name) { } } return Boolean( - new RegExp(`\\bexport\\s+\\*\\s+as\\s+${escaped}\\s+from\\b`).test(source), + new RegExp(`\\bexport\\s*\\*\\s*as\\s+${escaped}\\s+from\\b`).test(source), ); } @@ -135,14 +157,26 @@ export function moduleHasExport( if (moduleExportsName(source, name)) { return true; } - for (const match of source.matchAll( - /\bexport\s+\*\s+from\s*["'](\.[^"']+)["']/g, - )) { + for (const match of source.matchAll(STAR_REEXPORT_PATTERN)) { const target = resolveRelativeImport(relativePath, match[1]); if (target && moduleHasExport(files, target, name, visiting)) { return true; } } + for (const match of source.matchAll(NAMED_REEXPORT_PATTERN)) { + const target = resolveRelativeImport(relativePath, match[2]); + if (!target) { + continue; + } + for (const binding of parseSpecifierBindings(match[1] ?? "")) { + if ( + binding.exported === name && + moduleHasExport(files, target, binding.local, visiting) + ) { + return true; + } + } + } return false; } @@ -160,11 +194,16 @@ function resolveRelativeImport(fromRelativePath, specifier) { * Inspect a JavaScript module graph for empty files, dangling relative ESM * imports, and named imports that the target module does not export. * `files` is a map of posix-relative path → file bytes. + * Named-export checks are limited to main-process files; renderer/preload + * bundles and `node_modules` are skipped to avoid CJS/minifier false positives. */ export function inspectJavaScriptGraph(files) { const problems = []; for (const [relativePath, bytes] of Object.entries(files)) { + if (isNodeModulesPath(relativePath)) { + continue; + } if (isEmptyBytes(bytes)) { problems.push({ kind: "empty", @@ -185,7 +224,7 @@ export function inspectJavaScriptGraph(files) { }); continue; } - if (!(target in files)) { + if (!(target in files) || isNodeModulesPath(target)) { problems.push({ kind: "missing-import", file: relativePath, @@ -202,9 +241,18 @@ export function inspectJavaScriptGraph(files) { } } + if (!isMainProcessJs(relativePath)) { + continue; + } + for (const { specifier, names } of extractNamedImports(source)) { const target = resolveRelativeImport(relativePath, specifier); - if (!target || !(target in files) || isEmptyBytes(files[target])) { + if ( + !target || + !(target in files) || + isNodeModulesPath(target) || + isEmptyBytes(files[target]) + ) { continue; } for (const name of names) { @@ -259,7 +307,7 @@ export function inspectAsar(asarPath, resolveFrom = process.cwd()) { const extractErrors = []; for (const listedPath of listed) { const relativePath = asarEntryPath(listedPath); - if (!JS_FILE_PATTERN.test(relativePath)) { + if (!shouldInspectJsPath(relativePath)) { continue; } try { diff --git a/scripts/publish-verified-release.mjs b/scripts/publish-verified-release.mjs index bf58ca0..98727cf 100644 --- a/scripts/publish-verified-release.mjs +++ b/scripts/publish-verified-release.mjs @@ -3,7 +3,13 @@ import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { githubReleaseEditArgs } from "./release-helpers.mjs"; -const RELEASE_LIST_LIMIT = "1000"; +export const GITHUB_PUBLISHED_RELEASE_TAGS_ARGS = [ + "api", + "--paginate", + "repos/{owner}/{repo}/releases?per_page=100", + "--jq", + ".[] | select(.draft == false) | .tag_name", +]; function run(command, args) { const result = spawnSync(command, args, { encoding: "utf8" }); @@ -17,19 +23,17 @@ function run(command, args) { return result.stdout ?? ""; } +export function parsePublishedReleaseTagLines(stdout) { + return String(stdout) + .split(/\r?\n/) + .map((line) => line.trim().replace(/^"|"$/g, "")) + .filter(Boolean); +} + export function listPublishedReleaseTags() { - const listed = JSON.parse( - run("gh", [ - "release", - "list", - "--exclude-drafts", - "--limit", - RELEASE_LIST_LIMIT, - "--json", - "tagName", - ]) || "[]", + return parsePublishedReleaseTagLines( + run("gh", GITHUB_PUBLISHED_RELEASE_TAGS_ARGS), ); - return listed.map((release) => release.tagName); } export function publishVerifiedRelease(tag) { diff --git a/scripts/release-helpers.mjs b/scripts/release-helpers.mjs index 6c0b8b3..34147f7 100644 --- a/scripts/release-helpers.mjs +++ b/scripts/release-helpers.mjs @@ -32,6 +32,10 @@ export function isDeveloperIdSignature(signature) { ); } +export function isUsableDeveloperIdSignature(codesignStatus, signature) { + return codesignStatus === 0 && isDeveloperIdSignature(signature); +} + function parsePrereleaseIdentifiers(prerelease) { return prerelease.split(".").map((part) => { const value = Number.parseInt(part, 10); @@ -110,10 +114,18 @@ export function compareReleaseTags(left, right) { return comparePrerelease(leftVersion.prerelease, rightVersion.prerelease); } +function isStableReleaseTag(tag) { + const parsed = parseReleaseTag(tag); + return Boolean(parsed && !parsed.prerelease); +} + export function shouldMarkReleaseLatest(tag, publishedTags) { - return publishedTags.every( - (publishedTag) => compareReleaseTags(tag, publishedTag) >= 0, - ); + if (!isStableReleaseTag(tag)) { + return false; + } + return publishedTags + .filter((publishedTag) => isStableReleaseTag(publishedTag)) + .every((publishedTag) => compareReleaseTags(tag, publishedTag) >= 0); } export function githubReleaseEditArgs(tag, publishedTags) { diff --git a/scripts/verify-macos-release.mjs b/scripts/verify-macos-release.mjs index cb9f65c..01d4a80 100644 --- a/scripts/verify-macos-release.mjs +++ b/scripts/verify-macos-release.mjs @@ -4,7 +4,7 @@ import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { assertJavaScriptGraph, inspectAsar } from "./inspect-packaged-js.mjs"; -import { isDeveloperIdSignature } from "./release-helpers.mjs"; +import { isUsableDeveloperIdSignature } from "./release-helpers.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(scriptDir, ".."); @@ -50,8 +50,12 @@ function verifyRelease() { const asarPath = path.join(appPath, "Contents", "Resources", "app.asar"); assertJavaScriptGraph(inspectAsar(asarPath, repoRoot), asarPath); - const signature = run("codesign", ["-dv", "--verbose=4", appPath]); - if (!isDeveloperIdSignature(signature)) { + const display = spawnSync("codesign", ["-dv", "--verbose=4", appPath], { + cwd: repoRoot, + encoding: "utf8", + }); + const signature = `${display.stdout ?? ""}${display.stderr ?? ""}`.trim(); + if (!isUsableDeveloperIdSignature(display.status ?? 1, signature)) { console.log( `✅ Verified unsigned Pi Desktop ${version} artifacts and JavaScript graph.`, ); diff --git a/tests/integration/inspect-packaged-js.spec.ts b/tests/integration/inspect-packaged-js.spec.ts index d1543bc..d5d10b4 100644 --- a/tests/integration/inspect-packaged-js.spec.ts +++ b/tests/integration/inspect-packaged-js.spec.ts @@ -9,7 +9,9 @@ import { inspectAsar, inspectDirectory, inspectJavaScriptGraph, + isMainProcessJs, moduleExportsName, + moduleHasExport, packDirectoryAndInspect, } from "../../scripts/inspect-packaged-js.mjs"; @@ -31,10 +33,10 @@ afterEach(() => { describe("inspectJavaScriptGraph", () => { it("accepts a self-contained ESM graph", () => { const problems = inspectJavaScriptGraph({ - "index.js": Buffer.from( + "out/main/index.js": Buffer.from( 'import { ot } from "./chunks/lib.js";\nexport { ot };\n', ), - "chunks/lib.js": Buffer.from("export const ot = 1;\n"), + "out/main/chunks/lib.js": Buffer.from("export const ot = 1;\n"), }); expect(problems).toEqual([]); @@ -64,27 +66,29 @@ describe("inspectJavaScriptGraph", () => { it("reports a relative import whose target is missing", () => { const problems = inspectJavaScriptGraph({ - "index.js": Buffer.from('import { ot } from "./missing.js";\n'), + "out/main/index.js": Buffer.from('import { ot } from "./missing.js";\n'), }); expect(problems).toEqual([ expect.objectContaining({ kind: "missing-import", - file: "index.js", + file: "out/main/index.js", }), ]); }); it("reports a multiline named import whose target does not export that name", () => { const problems = inspectJavaScriptGraph({ - "index.js": Buffer.from('import {\n ot\n} from "./chunks/lib.js";\n'), - "chunks/lib.js": Buffer.from("export const other = 1;\n"), + "out/main/index.js": Buffer.from( + 'import {\n ot\n} from "./chunks/lib.js";\n', + ), + "out/main/chunks/lib.js": Buffer.from("export const other = 1;\n"), }); expect(problems).toEqual([ expect.objectContaining({ kind: "missing-export", - file: "index.js", + file: "out/main/index.js", detail: "imports ot from ./chunks/lib.js but that export is missing", }), ]); @@ -92,24 +96,137 @@ describe("inspectJavaScriptGraph", () => { it("follows export * from instead of treating star re-exports as universal", () => { const missing = inspectJavaScriptGraph({ - "index.js": Buffer.from('import { ot } from "./barrel.js";\n'), - "barrel.js": Buffer.from('export * from "./lib.js";\n'), - "lib.js": Buffer.from("export const other = 1;\n"), + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export * from "./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const other = 1;\n"), }); expect(missing).toEqual([ expect.objectContaining({ kind: "missing-export", - file: "index.js", + file: "out/main/index.js", }), ]); const present = inspectJavaScriptGraph({ - "index.js": Buffer.from('import { ot } from "./barrel.js";\n'), - "barrel.js": Buffer.from('export * from "./lib.js";\n'), - "lib.js": Buffer.from("export const ot = 1;\n"), + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export * from "./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const ot = 1;\n"), + }); + expect(present).toEqual([]); + }); + + it("follows named export { local as exported } from instead of treating it as local", () => { + const missingLocal = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from( + 'export { missing as ot } from "./lib.js";\n', + ), + "out/main/lib.js": Buffer.from("export const other = 1;\n"), + }); + expect(missingLocal).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "missing-export", + file: "out/main/index.js", + }), + ]), + ); + + const missingExported = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export { ot } from "./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const other = 1;\n"), + }); + expect(missingExported).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "missing-export", + file: "out/main/index.js", + }), + ]), + ); + + const present = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from( + 'export { source as ot } from "./lib.js";\n', + ), + "out/main/lib.js": Buffer.from("export const source = 1;\n"), }); expect(present).toEqual([]); }); + + it("follows minified export*from and export{ot}from", () => { + const starPresent = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export*from"./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const ot = 1;\n"), + }); + expect(starPresent).toEqual([]); + + const namedPresent = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export{ot}from"./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const ot = 1;\n"), + }); + expect(namedPresent).toEqual([]); + + const namedMissing = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from('import { ot } from "./barrel.js";\n'), + "out/main/barrel.js": Buffer.from('export{ot}from"./lib.js";\n'), + "out/main/lib.js": Buffer.from("export const other = 1;\n"), + }); + expect(namedMissing).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "missing-export", + file: "out/main/index.js", + }), + ]), + ); + }); + + it("does not require named exports from renderer bundles", () => { + const problems = inspectJavaScriptGraph({ + "out/renderer/index.js": Buffer.from( + 'import { ot } from "./chunk.js";\n', + ), + "out/renderer/chunk.js": Buffer.from("export const other = 1;\n"), + }); + expect(problems).toEqual([]); + }); + + it("still reports empty renderer files", () => { + const problems = inspectJavaScriptGraph({ + "out/renderer/index.js": Buffer.from('import "./chunk.js";\n'), + "out/renderer/chunk.js": Buffer.from(""), + }); + expect(problems).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "empty", + file: "out/renderer/chunk.js", + }), + ]), + ); + }); + + it("ignores node_modules JavaScript", () => { + const problems = inspectJavaScriptGraph({ + "out/main/index.js": Buffer.from("export const main = 1;\n"), + "node_modules/node-pty/lib/index.js": Buffer.from(""), + }); + expect(problems).toEqual([]); + }); +}); + +describe("isMainProcessJs", () => { + it("matches packaged and Vite main-process paths", () => { + expect(isMainProcessJs("out/main/index.js")).toBe(true); + expect(isMainProcessJs("main/index.js")).toBe(true); + expect(isMainProcessJs("out/renderer/index.js")).toBe(false); + expect(isMainProcessJs("out/preload/index.js")).toBe(false); + }); }); describe("moduleExportsName", () => { @@ -127,6 +244,27 @@ describe("moduleExportsName", () => { it("does not treat export * from as exporting every name", () => { expect(moduleExportsName('export * from "./lib.js";\n', "ot")).toBe(false); }); + + it("does not treat named re-exports as local exports", () => { + expect(moduleExportsName('export { ot } from "./lib.js";\n', "ot")).toBe( + false, + ); + expect(moduleExportsName('export{ot}from"./lib.js";\n', "ot")).toBe(false); + expect(moduleExportsName("export { ot };\n", "ot")).toBe(true); + }); +}); + +describe("moduleHasExport", () => { + it("walks aliased named re-exports to the local name", () => { + const files = { + "out/main/barrel.js": Buffer.from( + 'export { source as ot } from "./lib.js";\n', + ), + "out/main/lib.js": Buffer.from("export const source = 1;\n"), + }; + expect(moduleHasExport(files, "out/main/barrel.js", "ot")).toBe(true); + expect(moduleHasExport(files, "out/main/barrel.js", "source")).toBe(false); + }); }); describe("inspectDirectory", () => { @@ -148,6 +286,19 @@ describe("inspectDirectory", () => { expect(() => inspectDirectory(rootDir)).toThrow(/No JavaScript files/); }); + + it("does not walk node_modules", () => { + const rootDir = createFixtureRoot(); + mkdirSync(path.join(rootDir, "out/main"), { recursive: true }); + mkdirSync(path.join(rootDir, "node_modules/node-pty"), { recursive: true }); + writeFileSync( + path.join(rootDir, "out/main/index.js"), + "export const n = 1;\n", + ); + writeFileSync(path.join(rootDir, "node_modules/node-pty/index.js"), ""); + + expect(inspectDirectory(rootDir)).toEqual([]); + }); }); describe("inspectAsar", () => { @@ -201,6 +352,23 @@ describe("inspectAsar", () => { expect(problems).toEqual([]); }); + it("skips node_modules entries in the asar", async () => { + const asar = require("@electron/asar"); + const rootDir = createFixtureRoot(); + const appDir = path.join(rootDir, "app"); + const asarPath = path.join(rootDir, "app.asar"); + mkdirSync(path.join(appDir, "out/main"), { recursive: true }); + mkdirSync(path.join(appDir, "node_modules/node-pty"), { recursive: true }); + writeFileSync( + path.join(appDir, "out/main/index.js"), + "export const main = 1;\n", + ); + writeFileSync(path.join(appDir, "node_modules/node-pty/index.js"), ""); + await asar.createPackage(appDir, asarPath); + + expect(inspectAsar(asarPath, process.cwd())).toEqual([]); + }); + it("packs a directory to asar and inspects the archive", async () => { const rootDir = createFixtureRoot(); mkdirSync(path.join(rootDir, "out/main"), { recursive: true }); diff --git a/tests/integration/publish-verified-release.spec.ts b/tests/integration/publish-verified-release.spec.ts new file mode 100644 index 0000000..b5a01bb --- /dev/null +++ b/tests/integration/publish-verified-release.spec.ts @@ -0,0 +1,51 @@ +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + GITHUB_PUBLISHED_RELEASE_TAGS_ARGS, + parsePublishedReleaseTagLines, +} from "../../scripts/publish-verified-release.mjs"; + +describe("publish verified release", () => { + it("paginates GitHub releases instead of capping at 1000", () => { + expect(GITHUB_PUBLISHED_RELEASE_TAGS_ARGS).toContain("--paginate"); + expect(GITHUB_PUBLISHED_RELEASE_TAGS_ARGS.join(" ")).not.toMatch( + /--limit\s+1000/, + ); + expect(GITHUB_PUBLISHED_RELEASE_TAGS_ARGS.join(" ")).toContain( + "select(.draft == false)", + ); + }); + + it("parses paginated jq tag lines including quoted names", () => { + expect(parsePublishedReleaseTagLines('"v0.9.5"\nv0.9.3\n\n')).toEqual([ + "v0.9.5", + "v0.9.3", + ]); + }); +}); + +describe("CI packs a real unsigned electron-builder asar", () => { + it("uses pack:mac:dir instead of packing Vite out/", () => { + const ci = readFileSync( + path.resolve(process.cwd(), ".github/workflows/ci.yml"), + "utf8", + ); + const desktopPackage = JSON.parse( + readFileSync( + path.resolve(process.cwd(), "apps/desktop/package.json"), + "utf8", + ), + ); + + expect(ci).toContain("pack:mac:dir"); + expect(ci).toContain("CSC_IDENTITY_AUTO_DISCOVERY"); + expect(ci).not.toContain("--pack-dir"); + expect(ci).toMatch(/timeout-minutes:\s*45/); + expect(desktopPackage.scripts["pack:mac:dir"]).toContain("--mac dir"); + expect(desktopPackage.scripts["pack:mac:dir"]).toContain( + "CSC_IDENTITY_AUTO_DISCOVERY=false", + ); + }); +}); diff --git a/tests/integration/release-helpers.spec.ts b/tests/integration/release-helpers.spec.ts index c918b0f..d92613a 100644 --- a/tests/integration/release-helpers.spec.ts +++ b/tests/integration/release-helpers.spec.ts @@ -4,6 +4,7 @@ import { compareReleaseTags, githubReleaseEditArgs, isDeveloperIdSignature, + isUsableDeveloperIdSignature, shouldMarkReleaseLatest, withBuildHeap, } from "../../scripts/release-helpers.mjs"; @@ -27,6 +28,17 @@ describe("release helpers", () => { ).toBe(false); }); + it("treats a failed codesign -dv as unsigned instead of a hard failure", () => { + const developerId = + "Authority=Developer ID Application: Example (TEAMID)\nTeamIdentifier=TEAMID\n"; + expect(isUsableDeveloperIdSignature(0, developerId)).toBe(true); + expect(isUsableDeveloperIdSignature(1, developerId)).toBe(false); + expect(isUsableDeveloperIdSignature(1, "")).toBe(false); + expect( + isUsableDeveloperIdSignature(0, "code object is not signed at all"), + ).toBe(false); + }); + it("only marks a release latest when its tag is newest", () => { expect(compareReleaseTags("v0.9.5", "v0.9.3")).toBeGreaterThan(0); expect(shouldMarkReleaseLatest("v0.9.5", ["v0.9.3", "v0.9.5"])).toBe(true); @@ -38,6 +50,16 @@ describe("release helpers", () => { expect(shouldMarkReleaseLatest("v1.0.0-beta.1", ["v1.0.0"])).toBe(false); }); + it("never marks a prerelease latest and compares stables only to stables", () => { + expect(shouldMarkReleaseLatest("v1.0.0-beta.1", [])).toBe(false); + expect(shouldMarkReleaseLatest("v1.0.0-beta.1", ["v0.9.5"])).toBe(false); + expect(shouldMarkReleaseLatest("v0.9.5", ["v1.0.0-beta.1"])).toBe(true); + expect(shouldMarkReleaseLatest("v0.9.5", ["v0.9.3", "v1.0.0-beta.1"])).toBe( + true, + ); + expect(shouldMarkReleaseLatest("not-a-tag", ["v0.9.5"])).toBe(false); + }); + it("explicitly sets --latest=false so draft publish cannot steal latest", () => { expect(githubReleaseEditArgs("v0.9.5", ["v0.9.3"])).toEqual([ "release", @@ -53,5 +75,19 @@ describe("release helpers", () => { "--draft=false", "--latest=false", ]); + expect(githubReleaseEditArgs("v1.0.0-beta.1", [])).toEqual([ + "release", + "edit", + "v1.0.0-beta.1", + "--draft=false", + "--latest=false", + ]); + expect(githubReleaseEditArgs("v0.9.5", ["v1.0.0-beta.1"])).toEqual([ + "release", + "edit", + "v0.9.5", + "--draft=false", + "--latest=true", + ]); }); }); From e547f302cfc851f17316d34f3f9f70ac64d7df24 Mon Sep 17 00:00:00 2001 From: Tan Date: Wed, 16 Sep 2026 14:22:01 -0400 Subject: [PATCH 4/4] fix: close release inspection review gaps --- scripts/assert-main-build-layout.mjs | 13 +-- scripts/inspect-packaged-js.mjs | 85 +++++++------------ .../assert-main-build-layout.spec.ts | 7 +- tests/integration/inspect-packaged-js.spec.ts | 27 +++--- 4 files changed, 48 insertions(+), 84 deletions(-) diff --git a/scripts/assert-main-build-layout.mjs b/scripts/assert-main-build-layout.mjs index 2358c2f..f50b9d7 100644 --- a/scripts/assert-main-build-layout.mjs +++ b/scripts/assert-main-build-layout.mjs @@ -1,5 +1,5 @@ #!/usr/bin/env node -import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -16,15 +16,10 @@ export function assertMainProcessLayout(mainOutDir) { } const chunksDir = path.join(mainOutDir, "chunks"); - if (existsSync(chunksDir) && statSync(chunksDir).isDirectory()) { - const sharedChunks = readdirSync(chunksDir).filter((name) => - /\.(?:[cm]?js)$/.test(name), + if (existsSync(chunksDir)) { + throw new Error( + `Shared main-process chunks directory exists: ${chunksDir}`, ); - if (sharedChunks.length > 0) { - throw new Error( - `Shared main-process chunks were emitted: ${sharedChunks.join(", ")}`, - ); - } } const sessionFiles = readdirSync(mainOutDir).filter((name) => diff --git a/scripts/inspect-packaged-js.mjs b/scripts/inspect-packaged-js.mjs index e5b8aaf..5bf2c29 100644 --- a/scripts/inspect-packaged-js.mjs +++ b/scripts/inspect-packaged-js.mjs @@ -1,6 +1,5 @@ -import { existsSync, mkdtempSync, readdirSync, readFileSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; import { createRequire } from "node:module"; -import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -145,39 +144,44 @@ export function moduleHasExport( name, visiting = new Set(), ) { - if (visiting.has(relativePath)) { + const visitKey = `${relativePath}\0${name}`; + if (visiting.has(visitKey)) { return false; } - visiting.add(relativePath); - const bytes = files[relativePath]; - if (!bytes || isEmptyBytes(bytes)) { - return false; - } - const source = bytes.toString("utf8"); - if (moduleExportsName(source, name)) { - return true; - } - for (const match of source.matchAll(STAR_REEXPORT_PATTERN)) { - const target = resolveRelativeImport(relativePath, match[1]); - if (target && moduleHasExport(files, target, name, visiting)) { - return true; + visiting.add(visitKey); + try { + const bytes = files[relativePath]; + if (!bytes || isEmptyBytes(bytes)) { + return false; } - } - for (const match of source.matchAll(NAMED_REEXPORT_PATTERN)) { - const target = resolveRelativeImport(relativePath, match[2]); - if (!target) { - continue; + const source = bytes.toString("utf8"); + if (moduleExportsName(source, name)) { + return true; } - for (const binding of parseSpecifierBindings(match[1] ?? "")) { - if ( - binding.exported === name && - moduleHasExport(files, target, binding.local, visiting) - ) { + for (const match of source.matchAll(STAR_REEXPORT_PATTERN)) { + const target = resolveRelativeImport(relativePath, match[1]); + if (target && moduleHasExport(files, target, name, visiting)) { return true; } } + for (const match of source.matchAll(NAMED_REEXPORT_PATTERN)) { + const target = resolveRelativeImport(relativePath, match[2]); + if (!target) { + continue; + } + for (const binding of parseSpecifierBindings(match[1] ?? "")) { + if ( + binding.exported === name && + moduleHasExport(files, target, binding.local, visiting) + ) { + return true; + } + } + } + return false; + } finally { + visiting.delete(visitKey); } - return false; } function isEmptyBytes(bytes) { @@ -346,35 +350,10 @@ export function assertJavaScriptGraph(problems, label) { ); } -export async function packDirectoryAndInspect( - rootDir, - resolveFrom = process.cwd(), -) { - const asar = loadAsarModule(resolveFrom); - const tempDir = mkdtempSync(path.join(tmpdir(), "pi-pack-asar-")); - const asarPath = path.join(tempDir, "app.asar"); - await asar.createPackage(path.resolve(rootDir), asarPath); - const problems = inspectAsar(asarPath, resolveFrom); - assertJavaScriptGraph(problems, asarPath); - console.log( - `JavaScript graph OK: ${asarPath} (packed from ${path.resolve(rootDir)})`, - ); - return problems; -} - function runCli(argv) { const target = argv[0]; - if (target === "--pack-dir") { - const directory = argv[1]; - if (!directory) { - throw new Error("Usage: inspect-packaged-js.mjs --pack-dir "); - } - return packDirectoryAndInspect(directory); - } if (!target) { - throw new Error( - "Usage: inspect-packaged-js.mjs | --pack-dir ", - ); + throw new Error("Usage: inspect-packaged-js.mjs "); } const resolved = path.resolve(target); const problems = resolved.endsWith(".asar") diff --git a/tests/integration/assert-main-build-layout.spec.ts b/tests/integration/assert-main-build-layout.spec.ts index 875f51b..76bbb55 100644 --- a/tests/integration/assert-main-build-layout.spec.ts +++ b/tests/integration/assert-main-build-layout.spec.ts @@ -53,7 +53,7 @@ describe("assertMainProcessLayout", () => { ); }); - it("rejects shared chunks", () => { + it("rejects the shared chunks directory even when it is empty", () => { const mainOutDir = createFixtureRoot(); mkdirSync(path.join(mainOutDir, "chunks")); writeFileSync( @@ -64,11 +64,6 @@ describe("assertMainProcessLayout", () => { path.join(mainOutDir, "agent-host-session-server-entry-abc123.js"), "export const start = 1;\n", ); - writeFileSync( - path.join(mainOutDir, "chunks", "lib-WNnjYEnG.js"), - "export const ot = 1;\n", - ); - expect(() => assertMainProcessLayout(mainOutDir)).toThrow( /Shared main-process chunks/, ); diff --git a/tests/integration/inspect-packaged-js.spec.ts b/tests/integration/inspect-packaged-js.spec.ts index d5d10b4..cf00cc7 100644 --- a/tests/integration/inspect-packaged-js.spec.ts +++ b/tests/integration/inspect-packaged-js.spec.ts @@ -12,7 +12,6 @@ import { isMainProcessJs, moduleExportsName, moduleHasExport, - packDirectoryAndInspect, } from "../../scripts/inspect-packaged-js.mjs"; const require = createRequire(import.meta.url); @@ -265,6 +264,17 @@ describe("moduleHasExport", () => { expect(moduleHasExport(files, "out/main/barrel.js", "ot")).toBe(true); expect(moduleHasExport(files, "out/main/barrel.js", "source")).toBe(false); }); + + it("checks an aliased re-export after a star re-export visits the same file", () => { + const files = { + "out/main/barrel.js": Buffer.from( + 'export * from "./lib.js";\nexport { source as ot } from "./lib.js";\n', + ), + "out/main/lib.js": Buffer.from("export const source = 1;\n"), + }; + + expect(moduleHasExport(files, "out/main/barrel.js", "ot")).toBe(true); + }); }); describe("inspectDirectory", () => { @@ -368,19 +378,4 @@ describe("inspectAsar", () => { expect(inspectAsar(asarPath, process.cwd())).toEqual([]); }); - - it("packs a directory to asar and inspects the archive", async () => { - const rootDir = createFixtureRoot(); - mkdirSync(path.join(rootDir, "out/main"), { recursive: true }); - writeFileSync( - path.join(rootDir, "out/main/index.js"), - 'import { ot } from "./lib.js";\nexport { ot };\n', - ); - writeFileSync( - path.join(rootDir, "out/main/lib.js"), - "export const ot = 1;\n", - ); - - await expect(packDirectoryAndInspect(rootDir)).resolves.toEqual([]); - }); });