Conversation
Signed-off-by: caven <caven@minimaxi.com>
huangruiteng
left a comment
There was a problem hiding this comment.
审阅 exact head:93a7827d6b23156fd22f18f12e1997ac242dfeff
动机
这个修复针对一个真实且影响面不小的源码运行问题:scripts/loopx 原来只按 LOOPX_PYTHON -> .loopx-python -> python3 选择解释器,没有复用仓库已有的 scripts/loopx-python.sh。因此项目明明已经有可用的 Python 3.11+ .venv,只要未激活且系统 python3 偏旧,嵌套 CLI 调用仍会在进入 LoopX 前失败。
我用真实 launcher 验证了主路径:在 PATH=/usr/bin:/bin 且未设置 LOOPX_PYTHON 时,当前 head 会选中工作树 .venv/bin/python,scripts/loopx --format json version 正常返回 1.1.0。这个问题和修复方向都成立;但 exact head 尚未完整满足兼容性与本地门禁,因此当前不能批准。
改动思路
架构方向是对的:正常 release/source 布局把解释器决策交给已经存在的 selector;只有 companion helper 缺失的旧 snapshot 才保留旧逻辑。这样避免在 launcher 中再维护一套 .venv、版本化 Python、PATH 和 fallback 顺序。
最强的“不该直接合入”理由不在主路径,而在共享失败语义:helper 不只扩大成功候选,也带入了它自己的错误文本与“显式配置/记录值”区分。这个差异没有在 PR 的行为变化说明中体现,现有 launcher 回归测试也未随之迁移。
具体改动
关键代码讲解
scripts/loopx:66-70:helper 存在时执行loopx-python.sh并捕获唯一解释器路径。它把 Python 选择的决策权收敛到现有 owner,之后仍复用原有LOOPX_RELEASE_ROOT、PYTHONPATH和模块启动路径。scripts/loopx:72-86:helper 缺失时保留旧 snapshot 的LOOPX_PYTHON/.loopx-python/python3兼容路线,边界明确,没有再引入新持久状态。tests/test_loopx_launcher_python_selection.py:20-49:构造未激活的项目.venvstub,通过真实 Bash launcher 记录被选解释器;这个测试确实覆盖了作者声称的生产入口,而不是 mock selector 的返回值。
未来演进角度看,本 PR 已经采用了最有价值的相关重构:删除正常路径上的重复选择权。此处不需要再加新的 Python abstraction;接下来只需把 launcher 边界的失败语义和现有测试对齐。
对主干的风险
[P1] selector 复用改变了既有 launcher 失败契约,但没有迁移契约和回归测试
触发条件:把 LOOPX_PYTHON 指向不存在的可执行文件,然后让一个不满足 native scheduler fast-path 条件的命令经过 scripts/loopx。
基线会以 exit 2 返回并包含 configured Python executable not found;当前 head 仍是 exit 2,但 stderr 改成 does not resolve to a Python 3.11+ interpreter。这不是我从实现猜出的边角情况:
- 基线
0ea9b5b1c...上test_unix_launcher_keeps_unbound_calls_on_the_python_compatibility_route:1 passed; - 当前 exact head 的 launcher/discovery/scheduler 聚焦套件:
12 passed, 1 failed,失败正是tests/test_scheduler_native_launcher.py:97的既有诊断契约; - 新增
.venv正向测试和真实 version 入口均通过,所以这是与主修复不同的共享负向语义漂移。
最小修复有两种合法方向:要么在 launcher 边界保留既有 missing-executable 诊断;要么明确接受 selector 的更丰富语义,同时更新既有测试和 PR 行为变化说明,并分别覆盖“路径不存在”和“解释器存在但版本过旧”。修复后请重跑:
uv run --extra test python -m pytest -q tests/test_loopx_launcher_python_selection.py tests/test_python_discovery.py tests/test_scheduler_native_launcher.py
语义与 CI 对齐
静态证据通过:Ruff、bash -n、git diff --check 均为 green。标准 premerge 中 direct checks 与 2 个 catalog canary 通过,7/8 install/release risk smokes 通过;但 examples/install-local-smoke.py 在 120 秒超时,导致 exact-head premerge 总门禁为 failed。这个 timeout 尚未完成 base/head 归因,不能代替上面的确定性 blocker,也不能被算作通过;修复诊断契约后需在最终 head 重跑门禁并给出 readback。
我的整体评价
这是一个范围合适、owner 选择正确、正向真实路径已经成立的修复;没有新增持久状态、权限或调度语义,旧 snapshot fallback 也有明确兼容理由。但当前 head 对共享 launcher 错误语义产生了未披露漂移,并打破现有聚焦测试;同时 premerge 仍为 red。因此本轮结论为 REQUEST_CHANGES。修复上述 P1、刷新 exact-head 本地验证后,我可以复审。
English verdict: REQUEST_CHANGES - The selector reuse is the right architectural fix and the real project-venv path works, but this exact head silently changes a tested launcher failure diagnostic, leaving the focused suite at 12 passed / 1 failed; reconcile and disclose that contract, then rerun the focused suite and premerge gate.
Goal And Delivered Outcome
python3, whilescripts/loopx-python.shalready owned the supported precedence: explicitLOOPX_PYTHON, installer record, project.venv, then compatible installed interpreters. On hosts where systempython3is older than 3.11,python -m pytestcould therefore start under a valid project environment while nestedscripts/loopxcalls failed before reaching the CLI..venvpresent but not activated andPATH=/usr/bin:/bin, the launcher previously exited with “Python 3.11+ is required; selected Python is 3.9”. It now delegates selection to the existing helper and successfully runs the project interpreter.main. Related to the interpreter-selection ownership established in fix(runtime): discover Python interpreters without fixed minor lists #4482.中文说明
源码检出中的
scripts/loopx原先维护了第二套 Python 选择逻辑,未复用已有的scripts/loopx-python.sh。因此即使仓库已经创建 Python 3.11+ 的.venv,只要系统python3仍是 3.9,通过 pytest 间接调用 launcher 的 CLI 测试就会批量失败。本 PR 让 launcher 复用已有选择器,并增加“项目虚拟环境未激活、系统 Python 过旧”的回归测试。Scope And Continuation
scripts/loopxnow reuses the existing project/interpreter selector. A compatibility fallback remains for old release snapshots that do not contain the helper.Validation
93a7827d6b23156fd22f18f12e1997ac242dfeffregression_paritypassed.venvwith only an older systempython3on PATH;tests/test_loopx_launcher_python_selection.pyproves the project interpreter is selected.unitpassedpython -m pytest -q tests/test_loopx_launcher_python_selection.py tests/test_python_discovery.py: 8 passed.integrationpassedscripts/loopx: 322 passed, 1 skipped.real_entrypointpassedenv -u LOOPX_PYTHON PATH=/usr/bin:/bin scripts/loopx --format json versionreturns the checkout version through the project.venv.staticpassedbash -n scripts/loopx scripts/loopx-python.sh, andgit diff --check.integrationpassedloopx canary premerge --from-git-diff: 2 catalog checks and 8 install/release risk-profile smokes passed; no manual holds.integrationfailedFrontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
N/A. This PR does not claim progress against the TypeScript control-plane migration or Shared Goal Authority RFC.
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer.