feat: add desktop auto-update install flow - #1256
Conversation
github-actions
Bot
added
ai
ci/cd
documentation
🤖 自动审查报告
📁 修改的文件
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d617f056d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Get-ChildItem -Path 'apps/dsa-desktop/dist' -File | | ||
| Where-Object { $_.Name -eq 'latest.yml' -or $_.Name -like '*.blockmap' } | | ||
| ForEach-Object { | ||
| Copy-Item -Path $_.FullName -Destination "dist/release-assets/$($_.Name)" -Force |
There was a problem hiding this comment.
In the Windows release job, these new latest.yml/*.blockmap files are copied into dist/release-assets, but the immediately following actions/upload-artifact step still only uploads dist/release-assets/*.exe and *.zip, and the publish job uploads only the downloaded artifacts. This means every GitHub Release will still be missing latest.yml (and blockmaps), so electron-updater's GitHub provider cannot find the update metadata and the new auto-update flow will fail before downloading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a Windows NSIS desktop “download → prompt → quitAndInstall” auto-update loop (via electron-updater), while keeping the existing “prompt + open Releases page” flow for dev mode, portable builds, and macOS DMG. It also updates the Web Settings page to surface downloading/downloaded/installing states and an explicit “重启安装” action, plus adjusts the release workflow/docs to support the required updater artifacts.
Changes:
- Desktop: integrate
electron-updater, track richer update state, and expose aninstallDownloadedUpdateIPC/bridge API. - Web: extend Settings “桌面端更新” UI to handle downloading/downloaded/installing states and trigger install.
- Release/docs: document the new behavior and attempt to preserve/upload updater metadata (
latest.yml,*.blockmap, original*Setup*.exe).
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/desktop-package.md | Documents Windows installer auto-update behavior and required Release artifacts. |
| docs/CHANGELOG.md | Adds an [Unreleased] entry for the new Windows auto-update capability and workflow changes. |
| apps/dsa-web/src/pages/SettingsPage.tsx | Adds desktop update mode/progress fields and a “重启安装” install action. |
| apps/dsa-web/src/pages/tests/SettingsPage.test.tsx | Adds coverage for the downloaded-update UI and install trigger. |
| apps/dsa-desktop/preload.js | Exposes installDownloadedUpdate() over the preload bridge. |
| apps/dsa-desktop/tests/preload.test.js | Verifies the new bridge method and IPC channel wiring. |
| apps/dsa-desktop/main.js | Implements electron-updater based update checking/downloading, install prompting, and quitAndInstall entrypoint. |
| apps/dsa-desktop/package.json | Adds electron-updater dependency and GitHub publish config for Windows NSIS. |
| apps/dsa-desktop/package-lock.json | Locks electron-updater and transitive dependencies. |
| .github/workflows/desktop-release.yml | Copies updater metadata into release-assets (but currently doesn’t upload them). |
| $exeTarget = "dist/release-assets/daily-stock-analysis-windows-installer-$env:RELEASE_TAG.exe" | ||
| $zipTarget = "dist/release-assets/daily-stock-analysis-windows-noinstall-$env:RELEASE_TAG.zip" | ||
| Copy-Item -Path $installerExe.FullName -Destination $exeTarget -Force | ||
| Copy-Item -Path $installerExe.FullName -Destination "dist/release-assets/$($installerExe.Name)" -Force | ||
| Compress-Archive -Path $winUnpacked -DestinationPath $zipTarget -CompressionLevel Optimal -Force | ||
| Get-ChildItem -Path 'apps/dsa-desktop/dist' -File | | ||
| Where-Object { $_.Name -eq 'latest.yml' -or $_.Name -like '*.blockmap' } | | ||
| ForEach-Object { | ||
| Copy-Item -Path $_.FullName -Destination "dist/release-assets/$($_.Name)" -Force | ||
| } |
| } | ||
|
|
||
| updater.autoDownload = true; | ||
| updater.autoInstallOnAppQuit = true; |
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows 安装版自动更新补齐了桌面端“发现新版本后仍需手动下载安装”的实际体验缺口。
- 是否有对应 issue：无；PR 描述已说明无 Issue，并给出验收标准。
- PR 类型：feat/test。新增桌面端自动更新安装流程，并补充 preload 与设置页相关测试。
- description 完整性：完整。已覆盖背景、范围、验证、兼容性风险、官方约束链接、回滚方案和 CHANGELOG/文档同步。
- 是否可直接合入：不可。Release artifact 上传路径遗漏自动更新必需的
latest.yml/*.blockmap，会导致本 PR 的核心自动更新链路在正式 Release 上不可用。
主要问题
- [Correctness blocker]
.github/workflows/desktop-release.yml：Prepare release artifact (Windows)已把原始*Setup*.exe、latest.yml、*.blockmap复制到dist/release-assets/，但后续actions/upload-artifact的path仍只上传dist/release-assets/*.exe和dist/release-assets/*.zip。publish-release只会发布下载到的 artifact，因此 Release 上不会包含latest.yml/ blockmap；electron-updater的 GitHub provider 将无法拿到 Windows 更新元数据，本 PR 声称的 Windows NSIS 安装版后台下载/校验更新闭环会失败。建议把dist/release-assets/latest.yml和dist/release-assets/*.blockmap加入上传路径，或直接上传dist/release-assets/*，并最好在发布前增加一次资产列表校验。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1380149d5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Get-ChildItem -Path 'apps/dsa-desktop/dist' -File | | ||
| Where-Object { $_.Name -eq 'latest.yml' -or $_.Name -like '*.blockmap' } | | ||
| ForEach-Object { | ||
| Copy-Item -Path $_.FullName -Destination "dist/release-assets/$($_.Name)" -Force |
There was a problem hiding this comment.
When this job uploads latest.yml, that file was generated by electron-builder from apps/dsa-desktop/package.json's version, not from RELEASE_TAG. I checked .github/workflows/auto-tag.yml, and the normal tag path creates v* tags without updating this package version, so a release such as v3.16.1 can publish artifacts named with v3.16.1 while latest.yml still advertises 3.16.0; installed 3.16.0 clients will then see no newer updater version and the new auto-update path will never install that release unless someone manually keeps the package version in sync before tagging.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过，Windows 安装版自动更新补齐了桌面端发现新版本后仍需手动下载安装的体验缺口。
- 是否有对应 issue：无，PR 描述已说明无 Issue，并给出验收标准。
- PR 类型：feat/test，新增桌面端自动更新安装流程，同时补充 preload 与设置页测试。
- description 完整性：不完整，缺少 Windows 桌面端实际构建或 release workflow 级验证结果，也未说明受平台限制无法验证。
- 是否可直接合入：不可，当前 CI 成功但未覆盖本 PR 最关键的 Windows NSIS/electron-builder 自动更新产物链路。
主要问题
- [Process blocker]
.github/workflows/desktop-release.yml/apps/dsa-desktop/package.json/apps/dsa-desktop/main.js：本 PR 的核心行为依赖 electron-builder 生成安装包、latest.yml、*.blockmap以及包内 updater 配置，但验证只覆盖了 node tests、Web lint/build、workflow 文本检查和 audit；当前 CI 摘要也没有 desktop-release 或 Windows Electron build。按仓库 AGENTS.md 的桌面端验证要求，桌面端改动默认需要覆盖 Web 构建后再验证桌面端构建；若受平台限制无法完整验证，也需要明确说明未验证项和风险。缺少这部分证据时，publish配置、版本同步、Release 附件和 updater metadata 的问题只会在发版时暴露。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows NSIS 安装版自动下载并由用户确认重启安装，补齐了桌面端更新闭环。
- 是否有对应 issue：无；PR 描述明确说明无 Issue，并给出了验收标准。
- PR 类型：feat/test + docs；新增桌面端自动更新安装流程，补充 Web/desktop 测试，并同步
docs/desktop-package.md与docs/CHANGELOG.md。 - description 完整性：不完整。已覆盖背景、范围、兼容性、回滚和多项验证，但缺少本 PR 最关键的 Windows NSIS / electron-builder 打包或 release workflow 级验证结果，也未说明受平台限制无法完成该验证。
- 是否可直接合入：不可。当前 CI 成功，但 CI 摘要未覆盖
desktop-release发布链路；本 PR 的核心行为依赖 Windows 安装包、latest.yml、blockmap 与 Release 附件一致性，现有验证不足以支撑直接合入。
主要问题
- [Process blocker]
.github/workflows/desktop-release.yml/apps/dsa-desktop/main.js：本 PR 改动的核心链路是 Windows NSIS 安装版通过electron-updater下载并安装更新，但 PR 验证只包含桌面端 node 单测、workflow 文本检查、Web lint/build/vitest 和依赖审计，未执行apps/dsa-desktop打包，也未提供 Windows 发布链路或desktop-releaseworkflow dry-run/实际运行证据。按照仓库 AGENTS.md 的桌面端验证要求，桌面端改动默认应先构建 Web，再构建桌面端；如受平台限制未完整验证，需要明确说明 Electron 构建、Web 构建产物和 Release 工作流影响的验证缺口。这里缺口正好覆盖自动更新能否真实可用的关键产物，建议补充 Windows/NSIS 或至少 electron-builder 产物验证，并确认 Release 中latest.yml、原始*Setup*.exe、*.blockmap与 tag/package version 一致。 - [Nice to have]
apps/dsa-desktop/node_modules_exists.txt、apps/dsa-web/node_modules_ls_check.txt、root_node_modules_exists.txt：这 3 个空文件看起来像本地检查痕迹，不属于桌面自动更新功能、文档或测试资产。风险不高，但会扩大无关变更面，建议确认是否确有用途；若只是临时探测文件，应从 PR 中移除。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1cb6bbf44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| logLine('[update] quit and install requested'); | ||
| stopBackend(); | ||
| updater.quitAndInstall(false, true); |
There was a problem hiding this comment.
On Windows NSIS installs where users have the app's .env, data/stock_analysis.db, and logs next to the executable (as documented and as resolveAppDir() does), this new auto-install path runs the electron-builder NSIS updater against $INSTDIR; I checked the bundled electron-builder NSIS template and its update uninstall path moves/removes the whole install directory (RMDir /r $INSTDIR), so confirming “立即重启安装” can wipe the local config/database before the new version is installed. Please either move/preserve runtime data outside the install dir or add installer hooks to keep these files before enabling quitAndInstall for installed users.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows NSIS 安装版自动更新补齐了桌面端发现新版本后仍需手动下载安装的体验缺口。
- 是否有对应 issue：无，PR 描述已说明无 Issue，并给出验收标准。
- PR 类型：feat（附带 test/docs），新增桌面端自动更新安装流程，同时补充 Web/desktop 测试与桌面打包文档。
- description 完整性：不完整，缺少 Windows NSIS/electron-builder 实际打包或发布链路验证结果，也缺少自动安装路径对安装目录旁用户数据保留的证据。
- 是否可直接合入：不可。当前 CI 成功，
mergeable_state=blocked本身不作为阻断；但自动更新安装链路仍有兼容性/验证缺口需要先补齐。
主要问题
- [Compatibility blocker]
apps/dsa-desktop/main.js/docs/desktop-package.md：本 PR 将 Windows 安装版从“提醒 + 跳转下载页”升级为下载完成后quitAndInstall()执行安装器；而现有文档明确安装目录旁会生成/读取.env、data/stock_analysis.db和logs/desktop.log。当前改动未看到更新前保留/迁移这些用户运行时数据的实现，也未提供 Windows NSIS 覆盖更新后这些文件仍保留的验证证据。风险是自动更新路径扩大了用户配置、数据库或日志被覆盖/清理的影响面；需要补充保留机制，或给出可复核的 Windows 安装版更新 smoke 证据和恢复说明。置信度：中。 - [Process blocker]
.github/workflows/desktop-release.yml/ PR 描述：本 PR 的关键行为依赖 electron-builder 产出latest.yml、*.blockmap、原始*Setup*.exe并能被安装版消费，但当前验证只有 desktop node tests、workflow 文本/YAML 检查、Web lint/build/vitest 和 audit；未见apps/dsa-desktop npm run build、Windows NSIS 产物检查或desktop-release实际运行证据。按 AGENTS.md，桌面端与发布链路改动应覆盖 Web 构建、桌面构建及受影响发布路径；当前缺口会让 CI 通过但发布附件或自动更新安装链路仍可能不可用。 docs/desktop-package.md：新增验证步骤中，先cd apps/dsa-web后再写cd ../apps/dsa-desktop，实际会解析到apps/apps/dsa-desktop，命令路径错误；另外标题“当前 PR 验收”带有临时 PR 语境，不适合作为长期打包文档内容。建议改成仓库实际可执行路径，并调整为通用的发版前验证说明。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
b1cb6bb to
00d3cf7
Compare
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows NSIS 安装版自动更新补齐了桌面端发现新版本后仍需手动下载安装的体验缺口。
- 是否有对应 issue：无。PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat（附带 test/docs）。新增桌面端自动更新安装流程，同时补充 preload / 设置页测试与桌面打包文档。
- description 完整性：不完整。已覆盖背景、范围、兼容性、回滚和部分验证，但缺少 Windows NSIS / electron-builder 实际打包或
desktop-release产物验证结果，也缺少数据库 WAL 场景下运行时数据保留的验证证据。 - 是否可直接合入：不可。当前 CI 成功且 merge 状态未显示冲突，但自动安装前的运行时数据备份仍有正确性风险，需要先修复或补足可复核证据。
主要问题
- [Correctness blocker]
apps/dsa-desktop/main.js的自动更新备份清单只覆盖.env、data/stock_analysis.db、logs/desktop.log，但仓库当前 SQLite 默认启用 WAL（src/config.py中SQLITE_WAL_ENABLED默认为 true，src/storage.py会对文件 SQLite 设置PRAGMA journal_mode=WAL）。在quitAndInstall前若存在未 checkpoint 的stock_analysis.db-wal/stock_analysis.db-shm，只备份并恢复主.db可能丢失最近写入或恢复出不一致数据库；同时stopBackend()在 Windows 上通过异步taskkill返回后立即备份，也没有等待后端退出或显式 checkpoint。建议在安装前确保后端已退出并完成 WAL checkpoint，或把.db-wal/.db-shm纳入一致性备份，并补充对应回归/手工验证证据。 - [Process blocker] 本 PR 修改
apps/dsa-desktop、electron-builder配置和.github/workflows/desktop-release.yml，按 AGENTS.md 的桌面端/发布链路验证要求，至少需要说明桌面端构建、NSIS 产物、latest.yml、*.blockmap与 release tag 一致性的验证结果；当前描述只有单测、Web 构建、workflow 文本检查和 audit，不能证明新增自动更新安装链路在实际 Windows 打包产物中可用。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows 安装版自动下载并由用户确认重启安装，补齐了桌面端更新闭环。
- 是否有对应 issue：无；PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat，新增桌面端自动更新安装流程，附带测试、工作流和文档更新。
- description 完整性：不完整。已覆盖背景、范围、兼容性、回滚和多项验证，但缺少 Windows NSIS / electron-builder 实际打包或
desktop-release产物级验证结果，也未明确说明该关键路径受平台限制无法验证。 - 是否可直接合入：不可。当前 CI 为 success，
mergeable_state=blocked本身不作为阻断；阻断点是本 PR 的核心行为依赖 Windows 安装包、latest.yml、*.blockmap和 updater metadata，但现有验证只覆盖 node tests、Web lint/build、SettingsPage 测试和 workflow 文本检查，尚未证明真实打包产物能被 electron-updater 正确消费。
主要问题
- [Process blocker]
.github/workflows/desktop-release.yml、apps/dsa-desktop/main.js、apps/dsa-desktop/package.json：本 PR 改动的是 Windows NSIS 自动更新安装链路，AGENTS.md 对桌面端改动要求默认先构建 Web、再构建桌面端；如平台受限未完整验证，需要说明 Web 构建产物、Electron 构建和 Release 工作流影响。当前 PR 描述未提供apps/dsa-desktopElectron 构建、Windows NSIS 打包、latest.yml/*.blockmap与安装包文件名匹配的实际验证证据，也未说明无法验证的限制。建议补充一次桌面构建或发布产物级检查结果；如果无法在当前环境跑 Windows NSIS，也需要明确平台限制和由谁/在哪个发布链路复核。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
4b0e293 to
bff7a0c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bff7a0c02a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert.equal(destroyed, true); | ||
| }); | ||
|
|
||
| test('desktop update backup list includes WAL and SHM artifacts', (t) => { |
There was a problem hiding this comment.
These new tests/main.test.js cases are never exercised by the desktop test command: apps/dsa-desktop/package.json still runs only node --test tests/preload.test.js (confirmed by running npm test, which reported just the 3 preload tests). As a result, the backup/restore and stopBackend regressions this commit is trying to guard against will be missed by the documented npm test/release validation path unless the script is broadened to include this file.
Useful? React with 👍 / 👎.
b3b8226 to
b1b0368
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1b0368937
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "test": "node --test tests/*.test.js" | ||
| }, | ||
| "dependencies": { | ||
| "electron-updater": "^6.8.3" |
There was a problem hiding this comment.
When this new runtime dependency is added, a local or self-hosted build that already has apps/dsa-desktop/node_modules from before the change will still skip dependency installation because both scripts/build-desktop.ps1 and scripts/build-desktop-macos.sh only run npm install when the directory is absent. In that scenario the packaged app does not contain electron-updater, getElectronAutoUpdater() catches Cannot find module 'electron-updater', and the Windows installed build silently falls back to no automatic install support despite the release metadata being present. Please make the build path refresh/install dependencies when package-lock.json changes or before packaging this new module.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70aa49c9cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| ensureDirectory(path.dirname(target)); | ||
| fs.copyFileSync(source, target); |
There was a problem hiding this comment.
If the first launch after an update finds a backup manifest but any restored file cannot be copied (for example the target is locked or the install directory becomes unwritable), this copyFileSync exception bubbles out of createWindow() before logging and before a BrowserWindow is created. Because the backup directory is only cleaned after the loop, every subsequent launch will retry the same failing restore and the desktop app can get stuck opening no UI instead of falling back to manual recovery or skipping the failed file.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过，Windows NSIS 安装版自动下载并重启安装补齐了桌面端更新闭环。
- 是否有对应 issue：无；PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat + test + docs；新增桌面端自动更新安装流程，并同步补充测试、Release 工作流和桌面打包文档。
- description 完整性：基本完整；已覆盖背景、范围、兼容性、验证、回滚和官方约束链接。
- 是否可直接合入：不可；当前 CI 成功且可合并状态不是阻断点，但运行时文件恢复失败时会静默丢弃备份，存在用户配置/数据库丢失风险。
主要问题
- [Correctness blocker]
apps/dsa-desktop/main.js/apps/dsa-desktop/tests/main.test.js：新增的 Windows NSIS 自动更新会在安装前备份.env、数据库 WAL/SHM 和日志，并在首次启动恢复；但当前测试明确覆盖的是restorePackagedRuntimeStateFromBackup遇到copyFileSync失败时“不抛错并清理 backupRoot”。这意味着如果目标文件被锁定、安装目录不可写、磁盘/权限异常等导致任一运行时文件恢复失败，备份会被静默删除，用户没有可见提示，也没有恢复入口，可能造成配置或数据库文件不可恢复丢失。该风险正好落在 AGENTS.md 的“配置与运行入口”“工作流/发布/打包”和“运行时配置迁移/恢复需有提示与回归测试”范围内。建议改为失败时保留备份目录和 manifest，记录明确错误，并向用户提供可见提示或可重试/手动恢复路径；对应测试也应断言失败时不会清理备份。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过。Windows NSIS 安装版自动更新补齐了桌面端发现新版本后仍需手动下载安装的体验闭环。
- 是否有对应 issue：无；PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat；新增桌面端自动下载与重启安装流程，并附带 test/docs/workflow 更新。
- description 完整性：不完整；已覆盖背景、范围、兼容性、回滚和官方约束，但缺少 Windows NSIS/electron-builder 产物级验证结果或未验证原因在 PR 验证段的明确说明。
- 是否可直接合入：不可。当前 CI 成功，
mergeable_state=blocked本身不是阻断点；阻断点是自动安装确认弹窗路径缺少失败兜底，影响核心安装流程。
主要问题
- [Correctness blocker]
apps/dsa-desktop/main.js：update-downloaded事件中以void maybePromptInstallDownloadedUpdate(nextState)启动弹窗；用户点击“立即重启安装”后，maybePromptInstallDownloadedUpdate直接await installDownloadedUpdate()，没有捕获stopBackend、运行时文件备份失败或quitAndInstall抛错。设置页“重启安装”路径已有 try/catch，但自动弹窗主路径没有，失败时可能产生未处理 rejection，导致主进程异常、状态停在 installing，或用户看不到可恢复提示。建议在该弹窗路径 catch 错误，记录日志并把更新状态回落到 error，同时保留手动下载/重试入口。 - [Verification gap] 本 PR 改动
apps/dsa-desktop、打包脚本和desktop-release工作流，按仓库AGENTS.md的桌面端验证要求，需要覆盖 Web 构建与桌面端构建；当前描述有 desktop node tests、Web lint/build、workflow 文本检查和依赖审计，但没有npm run build、Windows NSIS 产物、latest.yml/*.blockmap与安装包版本一致性的实际验证结果。docs/desktop-package.md已补充通用核验说明，但本次 PR 的验证结论仍应明确该关键路径是否受平台限制未执行，以及由哪条发布链路补验。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b29bf1b7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| logLine('[update] quit and install requested'); | ||
| updater.quitAndInstall(false, true); |
There was a problem hiding this comment.
When quitAndInstall() throws after backupPackagedRuntimeState() has written a manifest, the app stays on the old version with .dsa-desktop-update-backup still present. If the user keeps using the app and changes .env or the database, the next startup will run restorePackagedRuntimeStateFromBackup() and overwrite those current files with the stale pre-install backup even though no update was installed. Please remove the backup/manifest on this failure path or only leave it behind once the updater is actually quitting into an install.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过，Windows NSIS 安装版自动下载并重启安装补齐了桌面端更新闭环。
- 是否有对应 issue：无，PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat，新增桌面端自动更新安装流程，并同步 test/docs/workflow 更新。
- description 完整性：基本完整，已覆盖背景、范围、兼容性、验证、回滚和官方约束；但建议补充安装失败后备份清理/保留策略的说明。
- 是否可直接合入：不可，当前 CI 成功且
mergeable_state=blocked本身不是阻断点，但安装启动失败后的运行时备份残留仍有用户数据回滚风险。
主要问题
- [Correctness blocker]
apps/dsa-desktop/main.js：自动更新安装前会备份安装目录旁的.env、数据库和日志等运行时文件；但当quitAndInstall()抛错时，测试只覆盖了状态变为 error，没有证明已清理或标记这次失败安装产生的.dsa-desktop-update-backup。如果应用仍停留在旧版本继续运行，用户后续修改配置或数据库后再重启，首次启动恢复逻辑可能把旧备份覆盖回安装目录，造成静默回滚用户运行时状态。建议在安装启动失败路径清理本次备份，或用明确状态避免旧版本下次启动误恢复，并补充对应回归测试。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
2b29bf1 to
389b2ac
Compare
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过，Windows NSIS 安装版自动下载并确认重启安装补齐了桌面端更新闭环。
- 是否有对应 issue：无，PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat，新增桌面端自动更新安装流程，并同步补充 tests/docs/workflow。
- description 完整性：完整，已覆盖背景、范围、兼容性、验证结果、官方约束和回滚方案，且符合 AGENTS.md 对用户可见变更同步
docs/CHANGELOG.md与专题文档的要求。 - 是否可直接合入：不可，CI 成功且
mergeable_state=blocked本身不是阻断点，但当前 head 仍存在运行时正确性问题，需要先修复。
主要问题
- [Correctness blocker]
apps/dsa-desktop/main.js：当前成功启动路径中引用了restoreFailed，但现有证据显示该变量未在main.js中声明。风险是应用完成后端启动后直接触发ReferenceError，导致桌面端正常启动路径被新引入的自动更新恢复逻辑打断。需要补齐变量定义或调整恢复结果判断，并增加覆盖该成功启动路径的测试。 scripts/verify-desktop-updater-artifacts.ps1：新增校验脚本文档化了-ReleaseTag/-DistDir参数用法，但 PowerShell 脚本的param(...)前存在$ErrorActionPreference，会导致具名参数绑定不符合预期。风险是 AGENTS.md 要求的发布/打包验证命令在维护者本地或 CI 复核时不可用。建议将param(...)放到脚本首部再设置错误策略。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afbc466020
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async function createWindow() { | ||
| const restoreResult = isWindowsNsisInstalledApp() ? restorePackagedRuntimeStateFromBackup() : null; | ||
| initLogging(); | ||
| const restoreFailed = Boolean(restoreResult && (restoreResult.failed.length || restoreResult.skipped.length)); |
There was a problem hiding this comment.
When an NSIS update is canceled or aborts and the user relaunches the same app version, restorePackagedRuntimeStateFromBackup() now discards the stale backup and records that in skipped, but this line treats any skipped entry as a restore failure. That leaves the app showing a manual-recovery error for a backup directory that was already deleted and suppresses the automatic update check for that launch, even though no restore failed. Only actual copy failures should put startup into the error/manual path.
Useful? React with 👍 / 👎.
| $installerFiles = Get-ChildItem -Path $distDirPath -Filter 'daily-stock-analysis-windows-installer-*.exe' -File -ErrorAction SilentlyContinue | ||
| if (-not $installerFiles) { | ||
| throw 'No daily-stock-analysis-windows-installer-*.exe found in dist; expected release attachment is missing.' |
There was a problem hiding this comment.
With the script's default DistDir and the documented flow after npm run build, apps/dsa-desktop/dist contains electron-builder's *Setup*.exe, latest.yml, and blockmaps, but the daily-stock-analysis-windows-installer-*.exe alias is only created later in the workflow's "Prepare release artifact (Windows)" step under dist/release-assets. This check therefore fails otherwise valid local updater builds unless the caller overrides -DistDir to the post-processed release-assets directory; split the dist metadata check from the release-attachment alias check or change the default/docs accordingly.
Useful? React with 👍 / 👎.
9020b7d to
bcb0648
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcb06487a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $DistDir = Join-Path $PSScriptRoot '..\apps\dsa-desktop\dist' | ||
| } | ||
|
|
||
| $distDirPath = (Resolve-Path $DistDir -ErrorAction SilentlyContinue)?.Path |
There was a problem hiding this comment.
When this verifier is run in the Windows release-check environment documented in docs/desktop-package.md using powershell.exe/Windows PowerShell 5.1, this line fails to parse because ?. is the null-conditional operator Microsoft documents as a PowerShell 7.1+ feature, so the artifact check never reaches the latest.yml/blockmap validation. The repo's Windows docs still use powershell -ExecutionPolicy ... for these scripts, so either require pwsh explicitly or rewrite the Resolve-Path assignments without ?..
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过，Windows NSIS 安装版自动下载并确认重启安装补齐了桌面端更新闭环。
- 是否有对应 issue：无，PR 描述明确说明无 Issue，并给出验收标准。
- PR 类型：feat，新增桌面端自动更新安装流程，并同步补充测试、发布工作流和文档。
- description 完整性：完整，已覆盖背景、范围、兼容性、验证结果、官方约束、风险边界和回滚方案，符合 AGENTS.md 对用户可见变更同步专题文档与
docs/CHANGELOG.md的要求。 - 是否可直接合入：可，当前 CI 全部通过；
mergeable_state=blocked仅表示受分支保护限制，不构成本次代码风险阻断。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
PR Type
Background And Problem
当前桌面端已经能检测 GitHub Releases 新版本，但只提醒并跳转下载页，Windows 安装版用户仍需要手动下载安装包。这个 PR 为 Windows NSIS 安装版补上自动下载与用户确认后重启安装的闭环，同时保留开发态、免安装包和 macOS DMG 的手动下载兼容路径。
Scope Of Change
apps/dsa-desktop/: 引入electron-updater，在安装版 Windows 中启用自动检查、后台下载、下载完成提示与quitAndInstall安装入口。apps/dsa-web/: 设置页桌面端更新区域新增 downloading / downloaded / installing 状态与“重启安装”操作。.github/workflows/desktop-release.yml: Windows Release 附件额外保留 updater 需要的原始*Setup*.exe、latest.yml、*.blockmap；构建前用RELEASE_TAG同步桌面端 package 版本，并校验latest.yml版本与 tag 一致。docs/desktop-package.md/docs/CHANGELOG.md: 记录 Windows 安装版自动更新能力与 macOS/免安装包边界。Issue Link
无 Issue。验收标准：Windows NSIS 安装版检测到新版本后可后台下载，下载完成后由用户确认重启安装；没有自动更新条件的平台继续显示下载页兼容路径。
Verification Commands And Results
关键输出/结论：
latest.ymlguard are present.--omit=dev.Compatibility And Risk
latest.yml、原始*Setup*.exe与 blockmap 文件。RELEASE_TAG#v，并在上传前校验latest.yml的version:与 tag 匹配；这不会提交版本文件，只影响 CI workspace 中生成的安装包与 updater metadata。latest-mac.yml。Rollback Plan
Revert this PR 即可回到原来的“检测更新 + 跳转 GitHub Releases 下载页”行为；不涉及数据迁移或用户本地配置迁移。
EXTRACT_PROMPT Change (if applicable)
不适用。
展开 / Expand: Full EXTRACT_PROMPT
Checklist
docs/CHANGELOG.md；README.md仅在首页级信息变化时更新，细节优先写入docs/*.md/ If user-visible changes are included, relevant docs anddocs/CHANGELOG.mdare updated;README.mdis updated only for homepage-level changes, with details kept indocs/*.md