Skip to content

fix(doctor): respect installation owner in repair advice - #8126

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
HaoooLee:codex/fix-doctor-install-owner
Nov 5, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
HaoooLee:codex/fix-doctor-install-owner

Conversation

@HaoooLee

@HaoooLee HaoooLee commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

fix(doctor): respect the original package manager in repair advice

Goal And Delivered Outcome

The default Fix advice in loopx doctor recommends pip even when LoopX identifies the installation as owned by pipx, uv, or a custom installer. For pipx, the same report already contains the correct upgrade command in install_freshness.

Repair advice should follow the installation's package manager. A user should be able to follow the Fix section without reconciling conflicting commands elsewhere in the report.

This change reuses the existing upgrade command and preserves the recorded pipx environment name. When the installation owner has no supported upgrade command, the Fix section keeps the repair steps and directs users to their original package manager.

For example, a pipx environment named loopx-preview now receives pipx upgrade loopx-preview rather than pip upgrade advice.

Related upstream work:

  • #3566 established installation ownership and manager-specific upgrade commands.
  • #6204 fixed pip command quoting on Windows.
  • #8107 fixed POSIX checkout repair-command quoting.

Those changes address the installation contract and command rendering. This PR fixes the remaining hard-coded pip advice in the default Fix section.

Author Declaration

  • Written by: model_agent - OpenAI Codex (GPT-6 family), under the author's direction.

Implemented against

The existing installation contract in Installing LoopX: "Choose one installation owner and keep it authoritative."

Criterion Disposition Implementation / evidence
Keep the original package manager authoritative implemented collect_doctor() uses the existing installation-owner upgrade command
Preserve the recorded pipx environment implemented CLI regression expects pipx upgrade loopx-preview
Preserve existing recovery paths implemented Focused tests cover pip quoting, source installations and ZCode recovery

Self-check included the complete two-file diff, related PR coverage, failing-before tests, CLI JSON output and Markdown repair advice.

Scope And Continuation

Complete within the repair-advice scope. The change uses the existing Python installation boundary and introduces no new installer policy, configuration, permissions or persisted state.

Package installation and upgrade execution are unchanged.

Validation

  • Tested revision: 68dc9e92c6176bbe99c3436d74021dccf291f857
  • Run state: finished
  • Input classes: synthetic, public_fixture

Focused checks

Check kind Result Evidence
regression_parity passed The three new cases fail with the original doctor module from 9ee0c75d5; two controls pass. All three cases pass after the fix.
real_entrypoint passed Temporary distribution metadata is supplied at the discovery boundary. Owner detection, in-process CLI dispatch, JSON serialization and Markdown rendering run normally.
unit passed 93 focused and adjacent tests pass on Windows. The three new CLI cases also pass on Ubuntu.
static passed Ruff on the changed files and CI scope, git diff --check, and the staged bilingual RFC check.
static passed The semantic smoke passed on a serial rerun with the original 120-second limit. Its first run timed out; that result is retained.

Focused regression command:

python -m pytest -q \
  tests/test_python_install_owner_platform.py \
  tests/test_doctor_install_freshness.py \
  tests/test_doctor_installation_scope.py \
  tests/test_zcode_doctor_skill_delivery.py

Broader checks and limits

Check kind Result Evidence / limitation
static failed Configured mypy reports 13 Windows platform-API errors in three files. The same errors were observed during the baseline comparison.
integration failed The earlier standard premerge run had three installer/update smoke failures, also reproduced on a clean baseline, plus an inherited maintainability advisory. No complete premerge pass is claimed.
integration failed The CLI budget smoke failed while reading a generated plan in its long-path fixture; cleanup also failed on Windows.
unit failed The full-suite attempt stopped during collection because a benchmark test imports fcntl, which is unavailable on Windows.
manual not_run Actual package upgrades were not tested.

The full standard premerge comparison used the earlier 9ee0c75d5 source snapshot. The doctor source is identical between that baseline and the later upstream base; the 93 focused tests were rerun on the submitted commit.

The public-boundary check completed with zero errors and 49 warnings. The PR text and two-file diff were also scanned separately, with zero hits and no unscanned inputs.

Frontend / Visual Evidence

  • UI impact: none.
  • This changes terminal repair guidance without changing a frontend or layout.
  • Screenshots, viewport, attention and craft review: not applicable.
  • Before/after output behavior is covered by the CLI and Markdown checks above.

Type of Change

  • Bug fix
  • Test update

LoopX Area

  • Build, packaging, installer, or CI

Technical Direction

N/A - a bounded installation-diagnostic repair using the existing installation-owner contract.

Shared-authority RFC fixture impact

N/A - no shared-authority schema, provider, runtime-routing or compatibility-projection change.

Boundary Checklist

  • The diff and description contain no private state, credentials, raw traces, internal links or local machine paths.
  • The change stays within the reproduced defect and does not duplicate maintainer-owned benchmark work.
  • No paired RFC changed; the bilingual diff check passed.
  • UI impact is marked none, with the evidence boundary explained above.
  • Every contribution commit includes a DCO Signed-off-by trailer.

Signed-off-by: ZKDplz <2646995705@qq.com>

@loopx-agent loopx-agent 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.

Reviewer: actor_kind=model_agent; model=gpt-6.1-sol; provider=OpenAI; declaration_source=runtime_reported; reasoning_effort=max.

动机 / Motivation

通过 pipx、uv 或其他管理器安装 LoopX 的用户,在 doctor 的修复区可能收到与安装来源冲突的 pip 建议。

例如原 pipx 环境叫 original-preview-env:之前诊断数据知道这个名字,修复区却建议直接运行 pip;现在修复区复用原命令,指向同一个 pipx 环境,未支持的管理器则明确交还原管理器处理。

六类安装来源的成对 CLI 检查显示,仅修复文本按预期变化;JSON 与 Markdown 使用相同建议,source checkout 的规范化输出保持一致。

本 PR 不新增安装器、自动升级、权限或持久状态,也不承诺能为每个自定义管理器生成可直接运行的升级命令。

改动思路 / Approach

包管理器决策已经由安装 owner 生成,Fix 应投影该命令;再复制一套安装分支会制造第二个建议来源。 当前 PR 只统一既有 doctor 的修复建议及回归测试,保留不支持管理器的人工恢复边界。

不改会留下可独立复现的矛盾;在 Fix 内为每个管理器再写分支,则需要同步两套升级决策。当前路径由 distribution RECORD 确认被导入模块归属,再由既有 PythonInstallOwner 和 freshness 生成命令,doctor 只决定如何展示。支持的命令以完整代码块展示,不支持的管理器仍交给原管理器人工处理。建议被忽略不会触发新任务、自动升级或权限;现有诊断状态和重试 owner 不变。

具体改动 / Changes

本次审查完整 68dc9e92c6176bbe99c3436d74021dccf291f857:生产部分 +10/-4,测试 +47/-4。依据已接受的 docs/guides/installing-loopx.md,固定规格版本 68318d14d2db02bd4855fa03a9af46ebc64be2bc,不是用本 PR 的实现反推要求。以下 OWNER 标识仅用于映射原文要求,没有创造新的产品协议。

规格映射 该 head 的处理与独立证据
OWNER-1:保留一个权威安装 owner implemented;Fix 复用 freshness 的 owner 命令,pip/pipx/uv/custom/unknown 的 CLI 对照可区分旧错误建议。
OWNER-2:pipx 继续使用其原环境 implemented;合成 metadata 中环境为 original-preview-env,读回命令仍是 pipx upgrade original-preview-env。
OWNER-3:source checkout 与原生 App 更新边界独立 implemented;source checkout 规范化成对输出一致,原有 host/ZCode 路径和相邻测试保留;没有新增 App 更新入口。

关键代码讲解

  1. python_distribution_install 先核对 RECORD 是否真正拥有当前导入的模块,再读取原管理器及 pipx 环境。它未被修改,避免用诊断 prose 或无关包推断归属。
  2. collect_doctor 的 Fix 分支 有 upgrade_command 时直接展示现成命令;缺失时保留 host material 的修复提示并明确原管理器。pip/pipx 的升级、刷新 workflow/slash、再诊断顺序来自既有命令,source 和 host-managed 外层分支未挪动。
  3. test_doctor_fix_preserves_installation_owner 将临时 distribution metadata 接到 discovery 边界,运行真实进程内 CLI、JSON 序列化及 Markdown 渲染,不只比较新 helper。它检查 pipx 环境、uv/custom 不泄漏 pip,以及刷新命令和完整 Fix 读回。

对主干的风险 / Risks

没有发现阻塞问题。最强反例是 freshness 已正确、Fix 却仍打印 pip,这会让只读 Fix 的用户绕过原管理器;旧 helper 测试即使通过也发现不了它。独立的六类 CLI 对照在基线观察到了这个错误,而当前 head 正确选择 pip/pipx 或明确的人工 fallback。只规范化 checkout 路径,未抹掉管理器、环境、命令参数或诊断文本的差异。安装来源、status、exit 和临时 runtime 目录行为保持一致,未创建 registry;没有执行输出中的升级命令。

四个相邻测试文件:基线 87 passed、当前 head 90 passed,双方均有 3 项实际 PowerShell 调用因缺少 pwsh 跳过。配置的 mypy 在双方通过(17 source files),变更文件 Ruff 通过;quick premerge 的 diff、编译以及 install-local、codex-cli-packaged-install、loopx-update 三个隔离 fixture smoke 通过。公开边界检查最终 exit 0、errors/warnings 0/0;初次缺少隔离 registry 的检查失败保留,创建空测试 registry 后完成读回。

完整 JSON 对照中,target main 与该 head 的唯一未审查项均为 _admitted_interaction_next_cli_actions:62 decision points,阈值 60;该函数、检查器及配置逐字节未改,完整结构化报告仅去除 repository_root 后一致。保留 failed,不修改阈值,作为无关既有债务。 作者早前 Windows mypy 和 broader-run 失败保留为原平台的诊断,当前 macOS 通过不证明那些原因已修复。按 wait_for_ci=false 未查询、轮询或等待远端 CI。uv/custom 的具体升级命令、真实用户升级和 Windows 可执行效果均未在本次测试中证明。

我的整体评价 / Verdict

交付判断为 goal_achieved,限于统一既有诊断建议。long_horizon 为 improved:以后诊断不会反复引导安装 owner 漂移;这是指令一致性的局部收益,未测量长期模型采用或业务效果。user_experience 为 improved:用户无需在报告两处自行调和冲突命令,仍是一次诊断后由本人执行必要修复,没有新信息重填、配置或确认轮次。这是 CLI 诊断建议修复,未修改 frontend/Lark 渲染或页面布局;检查了真实 JSON 序列化和 Markdown Fix 读回。 未验证真实包管理器升级效果或原生 Windows 执行。

包管理器决策已经由安装 owner 生成,Fix 应投影该命令;再复制一套安装分支会制造第二个建议来源。 当前 PR 只统一既有 doctor 的修复建议及回归测试,保留不支持管理器的人工恢复边界。 这次行为变化已明确披露,没有宣称 default-off 功能,也没有新增状态、安装权限或协议版本。复用既有 owner 比扩展新安装器更合适;exact-scope CQR 有效,完整原始维护性失败继续保留。批准仅针对这个未变 head,合并权限、原生平台资格和合并就绪度仍需分别判断。

English verdict: APPROVE — the exact-head CLI preserves the original installation owner in Fix advice and retains explicit unsupported-manager recovery. Actual upgrades, native Windows execution and long-term adoption are not proven by this review.

@huangruiteng
huangruiteng merged commit 4f28945 into loopx-project:main Nov 5, 2026
25 of 37 checks passed
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.

3 participants