Skip to content

test(install): isolate Claude smoke host roots - #5290

Merged
huangruiteng merged 1 commit into
mainfrom
codex/claude-install-smoke-home-isolation-20260929
Sep 30, 2026
Merged

huangruiteng merged 1 commit into
mainfrom
codex/claude-install-smoke-home-isolation-20260929

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

The Claude install smoke set a synthetic HOME but inherited host-root overrides such as CODEX_HOME. A normal default-install check could therefore write slash skills into the caller's live Codex home while still passing its Claude-only assertion.

The existing smoke now launches the real local installer with synthetic Codex, Claude, and OpenCode roots and no inherited LoopX install paths. It injects outside host-root overrides into the parent process, checks that the Codex and Claude /loopx skills land inside the fixture, and verifies the outside root remains untouched. The existing adapter-off, project/user scope, --harden, and permission-preservation checks remain in the same smoke. This repairs a pre-existing fixture gap observed during #5151 qualification; installer behavior is unchanged.

Validation: the real claude-install-optin-smoke.py passed twice, the adjacent Claude installer smoke passed, and Ruff, Python compilation, diff hygiene, and the changed-file public-boundary scan passed. The selected premerge catalog is still red: install-local-smoke.py exceeded its existing 120-second per-check limit on both attempts; one run of semantic-vocabulary-drift-smoke.py lacked npm development dependencies and passed after npm ci --ignore-scripts; codex-cli-packaged-install-smoke.py passed once and then hit a temporary-directory cleanup race on retry. The other selected canaries passed. These catalog checks do not import the changed smoke. Keep this draft unmerged until independent review and CI resolve the remaining validation gap.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
@huangruiteng
huangruiteng marked this pull request as ready for review September 29, 2026 11:01

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

评审 head:a97c75a705dbe53209f696354d6933b9eb5d23f7。结论:APPROVE,未发现本 PR 的阻断项。按 LoopX PR review capability 完成 smoke 价值及失败归因检查;未执行合并。

动机

仅设置临时 HOME 并不足以隔离安装 smoke:继承的 CODEX_HOME 等覆盖项仍能让真实 installer 写到测试目录以外。这个 PR 修复测试本身的写入边界,防止开发者验证产品时修改自己的已安装 skills,而不是把绿色 smoke 当作真实隔离证据。

改动思路

保留实际 bash installer 和 Claude installer 子进程,改为构造明确的 fixture 环境。父进程故意注入 fixture 外的各类根目录,子进程只收到允许的环境项;检查预期 skills 确实出现在 fixture 内,同时外部目录仅保留原有哨兵。project/user scope、dry-run、hooks opt-in 和权限保留测试继续走真实脚本。没有修改生产安装器,也没有给宿主安装更多权限。

具体改动

仅修改既有 smoke,增加 44 行、删除 15 行,没有新增一次性示例或生产模块。

关键代码讲解

  1. _isolated_host_env(examples/claude-install-optin-smoke.py:40)明确提供测试 HOME、PATH、shell 和三个宿主配置根,不再复制整个父进程环境。其职责是测试 fixture 隔离,不改变生产 installer 解释用户配置的方式。
  2. main(同文件:51)的默认安装分支为父进程设置外部根目录反例,再调用真实 install-local.sh;新增 Codex skill 的内部存在检查以及外部目录/哨兵不变检查。因此“测试没写任何东西”不能伪装成隔离成功。
  3. _install_py(同文件:36,未修改)的真实调用在 project/user 分支使用相同隔离环境。现有无 scope 拒绝、dry-run 无写入、project 不写全局目录、默认不写 hooks、harden 保留既有 deny 和用户 hook 的断言继续有效。显式 project harden 分支只写其临时 project;未扩大到全局宿主。

对主干的风险

实际 base/head 反例使用同一受控外部 Codex 根和相同未修改 smoke:基线 smoke 返回成功,却写入外部根的 14 个 skill 文件;head 同样返回成功,但外部根只剩原有哨兵,内容不变。这证明新增断言不是只重复检查成功码。基线为 ee1ea64b0aef45fdda81d2d7e48da356a1750eab,配对 fixture 指纹为 e11a812d36180feec10acb147092ce54d4422a0f1a7e8eb0cedfcf258773c48f。全部外部路径都属于临时合成 fixture,未触碰真实用户根。

修改后的原始 smoke 独立重跑通过,配对 head 的完整 smoke 也通过;相邻 no-system-mutation smoke、Ruff 和编译检查通过。首次原始运行曾在 candidate doctor 阶段失败,未获得细分原因;保留这一历史失败,不把它编造成外部故障。后续相同 head、未修改 smoke 的独立重跑及配对执行通过,独立 deep installation doctor 的 6 项 required 检查也通过;因此当前验收证据不是首次失败的成功码改写。

选中 premerge 的 4 项直接检查通过,6 项选中检查中 5 项通过。唯一剩余失败为未修改的 install-local-smoke 超时:同一命令、同一 120 秒预算在不可变 base 和本 head 均为 timed_out,returncode 为空且完整输出均为空。脚本及生产 install-local/Claude installer 的 base/head SHA-256 一致;该脚本不调用本 PR 修改的 smoke。结合上面的独立隔离反例通过,将此失败归为 pre_existing_unrelated,保留其安装验证预算排查,不增加 timeout 或宣称全部 canary 绿色。当前策略不查询或等待远端 CI。

已扫描 install-local、Claude no-system-mutation、slash installer 的既有覆盖及同作者近期安装相关 PR:它们没有覆盖此 smoke 子进程继承宿主根的漏洞;#5151 的生产安装 guard 修复、#5307 的 deprecated skills alias 清理也不是本次 fixture 隔离修复。增加断言位于原有 smoke,具备防止验证污染开发者环境的持续价值,不是重复批量 scaffolding。

我的整体评价

long_horizon:improved,反复执行安装验证不会积累测试外 skill 写入。user_experience:improved,维护者可以安全运行原有命令,scope、opt-in 和用户权限保留的产品合同不变。机制与问题相称;向前精简检查已把重复环境构造收束为局部 helper,无需提升成公共 capability。它不修改产品界面或共享控制面语义,不能据此声称整套 installer 性能验收完成。基线/head 的 120 秒超时须另行诊断,但不应让一个已经证明无关的红检查把本 PR 变成 request changes。

English verdict: APPROVE - a97c75a. Real baseline/head execution proves the smoke no longer writes inherited host roots; focused rerun, deep doctor and adjacent boundary validation passed. The unchanged installer canary timed out identically on immutable base and head and is separately attributed, not a PR regression. No merge or remote CI polling.

@huangruiteng
huangruiteng merged commit 039f1d0 into main Sep 30, 2026
41 of 53 checks passed
@huangruiteng
huangruiteng deleted the codex/claude-install-smoke-home-isolation-20260929 branch September 30, 2026 03:13
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.

1 participant