From 7b1e07d6f80e4b3ef5df4314eeda3dac90f9d108 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:25:07 +0700 Subject: [PATCH] feat: add windows desktop installer build, docs, and win32 fixes (#1897) * feat: add Windows desktop installer build, docs, and win32 fixes * fix: failing tests * refactor: update the code based on feedback --- .github/workflows/desktop-windows.yml | 91 +++++++++++++++++++ electron-builder.config.mjs | 4 + electron/main.mjs | 15 ++- scripts/dev-process-utils.mjs | 28 +++++- scripts/download-uv.mjs | 22 ++--- .../mock-llm-conversation.spec.ts | 24 ++++- tests/e2e/mock-llm/utils/mock-llm-helpers.ts | 8 ++ 7 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/desktop-windows.yml diff --git a/.github/workflows/desktop-windows.yml b/.github/workflows/desktop-windows.yml new file mode 100644 index 0000000000..1d3f874b0f --- /dev/null +++ b/.github/workflows/desktop-windows.yml @@ -0,0 +1,91 @@ +name: Desktop (Windows) + +# Builds the Agent Canvas Windows NSIS installer (npm run build:desktop on a +# Windows runner). +# pull_request: paths-filtered smoke build — the uploaded artifact is +# what a tester downloads to verify a PR on Windows. +# release published: rebuilds from the release tag and attaches the .exe to +# the GitHub Release created by release-please. +# workflow_dispatch: manual escape hatch for any ref. +# The installer is not code-signed (no signing certs exist for any platform); +# Windows SmartScreen therefore warns on first run — dismiss it via +# "More info" → "Run anyway". +on: + workflow_dispatch: + pull_request: + paths: + - .github/workflows/desktop-windows.yml + - electron/** + - electron-builder.config.mjs + - scripts/download-uv.mjs + - scripts/download-node.mjs + release: + types: [published] + +concurrency: + group: desktop-windows-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +# `gh release upload` needs contents: write on release events. Fork PR runs +# are downgraded to a read-only token by GitHub automatically. +permissions: + contents: write + +jobs: + build-installer: + name: Build Windows installer + runs-on: windows-latest + timeout-minutes: 30 + + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Set up Node.js with npm cache + uses: actions/setup-node@v6 + with: + node-version: '24' + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Build Windows installer + env: + # Production analytics key only for release builds (same split as + # docker.yml); PR/manual runs get the staging key. Both are public + # client-side keys stored as repo vars — empty on fork PRs, which + # simply disables analytics in the built app. + VITE_POSTHOG_API_KEY: ${{ github.event_name == 'release' && vars.POSTHOG_PROD_KEY || vars.POSTHOG_STAGING_KEY }} + # Authenticates download-uv.mjs's GitHub API version lookup so it + # doesn't hit the unauthenticated per-IP rate limit on shared runners. + GITHUB_TOKEN: ${{ github.token }} + run: npm run build:desktop + + - name: Verify installer output + shell: bash + run: | + ls -la dist-electron + exe_count=$(find dist-electron -maxdepth 1 -name '*.exe' | wc -l) + if [ "$exe_count" -ne 1 ]; then + echo "::error::Expected exactly one NSIS installer in dist-electron/, found $exe_count" + exit 1 + fi + + - name: Upload installer artifact + uses: actions/upload-artifact@v7 + with: + name: agent-canvas-windows-installer + path: dist-electron/*.exe + if-no-files-found: error + # The installer is large (~200 MB); keep PR artifacts long enough + # for manual QA without hoarding storage. + retention-days: 14 + + - name: Attach installer to GitHub release + if: github.event_name == 'release' + shell: bash + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ github.event.release.tag_name }} + run: gh release upload "$RELEASE_TAG" dist-electron/*.exe --clobber diff --git a/electron-builder.config.mjs b/electron-builder.config.mjs index eb9c11a0e9..4b84f79e4e 100644 --- a/electron-builder.config.mjs +++ b/electron-builder.config.mjs @@ -334,6 +334,10 @@ const config = { allowToChangeInstallationDirectory: true, createDesktopShortcut: true, createStartMenuShortcut: true, + // The default artifact name is "Agent Canvas Setup .exe"; + // GitHub release assets mangle spaces, so ship a space-free name. + // ${version}/${ext} are electron-builder macros, not JS interpolation. + artifactName: "Agent-Canvas-Setup-${version}.${ext}", }, // ── Linux ────────────────────────────────────────────────────────────────── diff --git a/electron/main.mjs b/electron/main.mjs index 1ad46237cb..3f2e5eb6f4 100644 --- a/electron/main.mjs +++ b/electron/main.mjs @@ -693,6 +693,12 @@ app.whenReady().then(async () => { // → before-quit fires (first time) → we preventDefault + send SIGTERM // → SIGTERM handler kills all children, calls process.exit(0) // → before-quit fires again (cleanupStarted=true) → we return, Electron exits +// +// Windows has no real POSIX signals: process.kill(pid, "SIGTERM") would +// terminate this process WITHOUT running the "SIGTERM" listener, skipping +// cleanup and orphaning the children on ports 8000/18000/18001 (the next +// launch then fails at startup). process.emit("SIGTERM") runs the same +// registered handler in-process instead. let cleanupStarted = false; @@ -703,7 +709,14 @@ app.on("before-quit", (event) => { event.preventDefault(); console.log("[desktop] Stopping backend services…"); - process.kill(process.pid, "SIGTERM"); + if (process.platform === "win32") { + // Run the cleanup handler in-process (see header note). emit() returns + // false when no listener is registered — the stack never started, so + // there is nothing to clean up and we can exit immediately. + if (!process.emit("SIGTERM")) app.exit(0); + } else { + process.kill(process.pid, "SIGTERM"); + } // Safety net: if the SIGTERM handler doesn't finish within 6 s, force-quit. const t = setTimeout(() => { diff --git a/scripts/dev-process-utils.mjs b/scripts/dev-process-utils.mjs index ec9eeabbe4..78dac1df6f 100644 --- a/scripts/dev-process-utils.mjs +++ b/scripts/dev-process-utils.mjs @@ -83,7 +83,9 @@ export function signalProcessTree(proc, signal) { } try { - if (process.platform === "win32" || !proc.pid) { + if (process.platform === "win32" && proc.pid) { + killWindowsProcessTree(proc, signal); + } else if (!proc.pid) { proc.kill(signal); } else { process.kill(-proc.pid, signal); @@ -97,6 +99,30 @@ export function signalProcessTree(proc, signal) { } } +/** + * Windows has no POSIX process groups: ChildProcess#kill reaches only the + * direct child (e.g. the uvx wrapper), leaving grandchildren — the actual + * python agent-server holding its port — running. `taskkill /t` walks the + * child tree instead. Windows also has no graceful tree signal (taskkill + * without /f posts WM_CLOSE, which console processes ignore), so SIGTERM and + * SIGKILL both map to the same forceful /f kill; callers' delayed SIGKILL + * pass skips already-exited trees via isProcessRunning, so the repeat is a + * no-op. A non-zero taskkill exit just means the tree already exited — only + * a failure to spawn taskkill itself falls back to the direct kill. + */ +function killWindowsProcessTree(proc, signal) { + const result = spawnSync( + "taskkill", + ["/pid", String(proc.pid), "/t", "/f"], + // windowsHide avoids a console window flash when invoked from the + // packaged (GUI) Electron process. + { stdio: "ignore", windowsHide: true }, + ); + if (result.error) { + proc.kill(signal); + } +} + export function createShutdownHookRegistry(onError) { const hooks = new Set(); diff --git a/scripts/download-uv.mjs b/scripts/download-uv.mjs index 48b4e114df..c8fda754fc 100644 --- a/scripts/download-uv.mjs +++ b/scripts/download-uv.mjs @@ -138,19 +138,15 @@ function extract(archivePath, targetDir, ext) { // Both tar.gz and zip are handled by the system 'tar' command: // macOS/Linux: GNU/BSD tar natively supports .tar.gz // Windows 10+: built-in bsdtar supports both .tar.gz and .zip - // --strip-components=1 removes the top-level archive directory so - // uv/uvx end up directly in targetDir. - execFileSync( - "tar", - [ - "-xf", - archivePath, - "-C", - targetDir, - "--strip-components=1", - ], - { stdio: "inherit" } - ); + // The tar.gz archives wrap uv/uvx in a top-level `uv-/` directory, + // which --strip-components=1 removes. uv's Windows .zip is flat (uv.exe / + // uvx.exe at the archive root) — stripping there would skip every entry + // and extract nothing. + const args = ["-xf", archivePath, "-C", targetDir]; + if (ext === "tar.gz") { + args.push("--strip-components=1"); + } + execFileSync("tar", args, { stdio: "inherit" }); } // ── Main ────────────────────────────────────────────────────────────────────── diff --git a/tests/e2e/mock-llm/conversations/mock-llm-conversation.spec.ts b/tests/e2e/mock-llm/conversations/mock-llm-conversation.spec.ts index 78e21b60b5..3b9f0e2290 100644 --- a/tests/e2e/mock-llm/conversations/mock-llm-conversation.spec.ts +++ b/tests/e2e/mock-llm/conversations/mock-llm-conversation.spec.ts @@ -189,7 +189,11 @@ test.describe("mock-LLM agent-server conversation", () => { // Verify the "Active" badge appears on our profile. // Poll with reload instead of a fixed timeout — the mutation may take - // more than 1s to persist on a loaded CI runner. + // more than 1s to persist on a loaded CI runner. The poll also + // RE-ATTEMPTS the activation when the badge is still missing: the + // "Set as active" PATCH above can be lost when it races the + // useEnsureActiveProfile reconciliation write, and a read-only poll + // would then time out with no profile active at all. await expect .poll( async () => { @@ -201,14 +205,28 @@ test.describe("mock-LLM agent-server conversation", () => { const row = rows.nth(i); const text = await row.textContent(); if (text?.includes(PROFILE_NAME)) { - return (await row.getByTestId("profile-active-badge").count()) > 0; + if ( + (await row.getByTestId("profile-active-badge").count()) > 0 + ) { + return true; + } + // Badge absent — re-attempt activation before the next poll. + await row.getByTestId("profile-menu-trigger").click(); + await waitForTestId(page, "profile-actions-menu"); + const retrySetActive = page.getByTestId("profile-set-active"); + if (await retrySetActive.isEnabled()) { + await retrySetActive.click(); + } else { + await page.keyboard.press("Escape"); + } + return false; } } return false; }, { message: `Profile "${PROFILE_NAME}" should have an "Active" badge`, - timeout: 15_000, + timeout: 25_000, intervals: [1_000, 2_000, 3_000], }, ) diff --git a/tests/e2e/mock-llm/utils/mock-llm-helpers.ts b/tests/e2e/mock-llm/utils/mock-llm-helpers.ts index 792c3af445..d51cad6534 100644 --- a/tests/e2e/mock-llm/utils/mock-llm-helpers.ts +++ b/tests/e2e/mock-llm/utils/mock-llm-helpers.ts @@ -887,6 +887,14 @@ export async function setChatInput( text: string, testId = "chat-input", ) { + // page.evaluate has no auto-waiting: right after a goto with + // `domcontentloaded`, the React app can take longer than + // dismissAnalyticsModal's give-up window to paint the composer on a + // loaded CI runner, and the querySelector below would throw. Wait for + // the input the way a locator action would before setting text. + await page + .getByTestId(testId) + .waitFor({ state: "visible", timeout: 30_000 }); await page.evaluate( ({ tid, inputText }) => { const el = document.querySelector(`[data-testid="${tid}"]`);