feat(release): verify Windows automatic updates end to end - #3240
Conversation
The updater feed was hardcoded to the production GitHub repository, so no harness could drive a real electron-updater download/install cycle on a packaged Windows build -- the Windows auto-update path shipped without end-to-end evidence. Add MAKA_UPDATE_TEST_FEED, resolved by an exported pure function that accepts exactly http://127.0.0.1:<port>[/path] and maps it to a generic provider. Anything else set -- a remote host, localhost, another loopback alias, HTTPS, userinfo, a query string, or a malformed URL -- throws at construction: a mistyped override must never silently fall back to the production feed, because a test run quietly installing a real release is exactly the failure this shape exists to prevent. This is not an update-hijack vector: setting an environment variable on the app's process already requires code execution as the same user, and the per-user NSIS install model means that user can rewrite the installation directory directly -- the override grants no capability across any privilege boundary. With the variable unset the feed configuration is byte-identical to production, and the override changes the feed URL and nothing else: every other updater setting and the whole status machine behave identically under it. Update signature verification (once a certificate exists) applies to overridden feeds exactly as to the GitHub feed. Unit tests pin the accepted shape (two positive forms, eleven rejected confusion shapes), the generic-provider mapping, the unchanged production feed without the variable, and the fail-closed constructor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
verify-windows-autoupdate installs the candidate build, points its packaged updater at a loopback feed serving the version-bumped installer, and asserts the complete in-app path stage by stage: renderer-driven check, downloaded status with the exact version pair, feed request log (latest.yml and installer actually fetched, zero unexpected paths), installUpdate handoff, old-process exit, NSIS upgrade, automatic relaunch whose on-disk image carries the new ProductVersion, a full packaged smoke of the upgraded install that also asserts app.getVersion() via the renderer bridge, and a clean silent uninstall. Every stage has its own deadline and named failure; cleanup mirrors the installer-lifecycle verifier (kill stragglers, best-effort uninstall, AggregateError cause chaining). The relaunched instance is observed and then stopped promptly: NSIS isForceRunAfter restarts without our CDP/user-data arguments, so its existence and image version are the assertions, and the inherited environment still points at the isolated home. verify-packaged-app generalizes its CDP internals into exported evaluateInRenderer/findRendererTarget/reserveTcpPort/stopChild with the smoke's assertions unchanged. Both Windows release workflows build the bumped installer and run the verification before uploads, with timeouts raised to absorb the ~8-10 minute addition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
The support baseline now states what the new gate proves -- a running installed candidate discovers, downloads, and installs a newer build through its packaged electron-updater against a loopback feed, relaunches as the new version, and passes the full packaged smoke, with feed requests, updater states, and the final version asserted individually -- and keeps the unproven remainder explicit: update signature verification awaits an Authenticode certificate (the production GitHub feed configuration is pinned by unit tests and exercised on real releases), business-data migration, and mid-install rollback stay uncovered. EN and zh sections updated together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummaryThis PR adds end-to-end verification for Windows automatic updates on the unsigned x64 preview channel. It verifies discovery, download, installation, NSIS upgrade, relaunch, version validation, packaged smoke testing, and uninstall. The PR extends the existing update service and packaged-app verification path. It does not replace the production update path. The loopback-only The solution is the smallest coherent path shown by the diff. Shared verifier exports avoid duplicate process and CDP logic. The patch-version-bumped installer is required to test the upgrade flow. No deletion or simplification is apparent without weakening coverage. Complexity delta
The added complexity is necessary to verify the previously untested upgrade path. Total maintenance complexity increases, but the increase has direct verification value. No code or tests can be deleted or simplified based on the available diff without weakening update-flow coverage or regression protection. Validation and risksReported validation includes clean typechecking and update-service tests. The full local Windows E2E run was unavailable because Visual Studio Build Tools were missing. Windows CI is the authoritative verification path. The final status of required checks is unverified from the available evidence. The verifier validates candidate and upgraded artifacts, exact raw feed paths, query support, differential-download probing, installer transfer responses, update state, installer handoff, automatic relaunch, final version, packaged smoke checks, silent uninstall, and failure-safe cleanup. Remaining coverage gaps include Authenticode signature verification, persisted-data migration, and rollback behavior. Review-relevant risks
The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe PR adds validated loopback feeds, Windows installer packaging, end-to-end autoupdate verification, CI release gates, and updated Windows support documentation. ChangesWindows automatic update verification
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR adds an end-to-end Windows update verification gate, but the verifier may accept non-canonical artifact paths and therefore fail to enforce its exact feed-path contract; this is a bounded merge-readiness risk that should receive explicit owner follow-up. Sequence Diagram(s)sequenceDiagram
participant CI
participant Packager
participant LoopbackFeed
participant CandidateApp
participant NSISInstaller
participant UpgradedApp
CI->>Packager: build version-bumped installer
Packager->>CI: return installer and update metadata
CI->>LoopbackFeed: serve update artifacts
CI->>CandidateApp: install and launch candidate
CandidateApp->>LoopbackFeed: fetch latest.yml and installer
CandidateApp->>NSISInstaller: hand off update
NSISInstaller->>UpgradedApp: install and relaunch
CI->>UpgradedApp: run packaged smoke checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoVerify Windows automatic updates end to end
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/verify-windows-autoupdate.mjs (1)
383-418: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated launch-and-wait-for-renderer block.
Lines 383-418 repeat lines 204-253 almost exactly: the same four
mkdircalls onisolatedUserEnv, the same three spawn flags, and the same poll onRENDERER_STATE_EXPRESSIONuntilisPackagedRendererUsable. The two copies differ only instdioand the stderr capture.Extract one local helper in this file that spawns the executable with an isolated home and returns the CDP target after the renderer becomes usable. Both call sites then pass only what differs. The helper stays local because the stderr-capture variant is specific to this verifier; promoting it to
scripts/verify-packaged-app.mjswould widen that module's public surface for one consumer.Disposition: optional. The duplication is correct today, and the risk is that a future change to the launch shape is applied to only one copy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e6b083c3-60f6-4683-82b7-41c1757dea62
📒 Files selected for processing (10)
.github/workflows/release-desktop.yml.github/workflows/release-windows-check.ymlapps/desktop/src/main/__tests__/app-update-service.test.tsapps/desktop/src/main/app-update-service.tsapps/desktop/src/main/runtime-host-boot.tsdocs/windows-support.mdpackage.jsonscripts/package-windows-autoupdate-next.mjsscripts/verify-packaged-app.mjsscripts/verify-windows-autoupdate.mjs
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The overall E2E shape is good: the production feed remains unchanged without the test override, the override is loopback-only and fail-closed, fake next-version artifacts stay outside release upload globs, metadata hashes are verified, and install/home/process cleanup is isolated.\n\nThe current head is not runnable as a release gate because of the deterministic old-blockmap failure below. Existing Qodo threads also correctly cover three additional gaps, which I am not duplicating inline: basename-only path validation, expectedVersion accidentally disabling current sandbox/disclaimer checks, and incomplete workflow path filters for the production feed wiring. Those must be resolved before this can serve as release evidence.\n\nCurrent CI has the Windows package failure, a cancelled Windows CLI validation, and one check still running.\n\nReviewed with Codex using three independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the exact head, live Windows request trace, updater fallback behavior, workflow permissions/artifacts, package-verifier semantics, existing threads, and live CI.\n\n
中文
\n\n整体 E2E 结构不错：没有 test override 时 production feed 不变；override 仅允许 loopback 且 fail closed；假的 next-version artifact 不会进入 release upload glob；metadata hash 会校验；install/home/process cleanup 也相互隔离。\n\n但当前 head 不能作为 release gate 运行，因为下面的旧 blockmap 失败是确定性的。已有 Qodo 线程还正确指出三个缺口，我不重复发布行内问题：只按 basename 校验路径；expectedVersion 意外关闭当前 sandbox/disclaimer 检查；以及 production feed wiring 的 workflow path filter 不完整。这些都需修复后才能作为 release evidence。\n\n当前 CI 有 Windows package failure、一项 Windows CLI validation cancelled，另有一项仍在运行。\n\n本次由 Codex 配合三个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我核验了精确 head、实时 Windows request trace、updater fallback、workflow permissions/artifacts、package verifier 语义、已有线程和实时 CI。\n\n
The first CI run proved the whole in-app path (latest.yml fetch, new blockmap, full installer download, downloaded state) and failed only on the harness's own too-strict feed assertion: electron-updater probes the *previous* version's blockmap to attempt a differential download, and the 404 for that legitimate probe was counted as an unexpected request. Serve the candidate's own blockmap exactly as a GitHub release hosts the previous version's assets, so the differential path is real; when the file is absent next to the candidate installer, the mapped-but- absent 404 no longer counts as unexpected and the updater's documented fallback to the full download proceeds. Truly unknown paths still fail the run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify-windows-autoupdate.mjs (1)
64-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject non-canonical feed paths.
basename(pathName)accepts arbitrary paths that end in an allowed filename. For example,GET /unexpected/latest.ymlreturnslatest.ymland passes the laterserved('latest.yml')assertion.Require the exact expected path, such as
pathName === \/${name}``, before serving the mapped file. This preserves the stated invariant that unknown paths return 404.Disposition: fix-now.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2efaeaf9-3774-4d2e-910e-0213220328f4
📒 Files selected for processing (1)
scripts/verify-windows-autoupdate.mjs
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The old-installer blockmap behavior is now modeled correctly, so the earlier deterministic 404 failure is resolved. The loopback feed, next-version artifact, hash checks, isolated install/home, and process scoping remain good foundations.
I’m keeping COMMENT because the existing release-evidence gaps still apply on this head:
- exact feed request shape is still reduced to
basename, so nested/unexpected paths can be served and accepted; - passing
expectedVersionfor the freshly built update still disables sandbox/disclaimer resource checks; - the Windows PR path filter still misses the production feed wiring and release workflow;
- the docs still overstate the updater states actually observed.
Those issues are already covered by existing review threads, so I have not duplicated them inline. I added one new P2 inline for repeatedly buffering the full installer when the differential downloader makes range requests. The current Windows release check is also still running.
The simplest final state is one exact pathname allowlist shared by serving and assertions, version checking separated from explicit resource requirements, and range-aware streaming (or one bounded immutable file cache) for the large installer.
AI-assisted review disclosure: two Codex reviewers independently re-reviewed exact head df30ccb, the 34-line delta, current Windows CI, release artifacts, and existing feedback; no external model was used.
中文说明
旧 installer blockmap 的 404 已正确处理。当前仍需解决已有线程中的几个 release evidence 缺口：feed 仍按 basename 而非精确 pathname 校验；新构建版本因为传入 expectedVersion 而关闭 sandbox/disclaimer 检查；Windows path filter 仍漏掉生产 wiring/release workflow；文档仍夸大实际断言的 updater state。另新增一个 P2 行内评论：range 请求会反复把完整 installer 读入内存。最简终态是共享精确路径 allowlist、把版本校验与资源豁免拆开，并对大 installer 做 range-aware streaming 或有界缓存。
Four review findings, each accepted: - Exact-path feed matching (Qodo): the allowlist compared basenames, so a nested /x/latest.yml would have been served as expected. The server and the evidence assertions now match full root-level request paths; anything else counts as unexpected and fails the run. - In-memory feed bodies (CodeRabbit): ranged installer requests re-read a multi-hundred-megabyte file per request, risking the 180s download deadline on slow runners. Bodies are read once at startup. - Full verification of the upgraded install (Qodo): expectedVersion historically marks a released baseline and silently exempted the sandbox and disclaimer checks. Those exemptions are now explicit options defaulting to the old behavior, and the auto-update gate passes requireWindowsSandbox/requireDisclaimer: true -- the upgraded install is a current build and gets the full packaged verification. - Boot wiring in the Windows gate paths (Qodo): a change limited to the runtime-host-boot.ts propagation of MAKA_UPDATE_TEST_FEED would not have triggered release-windows-check; the filter now includes it. Also per Astro-Han's P1 (the differential probe, fixed in the previous commit): the probe is now asserted explicitly -- the run fails if the updater never asked for the previous blockmap, and the log records whether the differential or the full-download path was exercised. Docs narrowed to match: transient updater states are not individually asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 789ecaed-4363-427d-acbb-ed313915f86b
📒 Files selected for processing (4)
.github/workflows/release-windows-check.ymldocs/windows-support.mdscripts/verify-windows-autoupdate.mjsscripts/verify-windows-x64.mjs
Included review availability: Your plan provides up to 3 included reviews per hour; 1 remains after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
This head fixes the prior full-installer re-read, adds the missing boot workflow trigger, and explicitly verifies the upgraded sandbox/disclaimer resources. One new deterministic blocker remains, and the three current CodeRabbit threads still need disposition; I did not duplicate those inline.
The dedicated Windows package job is still running, so the new release gate has not completed successfully yet.
AI-assisted review disclosure: Codex reviewed exact head 055ec02, including the delta from the last Astro-Han review, current CI, and all live review threads, with two independent reviewer passes.
中文说明
当前 head 已修复重复读取完整安装包、补上 boot wiring 的 workflow 触发，并显式验证升级后安装包中的 sandbox/disclaimer 资源。但新增的 smoke 工作目录没有在首次写文件前创建，会让自动更新门禁确定性失败；另外 3 条当前 CodeRabbit inline 仍需处理，这里没有重复发布。专用 Windows package job 仍在运行。
Four defects in the gate's own previous round of fixes, each confirmed before changing anything: - Create the smoke directory before verifyPackagedWindowsApp: with requireWindowsSandbox now true, the sandbox probe writes its manifest into workingDirectory before any renderer smoke mkdir would have created it, so the upgraded-install smoke deterministically threw ENOENT (Astro-Han P1). - Match the raw request target: decoding and query-stripping before the allowlist meant /%6catest.yml and /latest.yml?cache=1 passed an assertion that claims exact root-level paths (CodeRabbit). - Report the download mode from the installer transfer responses: a 200 on the previous blockmap proves the feed served it, not that the updater downloaded differentially. The log now records the blockmap status and the actual installer response sequence (CodeRabbit). - Treat expectedVersion: null like undefined for the sandbox/disclaimer defaults, matching the version fallback's nullish semantics (CodeRabbit). Astro-Han's range re-buffering P2 was already fixed in the previous commit: bodies are read once at startup and ranged responses serve subarrays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
…aths The previous round matched the raw request target including the query, per review -- and the live gate immediately proved that shape wrong: electron-updater cache-busts its channel request as latest.yml?noCache=<id>, so the exact-target match 404'd the updater's own documented request and the check failed with "Cannot find channel latest.yml". Match the raw, undecoded path segment (so /%6catest.yml still cannot alias /latest.yml) and allow the query, recording the full target in the request log as evidence. Truly unknown paths still count as unexpected and fail the run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-by: Claude Fable 5
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Approved on exact head 72c409ee445432adf0ba99651b848f2189ba2aa1.
The Windows automatic-update gate is end-to-end rather than a packaging proxy: it exercises 0.1.11 → 0.1.12 through the loopback feed, metadata and blockmap probe, installer download, installUpdate, NSIS relaunch, ProductVersion validation, packaged sandbox/disclaimer smoke, and uninstall cleanup. Feed override handling stays test-only and fail-closed for non-loopback URLs.
No remaining P0-P2 findings. Current CI is fully green, mergeability is CLEAN, and there are no unresolved review threads. This changes release verification rather than production UI/UX, so a screenshot is not applicable. The PR body names Claude Fable 5 and all seven commits carry matching Generated-by: trailers.
AI-assisted review disclosure: Codex reviewed the exact-head release workflow, feed isolation, artifact/update lifecycle, tests, current CI, threads, and provenance metadata. No external model was used. Astro-Han authorized this review campaign.
中文说明
Windows 自动更新门禁验证的是完整真实链路，而不只是打包结果：覆盖旧版到新版更新、feed/metadata/blockmap、下载安装、NSIS 自动重启、版本检查、packaged smoke 与卸载清理。
当前没有剩余 P0-P2，CI 全绿、CLEAN、无未解决线程；该 PR 不改变生产 UI/UX，无需截图。PR body 与 7 个 commit 的 AI disclosure 也完整一致。
|
Human merge confirmation: Astro-Han reviewed the final diff at |
2 participants
What & why
Closes the automatic updates item of #2142 Phase 3 (distribution): the update service,
latest.ymlproduction, and the closed-app NSIS upgrade gate all existed, but no evidence ever exercised the actual in-app path — a running packaged app discovering, downloading, and installing a newer build through its own electron-updater. This PR adds that evidence end to end on the unsigned Windows x64 preview channel.What this does
1. Loopback-only test feed override (
apps/desktop/src/main/app-update-service.ts).MAKA_UPDATE_TEST_FEEDis resolved by an exported pure function that accepts exactlyhttp://127.0.0.1:<port>[/path]and maps it to a generic provider. Any other value — remote host,localhost, another loopback alias, HTTPS, userinfo, query, malformed — throws at construction: a mistyped override must never silently fall back to the production GitHub feed. The override changes the feed URL and nothing else; with the variable unset the configuration is byte-identical to production (pinned by the existing unit test). Security posture: setting an env var on the app's process already requires same-user code execution, which under the per-user NSIS install model can rewrite the install directory directly — the hook crosses no privilege boundary, and update signature verification (once a certificate exists) applies to overridden feeds unchanged. Unit tests pin two accepted forms and eleven rejected confusion shapes.2. Version-bumped installer builder (
scripts/package-windows-autoupdate-next.mjs). Reuses the workspace build from a fullpackage:windows-x64run and repackages NSIS-only withextraMetadata.version= patch+1, so the upgraded app genuinely reports the new version viaapp.getVersion(). Output goes toapps/desktop/release-autoupdate-next, outside the release upload globs, so fake-versioned artifacts can never reach a draft release. Asserts properties, not exit codes: exe/latest.yml/blockmap exist, latest.yml advertises exactly the bumped version and installer name, and its sha512 matches the installer byte for byte.3. End-to-end harness (
scripts/verify-windows-autoupdate.mjs). Installs the candidate, launches it against a loopback feed serving the bumped build, and asserts stage by stage with individual deadlines and named failures:checkForUpdatesvia CDP (window.maka.app), polled to thedownloadedstatus with the exactcurrent → latestversion pair;latest.ymland the installer were actually fetched (200/206), zero unexpected paths;installUpdatehandoff (an evaluation dropped by the app quitting is the expected success shape; a structured refusal fails);ProductVersion;app.getVersion()reports the bumped version;verify-packaged-app.mjsgeneralizes its CDP internals into exportedevaluateInRenderer/findRendererTarget/reserveTcpPort/stopChild; the existing smoke's assertions are unchanged.4. CI wiring. Both
release-windows-check.yml(PR gate; paths filter extended to the new scripts andapp-update-service.ts) andrelease-desktop.yml(before asset upload, so a failure blocks the release) build the bumped installer and run the verification; timeouts raised 60→75 and 45→60 to absorb the ~8–10 minute addition.5. Docs.
docs/windows-support.md(EN + zh) now records what the gate proves and keeps the unproven remainder explicit: signature verification awaits an Authenticode certificate, business-data migration and mid-install rollback stay uncovered.Verification
npm --workspace @maka/desktop run typecheckclean; desktop unit tests: the update-service suites pass 19/19 including the new feed-override tests (the workspace run shows 22 pre-existing local-environment failures —EBUSYSQLite cleanup andEPERMsymlink — unrelated to this change and present onmainlocally).node-ptyfor Electron (and a pre-existing localEBUSYflake inrelease-cli-eval-support.test.mjsblockscheck:releasethere as well). The authoritative end-to-end evidence is this PR's ownwindows_sandbox-style gate:release-windows-checkruns the complete package → verify → autoupdate chain onwindows-2025. Treat that lane's result as the verification for this PR; I will iterate on it if it surfaces harness issues.AI use
Select exactly one:
Tool(s) and scope: Claude Fable 5 (Anthropic), driven by the human contributor of record, authored the feed-override hook and tests, the bumped-installer builder, the end-to-end harness, the CI wiring, and the documentation updates, all under human review. Commits carry
Generated-by: Claude Fable 5trailers; the final squash commit must retain them.