fix: register Windows IME from NSIS installer#268
Conversation
PR Reviewer Guide 🔍(Review updated until commit ec9f658)Here are some key observations to aid the review process:
|
|
Addressed the two review items in
Validation run:
|
![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: e67755d04e
ℹ️ 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".
| !macro OPENLESS_IME_UNREGISTER_X86 | ||
| DetailPrint "Unregistering OpenLess x86 TSF IME" | ||
| ExecWait '"$WINDIR\SysWOW64\regsvr32.exe" /s /u "$INSTDIR\windows-ime\x86\OpenLessIme.dll"' $0 | ||
| !insertmacro OPENLESS_IME_ABORT_IF_FAILED $0 "x86 unregistration" |
There was a problem hiding this comment.
Calling OPENLESS_IME_ABORT_IF_FAILED in the uninstall hooks makes the entire NSIS uninstall fail whenever regsvr32 /u returns non-zero (for example if the DLL was already removed/corrupted or COM state is partially broken). In that state users cannot remove OpenLess at all, which is a regression from the MSI path (openless-ime.wxs) where uninstall unregistration is Return="ignore" specifically to avoid blocking removal on cleanup errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the follow-up commits on codex/register-windows-ime-nsis-installer:
- NSIS uninstall now logs x64/x86
regsvr32 /uexit codes instead of aborting, matching the MSIReturn="ignore"cleanup behavior. - The packaging assertion now explicitly verifies registration still aborts during install, while unregistration does not abort during uninstall.
Validation run locally:
node scripts/windows-package-msvc.test.mjs- PowerShell parser check for
scripts/windows-ime-install-smoke.ps1
|
Persistent review updated to latest commit e67755d |
|
Persistent review updated to latest commit 68ee9df |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ee9dfae4
ℹ️ 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".
| powershell.exe -NoProfile -ExecutionPolicy Bypass -File .\scripts\windows-ime-install-smoke.ps1 -InstallerPath $nsis.FullName -InstallerKind nsis | ||
| powershell.exe -NoProfile -ExecutionPolicy Bypass -File .\scripts\windows-ime-install-smoke.ps1 -InstallerPath $msi.FullName -InstallerKind msi |
There was a problem hiding this comment.
In the Windows verify step, both smoke scripts are invoked sequentially but there is no per-command exit check. On pwsh, GitHub Actions reports the step status from the last $LASTEXITCODE, so a failing NSIS smoke run can be masked if the MSI smoke run succeeds afterward. That lets a broken NSIS installer pass release verification even though this step is intended to validate both artifacts.
Useful? React with 👍 / 👎.
| assert.match(nsisHook, /OPENLESS_IME_ABORT_IF_FAILED \$0 "x64 unregistration"/, "NSIS uninstall should fail if x64 TSF unregistration fails"); | ||
| assert.match(nsisHook, /OPENLESS_IME_ABORT_IF_FAILED \$0 "x86 unregistration"/, "NSIS uninstall should fail if x86 TSF unregistration fails"); |
There was a problem hiding this comment.
These assertions require uninstall abort macros that are not present in src-tauri/nsis/openless-ime-hooks.nsh, so the packaging test now fails deterministically (node scripts/windows-package-msvc.test.mjs fails at this check). Keeping this stale expectation breaks the repo’s own validation script and can block or confuse release-script verification.
Useful? React with 👍 / 👎.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Persistent review updated to latest commit 933e014 |
|
Persistent review updated to latest commit eca76de |
|
Persistent review updated to latest commit eca76de |
|
Persistent review updated to latest commit 4a7a1b6 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a7a1b6b8c
ℹ️ 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.match(ciWorkflow, /InstallerKind nsis[\s\S]*\$LASTEXITCODE -ne 0[\s\S]*NSIS installer smoke failed/, "CI should fail immediately when the NSIS smoke run fails"); | ||
| assert.match(ciWorkflow, /InstallerKind msi[\s\S]*\$LASTEXITCODE -ne 0[\s\S]*MSI installer smoke failed/, "CI should fail when the MSI smoke run fails"); |
There was a problem hiding this comment.
These assertions require the workflow to check $LASTEXITCODE after each windows-ime-install-smoke.ps1 invocation, but .github/workflows/release-tauri.yml currently runs both installer smokes without those guards. As a result, node scripts/windows-package-msvc.test.mjs fails deterministically at this point, so the repository’s own packaging validation cannot pass until the workflow and test expectations are brought back into sync.
Useful? React with 👍 / 👎.
|
Addressed the current CI smoke exit-code feedback in
Validation run locally:
|
|
Persistent review updated to latest commit b303ea4 |
millionart
left a comment
millionart
left a comment
There was a problem hiding this comment.
Addressed the persistent review guide items in ec9f658:
- NSIS now rolls back the already-registered x64 IME if x86 registration fails before aborting install, avoiding partial machine registration.
windows-ime-install-smoke.ps1now asserts every backend-required registry key inExpectedBackendKeysinstead of only tracing them.
Validation run locally:
node scripts/windows-package-msvc.test.mjs- PowerShell parser check for
scripts/windows-ime-install-smoke.ps1 git diff --check/git diff --cached --check
|
Persistent review updated to latest commit ec9f658 |
main 上 Open-Less#268 (millionart/codex/register-windows-ime-nsis-installer) 与 Open-Less#279 (本人) 都修了 NSIS 注册 TSF DLL，merge Open-Less#279 时未手动解决冲突，导致： 1. tauri.conf.json 出现两个 windows.nsis block（line 92-95 和 105-108）， serde_json 用最后写入的覆盖前面，结果 Open-Less#268 的 nsis/openless-ime-hooks.nsh 被丢弃，实际生效的是 Open-Less#279 的 installer.nsh。 2. Open-Less#279 的 bundle.resources 把 dll 多打一份到 $INSTDIR\tsf-ime\，与 Open-Less#268 PREINSTALL File 直接装到 $INSTDIR\windows-ime\ 并存。 3. installer.nsh 与 openless-ime-payload/ 残留为死代码。 Open-Less#268 方案更稳：x64 注册带 Sysnative + DisableX64FSRedirection 双保险，有 POSTUNINSTALL 清理 hook，有测试覆盖（4a7a1b6b/test、933e014a/test），且 不污染 MSI（不用 bundle.resources，避免 dll 重复）。 清理： - 删除 tauri.conf.json 重复 nsis block，保留 nsis/openless-ime-hooks.nsh - 删除 bundle.resources（Open-Less#268 不需要） - 删除死文件 installer.nsh - 删除死目录 openless-ime-payload/ release-tauri.yml 里"复制 dll 到 openless-ime-payload"步骤、build-mac.sh 与 Linux build 的 --config '{"bundle":{"resources":{}}}' override 暂留 — 没有 bundle.resources 后这些是 noop，不会破坏 build；当前 token 没有 workflow scope，等下一次有需求再清。
主要修复 ───────── - fix(windows): NSIS 安装包注册 OpenLess TSF DLL（含 32-bit COM 注册到 Wow6432Node + Sysnative 绕过 32-bit installer 的 WOW64 重定向）； 改 perMachine 安装让 regsvr32 能写 HKLM。修「Windows 输入法后端：不可用」 自 1.2.15 起的长期 bug。 - fix(local-asr): 切走/删模型/手动释放时引擎真正归还 1+ GB RAM；下载稳定性 4 次指数退避 + Range 分块 + 取消正确传播；点击下载即崩（tokio::spawn → tauri::async_runtime）。 - fix(tauri): Rust crate tauri 锁 ~2.10，避开 npm 端 @tauri-apps/api 尚未发布 2.11 导致的跨 minor 版本检查 fail。 - fix(capsule): AudioBars 用 cubic-bezier 缓动 + 0.18s transition，圆点→ 长条形变在 60Hz audio level 高频更新下平滑混合，不再阶梯式跳变。 - fix(recorder): liveness watchdog + 全局超时；watchdog 从首次回调开始计时 避免慢启动误报（issue #238）。 - fix(windows): hotkey trigger leak、capsule 视觉异常、音量计稳定（多 PR）。 - fix(credentials): 凭据状态按 active provider 判断。 新功能 ───────── - feat(asr): 本地 Qwen3-ASR-Flash 全链路 — 模型注册表、HF/HF-Mirror 4 并发 分块下载、Settings → 模型设置页、Coordinator 第三分支接入。 - feat(i18n): 日本語 / 한국어 完整翻译（217 string × 2 语种）+ 5 locale 在语言名后加 Beta 标识。 - feat(translation): 输出语言扩展为日 / 韩，与翻译链路解耦。 - feat(recording): 录音时静音系统输出，多会话共享 mute guard。 - feat(i18n): 繁體中文 UI 完整 + 简繁脚本随 UI 切换同步。 - feat(settings): 合并设置内外两个「关于」（QQ 群 / 反馈 / 源码 / 隐私 全部并入弹窗的关于）+ 一键导出错误日志（plugin-dialog 选保存位置， 跨 macOS / Windows）。 工程 ───────── - chore(ci): release-tauri.yml + ci.yml checkout 拉 submodule，修 mac 端 qwen-asr build 缺源文件。 - chore(release-prep): 清理 #268/#279 双 NSIS 方案残留。 - chore(deps): 引入 tauri-plugin-dialog ~2 给前端 save dialog 用。
2 participants
User description
Summary
OpenLessIme.dllartifacts and invoking the matchingregsvr32during install/uninstall hooks..exeand repaired MSI artifacts after bundle creation.Validation
node scripts/windows-package-msvc.test.mjsscripts/windows-ime-install-smoke.ps1cargo check --manifest-path src-tauri\Cargo.tomlnpm.cmd run buildscripts\windows-ime-build.ps1 -Configuration Release -Platform x64scripts\windows-ime-build.ps1 -Configuration Release -Platform Win32npm.cmd run tauri -- build --bundles nsisOpenLess_1.2.19_x64-setup.exepassed and reportedWindows IME backend would report installed, then uninstalled.Note: local
cargo test --manifest-path src-tauri\Cargo.tomldid not reach tests on this machine because the test executable failed to start withSTATUS_ENTRYPOINT_NOT_FOUND; this appears to be a local runtime/linkage startup issue separate from this installer change.PR Type
Enhancement, Tests
Description
Add NSIS installer hooks for Windows TSF IME registration (x64/x86)
Create elevated PowerShell smoke test to verify IME installation
Integrate smoke test into Windows release CI for NSIS and MSI
Update test assertions to cover NSIS hooks, smoke script, and CI
Diagram Walkthrough
File Walkthrough
windows-ime-install-smoke.ps1
New Windows IME installer smoke test scriptopenless-all/app/scripts/windows-ime-install-smoke.ps1
IME registration
aligned with backend
executable, and all backend-required keys
flag
windows-package-msvc.test.mjs
Extend build test assertions for NSIS IME and smokeopenless-all/app/scripts/windows-package-msvc.test.mjs
test suite
rollback, and unregistration logic
installerHooks path
quoting, registry checks) and CI workflow integration
release-tauri.yml
CI step to run IME installer smoke on Windows.github/workflows/release-tauri.yml
installers
directories
on non-zero exit
tauri.conf.json
Configure NSIS for machine-wide IME registrationopenless-all/app/src-tauri/tauri.conf.json
requirement
openless-ime-hooks.nsh
NSIS installer hooks for Windows TSF IMEopenless-all/app/src-tauri/nsis/openless-ime-hooks.nsh
directory
paths (Sysnative/SysWOW64)
back prior registration
up IME files/directories