Skip to content

feat: add native Windows PowerShell support - #2769

Open
sxw228 wants to merge 5 commits into
huangruiteng:mainfrom
sxw228:codex/windows-pwsh
Open

feat: add native Windows PowerShell support#2769
sxw228 wants to merge 5 commits into
huangruiteng:mainfrom
sxw228:codex/windows-pwsh

Conversation

@sxw228

@sxw228 sxw228 commented Aug 4, 2026

Copy link
Copy Markdown

Summary

  • add a native PowerShell installer, launcher, and Windows-safe command and file-lock handling
  • keep release validation and self-update fail-closed on native Windows
  • make loopx doctor recognize uniquely routed Skills under both .codex/skills and .agents/skills, reject duplicate routes, and preserve external Skill management with -SkipSkills
  • document the native Windows workflow and add focused lifecycle regressions

Issue Or Task

  • Closes # n/a
  • Contributor task ID: n/a

Validation

  • python3 -m py_compile loopx/*.py (run with Python 3.11 on native Windows)
  • loopx check --scan-root . (clean public boundary scan; expected missing local registry warnings only)
  • Other: focused Windows/runtime suite: 48 passed
  • Other: real installed release 20260804T052114Z passed loopx doctor --deep; externally managed .agents/skills were ready and .codex/skills received no LoopX copies
  • Other: full native-Windows suite baseline: 1802 passed, 4 skipped, 124 failed in POSIX-only surfaces such as symlink privilege, executable-bit, SSH, benchmark, and extension assumptions

Boundary Checklist

  • I did not commit .loopx/, .codex/goals/, live ACTIVE_GOAL_STATE.md, credentials, private benchmark traces, verifier output, raw agent sessions, internal document links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to native Windows support and the installed-Skill diagnosis needed for its real runtime acceptance.

@huangruiteng

Copy link
Copy Markdown
Owner

Findings

[P1] A custom -InstallRoot produces a launcher that cannot find the installed release

scripts/install-windows.ps1 exposes -InstallRoot and passes it through to the Python installer. The installer then writes the active pointer to <InstallRoot>/current-release.json (loopx/windows_install.py:236-248), but the copied launcher only reads LOOPX_CURRENT_RELEASE_FILE or the fixed default $HOME/.local/share/loopx/current-release.json (scripts/loopx.ps1:4-7). Nothing in the install flow persists the custom pointer path for later shells.

The native regression currently masks this mismatch: after installing into a temporary non-default root, tests/test_windows_install.py:99-100 explicitly injects LOOPX_CURRENT_RELEASE_FILE before invoking the launcher. A real user following the public contract can run the installer successfully with a custom root, start a fresh shell, and then have loopx doctor fail because the launcher searches the default root instead.

Please make the launcher and pointer location one contract. For example, generate or configure the installed launcher with the selected pointer path, or store the pointer at a location deterministically discoverable from the launcher. Then add a native Windows regression that installs with a non-default -InstallRoot and invokes the installed launcher in a fresh environment without LOOPX_CURRENT_RELEASE_FILE.

Open questions / residual risk

The candidate-validation failure test proves that the pointer is preserved when validation fails, but post-validation promotion currently updates Skills and the launcher before writing the pointer (loopx/windows_install.py:233-248). Please clarify whether the fail-closed guarantee includes failures during those late promotion steps; otherwise an install error can leave the old runtime pointer beside partially updated user-facing assets.

The PR body also reports 124 failures in the full native-Windows suite. Their stated POSIX-only categories are plausible, but there are no GitHub checks or a checked-in expected-failure classification to distinguish unsupported surfaces from regressions. Before merge, please provide a bounded list or machine-readable skip/xfail boundary for the Windows-supported product surface.

Product / architecture judgment

The motivation is sound: native Windows users need a PowerShell entry and release lifecycle that does not depend on Bash, POSIX symlinks, or fcntl. The new command_invocation and file_lock modules are useful reusable boundaries, and keeping automatic archive update/rollback fail-closed is the right choice. Accepting uniquely routed Skills from either .codex/skills or .agents/skills also makes -SkipSkills operational rather than cosmetic.

The 1,339 additions / 209 deletions are substantial but mostly justified by the shipped lifecycle: 876 additions / 201 deletions are product, scripts, and Skill guidance, while 463 additions / 8 deletions are focused tests. I do not see a safe architectural reduction that preserves native install, launcher, lock, doctor, and release validation behavior. The custom-root bug is a missing lifecycle connection, not evidence that the overall design should be replaced.

Validation

  • Reviewed exact head 0087518d57777f1aaeadc0212c9720a44d5f45d5 against current origin/main.
  • Focused local suite: 17 passed, 5 native-Windows-only tests skipped on macOS.
  • Standard premerge canary: 17 selected checks passed, 0 failures, 0 warnings, no manual holds.
  • Changed Python files compiled successfully; git diff --check passed.
  • Merge-tree against current origin/main is clean.
  • Native PowerShell execution could not be rerun locally because this host has no pwsh; the PR body reports 48 focused native-Windows tests passed.
  • GitHub currently reports no status checks for this PR.

Merge decision

Hold / request changes: connect custom -InstallRoot to launcher discovery and cover the real fresh-shell invocation path before merge. The broader architecture is directionally good.

@huangruiteng
huangruiteng self-requested a review August 4, 2026 07:49

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:建议暂缓并请求修改。[P1] 自定义 -InstallRoot 安装成功后,新的 PowerShell launcher 在 fresh shell 中找不到实际 release pointer;此外当前 head 与 mainloopx/file_lock.py 存在内容冲突。

动机

这个 PR 要让 LoopX 在原生 Windows + PowerShell 7 上拥有完整的安装、启动、doctor、自更新和文件锁路径,不再依赖 Bash、POSIX 可执行位、symlink 或 fcntl。同时,它把 Skills 的有效安装位置扩展为 .codex/skills.agents/skills 的唯一有效路由,使 -SkipSkills 能保留外部管理的 Skill。这个需求来自真实的 Windows 安装和运行调用链;不合并意味着 Windows 用户仍缺少受支持的 release lifecycle。

改动思路

正向链路是:install-windows.ps1 将安装参数交给 loopx.windows_install,后者复制并验证候选 release,再安装 Skills、复制 loopx.ps1 launcher、写入 current-release.json;launcher 读取 pointer 后用隔离 Python 启动 scripts/loopx_entry.pycommand_invocation.pyfile_lock.py 分别抽出跨平台命令和锁边界,doctor 与 self-update 复用这些边界。失败链路原则上保持 fail-closed:候选验证失败时不切换当前 release pointer。

具体改动

精确 head 0087518d57777f1aaeadc0212c9720a44d5f45d5 相对原 base 共 18 个文件、1339+ / 209-。其中生产代码、脚本和 Skill 指南约 876+ / 201-,聚焦测试约 463+ / 8-。主要新增 windows_install.py(289 行)、PowerShell installer/launcher(93 行)和跨平台命令、锁适配;doctor、issue-fix/domain-state 与 self-update 改为消费这些边界。测试覆盖候选验证、安装、doctor、外部 Skills 路由、命令调用和跨进程锁。

对主干的风险

[P1] scripts/install-windows.ps1:6,34 公开接受并传递自定义 -InstallRootloopx/windows_install.py:238-248 将 pointer 写到该 root;但复制后的 scripts/loopx.ps1:4-7 只读取环境变量或固定的 $HOME/.local/share/loopx/current-release.json。真实用户在自定义 root 安装后打开 fresh shell,launcher 会查错位置并在运行 doctor 前失败。tests/test_windows_install.py:99-111 手工注入 LOOPX_CURRENT_RELEASE_FILE,恰好掩盖了这个合同断裂。最小修复是让安装后的 launcher 可确定性发现所选 pointer,并增加“不设置该环境变量”的 native Windows 回归。

当前 GitHub 还将 PR 标为 CONFLICTING;本地 merge-tree 定位到 loopx/file_lock.py,合并前必须基于最新 main 解决。另一个残余风险是 Skills/launcher 在 pointer 写入前更新,晚期 promotion 失败可能留下旧 runtime pointer 与部分新用户资产;建议明确该失败合同。PR 当前没有 GitHub checks,且本机没有 pwsh,因此无法独立复跑 native PowerShell 路径。

我的整体评价

方向和跨平台边界设计是合理的,体量虽大但与完整 Windows release lifecycle 基本匹配;我没有看到能保留同等行为与验证的安全大幅删减。验证结果:聚焦测试 17 passed, 5 skipped;标准 premerge canary 17 项全部通过;Python compile 与 diff check 通过。由于 custom-root 的公开安装合同仍会产生“安装成功、fresh shell 无法启动”的假成功,当前应保持 REQUEST_CHANGES;修复该链路、补 native 回归并解决 merge conflict 后再复审。

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: REQUEST_CHANGES

head 0087518d57777f1aaeadc0212c9720a44d5f45d5。这是对 feat: add native Windows PowerShell support 的完整 PR 解读,覆盖 18 个改动文件、Windows launcher/installer、跨平台命令与文件锁、doctor skill 路由、self-update 和测试。功能方向完整,但当前 head 有 merge conflict 且没有任何 GitHub checks,不能合并。

动机

LoopX 目前以 POSIX 脚本和 fcntl/executable-bit 假设为主,Windows 用户无法用原生 PowerShell 安装和运行。这个 PR 的目标是补齐 Windows 原生路径:PowerShell installer、loopx.ps1 wrapper、Windows-safe command invocation、file lock、self-update 和 doctor 对 .codex/skills / .agents/skills 双 skill 路由的诊断。

改动思路

按“原生 Windows 入口 + 共享跨平台抽象”展开:

  • 新增 command_invocation.py,统一解析 executable 与 .ps1
  • 新增 windows_install.pyscripts/install-windows.ps1,提供 PowerShell 安装;
  • scripts/loopx.ps1 + scripts/loopx_entry.py 作为 Windows wrapper;
  • file_lock.pydomain_state.pyissue_fix.pyself_update.py 等移除 POSIX-only 假设;
  • doctor.py 增加 Windows install command 与 skill route 检测;
  • 新增 Windows/runtime 聚焦测试。

具体改动

  • loopx/command_invocation.pyresolve_command_path() / command_argv()
  • loopx/windows_install.py:Windows 安装主逻辑。
  • scripts/install-windows.ps1 / scripts/loopx.ps1 / scripts/loopx_entry.py:PowerShell installer 与 launcher。
  • loopx/file_lock.pydomain_state.pydomain_packs/issue_fix.py:跨平台 file lock / domain state。
  • loopx/doctor.py:Windows install command、release root、skill route 诊断。
  • loopx/release_candidate.pyself_update.pyslash_command_install.py:Windows fail-closed 调整。
  • skills/loopx-project/SKILL.md:Windows 使用说明。
  • 测试:test_windows_install.pytest_command_invocation.pytest_file_lock_cross_process.pytest_self_update_runtime_activation.pytest_doctor_install_freshness.py 等。

关键代码讲解

  1. resolve_command_path():Windows 下先检查 LOOPX_COMMAND_PATH,再用 shutil.which,最后在 PATH 中找 .ps1;非 Windows 保持 which 行为。
  2. command_argv():对 .ps1 通过 pwsh -NoLogo -NoProfile -File 启动;pwsh 缺失时抛 FileNotFoundError
  3. exclusive_file_lock:从 fcntl 移植到跨平台文件锁,POSIX 与 Windows 共用同一调用面。
  4. local_install_command():Windows 返回 pwsh -File install-windows.ps1 -Python <python>,POSIX 保持 install-local.sh。
  5. doctor skill route:同时识别 .codex/skills.agents/skills,拒绝重复 route,保留外部 skill 管理。

正向路径

Windows 用户运行 pwsh -File scripts/install-windows.ps1;安装完成后 loopx.ps1 通过 command_argv 启动 loopx_entry.pyloopx doctor 能识别 release root 和双 skill route;file lock、self-update、domain state 不再依赖 fcntl

负向路径

pwsh 不在 PATH 时,.ps1 调用明确抛错而不是静默失败;mcp.json / skill route 冲突或重复时 doctor fail-closed;Windows 上 release validation 与 self-update 保持只读/失败封闭。当前 head 的 merge-tree 冲突也说明需要先解决 file_lock.pytest_doctor_install_freshness.py 的冲突。

对主干的风险

P1,必须修复:

  1. merge conflict。 git merge-tree origin/main HEADloopx/file_lock.pytests/test_doctor_install_freshness.py 产生 content conflict,mergeStateStatus=DIRTY / mergeable=CONFLICTING。必须 rebase 到最新 main 并解决冲突。
  2. 没有 GitHub checks。 statusCheckRollup 为空。这是跨平台 runtime/installer 大改动,必须让 build/pytest 在 CI 上跑出结果;当前本地测试在 macOS 上还跳过了一部分 Windows 专属用例(17 passed / 5 skipped),不能替代真实 Windows/CI 验证。
  3. Windows 原生验证不足。 PR body 提到 full native-Windows baseline 有 124 个 POSIX-only 失败。需要确认这些失败都是既有 POSIX 假设,而不是这个 PR 引入的新回归,并至少让 Windows 相关 suite 在 CI 中稳定通过。

P2:

  • LOOPX_COMMAND_PATH 作为环境变量覆盖入口,建议确认它只影响本地 wrapper,不进入公共协议/权限边界。
  • 新增 PowerShell 脚本应做路径注入与引号审查,避免用户可控路径拼接进 pwsh 命令。

我的整体评价

这个 PR 解决了一个真实缺口,Windows launcher、跨平台 file lock 和 doctor skill 路由的方向都合理;本地聚焦测试与 ruff 也通过。但当前无法合并:两个 merge conflict、无 CI checks、Windows 测试未在 CI 独立验证。先 rebase、解决冲突、让 Windows-focused CI 绿,再重新评审。


English Verdict

Request changes. Head 0087518d57777f1aaeadc0212c9720a44d5f45d5. The native Windows support design is reasonable and 17 focused tests pass locally (5 skipped), but the PR cannot merge: merge conflicts exist in loopx/file_lock.py and tests/test_doctor_install_freshness.py, no GitHub status checks are present, and Windows-specific tests are skipped locally. Rebase, resolve conflicts, add real Windows/CI validation, and re-review. Non-blocking note: review PowerShell path quoting and LOOPX_COMMAND_PATH usage.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

详细中文评审(重审确认)

精确评审头: 2769@0087518d57777f1aaeadc0212c9720a44d5f45d5

动机

按全队列重审要求复核本 PR(Windows PowerShell 原生支持,18 个文件)。exact head 自 08-08 的 CHANGES_REQUESTED 后未更新(updatedAt 2026-08-08T21:53:56Z),阻断项仍待作者修复。

改动思路

PR 引入 PowerShell launcher、windows_install 模块与安装脚本,方向合理,但存在两个未解决阻断项。

具体改动(关键内容讲解)

  • P1(blocking,安装路径):自定义 -InstallRoot 安装成功后,fresh shell 中新的 PowerShell launcher 找不到实际 release pointer(08-04 评审确认)。
  • P1(blocking,冲突):当前 head 与 mainloopx/file_lock.py 存在内容冲突,未合并前需 rebase 解决(08-04 评审确认)。
  • head 未变:自 08-08 完整复审(覆盖 18 个文件)后无新 commit,两个阻断项均未解决。

对主干的风险

Windows 安装路径错误会让用户安装“成功”但命令不可用,属于高影响缺陷;file_lock 冲突若不处理会破坏跨进程锁语义。风险高。

我的整体评价

REQUEST_CHANGES(确认既有结论)。 修复要求:(1) 修复 -InstallRoot 后 launcher 的 release pointer 解析;(2) rebase 并解决 file_lock.py 冲突;(3) 补充 Windows fresh-shell 安装回归验证。修复后需 exact-head 复审。


English Verdict

REQUEST_CHANGES — exact head 0087518d57777f1aaeadc0212c9720a44d5f45d5.

Confirms the existing blocking findings: custom -InstallRoot installs leave the PowerShell launcher unable to find the release pointer in a fresh shell, and the head conflicts with main in loopx/file_lock.py. No new commits since the 08-08 review; both blockers remain. Fix and rebase, add a fresh-shell Windows regression, then re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants