Skip to content

fix(runtime): use project Python selector in source launcher - #4864

Open
viafcccy wants to merge 1 commit into
loopx-project:mainfrom
viafcccy:codex/source-launcher-python-selection
Open

viafcccy wants to merge 1 commit into
loopx-project:mainfrom
viafcccy:codex/source-launcher-python-selection

Conversation

@viafcccy

Copy link
Copy Markdown

Goal And Delivered Outcome

  • Goal/source and gap: the source-checkout launcher had a second Python-selection path that fell back directly to python3, while scripts/loopx-python.sh already owned the supported precedence: explicit LOOPX_PYTHON, installer record, project .venv, then compatible installed interpreters. On hosts where system python3 is older than 3.11, python -m pytest could therefore start under a valid project environment while nested scripts/loopx calls failed before reaching the CLI.
  • Observable before → after: with a project .venv present but not activated and PATH=/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.
  • Issue/task and intended base: self-contained source-launcher bug; intended base is 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

  • Completed scope and remaining work: scripts/loopx now reuses the existing project/interpreter selector. A compatibility fallback remains for old release snapshots that do not contain the helper.
  • Slice boundary / successor: complete within this scope. The future-facing refactor pass reused the existing interpreter owner instead of adding another selection abstraction. No CLI protocol, install mutation, scheduler authority, or Python support-policy change is included.

Validation

  • Tested revision: 93a7827d6b23156fd22f18f12e1997ac242dfeff
  • Run state: finished
  • Input classes: synthetic, public_fixture
Check kind Result Public-safe evidence / limitation
regression_parity passed Isolated launcher fixture reproduces an unactivated project .venv with only an older system python3 on PATH; tests/test_loopx_launcher_python_selection.py proves the project interpreter is selected.
unit passed python -m pytest -q tests/test_loopx_launcher_python_selection.py tests/test_python_discovery.py: 8 passed.
integration passed All Python tests that invoke scripts/loopx: 322 passed, 1 skipped.
real_entrypoint passed env -u LOOPX_PYTHON PATH=/usr/bin:/bin scripts/loopx --format json version returns the checkout version through the project .venv.
static passed Focused Ruff, bash -n scripts/loopx scripts/loopx-python.sh, and git diff --check.
integration passed loopx canary premerge --from-git-diff: 2 catalog checks and 8 install/release risk-profile smokes passed; no manual holds.
integration failed A broader parallel Python run was not used as passing evidence: it was interrupted after 4,638 passes with one extension-timeout assertion and two quota/Turn-envelope assertions outside the changed files.
  • Coverage and gaps: the regression exercises the exact selector boundary; the existing discovery suite covers precedence and failure behavior; every Python test that invokes the launcher passed; install/release canaries cover packaged and overwrite flows. The broader-suite failures are disclosed above and are not treated as evidence for this change.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: N/A; no user-visible UI changed.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no public invocation change)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: core runtime and contributor reliability; no roadmap claim.

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

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the reproduced source-launcher bug.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer.

Signed-off-by: caven <caven@minimaxi.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

审阅 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/pythonscripts/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_ROOTPYTHONPATH 和模块启动路径。
  • scripts/loopx:72-86:helper 缺失时保留旧 snapshot 的 LOOPX_PYTHON/.loopx-python/python3 兼容路线,边界明确,没有再引入新持久状态。
  • tests/test_loopx_launcher_python_selection.py:20-49:构造未激活的项目 .venv stub,通过真实 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_route1 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 -ngit 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.

This branch has not been deployed

No deployments
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