test: discover compatible Python for Node-driven smokes - #4980
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Reviewed exact head 86c1f5839717ad47f3d8d904c609bc98bcf8b83d.
动机
要解决的是 Node/TypeScript 测试和浏览器 smoke 绕过工作树 Python 环境、误用系统 Python 3.9 的反复故障,并用测试阻止新绕过。共享选择器是合适方向;目前这份 PR 已修好一批入口,但未覆盖完整的活跃 TS 测试路径。
改动思路
scripts/test-python.mjs 先处理显式覆盖,再优先使用工作树 .venv;POSIX 复用已有 loopx-python.sh,Windows 则搜索并探测候选解释器。它用 sys.executable 和版本信息拒绝不兼容候选。多个 TS 测试、打包版浏览器 smoke 和 fixture exporter 改用该函数;新单测、类型声明、Windows CI 步骤以及中英双语开发文档一并落地。这个边界没有引入 Goal 状态或运行时授权。
具体改动
关键代码讲解
probe(scripts/test-python.mjs:9)验证候选确为 Python 3.11+;显式覆盖无效时resolveTestPython(:46)报错,不悄悄回退。test_python_runtime.test.ts验证版本、覆盖优先级、工作树 venv 和裸python3扫描;5 个测试及 TS 类型检查通过。受改动的 coordination 测试在没有python别名的 PATH 下仍能通过。- 问题在新的源扫描(
test_python_runtime.test.ts:80):正则只匹配字面量python3。活跃的monitor_successor.test.ts:60与quota_monitor_poll_commit.test.ts:624仍直接spawnSync("python", ...),完全绕开选择器。
对主干的风险
P1 阻断:只要 PATH 中没有 python 别名(即使工作树 .venv 存在),npm run test:control-plane 的这两处调用仍可能失败;若 PATH 指向旧解释器,则可能再次落到错误 Python。实测用限制后的 PATH 跑 monitor_successor 的 repository-codec 测试,子进程状态为 null、测试退出 1;同样环境下已改用选择器的 coordination 测试通过,而新增的 5 个选择器测试仍全部通过。因此当前单测约束不能兑现“以后不再绕过”的目标。请将这两处改用 resolveTestPython,并让源扫描拦截直接启动或回退到裸 python 和 python3(不要误伤作为数据的 validation argv);再在受限 PATH 下重跑对应测试。
此外,git diff --check 通过,但与当前 origin/main 的 merge-tree 在 examples/personal-workspace-browser/fixture.mjs 有内容冲突,合并前需要更新分支并解决。原生 Windows 和浏览器视觉 smoke 本地未验证;当前 Goal 配置 wait_for_ci=false,本评审没有查询远端 CI。
语义与 CI 对齐
没有改变 Goal/配额等共享状态语义;问题是测试入口覆盖与文档所声明的默认行为不一致。保留已有 launcher/覆盖优先级是合理的,缺口在未迁移的活跃 caller 和过窄的机器约束。
我的整体评价
REQUEST_CHANGES:核心设计和已迁移路径成立,范围也基本合适,但复现出的两个绕过点使原始故障仍可发生。完成上述最小修复并解决主干冲突后,再对更新 head 重新做全量结论。
English verdict: REQUEST_CHANGES - exact head 86c1f58 leaves two active bare-python subprocesses outside the new guard; a restricted-PATH TS test fails despite 5/5 selector tests passing, and the branch conflicts with current main.
86c1f58 to
27d9a55
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head: 27d9a5575ff8d1aac7a7adbe233cf86843ce6396
动机
要解决的是 Node/TypeScript 测试与浏览器 smoke「各自决定用哪个 Python」造成的漂移:spawnSync("python", ...)、process.env.LOOPX_PYTHON_BIN || "python3" 这类写法在本机没有 python 别名时会让子进程以 null 状态失败,而在别名存在但版本过旧时又会静默用错解释器——即使工作树里已经有兼容的 .venv。
上一轮对旧 head 的 review 判了 REQUEST_CHANGES,理由正是这个目标还没兑现:monitor_successor.test.ts 与 quota_monitor_poll_commit.test.ts 仍直接启动裸解释器,而机器约束(源扫描)只认字面量 python3,因此「以后不再绕过」的承诺不成立。本轮修的就是这条缺口,并顺带处理了 rebase 到当前 main 时出现的冲突与新调用点。
改动思路
核心是把「用哪个解释器」收敛成一个选择器 scripts/test-python.mjs,并把兼容性判据交给候选解释器自己报告:显式覆盖(LOOPX_TEST_PYTHON → LOOPX_PYTHON_BIN → LOOPX_PYTHON)优先,其次工作树 .venv,POSIX 上复用已有的 scripts/loopx-python.sh 优先级,Windows 上按已记录指针、版本化 PATH 候选与 py -3 探测;所有候选都必须报告绝对 sys.executable 且版本 ≥ 3.11。显式覆盖解析失败会直接抛错,不会悄悄回退——这是刻意选择的 fail-fast,而不是兼容性回退。
第二层是机器约束:静态扫描把这些目录里「裸别名直启 / ??、|| 回退 / const x = "python" 默认值」都判为违规,同时用负例保证「解析后的绝对路径」和「validation argv 数据」不被误伤。这样规则不再依赖每个新调用点自觉。
第三层是边界处理:rebase 到当前 main 时 examples/personal-workspace-browser/fixture.mjs 有冲突,main 已经把这段改成用隔离标志(-I)直接启动 chat server。冲突解决保留了 main 的新实现与隔离语义,只把解释器换成选择器结果;因为 resolveTestPython() 本身仍把 LOOPX_PYTHON_BIN 当显式覆盖,两者语义一致——显式安装的解释器仍然按它自己的包解析。
具体改动
scripts/test-python.mjs(+77)与 scripts/test-python.d.mts(+7)是唯一的新production模块;tsconfig.control-plane.json 让测试可 import 它;.github/workflows/python-tests.yml(+3)增加原生 Windows 探测步骤;docs/development/testing-and-quality.md(+17)与 skills/loopx-self-repair/references/repair-patterns.md(+1)记录契约;其余是十二个 examples/smoke 的两行调用点替换与六个 TS 测试/conformance 的解释器替换,外加 test_python_runtime.test.ts(+115)的守卫与选择器测试。
关键代码讲解
probe(scripts/test-python.mjs:9)是唯一的兼容性判据:候选必须零退出、报告绝对 sys.executable、且 version_info >= (3, 11),任何一个不满足都只算「这次没找到」,不会把旧解释器混进来。resolveTestPython(:46)负责优先级并在无解时抛出一条可执行的错误(提示 uv sync --extra test 或设置 LOOPX_TEST_PYTHON)。
tests/control_plane_ts/test_python_runtime.test.ts:77 的守卫把「不再绕过」变成可失败的断言:正则分别覆盖直启(:83)、回退(:84)与赋默认值(:85)三种形态,且同时匹配 python 与 python3;负例断言 validation_command_argv: ["python", ...]、/usr/bin/python3、/opt/loopx-qualification/bin/python 都不算违规,正是为了不误伤「把路径当数据」的既有测试。
两处被上一轮点名的 caller 现在都用选择器:monitor_successor.test.ts:61 的 repository-identity oracle 与 quota_monitor_poll_commit.test.ts:625 的旧指纹 oracle。前者同时把变量从 python 改名为 oracle,因为它装的是子进程结果而不是解释器。rebase 后新出现的 examples/chat-bundle-upgrade-browser-smoke.mjs:46 也一并迁移——它本来正是被放宽后的守卫抓出来的。
对主干的风险
最需要防的回归是「选择器挑到了与调用点原本语义不同的解释器」。具体场景是打包版浏览器 smoke:原来 process.env.LOOPX_PYTHON_BIN || "python3" 把显式安装的解释器排在前面,如果选择器忽略它,就会改用 checkout 里的包,验证对象变了。实现上这条被两层挡住——覆盖变量在探测顺序最前,且 fixture.mjs 在设置了 LOOPX_PYTHON_BIN 时仍传 -I,让安装版解释器按自己的包解析。
第二类风险是守卫误报:把「路径/命令当数据」的既有测试判成违规,会让改动变成噪音。本轮用负例断言固定了边界,并且守卫是在实际扫出 chat-bundle-upgrade-browser-smoke.mjs 之后才补上该迁移的——也就是说这条约束确有抓到真问题,而不是只对本次改动自证。
第三是证据边界:原生 Windows 分支只在新增的 windows-powershell CI 步骤里执行,本机是 macOS;浏览器 smoke 只做了调用点替换与语法检查,没有在本 head 上跑完整浏览器断言。本轮 review 依据该 Goal 解析出的 wait_for_ci=false,没有等待远端 CI,因此这两项在结论里保持未验证,而不是当作已通过。
可回滚性没有问题:显式覆盖(LOOPX_TEST_PYTHON)可以按次指定解释器,revert 本 PR 也只是恢复各调用点原有的解析方式;本改动不触碰 loopx/** 运行时行为、权限或持久化状态。
我的整体评价
方向正确、边界合适,而且上一轮 blocking 的缺口这一轮确实补上了:两处活跃的裸 python 调用点已迁移,守卫从「只认字面量 python3」扩到「python/python3 的直启、回退与默认值」,并带有防误伤的负例。
我按上一轮的复现路径做了独立验证:在受限 PATH(无 python 别名)下,monitor_successor.test.ts 5/5、quota_monitor_poll_commit.test.ts 19/19 通过;把任一处改回裸解释器会各自失败 1 条(4/5),把 tab-upgrade smoke 改回 || "python3" 会让守卫失败 1 条(4/5)——即原始故障模式确实被覆盖。完整 npm run test:control-plane 在本 head 上为 3037 tests / 3009 passed / 0 failed / 28 skipped;typecheck:control-plane、git diff --check、node --check 与 loopx canary premerge --from-git-diff(含 26 个改动路径的 public-boundary 扫描)均为 0 失败。需要说明的是,早前在机器负载很高时该完整套件出现过 effect runtime 连接被拒的成片失败,逐文件隔离重跑(authority_store 287、nokv_authority_store 292、nokv_jsonl_transport 291、sqlite_capacity 9、todo_continuation 36 等)全部通过,随后完整套件也全绿,因此判断为环境争用而非本 diff 引入。
建议合并,并在 Windows CI 回收后核对 windows-powershell 的结果。
English verdict: APPROVE - The selector, the migrated callers and the widened guard now form one enforced contract: a restricted-PATH rerun fixes the previously failing bare-python oracle, each deliberate mutation of a migrated call site fails a test, the full control-plane suite is green on this head, and the rebase keeps main's isolated chat-server body while routing the interpreter through the same selector.
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
The previous head still left active callers on a bare interpreter, so the original failure mode stayed reachable even with a compatible worktree environment present, and the static guard only recognized literal `python3`. Route the remaining callers through resolveTestPython(): the repository identity oracle in monitor_successor, the legacy fingerprint oracle in quota_monitor_poll_commit, and the tab-upgrade chat server smoke that arrived on main while this branch was open. Widen the guard to reject bare `python` and `python3` launches, fallbacks and assigned defaults, and add negative assertions so resolved paths such as `/usr/bin/python3` and validation argv data stay allowed. Validated with the full control-plane suite (3009 passed, 0 failed, 28 skipped), the guard and typecheck, and a restricted-PATH rerun where a bare `python` mutation fails monitor_successor while the selector passes both affected files. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
27d9a55 to
0b1de3a
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head: 0b1de3a909571521306a1485ea4e70bb4ab51fa1
动机
要解决的是 Node/TypeScript 测试与浏览器 smoke「各自决定用哪个 Python」造成的漂移:spawnSync("python", ...)、process.env.LOOPX_PYTHON_BIN || "python3" 这类写法在本机没有 python 别名时会让子进程以 null 状态失败,而在别名存在但版本过旧时又会静默用错解释器——即使工作树里已经有兼容的 .venv。
上一轮对旧 head 的 review 判了 REQUEST_CHANGES,理由正是这个目标还没兑现:monitor_successor.test.ts 与 quota_monitor_poll_commit.test.ts 仍直接启动裸解释器,而机器约束(源扫描)只认字面量 python3,因此「以后不再绕过」的承诺不成立。本轮修的就是这条缺口,并顺带处理了 rebase 到当前 main 时出现的冲突与新调用点。
改动思路
核心是把「用哪个解释器」收敛成一个选择器 scripts/test-python.mjs,并把兼容性判据交给候选解释器自己报告:显式覆盖(LOOPX_TEST_PYTHON → LOOPX_PYTHON_BIN → LOOPX_PYTHON)优先,其次工作树 .venv,POSIX 上复用已有的 scripts/loopx-python.sh 优先级,Windows 上按已记录指针、版本化 PATH 候选与 py -3 探测;所有候选都必须报告绝对 sys.executable 且版本 ≥ 3.11。显式覆盖解析失败会直接抛错,不会悄悄回退——这是刻意选择的 fail-fast,而不是兼容性回退。
第二层是机器约束:静态扫描把这些目录里「裸别名直启 / ??、|| 回退 / const x = "python" 默认值」都判为违规,同时用负例保证「解析后的绝对路径」和「validation argv 数据」不被误伤。这样规则不再依赖每个新调用点自觉。
第三层是边界处理:rebase 到当前 main 时 examples/personal-workspace-browser/fixture.mjs 有冲突,main 已经把这段改成用隔离标志(-I)直接启动 chat server。冲突解决保留了 main 的新实现与隔离语义,只把解释器换成选择器结果;因为 resolveTestPython() 本身仍把 LOOPX_PYTHON_BIN 当显式覆盖,两者语义一致——显式安装的解释器仍然按它自己的包解析。
具体改动
scripts/test-python.mjs(+77)与 scripts/test-python.d.mts(+7)是唯一的新production模块;tsconfig.control-plane.json 让测试可 import 它;.github/workflows/python-tests.yml(+3)增加原生 Windows 探测步骤;docs/development/testing-and-quality.md(+17)与 skills/loopx-self-repair/references/repair-patterns.md(+1)记录契约;其余是十二个 examples/smoke 的两行调用点替换与六个 TS 测试/conformance 的解释器替换,外加 test_python_runtime.test.ts(+115)的守卫与选择器测试。
关键代码讲解
probe(scripts/test-python.mjs:9)是唯一的兼容性判据:候选必须零退出、报告绝对 sys.executable、且 version_info >= (3, 11),任何一个不满足都只算「这次没找到」,不会把旧解释器混进来。resolveTestPython(:46)负责优先级并在无解时抛出一条可执行的错误(提示 uv sync --extra test 或设置 LOOPX_TEST_PYTHON)。
tests/control_plane_ts/test_python_runtime.test.ts:77 的守卫把「不再绕过」变成可失败的断言:正则分别覆盖直启(:83)、回退(:84)与赋默认值(:85)三种形态,且同时匹配 python 与 python3;负例断言 validation_command_argv: ["python", ...]、/usr/bin/python3、/opt/loopx-qualification/bin/python 都不算违规,正是为了不误伤「把路径当数据」的既有测试。
两处被上一轮点名的 caller 现在都用选择器:monitor_successor.test.ts:61 的 repository-identity oracle 与 quota_monitor_poll_commit.test.ts:625 的旧指纹 oracle。前者同时把变量从 python 改名为 oracle,因为它装的是子进程结果而不是解释器。rebase 后新出现的 examples/chat-bundle-upgrade-browser-smoke.mjs:46 也一并迁移——它本来正是被放宽后的守卫抓出来的。
对主干的风险
最需要防的回归是「选择器挑到了与调用点原本语义不同的解释器」。具体场景是打包版浏览器 smoke:原来 process.env.LOOPX_PYTHON_BIN || "python3" 把显式安装的解释器排在前面,如果选择器忽略它,就会改用 checkout 里的包,验证对象变了。实现上这条被两层挡住——覆盖变量在探测顺序最前,且 fixture.mjs 在设置了 LOOPX_PYTHON_BIN 时仍传 -I,让安装版解释器按自己的包解析。
第二类风险是守卫误报:把「路径/命令当数据」的既有测试判成违规,会让改动变成噪音。本轮用负例断言固定了边界,并且守卫是在实际扫出 chat-bundle-upgrade-browser-smoke.mjs 之后才补上该迁移的——也就是说这条约束确有抓到真问题,而不是只对本次改动自证。
第三是证据边界:原生 Windows 分支只在新增的 windows-powershell CI 步骤里执行,本机是 macOS;浏览器 smoke 只做了调用点替换与语法检查,没有在本 head 上跑完整浏览器断言。本轮 review 依据该 Goal 解析出的 wait_for_ci=false,没有等待远端 CI,因此这两项在结论里保持未验证,而不是当作已通过。
可回滚性没有问题:显式覆盖(LOOPX_TEST_PYTHON)可以按次指定解释器,revert 本 PR 也只是恢复各调用点原有的解析方式;本改动不触碰 loopx/** 运行时行为、权限或持久化状态。
我的整体评价
方向正确、边界合适,而且上一轮 blocking 的缺口这一轮确实补上了:两处活跃的裸 python 调用点已迁移,守卫从「只认字面量 python3」扩到「python/python3 的直启、回退与默认值」,并带有防误伤的负例。
我按上一轮的复现路径做了独立验证:在受限 PATH(无 python 别名)下,monitor_successor.test.ts 5/5、quota_monitor_poll_commit.test.ts 19/19 通过;把任一处改回裸解释器会各自失败 1 条(4/5),把 tab-upgrade smoke 改回 || "python3" 会让守卫失败 1 条(4/5)——即原始故障模式确实被覆盖。完整 npm run test:control-plane 在本 head 上为 3037 tests / 3009 passed / 0 failed / 28 skipped;typecheck:control-plane、git diff --check、node --check 与 loopx canary premerge --from-git-diff(含 26 个改动路径的 public-boundary 扫描)均为 0 失败。需要说明的是,早前在机器负载很高时该完整套件出现过 effect runtime 连接被拒的成片失败,逐文件隔离重跑(authority_store 287、nokv_authority_store 292、nokv_jsonl_transport 291、sqlite_capacity 9、todo_continuation 36 等)全部通过,随后完整套件也全绿,因此判断为环境争用而非本 diff 引入。
建议合并,并在 Windows CI 回收后核对 windows-powershell 的结果。
English verdict: APPROVE - The selector, the migrated callers and the widened guard now form one enforced contract: a restricted-PATH rerun fixes the previously failing bare-python oracle, each deliberate mutation of a migrated call site fails a test, the full control-plane suite is green on this head, and the rebase keeps main's isolated chat-server body while routing the interpreter through the same selector.
Merge note (maintainer self-merge)Changed surfaces: one new source-checkout test helper ( Decision evidence for this head (
Manual holds and unverified surfaces: the native Windows discovery branch is exercised only by Separately, the |
Goal And Delivered Outcome
python3(including 3.9) even when the source checkout had a compatible test environment.python3subprocess defaults in these source surfaces.main.Scope And Continuation
Review Follow-Up
python3.resolveTestPython(); rebasing onto currentmainsurfaced a third one (examples/chat-bundle-upgrade-browser-smoke.mjs) which is migrated in the same commit.pythonandpython3launches, fallbacks and assigned defaults, with negative assertions for resolved paths and validation argv data.tests/control_plane_ts/monitor_successor.test.tswas also renamed the local variable frompythontooraclesince it holds the subprocess result.main; theexamples/personal-workspace-browser/fixture.mjsconflict keeps main's chat-server body and isolation flag while selecting the interpreter throughresolveTestPython().Validation
0b1de3a909571521306a1485ea4e70bb4ab51fa1(rebased onto currentmain; the earlier green full-suite run was on the identical diff immediately before main added three unrelated quota commits).unitpassednode --no-warnings --experimental-strip-types --test tests/control_plane_ts/test_python_runtime.test.ts: 5 tests, including Python 3.9 rejection, worktree precedence, override failure, source provenance, and static guard mutation checks.regression_paritypassedcoordination_state_contract.test.ts: 12 tests with the system Python 3.9 directory first on PATH; the child still used the compatible worktree interpreter.integrationpassedunitpassedstaticpassednpm run typecheck:control-plane; syntax checks for modified.mjs;git diff --check; changed-path public/private scan.staticpassedloopx canary premerge --from-git-diff --git-diff-base <PR merge-base> --goal-id loopx-meta: 19 selected checks passed, zero failures or manual holds; exact-scope quality receiptcqr_e3430f377e2494fbd133was verified. The merge-base is pinned becausemainadvanced during earlier long-running checks.integrationpassedtodo_continuation36/36,nokv_authority_store292/292).negativepassedpythonalias):monitor_successor.test.ts5/5 andquota_monitor_poll_commit.test.ts19/19 passed through the selector; reverting one call site to barepythonfails that file 4/5, proving the guard covers the original failure mode.integrationnot_runwindows-powershellCI step; local host is macOS.loopx checkdiagnostic reports an existing finding in unchangedtests/control_plane/test_public_safety_text_budget.py; the changed-path scan passed.Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
docs/development/testing-and-quality.md.Shared-authority RFC fixture impact
Boundary Checklist
none.Signed-off-bytrailer (git commit -s).