From 563be1293d2049279a8cca4a0182be76771e2ec2 Mon Sep 17 00:00:00 2001 From: Raj D <25481060+radroid@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:41:05 -0400 Subject: [PATCH] fix(coil): verify every packaged layer, and fail on layers the gate cannot see (#102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream's Windows server.asar split (this sync) broke the release's bundle-verify gate both ways at once: the server graph silently stopped being scanned, and node-pty — mentioned only inside main.cjs's embedded WSL heredoc scripts, loadable from the sidecar where it genuinely lives — was reported unresolvable, failing a shippable build (release run 31837711136, publish skipped). The view now merges a sibling resources/server.asar (+ .unpacked) into the packaged-file map, so sidecar bundles are scanned again and sidecar packages resolve. And every FIRST_PARTY_BUNDLE_DIRS entry must contribute at least one scanned bundle — a packaging-topology change that hides a layer is now an error instead of an empty green result, which is the 'guard that cannot fail' shape #97 already taught us once. Also corrects the three comments still teaching the disproven GITHUB_REPOSITORY="" mechanism (main.ts, UpdateToast.tsx) and the workflow step's stale Windows-topology claim. Fixes #102. Co-Authored-By: Claude Fable 5 --- .github/workflows/coil-release.yml | 9 ++- apps/desktop/src/main.ts | 5 +- apps/web/src/components/coil/UpdateToast.tsx | 6 +- scripts/coil/desktop-bundle-size.test.ts | 53 +++++++++++++++- scripts/coil/verify-desktop-bundle.d.mts | 7 +++ scripts/coil/verify-desktop-bundle.mjs | 63 +++++++++++++++++--- 6 files changed, 125 insertions(+), 18 deletions(-) diff --git a/.github/workflows/coil-release.yml b/.github/workflows/coil-release.yml index 09c9a02325a..32d6631c7fd 100644 --- a/.github/workflows/coil-release.yml +++ b/.github/workflows/coil-release.yml @@ -647,9 +647,12 @@ jobs: # not depend on attribution: a bundle that imports what it does not contain is broken either way, # and an earlier draft that warned on unattributed gaps passed a build that could not start. # - # Runs on BOTH platforms: the Windows bundle unpacks node_modules outside the asar (the checker - # merges app.asar.unpacked back in) and its graph differs (WSL backend, Linux fff binaries), so a - # mac-only check would not cover it. + # Runs on BOTH platforms: the Windows artifact ships the server tree as a resources/server.asar + # sidecar next to app.asar (the checker discovers and merges it, plus both .unpacked siblings) + # and its graph differs (WSL backend, Linux fff binaries), so a mac-only check would not cover + # it. The checker also fails outright if any first-party bundle directory is invisible to it — + # the 2026-08-14 sync proved a packaging-topology change can otherwise leave a whole layer + # unverified (issue #102). - name: Verify the packaged bundle still resolves shell: bash run: | diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 7a71f95d8f9..87d960375a4 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -193,8 +193,9 @@ const desktopApplicationLayer = Layer.mergeAll( DesktopLinuxUrlHandler.layer, DesktopShellEnvironment.layer, // coil: fork-owned update delivery. Sits beside DesktopUpdates rather than replacing it — - // upstream's updater is silenced by building with GITHUB_REPOSITORY="" (no app-update.yml is - // packaged, so it self-disables), which costs no seam here. + // upstream's updater is silenced by building with T3CODE_DESKTOP_UPDATE_REPOSITORY: "disabled" + // (no app-update.yml is packaged, so it self-disables; a GITHUB_REPOSITORY override cannot work + // in Actions, see coil-release.yml), which costs no seam here. CoilUpdateDelivery.layer, desktopSshLayer, ).pipe( diff --git a/apps/web/src/components/coil/UpdateToast.tsx b/apps/web/src/components/coil/UpdateToast.tsx index 88934d8752b..903f0ddfc56 100644 --- a/apps/web/src/components/coil/UpdateToast.tsx +++ b/apps/web/src/components/coil/UpdateToast.tsx @@ -7,8 +7,10 @@ * subscription and the toast calls. * * There is exactly one update surface in a fork build. Upstream's two — the sidebar pill and - * `desktopUpdate.toast.tsx` — are both silenced by building with `GITHUB_REPOSITORY: ""`, which - * packages no `app-update.yml`, which makes electron-updater self-disable. See the design doc. + * `desktopUpdate.toast.tsx` — are both silenced by building with + * `T3CODE_DESKTOP_UPDATE_REPOSITORY: "disabled"` (a `GITHUB_REPOSITORY` override cannot work in + * Actions), which packages no `app-update.yml`, which makes electron-updater self-disable. See the + * design doc and coil-release.yml. */ import { useEffect, useMemo, useRef, useState } from "react"; diff --git a/scripts/coil/desktop-bundle-size.test.ts b/scripts/coil/desktop-bundle-size.test.ts index e7751b75e83..f5671914dfa 100644 --- a/scripts/coil/desktop-bundle-size.test.ts +++ b/scripts/coil/desktop-bundle-size.test.ts @@ -478,7 +478,7 @@ it.layer(NodeServices.layer)("the packaging seam agrees everywhere", (it) => { */ describe("verifyPackagedApp over a real asar", () => { /** Writes a minimal but format-correct asar: [4][8+n+pad][4+n+pad][n][json][pad][contents]. */ - const writeAsar = (dir: string, files: Record): string => { + const writeAsar = (dir: string, files: Record, name = "app.asar"): string => { const entries: Record = {}; const blobs: Buffer[] = []; let offset = 0; @@ -509,7 +509,7 @@ describe("verifyPackagedApp over a real asar", () => { preamble.writeUInt32LE(4 + json.length + pad, 8); preamble.writeUInt32LE(json.length, 12); - const asarPath = NodePath.join(dir, "app.asar"); + const asarPath = NodePath.join(dir, name); NodeFS.writeFileSync(asarPath, Buffer.concat([preamble, json, Buffer.alloc(pad), ...blobs])); return asarPath; }; @@ -527,6 +527,7 @@ describe("verifyPackagedApp over a real asar", () => { withTempDir((dir) => { const asarPath = writeAsar(dir, { "apps/server/dist/bin.mjs": 'import x from "effect";\n', + "apps/desktop/dist-electron/main.cjs": "const x = 1;", "node_modules/effect/package.json": '{"name":"effect"}', "node_modules/effect/dist/index.js": "module.exports = {};", }); @@ -584,6 +585,7 @@ describe("verifyPackagedApp over a real asar", () => { // The @noble/hashes shape that failed a good release before comments were blanked. "apps/server/dist/bin.mjs": "/**\n * @example\n * import { hmac } from '@noble/hashes/hmac';\n */\nconst x = 1;\n", + "apps/desktop/dist-electron/main.cjs": "const x = 1;", }); assert.ok(verifyPackagedApp(asarPath).ok, "a JSDoc example is not an import"); }); @@ -594,10 +596,57 @@ describe("verifyPackagedApp over a real asar", () => { const asarPath = writeAsar(dir, { "apps/desktop/dist-electron/main.cjs": 'require("electron");require("@t3tools/shared");require("node:fs");', + "apps/server/dist/bin.mjs": "const x = 1;", }); assert.ok(verifyPackagedApp(asarPath).ok); }); }); + + /* + * The 2026-08-14 regression (issue #102). Upstream moved the Windows server tree out of app.asar + * into a resources/server.asar sidecar: node-pty (mentioned by main.cjs's embedded WSL scripts) + * stopped resolving from app.asar and the gate went red on a shippable build — and, worse, the + * server bundles silently stopped being scanned at all. The sidecar is part of the shipped app; + * the view must include it. + */ + it("resolves imports from a sibling server.asar sidecar and scans its bundles", () => { + withTempDir((dir) => { + const asarPath = writeAsar(dir, { + "apps/desktop/dist-electron/main.cjs": 'require("node-pty");', + }); + writeAsar( + dir, + { + // The server bundle imports a package that exists nowhere: with the sidecar merged into + // the view this MUST fail, proving server bundles are scanned rather than merely stored. + "apps/server/dist/bin.mjs": 'import x from "not-shipped";\n', + "node_modules/node-pty/package.json": '{"name":"node-pty"}', + "node_modules/node-pty/lib/index.js": "module.exports = {};", + }, + "server.asar", + ); + const result = verifyPackagedApp(asarPath); + assert.deepStrictEqual(result.uncoveredBundleDirs, [], "sidecar bundles must be visible"); + assert.deepStrictEqual( + result.missing.map((entry) => entry.name), + ["not-shipped"], + "node-pty resolves from the sidecar; the sidecar's own broken import still fails", + ); + }); + }); + + // A packaging-topology change that hides a whole bundle directory must be an error, not an empty + // green result — the pre-#102 gate verified only the Electron bundle on Windows and passed. + it("fails when a first-party bundle directory is invisible to the checker", () => { + withTempDir((dir) => { + const asarPath = writeAsar(dir, { + "apps/desktop/dist-electron/main.cjs": 'require("electron");', + }); + const result = verifyPackagedApp(asarPath); + assert.isFalse(result.ok, "an unscanned layer must fail the release"); + assert.deepStrictEqual(result.uncoveredBundleDirs, ["apps/server/dist"]); + }); + }); }); /* diff --git a/scripts/coil/verify-desktop-bundle.d.mts b/scripts/coil/verify-desktop-bundle.d.mts index 17baf796228..28e090e4d46 100644 --- a/scripts/coil/verify-desktop-bundle.d.mts +++ b/scripts/coil/verify-desktop-bundle.d.mts @@ -27,6 +27,13 @@ export interface VerifyResult { readonly importedBy: string; readonly glob: string | undefined; }[]; + /** Sibling `server.asar` sidecars merged into the view (the Windows server tree since #102). */ + readonly sidecars: readonly string[]; + /** + * FIRST_PARTY_BUNDLE_DIRS entries that contributed no scanned bundle in any packaged layer. + * Non-empty fails the release: a layer the checker cannot see must not pass silently. + */ + readonly uncoveredBundleDirs: readonly string[]; readonly totalBytes: number; readonly totalFiles: number; readonly mapBytes: number; diff --git a/scripts/coil/verify-desktop-bundle.mjs b/scripts/coil/verify-desktop-bundle.mjs index fa1bafca294..ee5bbfafa23 100644 --- a/scripts/coil/verify-desktop-bundle.mjs +++ b/scripts/coil/verify-desktop-bundle.mjs @@ -15,12 +15,16 @@ * granularity of the risk, because every fork exclusion removes a whole package or a file type, * never an individual entry point. * - * BOTH HALVES OF THE APP, WHICH IS THE POINT ON WINDOWS. The Windows artifact sets - * `asarUnpack: ["apps/server/dist/**", "**\/node_modules/**"]`, so on that platform the bundles AND - * every dependency live in `app.asar.unpacked/` on disk and the asar is nearly empty. A checker that - * read only the archive would find nothing on Windows, blame the fork's globs for all of it, and fail - * every Windows release. So the two sources are merged into one view before anything is checked, and - * `--verify-app` is the supported entry point precisely because it finds both. + * EVERY LAYER OF THE APP, WHICH IS THE POINT ON WINDOWS. The Windows artifact ships the server tree + * as a separate `resources/server.asar` sidecar (plus its `.unpacked` sibling for natives) so the + * NSIS installer extracts a handful of archives instead of thousands of files; `app.asar` holds only + * the Electron main-process bundle. A checker that read only `app.asar` would verify a fraction of + * the app and could not fail on the rest — which happened: the 2026-08-14 sync imported that split + * and this gate red-flagged `node-pty` as unresolvable when it had merely moved into the sidecar + * (issue #102). So the view merges `app.asar`, `app.asar.unpacked/`, and any sibling `server.asar` + * (+ `.unpacked`) before anything is checked, and additionally requires that every entry in + * FIRST_PARTY_BUNDLE_DIRS contributed at least one scanned bundle — a layer this checker cannot see + * fails the release instead of silently passing it. * * ANY MISSING IMPORT FAILS, and an earlier draft of this file got that wrong in a way worth recording. * It split findings into "a fork glob removed this" (error) and "missing for some other reason" @@ -323,6 +327,8 @@ export function findExcludingGlob(name, globs = defaultAttributionGlobs()) { * ok: boolean, * checked: number, * missing: { name: string, importedBy: string, glob: string | undefined }[], + * sidecars: string[], + * uncoveredBundleDirs: string[], * totalBytes: number, * totalFiles: number, * mapBytes: number, @@ -330,8 +336,39 @@ export function findExcludingGlob(name, globs = defaultAttributionGlobs()) { */ export function verifyPackagedApp(asarPath) { const files = readPackagedFiles(asarPath); + + // Windows ships the server tree as a resources/server.asar sidecar beside app.asar (see the + // module header). Its archive and .unpacked sibling are part of the shipped app, so they join + // the view: server bundles get scanned again, and a package that moved into the sidecar + // (node-pty, for the WSL probe scripts) counts as loadable because it is. + /** @type {string[]} */ + const sidecars = []; + const serverAsarPath = NodePath.join(NodePath.dirname(asarPath), "server.asar"); + if (serverAsarPath !== asarPath && NodeFS.existsSync(serverAsarPath)) { + for (const [relative, file] of readPackagedFiles(serverAsarPath)) { + if (!files.has(relative)) files.set(relative, file); + } + sidecars.push(serverAsarPath); + } + const required = collectRequiredPackages(files); + // A bundle directory nobody scanned is a layer this checker cannot see — exactly how the + // pre-#102 gate verified only the Electron bundle on Windows and could not fail on the server + // half. Absence must be an error, not an empty (green) result. + const scannedDirs = new Set(); + for (const filePath of files.keys()) { + for (const dir of FIRST_PARTY_BUNDLE_DIRS) { + if ( + filePath.startsWith(`${dir}/`) && + [".js", ".mjs", ".cjs"].includes(NodePath.posix.extname(filePath)) + ) { + scannedDirs.add(dir); + } + } + } + const uncoveredBundleDirs = FIRST_PARTY_BUNDLE_DIRS.filter((dir) => !scannedDirs.has(dir)); + /** @type {{ name: string, importedBy: string, glob: string | undefined }[]} */ const missing = []; @@ -350,9 +387,11 @@ export function verifyPackagedApp(asarPath) { } return { - ok: missing.length === 0, + ok: missing.length === 0 && uncoveredBundleDirs.length === 0, checked: required.size, missing, + sidecars, + uncoveredBundleDirs, totalBytes, totalFiles: files.size, mapBytes, @@ -432,6 +471,7 @@ if (isEntryPoint()) { process.stdout.write( [ `app.asar: ${asarPath}`, + `sidecars: ${result.sidecars.length > 0 ? result.sidecars.join(", ") : "none"}`, `packaged size: ${formatMiB(result.totalBytes)} across ${result.totalFiles} files`, `source maps: ${formatMiB(result.mapBytes)}`, `imports checked: ${result.checked} packages`, @@ -439,7 +479,12 @@ if (isEntryPoint()) { ].join("\n"), ); - if (!result.ok) { + if (result.uncoveredBundleDirs.length > 0) { + process.stderr.write( + `::error::No bundles found under ${result.uncoveredBundleDirs.join(", ")} in any packaged layer. A layer this checker cannot see passes nothing — the app's packaging topology changed and this script must learn the new location.\n`, + ); + } + if (result.missing.length > 0) { process.stderr.write( `::error::${result.missing.length} package(s) are imported by the packaged bundles but are not loadable from the shipped app. This bundle would throw MODULE_NOT_FOUND at runtime.\n`, ); @@ -450,8 +495,8 @@ if (isEntryPoint()) { : ` ${entry.name} — imported by ${entry.importedBy}; removed by ${entry.glob} — fix that glob in scripts/coil/desktop-file-exclusions.mjs\n`, ); } - process.exit(1); } + if (!result.ok) process.exit(1); process.stdout.write("Desktop bundle verification passed.\n"); }