fix(installer): auto-detect SYSTEM account and default PATH scope to machine#4903
Conversation
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the fix, @yiliang114! The SYSTEM account PATH detection is a real and well-motivated problem.
Before diving into code review, this PR needs to fill in the PR template — it currently only has "What this PR does" and "Why it's needed". The missing sections are:
- Reviewer Test Plan — "How to verify", "Evidence (Before & After)", and the "Tested on" table. The template notes: "Maintainers prioritize PRs with a clear reviewer test plan — without it, review may be delayed." This is especially important for a Windows-only installer change that's hard for most reviewers to verify on their own machines.
- Risk & Scope — main risk/tradeoff, what's out of scope, breaking changes. Writing to Machine PATH (HKLM) requires admin privileges and affects all users on the machine — that's worth calling out.
- Linked Issues — the body mentions "Resolves #4901" at the bottom, but it should be in the dedicated section so GitHub auto-closes it.
- Chinese translation (
<details>block) — the template requires a full translation of the English body.
Please update the PR body and we'll continue the review. 🙏
中文说明
感谢 @yiliang114 的修复!SYSTEM 账户下 PATH 检测的问题确实存在,动机合理。
在进入代码审查之前,这个 PR 需要补充 PR 模板 中的必填部分——目前只有 "What this PR does" 和 "Why it's needed"。缺少的部分:
- Reviewer Test Plan — "How to verify"、"Evidence (Before & After)" 和 "Tested on" 表格。模板中明确说明:"Maintainers prioritize PRs with a clear reviewer test plan"。对于 Windows 安装脚本的改动,这一点尤其重要,因为大多数 reviewer 无法在自己的机器上直接验证。
- Risk & Scope — 主要风险/权衡、不在范围内的内容、破坏性变更。写入 Machine PATH(HKLM)需要管理员权限且影响机器上所有用户——这值得明确说明。
- Linked Issues — PR 底部提到了 "Resolves #4901",但应该放在专门的 section 中,以便 GitHub 自动关闭 issue。
- 中文翻译(
<details>块)— 模板要求完整翻译英文正文。
请补充后我们继续审查。🙏
— Qwen Code · qwen3.7-max
There was a problem hiding this comment.
Pull request overview
This PR updates the Windows standalone installer to default PATH persistence to Machine scope when running under the SYSTEM account, and introduces explicit controls to repair or choose PATH scope so qwen remains discoverable in new service-spawned sessions (SSM/WinRM/scheduled tasks).
Changes:
- Auto-detect SYSTEM (SID
S-1-5-18) ininstall-qwen-standalone.batand default PATH scope tomachine, plus new--repair-pathand--path-scope user|machineoptions (and env vars). - Uninstaller now removes both User and Machine PATH entries for the standalone shim directory.
- Adds CI/test/docs updates to validate hosted-asset packaging preserves the new flags.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/tests/install-script.test.js |
Extends workflow assertions to require hosted-asset validation and presence of new installer flags. |
scripts/installation/uninstall-qwen-standalone.ps1 |
Removes standalone bin dir from both User and Machine PATH and updates current-session PATH accordingly. |
scripts/installation/INSTALLATION_GUIDE.md |
Documents --repair-path, --path-scope, and related environment variables. |
scripts/installation/install-qwen-standalone.bat |
Adds SYSTEM detection for PATH scope defaulting, new CLI/env options, and PATH repair mode. |
scripts/build-hosted-installation-assets.js |
Adds hosted-installer behavior checks for new flags (needs tighter regex to avoid false positives). |
.github/workflows/sync-release-to-oss.yml |
Adds a stable-release validation step checking hosted .bat contains the new flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ — thorough description, clear before/after evidence, and the risk assessment is honest. On direction: this is a real, well-understood bug. SSM/Workbench sessions running as SYSTEM write to HKCU PATH, which new service-spawned sessions never re-read. Writing to Machine PATH (HKLM) when running as SYSTEM is the correct fix — it's resolved at process creation time regardless of session origin. Aligned with the platform distribution work. No CHANGELOG reference for this specific area, but the problem domain is clearly relevant. On approach: the scope feels right. 8 files, focused changes, no scope creep. The Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 描述详细,修复前后证据清晰,风险评估客观。 方向:这是一个真实且理解到位的 bug。SSM/Workbench 以 SYSTEM 运行时写入 HKCU PATH,新的服务派生 session 不会重新读取。在 SYSTEM 下写入 Machine PATH (HKLM) 是正确的修复方案——Machine PATH 在进程创建时解析,不受 session 来源影响。与平台分发方向一致。 方案:范围合理,8 个文件,聚焦改动,没有范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe diff is clean and well-structured. Here's what I found: What works well:
Nothing blocking. The changes follow existing TestingThis PR modifies Windows-only installer scripts ( The PR author's evidence (before/after in the PR body) shows:
The test plan in the PR body is thorough and actionable for Windows reviewers. 中文说明代码审查代码整洁,结构清晰。 做得好的部分:
无阻塞问题。 改动完全遵循现有 测试此 PR 修改 Windows 安装脚本( 作者提供的前后对比证据充分,测试计划对 Windows reviewer 来说可操作。 — Qwen Code · qwen3.7-max |
|
This PR does what it says: fixes a real Windows installer bug where SYSTEM account installs write PATH to the wrong registry hive, and new service-spawned sessions can't find The previous The code changes are well-scoped (8 files, no scope creep), follow existing Approving. ✅ 中文说明此 PR 修复了一个真实的 Windows 安装器 bug:SYSTEM 账户下 PATH 写入了错误的注册表 hive,导致新的服务派生 session 找不到 之前的 CHANGES_REQUESTED 是关于模板完整性,已完全解决。PR 正文现在包含所有必填部分,内容详尽。 代码改动范围合理(8 个文件,无范围蔓延),遵循现有 通过。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
|
||
| echo. | ||
| echo Qwen Code !INSTALLED_VERSION! is already installed. | ||
| echo PATH repaired for !PATH_SCOPE! scope: |
There was a problem hiding this comment.
[Suggestion] When --no-modify-path is set, the persistent PATH update is correctly skipped, but this message still prints PATH repaired for !PATH_SCOPE! scope:. A user (or CI operator) would believe the persistent PATH was fixed, but new terminals still won't find qwen.
Consider differentiating the message based on whether the persistent update actually ran:
| echo PATH repaired for !PATH_SCOPE! scope: | |
| if /i "!NO_MODIFY_PATH!"=="1" ( | |
| echo NOTE: --no-modify-path is set; persistent PATH was not changed. | |
| echo !INSTALL_BIN_DIR! | |
| ) else ( | |
| echo PATH repaired for !PATH_SCOPE! scope: | |
| echo !INSTALL_BIN_DIR! | |
| ) |
— qwen3.7-max via Qwen Code /review
| "- name: 'Verify Aliyun OSS Hosted Installation Assets'", | ||
| ); | ||
| expect(syncStepIndex).toBeGreaterThanOrEqual(0); | ||
| expect(packageHostedStepIndex).toBeGreaterThanOrEqual(0); |
There was a problem hiding this comment.
[Suggestion] The new packageHostedStepIndex assertion verifies the step exists (>= 0) but doesn't check it runs before syncHostedStepIndex. Since the packaging step produces artifacts that the sync step uploads, a future workflow edit could reorder them without this test catching it.
| expect(packageHostedStepIndex).toBeGreaterThanOrEqual(0); | |
| expect(packageHostedStepIndex).toBeGreaterThanOrEqual(0); | |
| expect(syncHostedStepIndex).toBeGreaterThan(packageHostedStepIndex); |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review
Local runtime verification reportSince the risky part of this PR is batch arg parsing + scope selection + real registry writes, I ran the changed installer end-to-end against a real Scenario matrix
Evidence: registry before/after for scenarios 7, 8, 11S7/S8 (machine scope; S8 via SYSTEM auto-detect, no flags): S11 (uninstall, both scopes seeded): Repo checks
Observations (non-blocking)
Not covered here
Verdict: every executable scenario matches the PR description, including the headline SSM/SYSTEM auto-detect behavior and the dual-scope uninstall cleanup; no regressions found. LGTM from the runtime side. |
|
Heads-up: #4915 just merged a fix for the same two |
…machine When the Windows standalone installer runs as the SYSTEM account (SID S-1-5-18), which is common for ECS Workbench SSM, WinRM, and scheduled tasks, the default --path-scope is now 'machine' instead of 'user'. Previously, persisting PATH to the SYSTEM user's HKCU hive had no effect on new sessions spawned by the SSM agent service, because the agent inherits its environment at service start time and never re-reads HKCU. Writing to the Machine PATH (HKLM) ensures new sessions pick up the qwen shim immediately. Also adds: - --repair-path flag to fix PATH for an existing install without reinstalling - --path-scope user|machine CLI option and QWEN_INSTALL_PATH_SCOPE env var - Uninstaller now cleans both User and Machine PATH entries - CI validation step for hosted installation assets Resolves #4901
…st fixtures - Remove windows-self-hosted-qwen-smoke.yml (operational tool, not part of fix) - Revert sync-release-to-oss.yml validate step (redundant with JS behavior guards) - Extract duplicated test fixture strings into shared constants - Pass normalized path to Remove-PathEntry to avoid double Get-NormalizedPath - Fix stale comment referencing "user PATH" when scope is now configurable
f878072 to
9bd2bf1
Compare
| if /i "!REPAIR_PATH!"=="1" ( | ||
| call :RepairPath | ||
| if !ERRORLEVEL! NEQ 0 exit /b !ERRORLEVEL! | ||
| endlocal & set "PATH=%INSTALL_BIN_DIR%;%PATH%" |
There was a problem hiding this comment.
[Suggestion] After :RepairPath returns, this endlocal & set "PATH=%INSTALL_BIN_DIR%;%PATH%" creates a duplicate PATH entry in the session. Inside :RepairPath, set "PATH=!INSTALL_BIN_DIR!;!PATH!" already prepends the bin dir to PATH within the setlocal scope. Batch parses the entire &-joined line before executing any part, so %PATH% expands to the already-modified PATH — then endlocal restores the original PATH and set writes INSTALL_BIN_DIR;<PATH that already contains INSTALL_BIN_DIR>.
The normal install flow (line ~290) uses the same endlocal & set idiom without prior in-setlocal PATH modification, so this is specific to the --repair-path code path.
Consider removing set "PATH=!INSTALL_BIN_DIR!;!PATH!" from inside :RepairPath, since this caller line already handles the PATH escape via the endlocal pattern.
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
There was a problem hiding this comment.
No issues found. LGTM! The SYSTEM account auto-detection via SID S-1-5-18 is the canonical approach, input validation correctly includes the new env vars, and the uninstall script properly cleans both scopes with graceful error handling. Existing Suggestion-level comments from @qwen-code-ci-bot cover the minor issues (duplicate PATH entry, misleading repair message). -- qwen-code-review via Qwen Code /review
What this PR does
When the Windows standalone installer runs under the SYSTEM account (SID
S-1-5-18),PATH_SCOPEnow defaults tomachineinstead ofuser. This ensures theqwenshim is written to Machine PATH (HKLM) and discoverable in new terminal sessions spawned by service processes (SSM agent, WinRM, scheduled tasks). Also adds--repair-pathand--path-scope user|machineoptions so users can fix PATH for an existing install without re-downloading, and the uninstaller now cleans both User and Machine PATH entries.Why it's needed
When installing via ECS Workbench (SSM, runs as SYSTEM), the installer persists PATH to the SYSTEM user's HKCU registry hive. New SSM sessions are spawned by the SSM agent service, which loaded its environment at service start time and never re-reads HKCU — so the updated User PATH is never inherited. The result:
qwenworks in the install session but every subsequent session getsCommandNotFoundException. Writing to Machine PATH (HKLM) fixes this because Machine PATH is resolved from the registry at process creation time, regardless of how the session is spawned.Reviewer Test Plan
How to verify
irm https://qwen-code-assets.oss-cn-hangzhou.aliyuncs.com/installation/install-qwen-standalone.ps1 | iexqwen --versionworks in the current sessionqwen --version— should output0.17.1(previously:CommandNotFoundException)--repair-path: runinstall-qwen-standalone.bat --repair-path --path-scope machineon an existing install — should report "PATH repaired" without downloading$env:QWEN_INSTALL_PATH_SCOPE = 'user'; irm ... | iexshould persist to User PATH as beforeEvidence (Before & After)
Before (new SSM session after install):
After (new SSM session after install with this fix):
Tested on
Environment (optional)
ECS Workbench SSM connection (免密连接) to Windows Server instance (us-east-1), running as SYSTEM account (
C:\Windows\system32\config\systemprofile).Risk & Scope
userscope. If the machine has multiple users, the qwen bin dir will appear in all users' PATH — acceptable since standalone installs are typically single-purpose servers..ps1wrapper'sUpdate-CurrentSessionPathstill only patches the current process — it does not callSetEnvironmentVariableitself (that remains the.bat's responsibility).install-qwen-standalone.bat --repair-path --path-scope machineto fix without reinstalling.Linked Issues
Resolves #4901
中文说明
这个 PR 做了什么
当 Windows standalone 安装器检测到运行在 SYSTEM 账户下(SID
S-1-5-18)时,PATH_SCOPE默认改为machine而非user。这确保qwenshim 写入 Machine PATH (HKLM),新的 SSM/WinRM/计划任务 session 能正确发现qwen命令。同时新增--repair-path和--path-scope user|machine选项,方便用户修复已有安装的 PATH 而无需重新下载。卸载脚本现在同时清理 User 和 Machine 两级 PATH。为什么需要
通过 ECS Workbench SSM 安装时(以 SYSTEM 运行),安装器将 PATH 写入 SYSTEM 用户的 HKCU 注册表。但新的 SSM session 由 SSM agent 服务进程派生,该服务在启动时加载环境变量后不会重新读取 HKCU——因此更新后的 User PATH 永远不会被新 session 继承。表现为:安装 session 中
qwen可用,但每个新 session 报CommandNotFoundException。写入 Machine PATH (HKLM) 可修复此问题,因为 Machine PATH 在进程创建时总是从注册表读取。Reviewer 测试计划
如何验证
irm https://qwen-code-assets.oss-cn-hangzhou.aliyuncs.com/installation/install-qwen-standalone.ps1 | iexqwen --version正常qwen --version— 应输出0.17.1(之前:CommandNotFoundException)--repair-path:对已有安装执行install-qwen-standalone.bat --repair-path --path-scope machine,应显示 "PATH repaired" 且不下载$env:QWEN_INSTALL_PATH_SCOPE = 'user'; irm ... | iex应写入 User PATH证据(修复前后对比)
修复前(安装后新开 SSM session):
CommandNotFoundException修复后(安装后新开 SSM session):输出
0.17.1测试平台
环境
ECS Workbench SSM(免密连接),Windows Server 实例 (us-east-1),SYSTEM 账户。
风险与范围
userscope。.ps1wrapper 的Update-CurrentSessionPath仍然只修改当前进程内存中的 PATH。install-qwen-standalone.bat --repair-path --path-scope machine修复。关联 Issue
Resolves #4901