From 9dd2abcf2ca875b074317d3d8b6521f797e79acb Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 18:03:55 -0400 Subject: [PATCH 1/6] fix(desktop): correct Windows process teardown, artifact guard, and taskbar identity Windows has no signal delivery, so child.kill terminated only the Host PID and left node-pty shells and subagent hosts holding the loopback port. Kill the tree with taskkill /T /F in the child adapter, where every caller routes. Also: the packaged-runtime guard tested for a forward slash and so never fired on a Windows exec path; set the Application User Model ID so taskbar pinning survives an installer upgrade; and require the win32 node-pty prebuilds at pack time. --- .changeset/desktop-windows-runtime.md | 5 ++ .../scripts/verify-packaged-runtime.ts | 10 +++ apps/desktop/src/host-supervisor.ts | 24 ++++++- apps/desktop/src/main.ts | 5 +- apps/desktop/tests/host-supervisor.spec.ts | 65 ++++++++++++++++++- .../tests/verify-packaged-runtime.spec.ts | 34 ++++++++++ 6 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 .changeset/desktop-windows-runtime.md diff --git a/.changeset/desktop-windows-runtime.md b/.changeset/desktop-windows-runtime.md new file mode 100644 index 00000000..a92387c8 --- /dev/null +++ b/.changeset/desktop-windows-runtime.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-desktop': patch +--- + +Fix Windows process-tree shutdown, packaged-runtime guards, and taskbar identity in the desktop app diff --git a/apps/desktop/scripts/verify-packaged-runtime.ts b/apps/desktop/scripts/verify-packaged-runtime.ts index 6deb0908..a51bd402 100644 --- a/apps/desktop/scripts/verify-packaged-runtime.ts +++ b/apps/desktop/scripts/verify-packaged-runtime.ts @@ -9,6 +9,12 @@ const REQUIRED_HOST_FILES = [ ['@pymodel', 'pythinker-code', 'dist-web', 'index.html'], ] as const +const REQUIRED_WINDOWS_NODE_PTY_ENTRIES = [ + ['node-pty', 'prebuilds', 'win32-x64', 'pty.node'], + ['node-pty', 'prebuilds', 'win32-x64', 'conpty.node'], + ['node-pty', 'prebuilds', 'win32-x64', 'conpty_console_list.node'], +] as const + /** * Verify the Host files required before the signed application can start. * @param context - Electron Builder's completed application directory. @@ -21,6 +27,10 @@ export async function afterPack(context: AfterPackContext): Promise { for (const segments of REQUIRED_HOST_FILES) { await access(join(resources, 'host', 'node_modules', ...segments)) } + if (context.electronPlatformName !== 'win32') return + for (const segments of REQUIRED_WINDOWS_NODE_PTY_ENTRIES) { + await access(join(resources, 'host', 'node_modules', ...segments)) + } } export default afterPack diff --git a/apps/desktop/src/host-supervisor.ts b/apps/desktop/src/host-supervisor.ts index bc09e555..2b759561 100644 --- a/apps/desktop/src/host-supervisor.ts +++ b/apps/desktop/src/host-supervisor.ts @@ -1,6 +1,6 @@ /** Supervise the loopback Web Host used by the first desktop application. */ -import { spawn, type ChildProcessByStdio } from 'node:child_process' +import { spawn, spawnSync, type ChildProcessByStdio } from 'node:child_process' import type { Readable } from 'node:stream' const READINESS_PREFIX = 'Pythinker server: ' @@ -297,6 +297,26 @@ export function spawnPythinkerServer(options: SpawnPythinkerServerOptions): Host return nodeChildAdapter(process) } +/** + * Terminate a child and its descendants. + * + * Windows has no signal delivery: `child.kill` calls TerminateProcess on one + * PID, so the Host's own children (node-pty shells, subagent hosts) survive and + * keep holding the loopback port. `taskkill /T` walks the tree instead. + * + * ponytail: /F makes every Windows stop a forced stop — Node cannot deliver a + * graceful SIGTERM to a Windows child at all. Add a stdin or IPC shutdown + * channel to the Host if graceful Windows teardown is ever needed. + */ +function killProcessTree(child: ChildProcessByStdio, signal: 'SIGTERM' | 'SIGKILL'): void { + if (process.platform !== 'win32' || child.pid === undefined) { + child.kill(signal) + return + } + const result = spawnSync('taskkill', ['/pid', String(child.pid), '/T', '/F'], { windowsHide: true }) + if (result.error !== undefined || result.status !== 0) child.kill(signal) +} + /** Adapt Node's event overloads to the supervisor's explicit ownership API. */ function nodeChildAdapter(child: ChildProcessByStdio): HostChild { return { @@ -312,7 +332,7 @@ function nodeChildAdapter(child: ChildProcessByStdio): return () => { child.off('error', listener) } }, kill(signal) { - child.kill(signal) + killProcessTree(child, signal) }, } } diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index fece9925..30522f41 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -2,7 +2,7 @@ import { randomUUID } from 'node:crypto' import { existsSync, readFileSync, writeFileSync } from 'node:fs' -import { join, resolve } from 'node:path' +import { isAbsolute, join, resolve } from 'node:path' import { app, BrowserWindow, @@ -109,7 +109,7 @@ function hostPaths(): { nodeExecutable: string; cliEntry: string; cwd: string; e } function assertHostArtifacts(paths: ReturnType): void { - if (paths.nodeExecutable.includes('/') && !existsSync(paths.nodeExecutable)) { + if (isAbsolute(paths.nodeExecutable) && !existsSync(paths.nodeExecutable)) { throw new Error(`desktop Node runtime is missing: ${paths.nodeExecutable}`) } if (!existsSync(paths.cliEntry)) { @@ -298,6 +298,7 @@ function requestAppQuit(): Promise { } async function boot(): Promise { + if (process.platform === 'win32') app.setAppUserModelId('com.pythinker.desktop') if (bootQuitPromise !== undefined) return initializeDesktopTelemetry() const paths = hostPaths() diff --git a/apps/desktop/tests/host-supervisor.spec.ts b/apps/desktop/tests/host-supervisor.spec.ts index 049d10f2..f862fdc4 100644 --- a/apps/desktop/tests/host-supervisor.spec.ts +++ b/apps/desktop/tests/host-supervisor.spec.ts @@ -1,4 +1,4 @@ -import { spawn } from 'node:child_process' +import { spawn, spawnSync } from 'node:child_process' import { afterEach, describe, expect, it, vi } from 'vitest' import { createHostSupervisor, @@ -314,4 +314,67 @@ describe('desktop Host process', () => { expect.objectContaining({ env: { PYTHINKER_DESKTOP: '1', ELECTRON_RUN_AS_NODE: '1' } }), ) }) + + it('kills the Windows Host process tree', async () => { + const child = { + pid: 4242, + stdout: { on: vi.fn(), off: vi.fn() }, + stderr: { on: vi.fn(), off: vi.fn() }, + on: vi.fn(), + off: vi.fn(), + kill: vi.fn(), + } + vi.mocked(spawn).mockReturnValue(child as never) + vi.mocked(spawnSync).mockReturnValue({ + pid: 4242, + output: [], + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + status: 0, + signal: null, + }) + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + + const { spawnPythinkerServer } = await import('../src/host-supervisor') + const host = spawnPythinkerServer({ + nodeExecutable: 'node', + cliEntry: '/tmp/launcher.mjs', + cwd: '/tmp', + env: {}, + }) + host.kill('SIGTERM') + + expect(spawnSync).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '4242', '/T', '/F'], + { windowsHide: true }, + ) + expect(child.kill).not.toHaveBeenCalled() + }) + + it('uses child.kill unchanged on non-Windows', async () => { + const child = { + pid: 4242, + stdout: { on: vi.fn(), off: vi.fn() }, + stderr: { on: vi.fn(), off: vi.fn() }, + on: vi.fn(), + off: vi.fn(), + kill: vi.fn(), + } + vi.mocked(spawn).mockReturnValue(child as never) + vi.mocked(spawnSync).mockClear() + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + + const { spawnPythinkerServer } = await import('../src/host-supervisor') + const host = spawnPythinkerServer({ + nodeExecutable: 'node', + cliEntry: '/tmp/launcher.mjs', + cwd: '/tmp', + env: {}, + }) + host.kill('SIGTERM') + + expect(child.kill).toHaveBeenCalledWith('SIGTERM') + expect(spawnSync).not.toHaveBeenCalled() + }) }) diff --git a/apps/desktop/tests/verify-packaged-runtime.spec.ts b/apps/desktop/tests/verify-packaged-runtime.spec.ts index ee624638..51576216 100644 --- a/apps/desktop/tests/verify-packaged-runtime.spec.ts +++ b/apps/desktop/tests/verify-packaged-runtime.spec.ts @@ -38,4 +38,38 @@ describe('packaged desktop runtime verification', () => { await rm(appOutDir, { recursive: true, force: true }) } }) + + it('rejects a Windows shell whose node-pty native closure was filtered out', async () => { + const appOutDir = await mkdtemp(join(tmpdir(), 'pythinker-packaged-runtime-')) + try { + const resources = join(appOutDir, 'resources', 'host', 'node_modules') + const cli = join(resources, '@pymodel', 'pythinker-code', 'dist', 'launcher.mjs') + const web = join(resources, '@pymodel', 'pythinker-code', 'dist-web', 'index.html') + await mkdir(join(cli, '..'), { recursive: true }) + await mkdir(join(web, '..'), { recursive: true }) + await writeFile(cli, '') + await writeFile(web, '') + + await expect(afterPack(context(appOutDir, 'win32'))).rejects.toMatchObject({ code: 'ENOENT' }) + } finally { + await rm(appOutDir, { recursive: true, force: true }) + } + }) + + it('does not require Windows node-pty entries for a Darwin shell', async () => { + const appOutDir = await mkdtemp(join(tmpdir(), 'pythinker-packaged-runtime-')) + try { + const resources = join(appOutDir, 'Pythinker.app', 'Contents', 'Resources', 'host', 'node_modules') + const cli = join(resources, '@pymodel', 'pythinker-code', 'dist', 'launcher.mjs') + const web = join(resources, '@pymodel', 'pythinker-code', 'dist-web', 'index.html') + await mkdir(join(cli, '..'), { recursive: true }) + await mkdir(join(web, '..'), { recursive: true }) + await writeFile(cli, '') + await writeFile(web, '') + + await expect(afterPack(context(appOutDir))).resolves.toBeUndefined() + } finally { + await rm(appOutDir, { recursive: true, force: true }) + } + }) }) From 70a22fc50c749b0c08eede9889e8d43143d06cf9 Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 18:13:07 -0400 Subject: [PATCH 2/6] feat(desktop): ship a Windows NSIS installer and release job Replace the win "dir" target with a per-user NSIS installer that allows elevation and a selectable installation directory, so electron-updater has a latest.yml feed to read on Windows. Add dist:win, which refuses to run anywhere but a native Windows x64 host: the staged Host closure resolves platform-gated native packages at deploy time, so a macOS-staged tree cannot produce a working Windows build. A companion verifier sniffs the DOS and PE headers of both artifacts rather than trusting the file extension. Windows artifacts are unsigned until WIN_CSC_LINK and WIN_CSC_KEY_PASSWORD are configured; the workflow passes them through so signing needs no code change. --- .changeset/desktop-windows-packaging.md | 5 ++ .github/workflows/desktop-release.yml | 54 ++++++++++++++ apps/desktop/README.md | 6 +- apps/desktop/package.json | 20 ++++- apps/desktop/scripts/release-win.ts | 37 ++++++++++ apps/desktop/scripts/verify-win-installer.ts | 36 +++++++++ apps/desktop/tests/packaging-config.spec.ts | 36 ++++++++- .../tests/verify-win-installer.spec.ts | 74 +++++++++++++++++++ package.json | 1 + 9 files changed, 265 insertions(+), 4 deletions(-) create mode 100644 .changeset/desktop-windows-packaging.md create mode 100644 apps/desktop/scripts/release-win.ts create mode 100644 apps/desktop/scripts/verify-win-installer.ts create mode 100644 apps/desktop/tests/verify-win-installer.spec.ts diff --git a/.changeset/desktop-windows-packaging.md b/.changeset/desktop-windows-packaging.md new file mode 100644 index 00000000..cf74b3cd --- /dev/null +++ b/.changeset/desktop-windows-packaging.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-desktop': patch +--- + +Add the Windows NSIS installer target, release script, and release workflow job diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index 9c8f863b..db55afc7 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -72,3 +72,57 @@ jobs: apps/desktop/dist/*.zip apps/desktop/dist/latest-mac.yml if-no-files-found: error + + windows: + runs-on: windows-latest + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # pinned from v4 + with: + fetch-depth: 0 + persist-credentials: true + + - uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # pinned from v6 + + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # pinned from v6 + with: + node-version-file: .nvmrc + cache: pnpm + + - run: pnpm install --frozen-lockfile + + - name: Stamp desktop version for tag builds + if: startsWith(github.ref, 'refs/tags/desktop-v') + shell: bash + env: + TAG_NAME: ${{ github.ref_name }} + run: | + export DESKTOP_VERSION="${TAG_NAME#desktop-v}" + node -e 'const fs = require("node:fs"); const path = "apps/desktop/package.json"; const packageJson = JSON.parse(fs.readFileSync(path, "utf8")); packageJson.version = process.env.DESKTOP_VERSION; fs.writeFileSync(path, `${JSON.stringify(packageJson, null, 2)}\n`);' + + - name: Build workspace + run: pnpm --workspace-root run build + + - name: Stage desktop runtime + working-directory: apps/desktop + run: node --import tsx scripts/stage-runtime.ts + + # Without WIN_CSC_* signing secrets, Windows artifacts are unsigned and + # installers trigger a SmartScreen warning on first run. + - name: Package and publish desktop release + working-directory: apps/desktop + run: pnpm exec electron-builder --win nsis --x64 --publish always + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + WIN_CSC_LINK: ${{ secrets.WIN_CSC_LINK }} + WIN_CSC_KEY_PASSWORD: ${{ secrets.WIN_CSC_KEY_PASSWORD }} + + - name: Upload Windows artifacts for manual runs + if: github.event_name == 'workflow_dispatch' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # pinned from v7 + with: + name: desktop-windows + path: | + apps/desktop/dist/*.exe + apps/desktop/dist/latest.yml + if-no-files-found: error diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 0572e176..1152b1b6 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -64,11 +64,15 @@ hdiutil detach "$MOUNT_POINT" rmdir "$MOUNT_POINT" ``` +### Windows + +Run `pnpm run dist:win` on a native Windows x64 host; cross-building from macOS is not possible because the staged Host closure contains platform-gated native packages. The output is `dist/Pythinker--x64-Setup.exe`, a per-user NSIS installer with no elevation required and a selectable installation directory. Artifacts are unsigned unless `WIN_CSC_LINK` and `WIN_CSC_KEY_PASSWORD` are set. + ## Known limitations The first desktop assembly uses a loopback HTTP Host. The renderer and Host protocol remain unchanged so the application can replace the transport with the IPC carrier reserved by the GUI architecture without changing product features. -The signed installer path currently targets macOS. Windows and Linux packaging creates unpacked applications; their installer formats and distribution signing remain release work. +The signed installer path currently targets macOS. Linux packaging creates an unpacked application; its installer format and distribution signing remain release work. ## Model Experience diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 4683e506..4df3a5bb 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -11,7 +11,8 @@ "dev": "pnpm -C ../pythinker-code run build && tsc -p tsconfig.json && tsdown && electron .", "package": "pnpm --workspace-root run build && node --import tsx scripts/stage-runtime.ts && electron-builder --dir", "dist": "pnpm --workspace-root run build && node --import tsx scripts/stage-runtime.ts && electron-builder", - "dist:mac": "node --import tsx scripts/release-mac.ts" + "dist:mac": "node --import tsx scripts/release-mac.ts", + "dist:win": "node --import tsx scripts/release-win.ts" }, "license": "MIT", "devDependencies": { @@ -67,9 +68,24 @@ "win": { "icon": "build/icon.png", "target": [ - "dir" + { + "target": "nsis", + "arch": [ + "x64" + ] + } ] }, + "nsis": { + "allowElevation": true, + "allowToChangeInstallationDirectory": true, + "artifactName": "Pythinker-${version}-${arch}-Setup.${ext}", + "createDesktopShortcut": true, + "createStartMenuShortcut": true, + "oneClick": false, + "perMachine": false, + "shortcutName": "Pythinker" + }, "linux": { "category": "Development", "target": [ diff --git a/apps/desktop/scripts/release-win.ts b/apps/desktop/scripts/release-win.ts new file mode 100644 index 00000000..97c3834b --- /dev/null +++ b/apps/desktop/scripts/release-win.ts @@ -0,0 +1,37 @@ +/** Build the Windows NSIS installer from a native Windows host. */ + +import { spawnSync } from 'node:child_process' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { verifyWindowsInstaller } from './verify-win-installer' + +function run(command: string, args: readonly string[], cwd: string): void { + const result = spawnSync(command, args, { cwd, stdio: 'inherit', shell: process.platform === 'win32' }) + if (result.error !== undefined) throw result.error + if (result.status !== 0) throw new Error(`${command} ${args.join(' ')} exited with ${String(result.status)}`) +} + +/** Build and verify the unsigned Windows installer. */ +export function releaseWin(): void { + if (process.platform !== 'win32') { + throw new Error('The Windows installer must be built on Windows: the staged Host closure contains platform-specific native packages') + } + if (process.arch !== 'x64') { + throw new Error(`The Windows installer targets x64; this host is ${process.arch}`) + } + const desktopRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..') + run('pnpm', ['--workspace-root', 'run', 'build'], desktopRoot) + run('node', ['--import', 'tsx', 'scripts/stage-runtime.ts'], desktopRoot) + run('pnpm', ['exec', 'electron-builder', '--win', 'nsis', '--x64', '--publish', 'never'], desktopRoot) + verifyWindowsInstaller(desktopRoot) +} + +const invokedPath = process.argv[1] +if (invokedPath !== undefined && resolve(invokedPath) === fileURLToPath(import.meta.url)) { + try { + releaseWin() + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)) + process.exitCode = 1 + } +} diff --git a/apps/desktop/scripts/verify-win-installer.ts b/apps/desktop/scripts/verify-win-installer.ts new file mode 100644 index 00000000..a47aa730 --- /dev/null +++ b/apps/desktop/scripts/verify-win-installer.ts @@ -0,0 +1,36 @@ +/** Reject a Windows release whose installer or unpacked shell is not a real PE binary. */ + +import { openSync, readSync, closeSync, statSync, readFileSync } from 'node:fs' +import { join } from 'node:path' + +const MINIMUM_PE_BYTES = 0x40 + 4 + +function assertPortableExecutable(path: string): void { + const stats = statSync(path) + if (!stats.isFile()) throw new Error(`Windows release artifact is not a regular file: ${path}`) + if (stats.size < MINIMUM_PE_BYTES) throw new Error(`Windows release artifact is too small to be a PE image: ${path}`) + const handle = openSync(path, 'r') + try { + const header = Buffer.alloc(0x40) + readSync(handle, header, 0, header.length, 0) + if (header.toString('latin1', 0, 2) !== 'MZ') throw new Error(`Windows release artifact has no DOS header: ${path}`) + const peOffset = header.readUInt32LE(0x3c) + if (peOffset + 4 > stats.size) throw new Error(`Windows release artifact has an out-of-range PE offset: ${path}`) + const signature = Buffer.alloc(4) + readSync(handle, signature, 0, 4, peOffset) + if (signature.toString('latin1') !== 'PE\0\0') throw new Error(`Windows release artifact has no PE signature: ${path}`) + } finally { + closeSync(handle) + } +} + +/** + * Verify the Windows artifacts electron-builder must have produced. + * @param desktopRoot - The apps/desktop directory containing dist/. + */ +export function verifyWindowsInstaller(desktopRoot: string): void { + const { version } = JSON.parse(readFileSync(join(desktopRoot, 'package.json'), 'utf8')) as { version: string } + assertPortableExecutable(join(desktopRoot, 'dist', `Pythinker-${version}-x64-Setup.exe`)) + assertPortableExecutable(join(desktopRoot, 'dist', 'win-unpacked', 'Pythinker.exe')) + console.log(`Windows release verified: Pythinker-${version}-x64-Setup.exe`) +} diff --git a/apps/desktop/tests/packaging-config.spec.ts b/apps/desktop/tests/packaging-config.spec.ts index 73ffbaad..ca381047 100644 --- a/apps/desktop/tests/packaging-config.spec.ts +++ b/apps/desktop/tests/packaging-config.spec.ts @@ -18,8 +18,24 @@ interface DesktopPackage { readonly icon: string readonly notarize: boolean } + readonly nsis: { + readonly allowElevation: boolean + readonly allowToChangeInstallationDirectory: boolean + readonly artifactName: string + readonly createDesktopShortcut: boolean + readonly createStartMenuShortcut: boolean + readonly oneClick: boolean + readonly perMachine: boolean + readonly shortcutName: string + } readonly productName: string - readonly win: { readonly icon: string } + readonly win: { + readonly icon: string + readonly target: readonly { + readonly target: string + readonly arch: readonly string[] + }[] + } } } @@ -79,9 +95,27 @@ describe('desktop packaging configuration', () => { expect(desktopPackage.build.mac.notarize).toBe(true) }) + it('configures the Windows x64 NSIS installer', () => { + expect(desktopPackage.build.win.target).toEqual([{ target: 'nsis', arch: ['x64'] }]) + expect(desktopPackage.build.nsis).toEqual({ + allowElevation: true, + allowToChangeInstallationDirectory: true, + artifactName: 'Pythinker-${version}-${arch}-Setup.${ext}', + createDesktopShortcut: true, + createStartMenuShortcut: true, + oneClick: false, + perMachine: false, + shortcutName: 'Pythinker', + }) + expect(desktopPackage.build.nsis.artifactName).toBe('Pythinker-${version}-${arch}-Setup.${ext}') + expect(desktopPackage.build.productName).toBe('Pythinker') + expect(desktopPackage.scripts['dist:win']).toBe('node --import tsx scripts/release-win.ts') + }) + it('exposes desktop commands at the repository root', () => { expect(rootPackage.scripts['dev:desktop']).toBe('pnpm -C apps/desktop run dev') expect(rootPackage.scripts['package:desktop']).toBe('pnpm -C apps/desktop run package') expect(rootPackage.scripts['dist:mac:desktop']).toBe('pnpm -C apps/desktop run dist:mac') + expect(rootPackage.scripts['dist:win:desktop']).toBe('pnpm -C apps/desktop run dist:win') }) }) diff --git a/apps/desktop/tests/verify-win-installer.spec.ts b/apps/desktop/tests/verify-win-installer.spec.ts new file mode 100644 index 00000000..a8455831 --- /dev/null +++ b/apps/desktop/tests/verify-win-installer.spec.ts @@ -0,0 +1,74 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { verifyWindowsInstaller } from '../scripts/verify-win-installer' + +const VERSION = '9.9.9' + +function portableExecutable(magic = 'MZ', signature = 'PE\0\0', offset = 0x80): Buffer { + const image = Buffer.alloc(offset + signature.length) + image.write(magic, 0, magic.length, 'latin1') + image.writeUInt32LE(offset, 0x3c) + image.write(signature, offset, signature.length, 'latin1') + return image +} + +async function withFixture(callback: (root: string) => Promise): Promise { + const root = await mkdtemp(join(tmpdir(), 'pythinker-win-installer-')) + try { + await callback(root) + } finally { + await rm(root, { force: true, recursive: true }) + } +} + +async function createFixture(root: string, installer?: Buffer): Promise { + const dist = join(root, 'dist') + await mkdir(join(dist, 'win-unpacked'), { recursive: true }) + await writeFile(join(root, 'package.json'), JSON.stringify({ version: VERSION })) + if (installer !== undefined) { + await writeFile(join(dist, `Pythinker-${VERSION}-x64-Setup.exe`), installer) + } + await writeFile(join(dist, 'win-unpacked', 'Pythinker.exe'), portableExecutable()) +} + +describe('Windows installer verification', () => { + it('accepts the installer and unpacked shell when both are PE binaries', async () => { + await withFixture(async (root) => { + await createFixture(root, portableExecutable()) + expect(() => verifyWindowsInstaller(root)).not.toThrow() + }) + }) + + it('rejects a missing installer', async () => { + await withFixture(async (root) => { + await createFixture(root) + expect(() => verifyWindowsInstaller(root)).toThrow('ENOENT') + }) + }) + + it('rejects an artifact without a DOS header', async () => { + await withFixture(async (root) => { + await createFixture(root, portableExecutable('ZZ')) + expect(() => verifyWindowsInstaller(root)).toThrow(/no DOS header/) + }) + }) + + it('rejects an artifact with a truncated PE offset', async () => { + await withFixture(async (root) => { + const truncated = Buffer.alloc(0x44) + truncated.write('MZ', 0, 2, 'latin1') + truncated.writeUInt32LE(0x80, 0x3c) + await createFixture(root, truncated) + expect(() => verifyWindowsInstaller(root)).toThrow(/out-of-range PE offset/) + }) + }) + + it('rejects an artifact without a PE signature', async () => { + await withFixture(async (root) => { + await createFixture(root, portableExecutable('MZ', 'NE\0\0')) + expect(() => verifyWindowsInstaller(root)).toThrow(/no PE signature/) + }) + }) +}) diff --git a/package.json b/package.json index c2930a0a..268323d8 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "dev:web": "pnpm -C apps/pythinker-web run dev", "package:desktop": "pnpm -C apps/desktop run package", "dist:mac:desktop": "pnpm -C apps/desktop run dist:mac", + "dist:win:desktop": "pnpm -C apps/desktop run dist:win", "dev:server": "pnpm -C apps/pythinker-code run dev:server", "build:plugin-marketplace": "pnpm -C apps/pythinker-code run build:plugin-marketplace", "dashboard": "pnpm -C apps/dashboard run dev", From 530dc19e200e976fc68e4666f89cbd21a0b37142 Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 19:02:09 -0400 Subject: [PATCH 3/6] fix(desktop): stage the runtime on Windows and skip empty signing credentials Node refuses to spawn a .cmd shim without a shell, so pnpm.cmd raised EINVAL and the Windows job never staged the Host closure. Spawn a bare pnpm through a shell on Windows and quote arguments cmd.exe would otherwise split. An unset GitHub secret interpolates to an empty string rather than to an absent variable, and electron-builder resolves an empty CSC_LINK as a certificate path -- path.resolve(appDir, '') is the app directory, so the mac job died on 'not a file'. Export only credentials that carry a value, and state the unsigned macOS path explicitly. --- .changeset/desktop-windows-ci-fixes.md | 5 +++ .github/workflows/desktop-release.yml | 45 +++++++++++++++++++----- apps/desktop/scripts/stage-runtime.ts | 43 ++++++++++++++++++++-- apps/desktop/tests/stage-runtime.spec.ts | 33 +++++++++++++++++ 4 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 .changeset/desktop-windows-ci-fixes.md create mode 100644 apps/desktop/tests/stage-runtime.spec.ts diff --git a/.changeset/desktop-windows-ci-fixes.md b/.changeset/desktop-windows-ci-fixes.md new file mode 100644 index 00000000..1e762bbb --- /dev/null +++ b/.changeset/desktop-windows-ci-fixes.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-desktop': patch +--- + +Fix Windows runtime staging and skip empty signing credentials in the desktop release workflow diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index db55afc7..fdeafbe7 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -51,16 +51,33 @@ jobs: # Without Developer ID signing secrets, electron-builder publishes an # ad-hoc/self-signed app. macOS auto-update will not accept unsigned updates, # but this still proves packaging and the feed shape. + # An unset GitHub secret interpolates to an empty string, and + # electron-builder resolves an empty CSC_LINK as a certificate path + # (path.resolve(appDir, '') === appDir), failing with "not a file". + # Export only the variables that carry a value. + - name: Resolve macOS signing credentials + shell: bash + env: + IN_CSC_LINK: ${{ secrets.MAC_CSC_LINK }} + IN_CSC_KEY_PASSWORD: ${{ secrets.MAC_CSC_KEY_PASSWORD }} + IN_APPLE_ID: ${{ secrets.APPLE_ID }} + IN_APPLE_APP_SPECIFIC_PASSWORD: ${{ secrets.APPLE_APP_SPECIFIC_PASSWORD }} + IN_APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + run: | + for name in CSC_LINK CSC_KEY_PASSWORD APPLE_ID APPLE_APP_SPECIFIC_PASSWORD APPLE_TEAM_ID; do + input="IN_${name}" + value="${!input:-}" + if [ -n "$value" ]; then printf '%s<<__EOF__\n%s\n__EOF__\n' "$name" "$value" >> "$GITHUB_ENV"; fi + done + if [ -z "${IN_CSC_LINK:-}" ]; then + echo 'CSC_IDENTITY_AUTO_DISCOVERY=false' >> "$GITHUB_ENV" + echo 'No macOS signing certificate configured; building unsigned.' + fi - name: Package and publish desktop release working-directory: apps/desktop run: pnpm exec electron-builder --mac dmg zip --publish always env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - CSC_LINK: ${{ secrets.MAC_CSC_LINK }} - CSC_KEY_PASSWORD: ${{ secrets.MAC_CSC_KEY_PASSWORD }} - APPLE_ID: ${{ secrets.APPLE_ID }} - APPLE_APP_SPECIFIC_PASSWORD: ${{ secrets.APPLE_APP_SPECIFIC_PASSWORD }} - APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} - name: Upload macOS artifacts for manual runs if: github.event_name == 'workflow_dispatch' @@ -107,15 +124,25 @@ jobs: working-directory: apps/desktop run: node --import tsx scripts/stage-runtime.ts - # Without WIN_CSC_* signing secrets, Windows artifacts are unsigned and - # installers trigger a SmartScreen warning on first run. + # Only non-empty WIN_CSC_* signing secrets are exported. Without them, + # Windows artifacts are unsigned and installers trigger a SmartScreen + # warning on first run. + - name: Resolve Windows signing credentials + shell: bash + env: + IN_WIN_CSC_LINK: ${{ secrets.WIN_CSC_LINK }} + IN_WIN_CSC_KEY_PASSWORD: ${{ secrets.WIN_CSC_KEY_PASSWORD }} + run: | + for name in WIN_CSC_LINK WIN_CSC_KEY_PASSWORD; do + input="IN_${name}" + value="${!input:-}" + if [ -n "$value" ]; then printf '%s<<__EOF__\n%s\n__EOF__\n' "$name" "$value" >> "$GITHUB_ENV"; fi + done - name: Package and publish desktop release working-directory: apps/desktop run: pnpm exec electron-builder --win nsis --x64 --publish always env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - WIN_CSC_LINK: ${{ secrets.WIN_CSC_LINK }} - WIN_CSC_KEY_PASSWORD: ${{ secrets.WIN_CSC_KEY_PASSWORD }} - name: Upload Windows artifacts for manual runs if: github.event_name == 'workflow_dispatch' diff --git a/apps/desktop/scripts/stage-runtime.ts b/apps/desktop/scripts/stage-runtime.ts index fa52c274..b04fe60a 100644 --- a/apps/desktop/scripts/stage-runtime.ts +++ b/apps/desktop/scripts/stage-runtime.ts @@ -5,6 +5,7 @@ import { existsSync } from 'node:fs' import { cp, lstat, mkdir, mkdtemp, readFile, readdir, realpath, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, resolve, sep } from 'node:path' +import { fileURLToPath } from 'node:url' const desktopRoot = resolve(import.meta.dirname, '..') const repositoryRoot = resolve(desktopRoot, '../..') @@ -14,9 +15,42 @@ const entry = join(staging, 'node_modules/@pymodel/pythinker-code/dist/launcher. const frontend = join(staging, 'node_modules/@pymodel/pythinker-code/dist-web/index.html') const workspaceState = join(repositoryRoot, 'node_modules/.pnpm-workspace-state-v1.json') +/** Windows characters that make an argument unsafe to hand to `cmd.exe` unquoted. */ +const WINDOWS_UNSAFE_ARGUMENT = /[\s"&()<>^|]/u + +/** + * Decide how to invoke a package manager on one platform. + * + * Node refuses to spawn a `.cmd` or `.bat` shim without a shell, so Windows + * needs `shell: true`. With a shell, Node does not quote arguments, so any + * argument carrying whitespace or a `cmd.exe` metacharacter is quoted here. + * @param platform - The value of `process.platform`. + * @param command - The package-manager binary name. + * @param args - Arguments in their unquoted form. + * @returns The command, arguments and shell flag to pass to `spawn`. + */ +export function packageManagerInvocation(platform: string, command: string, args: readonly string[]): { + readonly command: string + readonly args: readonly string[] + readonly shell: boolean +} { + if (platform !== 'win32') return { command, args, shell: false } + return { + command, + args: args.map(argument => (WINDOWS_UNSAFE_ARGUMENT.test(argument) ? `"${argument}"` : argument)), + shell: true, + } +} + async function run(command: string, args: readonly string[]): Promise { + const invocation = packageManagerInvocation(process.platform, command, args) await new Promise((accept, reject) => { - const child = spawn(command, args, { cwd: repositoryRoot, env: { ...process.env, CI: 'true' }, stdio: 'inherit' }) + const child = spawn(invocation.command, [...invocation.args], { + cwd: repositoryRoot, + env: { ...process.env, CI: 'true' }, + stdio: 'inherit', + shell: invocation.shell, + }) child.once('error', reject) child.once('exit', (code, signal) => { if (code === 0) accept() @@ -60,7 +94,7 @@ async function materializeLinks(): Promise { async function deploy(target: string): Promise { const savedWorkspaceState = existsSync(workspaceState) ? await readFile(workspaceState) : undefined try { - await run(process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm', [ + await run('pnpm', [ '--config.verify-deps-before-run=false', '--filter', deployPackage, 'deploy', '--legacy', '--prod', '--config.node-linker=hoisted', '--config.auto-install-peers=false', '--config.link-workspace-packages=true', target, ]) @@ -96,4 +130,7 @@ async function main(): Promise { console.log(`desktop runtime staged at ${staging}`) } -await main() +const invokedPath = process.argv[1] +if (invokedPath !== undefined && resolve(invokedPath) === fileURLToPath(import.meta.url)) { + await main() +} diff --git a/apps/desktop/tests/stage-runtime.spec.ts b/apps/desktop/tests/stage-runtime.spec.ts new file mode 100644 index 00000000..747d6483 --- /dev/null +++ b/apps/desktop/tests/stage-runtime.spec.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { packageManagerInvocation } from '../scripts/stage-runtime' + +describe('package manager invocation', () => { + it('leaves non-Windows invocations untouched', () => { + const args = ['--filter', 'x', '/tmp/a b'] + + expect(packageManagerInvocation('darwin', 'pnpm', args)).toEqual({ + command: 'pnpm', + args, + shell: false, + }) + }) + + it('uses a shell on Windows', () => { + expect(packageManagerInvocation('win32', 'pnpm', ['deploy']).shell).toBe(true) + }) + + it('quotes a Windows path containing a space', () => { + expect(packageManagerInvocation('win32', 'pnpm', ['deploy', 'C:\\Users\\John Doe\\tmp']).args) + .toEqual(['deploy', '"C:\\Users\\John Doe\\tmp"']) + }) + + it('leaves a safe Windows argument alone', () => { + expect(packageManagerInvocation('win32', 'pnpm', ['--config.node-linker=hoisted']).args) + .toEqual(['--config.node-linker=hoisted']) + }) + + it('quotes a Windows cmd metacharacter', () => { + expect(packageManagerInvocation('win32', 'pnpm', ['deploy&verify']).args) + .toEqual(['"deploy&verify"']) + }) +}) From 815b8921452a44bd890fc09f6c596410bfde35e8 Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 19:26:32 -0400 Subject: [PATCH 4/6] fix(desktop): give pnpm deploy a workspace-relative staging target pnpm joins its workspace root with the deploy target instead of resolving it, so an absolute path on another volume became D:\repo\C:\Users\... and the Windows job failed with ERR_PNPM_ENOENT. A relative target is correct whether pnpm joins or resolves, and cannot cross a drive letter, so the staging directory moves onto the repository's own volume. --- .changeset/desktop-windows-deploy-target.md | 5 +++++ apps/desktop/scripts/stage-runtime.ts | 25 +++++++++++++++++---- apps/desktop/tests/stage-runtime.spec.ts | 21 ++++++++++++++++- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 .changeset/desktop-windows-deploy-target.md diff --git a/.changeset/desktop-windows-deploy-target.md b/.changeset/desktop-windows-deploy-target.md new file mode 100644 index 00000000..1c0ab894 --- /dev/null +++ b/.changeset/desktop-windows-deploy-target.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-desktop': patch +--- + +Stage the desktop Host closure inside the workspace so pnpm deploy resolves the target on Windows diff --git a/apps/desktop/scripts/stage-runtime.ts b/apps/desktop/scripts/stage-runtime.ts index b04fe60a..0324afee 100644 --- a/apps/desktop/scripts/stage-runtime.ts +++ b/apps/desktop/scripts/stage-runtime.ts @@ -3,8 +3,7 @@ import { spawn } from 'node:child_process' import { existsSync } from 'node:fs' import { cp, lstat, mkdir, mkdtemp, readFile, readdir, realpath, rm, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join, resolve, sep } from 'node:path' +import { join, relative, resolve, sep } from 'node:path' import { fileURLToPath } from 'node:url' const desktopRoot = resolve(import.meta.dirname, '..') @@ -14,6 +13,7 @@ const deployPackage = '@pymodel/pythinker-code' const entry = join(staging, 'node_modules/@pymodel/pythinker-code/dist/launcher.mjs') const frontend = join(staging, 'node_modules/@pymodel/pythinker-code/dist-web/index.html') const workspaceState = join(repositoryRoot, 'node_modules/.pnpm-workspace-state-v1.json') +const stagingParent = join(repositoryRoot, 'node_modules', '.pythinker-desktop-staging') /** Windows characters that make an argument unsafe to hand to `cmd.exe` unquoted. */ const WINDOWS_UNSAFE_ARGUMENT = /[\s"&()<>^|]/u @@ -42,6 +42,21 @@ export function packageManagerInvocation(platform: string, command: string, args } } +/** + * Express a deploy target the way pnpm accepts it. + * + * pnpm joins its workspace root with the deploy target rather than resolving + * it, so an absolute path on another volume produces a concatenated, + * non-existent directory such as `D:\repo\C:\Users\…`. A workspace-relative + * target is correct whether pnpm joins or resolves. + * @param workspaceRoot - The pnpm workspace root, and the child process's cwd. + * @param target - The absolute staging directory. + * @returns The target expressed relative to the workspace root. + */ +export function deployTargetArgument(workspaceRoot: string, target: string): string { + return relative(workspaceRoot, target) +} + async function run(command: string, args: readonly string[]): Promise { const invocation = packageManagerInvocation(process.platform, command, args) await new Promise((accept, reject) => { @@ -96,7 +111,8 @@ async function deploy(target: string): Promise { try { await run('pnpm', [ '--config.verify-deps-before-run=false', '--filter', deployPackage, 'deploy', '--legacy', '--prod', - '--config.node-linker=hoisted', '--config.auto-install-peers=false', '--config.link-workspace-packages=true', target, + '--config.node-linker=hoisted', '--config.auto-install-peers=false', '--config.link-workspace-packages=true', + deployTargetArgument(repositoryRoot, target), ]) } finally { if (savedWorkspaceState === undefined) await rm(workspaceState, { force: true }) @@ -105,7 +121,8 @@ async function deploy(target: string): Promise { } async function main(): Promise { - const deployed = await mkdtemp(join(tmpdir(), 'pythinker-desktop-runtime-')) + await mkdir(stagingParent, { recursive: true }) + const deployed = await mkdtemp(join(stagingParent, 'runtime-')) try { await deploy(deployed) await rm(join(staging, 'node_modules'), { recursive: true, force: true }) diff --git a/apps/desktop/tests/stage-runtime.spec.ts b/apps/desktop/tests/stage-runtime.spec.ts index 747d6483..49d14651 100644 --- a/apps/desktop/tests/stage-runtime.spec.ts +++ b/apps/desktop/tests/stage-runtime.spec.ts @@ -1,5 +1,6 @@ +import { isAbsolute, join } from 'node:path' import { describe, expect, it } from 'vitest' -import { packageManagerInvocation } from '../scripts/stage-runtime' +import { deployTargetArgument, packageManagerInvocation } from '../scripts/stage-runtime' describe('package manager invocation', () => { it('leaves non-Windows invocations untouched', () => { @@ -31,3 +32,21 @@ describe('package manager invocation', () => { .toEqual(['"deploy&verify"']) }) }) + +describe('deploy target argument', () => { + it('is relative to the workspace root', () => { + expect(deployTargetArgument('/repo', '/repo/node_modules/.pythinker-desktop-staging/runtime-abc123')) + .toBe(join('node_modules', '.pythinker-desktop-staging', 'runtime-abc123')) + }) + + it('is never absolute', () => { + expect(isAbsolute(deployTargetArgument('/repo', '/repo/node_modules/.pythinker-desktop-staging/runtime-abc123'))) + .toBe(false) + }) + + it('never returns the target unchanged', () => { + const target = '/repo/node_modules/.pythinker-desktop-staging/runtime-abc123' + + expect(deployTargetArgument('/repo', target)).not.toBe(target) + }) +}) From 7321b89c8be5ef2bf17fb9624a34b0ad830bf16f Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 21:36:47 -0400 Subject: [PATCH 5/6] ci: gate the desktop suite and typecheck on pull requests apps/desktop shipped 65 vitest cases and a typecheck script that no required check ran: the root vitest projects list stopped at apps/pythinker-code, and the typecheck job looped packages/* plus apps/pythinker-code only. A desktop regression merged green. Register the project and add a per-package typecheck step rather than extending the tsgo loop, which would cover the source tsconfig and silently skip tests/tsconfig.json. --- .github/workflows/ci.yml | 2 ++ package.json | 2 +- vitest.config.ts | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a06d598..11c02f39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,3 +88,5 @@ jobs: run: pnpm --filter @pymodel/dashboard-server run typecheck - name: Typecheck dashboard-web run: pnpm --filter @pymodel/dashboard-web run typecheck + - name: Typecheck desktop + run: pnpm --filter @pymodel/pythinker-desktop run typecheck diff --git a/package.json b/package.json index 268323d8..403fdbcf 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "build:plugin-marketplace": "pnpm -C apps/pythinker-code run build:plugin-marketplace", "dashboard": "pnpm -C apps/dashboard run dev", "dev:docs": "pnpm -C docs install --ignore-workspace && pnpm -C docs run dev", - "typecheck": "pnpm run build:packages && pnpm -r --filter './packages/*' run typecheck && pnpm --filter @pymodel/pythinker-code run typecheck && pnpm --filter @pymodel/pythinker-web run typecheck && pnpm --filter @pymodel/dashboard-server run typecheck && pnpm --filter @pymodel/dashboard-web run typecheck", + "typecheck": "pnpm run build:packages && pnpm -r --filter './packages/*' run typecheck && pnpm --filter @pymodel/pythinker-code run typecheck && pnpm --filter @pymodel/pythinker-web run typecheck && pnpm --filter @pymodel/dashboard-server run typecheck && pnpm --filter @pymodel/dashboard-web run typecheck && pnpm --filter @pymodel/pythinker-desktop run typecheck", "lint": "oxlint --type-aware", "lint:fix": "pnpm run lint --fix", "lint:pkg": "pnpm --filter @pymodel/pythinker-code exec publint && npm_config_cache=${TMPDIR:-/tmp}/pythinker-code-npm-cache pnpm --filter @pymodel/pythinker-code exec attw --pack . --profile node16", diff --git a/vitest.config.ts b/vitest.config.ts index 55584ee2..d83c11c0 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,7 +2,7 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { - projects: ['packages/*', 'apps/pythinker-code'], + projects: ['packages/*', 'apps/pythinker-code', 'apps/desktop'], coverage: { provider: 'v8', include: ['packages/*/src/**/*.ts', 'apps/*/src/**/*.ts'], From d94ba4b68bf7777ead30f87385bb6d123c20d68a Mon Sep 17 00:00:00 2001 From: elkaix Date: Sat, 15 Aug 2026 21:56:29 -0400 Subject: [PATCH 6/6] fix(desktop): bound the Windows tree kill and correct the installer docs spawnSync blocks the Electron main loop, so a stalled taskkill could freeze shutdown indefinitely. Cap it and let the existing fallback degrade to a single-process kill. The README claimed a per-user installer with no elevation required. With oneClick and perMachine both false, electron-builder shows an install-mode page, so per-user is the default rather than the contract. Describe the real behavior and pin it with a test. --- .changeset/desktop-taskkill-timeout.md | 5 +++ apps/desktop/README.md | 2 +- apps/desktop/src/host-supervisor.ts | 8 +++- apps/desktop/tests/host-supervisor.spec.ts | 37 ++++++++++++++++++- apps/desktop/tests/packaging-config.spec.ts | 6 +++ .../tests/verify-win-installer.spec.ts | 10 ++--- 6 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 .changeset/desktop-taskkill-timeout.md diff --git a/.changeset/desktop-taskkill-timeout.md b/.changeset/desktop-taskkill-timeout.md new file mode 100644 index 00000000..c7d7cc35 --- /dev/null +++ b/.changeset/desktop-taskkill-timeout.md @@ -0,0 +1,5 @@ +--- +'@pymodel/pythinker-desktop': patch +--- + +Bound the Windows process-tree kill so a stalled taskkill cannot freeze desktop shutdown diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 1152b1b6..3e4f20cc 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -66,7 +66,7 @@ rmdir "$MOUNT_POINT" ### Windows -Run `pnpm run dist:win` on a native Windows x64 host; cross-building from macOS is not possible because the staged Host closure contains platform-gated native packages. The output is `dist/Pythinker--x64-Setup.exe`, a per-user NSIS installer with no elevation required and a selectable installation directory. Artifacts are unsigned unless `WIN_CSC_LINK` and `WIN_CSC_KEY_PASSWORD` are set. +Run `pnpm run dist:win` on a native Windows x64 host; cross-building from macOS is not possible because the staged Host closure contains platform-gated native packages. The output is `dist/Pythinker--x64-Setup.exe`, an assisted NSIS installer that defaults to a per-user install, offers a per-machine option that requires elevation, and lets you select the installation directory. Artifacts are unsigned unless `WIN_CSC_LINK` and `WIN_CSC_KEY_PASSWORD` are set. ## Known limitations diff --git a/apps/desktop/src/host-supervisor.ts b/apps/desktop/src/host-supervisor.ts index 2b759561..89f525c8 100644 --- a/apps/desktop/src/host-supervisor.ts +++ b/apps/desktop/src/host-supervisor.ts @@ -6,6 +6,7 @@ import type { Readable } from 'node:stream' const READINESS_PREFIX = 'Pythinker server: ' const DEFAULT_READINESS_TIMEOUT_MS = 90_000 const DEFAULT_SHUTDOWN_TIMEOUT_MS = 5_000 +const TASKKILL_TIMEOUT_MS = 5_000 const MAX_STARTUP_OUTPUT_CHARS = 32_768 /** Incremental parser for the Web Host's canonical readiness line. */ @@ -303,6 +304,8 @@ export function spawnPythinkerServer(options: SpawnPythinkerServerOptions): Host * Windows has no signal delivery: `child.kill` calls TerminateProcess on one * PID, so the Host's own children (node-pty shells, subagent hosts) survive and * keep holding the loopback port. `taskkill /T` walks the tree instead. + * Because this call is synchronous, keep it bounded so a stalled taskkill falls + * back to a single-process kill instead of blocking the Electron main loop indefinitely. * * ponytail: /F makes every Windows stop a forced stop — Node cannot deliver a * graceful SIGTERM to a Windows child at all. Add a stdin or IPC shutdown @@ -313,7 +316,10 @@ function killProcessTree(child: ChildProcessByStdio, s child.kill(signal) return } - const result = spawnSync('taskkill', ['/pid', String(child.pid), '/T', '/F'], { windowsHide: true }) + const result = spawnSync('taskkill', ['/pid', String(child.pid), '/T', '/F'], { + windowsHide: true, + timeout: TASKKILL_TIMEOUT_MS, + }) if (result.error !== undefined || result.status !== 0) child.kill(signal) } diff --git a/apps/desktop/tests/host-supervisor.spec.ts b/apps/desktop/tests/host-supervisor.spec.ts index f862fdc4..c347f3dd 100644 --- a/apps/desktop/tests/host-supervisor.spec.ts +++ b/apps/desktop/tests/host-supervisor.spec.ts @@ -331,7 +331,7 @@ describe('desktop Host process', () => { stdout: Buffer.alloc(0), stderr: Buffer.alloc(0), status: 0, - signal: null, + signal: 'SIGTERM', }) vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') @@ -347,11 +347,44 @@ describe('desktop Host process', () => { expect(spawnSync).toHaveBeenCalledWith( 'taskkill', ['/pid', '4242', '/T', '/F'], - { windowsHide: true }, + { windowsHide: true, timeout: 5_000 }, ) expect(child.kill).not.toHaveBeenCalled() }) + it('falls back to killing the Windows Host when taskkill times out', async () => { + const child = { + pid: 4242, + stdout: { on: vi.fn(), off: vi.fn() }, + stderr: { on: vi.fn(), off: vi.fn() }, + on: vi.fn(), + off: vi.fn(), + kill: vi.fn(), + } + vi.mocked(spawn).mockReturnValue(child as never) + vi.mocked(spawnSync).mockReturnValue({ + pid: 4242, + output: [], + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + status: null, + signal: null, + error: Object.assign(new Error('spawnSync taskkill ETIMEDOUT'), { code: 'ETIMEDOUT' }), + }) + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + + const { spawnPythinkerServer } = await import('../src/host-supervisor') + const host = spawnPythinkerServer({ + nodeExecutable: 'node', + cliEntry: '/tmp/launcher.mjs', + cwd: '/tmp', + env: {}, + }) + host.kill('SIGTERM') + + expect(child.kill).toHaveBeenCalledWith('SIGTERM') + }) + it('uses child.kill unchanged on non-Windows', async () => { const child = { pid: 4242, diff --git a/apps/desktop/tests/packaging-config.spec.ts b/apps/desktop/tests/packaging-config.spec.ts index ca381047..6c87e46c 100644 --- a/apps/desktop/tests/packaging-config.spec.ts +++ b/apps/desktop/tests/packaging-config.spec.ts @@ -112,6 +112,12 @@ describe('desktop packaging configuration', () => { expect(desktopPackage.scripts['dist:win']).toBe('node --import tsx scripts/release-win.ts') }) + it('offers an assisted installer that defaults to a per-user install', () => { + expect(desktopPackage.build.nsis.oneClick).toBe(false) + expect(desktopPackage.build.nsis.perMachine).toBe(false) + expect(desktopPackage.build.nsis.allowElevation).toBe(true) + }) + it('exposes desktop commands at the repository root', () => { expect(rootPackage.scripts['dev:desktop']).toBe('pnpm -C apps/desktop run dev') expect(rootPackage.scripts['package:desktop']).toBe('pnpm -C apps/desktop run package') diff --git a/apps/desktop/tests/verify-win-installer.spec.ts b/apps/desktop/tests/verify-win-installer.spec.ts index a8455831..37e5beef 100644 --- a/apps/desktop/tests/verify-win-installer.spec.ts +++ b/apps/desktop/tests/verify-win-installer.spec.ts @@ -37,21 +37,21 @@ describe('Windows installer verification', () => { it('accepts the installer and unpacked shell when both are PE binaries', async () => { await withFixture(async (root) => { await createFixture(root, portableExecutable()) - expect(() => verifyWindowsInstaller(root)).not.toThrow() + expect(() => { verifyWindowsInstaller(root) }).not.toThrow() }) }) it('rejects a missing installer', async () => { await withFixture(async (root) => { await createFixture(root) - expect(() => verifyWindowsInstaller(root)).toThrow('ENOENT') + expect(() => { verifyWindowsInstaller(root) }).toThrow('ENOENT') }) }) it('rejects an artifact without a DOS header', async () => { await withFixture(async (root) => { await createFixture(root, portableExecutable('ZZ')) - expect(() => verifyWindowsInstaller(root)).toThrow(/no DOS header/) + expect(() => { verifyWindowsInstaller(root) }).toThrow(/no DOS header/u) }) }) @@ -61,14 +61,14 @@ describe('Windows installer verification', () => { truncated.write('MZ', 0, 2, 'latin1') truncated.writeUInt32LE(0x80, 0x3c) await createFixture(root, truncated) - expect(() => verifyWindowsInstaller(root)).toThrow(/out-of-range PE offset/) + expect(() => { verifyWindowsInstaller(root) }).toThrow(/out-of-range PE offset/u) }) }) it('rejects an artifact without a PE signature', async () => { await withFixture(async (root) => { await createFixture(root, portableExecutable('MZ', 'NE\0\0')) - expect(() => verifyWindowsInstaller(root)).toThrow(/no PE signature/) + expect(() => { verifyWindowsInstaller(root) }).toThrow(/no PE signature/u) }) }) })