Skip to content

fix(daemon): 稳定桌面源码联调运行时 - #57

Merged
Mr-KID-github merged 2 commits into
mainfrom
codex/sdk-desktop-runtime-main
Aug 17, 2026
Merged

fix(daemon): 稳定桌面源码联调运行时#57
Mr-KID-github merged 2 commits into
mainfrom
codex/sdk-desktop-runtime-main

Conversation

@orulink-wugui

@orulink-wugui orulink-wugui commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Comparison

  • Target repository: git@github.com:orulink-ai/WatcheRobot_python_sdk.git
  • Current branch: codex/sdk-desktop-runtime-main
  • Base branch: origin/main
  • Comparison range: origin/main...HEAD
  • Generated at: 2026-08-17T19:00:09+08:00

What Changed

  • 修复桌面源码联调时的 Application 启动器选择、跨虚拟环境切换与本地 WebSocket 代理干扰问题
  • 收紧 POSIX 与 Windows 启动器的受管目录边界,并补充真实虚拟环境和路径逃逸回归测试
  • 保持 Daemon 透明路由及现有维护 REST API、依赖和协议版本不变
  • 修复 Windows 环境变量名大小写不敏感时 NO_PROXY/no_proxy 用户原值可能丢失的问题,并补齐跨平台测试门禁

Changed Files

  • M docs/contracts/runtime-profile-index.md
  • M src/watcherobot/cli.py
  • M src/watcherobot/runtime/daemon/__main__.py
  • M src/watcherobot/runtime/daemon/application/client.py
  • M src/watcherobot/runtime/daemon/application/launcher.py
  • M src/watcherobot/runtime/daemon/application/runtime.py
  • M tests/runtime/test_application_client.py
  • M tests/runtime/test_application_launcher.py
  • M tests/runtime/test_application_logging.py
  • M tests/runtime/test_application_runtime.py
  • A tests/runtime/test_daemon_entrypoint.py
  • M tests/runtime/test_daemon_runtime_routing.py
  • M tests/runtime/test_full_architecture_flow.py
  • M tests/test_cli_runtime.py

Commits

  • a41db0b fix(daemon): 收紧桌面源码联调运行时边界
  • 675da0c fix(runtime): 兼容 Windows 代理环境变量

Validation

  • SDK CI:Python 3.10–3.12 最低/最新依赖、BLE fake backend、发行包构建与 wheel 安装全部通过
  • 本机 Python 3.10.21 双虚拟环境切换及无效代理场景:通过
  • 运行时与 CLI 聚焦测试 71 项:通过
  • Windows Python 3.11 合并结果:收集 800 项测试,平台适用项全部通过,POSIX-only 项按预期跳过
  • mypy src/watcherobot:通过
  • pip check:通过
  • git diff --check:通过

Risk Checks

  • Git operation state: 正常:没有检测到 merge/rebase/cherry-pick/revert 等未完成操作。
  • Secret scan: 已人工复核。扫描命中 src/watcherobot/cli.py 中基础分支已存在的 password=password 参数透传;该行未被本 PR 修改,也不包含任何凭证值。
  • Whitespace check: 通过:没有检测到 diff 空白字符问题。

Related Issue

  • None

Notes

  • 本 PR 已按审查意见移除不相关的维护接口迁移,不删除维护 REST API,不调整依赖,不修改 DAEMON_CONTROL_PROTOCOL_VERSION,也不新增业务消息类型分支。
  • POSIX 启动器仅在默认源码 Application 根目录或受管 Application 根目录内保留虚拟环境入口;Windows 继续对解析后的真实目标执行严格目录边界校验。
  • macOS 本机全量测试中仅剩 7 项既有发行包夹具失败,原因是测试夹具固定寻找 linux-x64,与本 PR 运行时改动无关;PR 的跨平台 SDK CI 已全部通过。

@github-actions

Copy link
Copy Markdown

🤖 Luxiao PR 审查报告

🤖 PR 审查报告

PR: fix(daemon): 稳定桌面源码联调运行时
变更: 11 个文件,+273 / -17


关键问题

🔴 Python 启动器的符号链接保留被扩大到所有 Application,削弱了原有可执行文件边界

位置:src/watcherobot/runtime/daemon/application/launcher.py

本次代码先调用 _require_controlled_executable() 校验解析后的目标,但丢弃了它返回的 resolved_executable

_require_controlled_executable(...)

随后实际执行的是未经解析的原始路径:

command_executable = _python_executable_for_application(
    requested_executable,
    ...
)

测试也明确把这一行为扩展到了普通 Python Application:

def test_python_launcher_preserves_virtualenv_symlink_for_execution(...)

这带来两个问题:

  1. 校验对象和执行对象不再是同一个路径身份
    校验时检查的是符号链接当时指向的真实目标,执行时却重新通过符号链接路径查找目标。

  2. 存在符号链接替换的 TOCTOU 风险
    build_spec() 完成后、子进程启动前,如果链接被替换,实际运行的程序可能不是已通过 _require_controlled_executable() 校验的程序。

尤其测试允许以下结构:

  • 符号链接位于 controlled_root 外;
  • 只有链接当时的目标位于 controlled_root 内;
  • 最终命令仍执行外部的符号链接路径。

这实际上改变了普通第三方 Application 的安全边界,与 PR 描述中“只为 Workspace 官方默认 Application 增加例外”“不扩大普通 Python Application 的可执行文件边界”不完全一致。

建议:

  • 将“保留虚拟环境符号链接路径”的行为严格限制在 trusted_source_default 分支;
  • 普通 Python Application 继续执行 _require_controlled_executable() 返回的解析路径;
  • 如果普通 Application 也必须保留 venv 路径,需要重新定义受控边界:至少要求链接路径本身位于受管环境中,并解决校验与启动之间链接可变的问题,不能仅校验链接目标后执行原链接。

这是当前最主要的合并阻塞项。

⚠️ websockets.connect(proxy=None) 的运行时版本兼容性未被真实测试覆盖

位置:src/watcherobot/runtime/daemon/application/client.py

connect(self._desktop_url, max_size=None, proxy=None)

proxy 参数取决于项目支持的 websockets 版本。当前新增测试使用接受任意 **kwargs 的 Fake,因此只能验证“参数被传入”,不能证明项目声明的最低依赖版本实际支持该参数。

如果安装到较旧版本,参数可能继续向底层连接 API 传递,并在运行时产生 TypeError,导致 Application channel 完全无法连接。

合并前应确认:

  • 依赖清单已经把 websockets 最低版本约束到支持 proxy 参数的版本;
  • 或使用项目兼容版本范围内统一可用的代理绕过方式;
  • 最好增加一项使用真实 websockets.connect 签名或真实本地 WebSocket 服务的测试。

维度一:代码质量

🏗️ 架构视角 — ⚠️

整体方向符合现有架构:

  • SDK 仍然拥有 Daemon、Runtime 和 Application 控制面;
  • 通过 Daemon CLI 显式传入源码 Application 根目录与启动器,没有让 Daemon自行猜测 Workspace 布局;
  • 没有引入业务消息类型分支、设备旁路或 Desktop 对 Application 的直接管理;
  • 官方源码 Application 的授权以“根目录 + 启动器”成对配置,边界设计基本清晰;
  • RuntimeDaemon → ApplicationLauncher 的参数传递链路简单,没有增加新的跨层依赖。

但启动器改动超出了必要范围。PR 的核心需求是允许官方源码 watcher_default 使用 Workspace venv,而实现同时改变了全部 Python Application 的执行路径语义。

另外:

controlled_root = _resolved_executable_parent(requested_executable)

对于受信任源码默认 Application,controlled_root 被设置为已解析可执行文件自身的父目录。这样 _require_controlled_executable() 的目录约束基本退化为“可执行文件位于自己的父目录下”,真正的授权只剩前面的路径对比。

这并非必然错误,但代码表达容易让后续维护者误认为仍然存在独立的 controlled-root 安全检查。建议把“显式受信任启动器校验”和“普通受管目录校验”拆成清晰的两个分支,避免复用一个已经失去约束意义的 controlled_root

🎨 产品视角 — ✅

用户问题与修改目标对应明确:

  • macOS/Homebrew Python 符号链接解析导致 venv 语义丢失;
  • 系统代理劫持本机 WebSocket;
  • Workspace 默认 Application 目录和入口识别不稳定;
  • 多仓源码联调缺少统一入口说明。

显式设置 proxy=None,同时补齐 NO_PROXY/no_proxy,能够覆盖两类场景:

  • SDK 自身建立的本机 Application WebSocket;
  • Application 子进程继承代理环境后访问其他本机服务。

环境变量处理保留了已有值,而不是直接覆盖:

internal.example,127.0.0.1,localhost,::1

这对已有企业代理环境较友好。

文档也清楚说明了 Workspace、SDK、Desktop 和默认 Application 的职责边界,能够减少开发人员误用系统 Python、Conda 或桌面安装包 Runtime 的概率。

产品层面没有看到功能蔓延,主要风险来自实现安全边界,而不是需求本身。

📏 规范视角 — ⚠️

做得较好的部分:

  • 参数命名清晰:
    • source_default_application_root
    • source_default_launcher_executable
  • 成对配置在构造阶段 fail-fast;
  • 中英文 README 同步更新;
  • Runtime contract 明确记录授权边界;
  • 类型标注完整;
  • 针对路径不匹配、配置缺失、虚拟环境符号链接、代理环境增加了测试。

需要改进的部分:

  1. _require_controlled_executable() 返回值被忽略,语义不直观
    函数名看起来不仅是断言,还负责返回安全规范化后的路径。忽略返回值会让审查者误判实际执行对象。

  2. 重复进行路径处理

requested_executable = Path(os.path.abspath(executable))

同样的表达在可信路径比较和后续变量赋值中出现两次,可以先计算一次,降低两处逻辑未来不一致的风险。

  1. 错误信息可以更具体
"Source default Application must use its trusted source default launcher"

该错误同时覆盖根目录错误与 launcher 错误,不利于定位 Workspace 参数传递问题。建议区分:

  • Application root 不匹配;
  • launcher 不匹配。
  1. 缺少 CLI 参数链路测试
    当前测试主要集中于 ApplicationLauncher,但没有看到测试确认:
argparse
→ run_runtime()
→ RuntimeDaemon
→ ApplicationLauncher

能够完整保留 launcher 的未解析路径。这个链路正是本次 macOS 修复的核心。

  1. 代理测试只验证 Fake 调用参数
    没有覆盖真实 websockets 版本兼容性。

⚠️ 损伤视角 — 🔴

主要损伤风险是普通 Python Application 的执行安全语义被全局改变。

原逻辑大致是:

请求路径
→ resolve(strict=True)
→ 校验解析目标在受控目录内
→ 执行解析后的同一目标

新逻辑变成:

请求路径
→ 解析并校验当时的目标
→ 丢弃解析结果
→ 稍后重新通过原始符号链接执行

因此,验证通过的目标与最终执行目标之间不再具有稳定的一致性。

其他风险:

  • 如果依赖范围包含不支持 proxy 参数的 websockets 版本,Application client 会在建立连接时直接失败;
  • 描述中的完整 tests/runtime 仍有 6 项失败。虽然声明已在 origin/main 复现,但当前 PR 仍然缺少完全绿色的运行时验证结果。基线失败可以不阻塞合并,但应在 CI 或审查记录中保留可核验的基线对照,而不能只依赖文字说明;
  • 没有看到 Windows 下 source-default launcher 成对授权的专项测试。当前 Windows 路径仍可能经过 pythonw 转换,需要确认最终执行命令不会意外偏离被授权的 launcher 语义。

代理绕过本身没有明显性能问题,NO_PROXY 处理也不会覆盖用户已有配置。


维度二:意图分析

🎯 意图提炼

该 PR 的真实意图是建立一条稳定、可重复的 Workspace 桌面源码联调链路:让 Daemon 保留 Workspace venv 的启动语义,可靠启动官方源码 watcher_default,并避免本机 Application WebSocket 被系统代理劫持。

同时通过显式参数和文档明确 SDK、Desktop、Workspace 与第三方 Application 的所有权边界。

🔀 偏离检测

需求方向与项目架构一致,没有引入业务路由旁路,也没有把 Desktop 或 Server 的业务实现复制进 SDK。

但实现存在一处明显偏离:

  • 声明的例外边界是“Workspace 官方默认 Application”;
  • 实际符号链接执行语义却被扩展到所有 Python Application;
  • 新增测试还把这种扩展固化为预期行为。

因此,这不是单纯的实现细节,而是授权模型发生了未在 PR 描述和 contract 中说明的扩大。应当收窄到官方 source-default 分支,或者明确重新设计和记录所有 Python Application 的可执行文件信任模型。


Merge 建议

🔴 当前不建议合并

必须解决:

  1. 收窄符号链接保留范围,避免普通 Python Application 出现“校验解析目标、执行可变符号链接”的不一致。
  2. 确认并测试项目支持的 websockets 版本确实接受 proxy=None

建议同时补充:

  1. 增加 CLI 参数到 ApplicationLauncher 的端到端传递测试,验证 launcher 的 venv 路径不会在中途被 resolve()
  2. 增加普通第三方 Application 无法借用 source-default 授权的显式测试。
  3. 增加符号链接目标被替换或超出受控根目录时的拒绝测试。
  4. 在 CI 结果中保留 6 项基线失败在 origin/main 上的对照证据。

总结: 修复目标合理,代理与 Workspace 授权方向基本正确,但当前实现将 venv 符号链接执行策略扩展到了所有 Python Application,导致校验路径与实际执行路径不一致;在收紧这一安全边界并确认 websockets 版本兼容前,不建议合并。

@github-actions

Copy link
Copy Markdown

🤖 Luxiao PR 审查报告

🤖 PR 审查报告

PR: fix(daemon): 稳定桌面源码联调运行时
变更: 12 个文件,+551 / -40


维度一:代码质量

🏗️ 架构视角 — ✅

整体改动遵守现有架构边界,没有把桌面端编排、默认 Application 业务逻辑或第三方 Application 管理职责下沉到 SDK。

主要判断:

  • DaemonRuntime → ApplicationLauncher 的参数传递链路清晰,源码默认 Application 的特殊授权集中在启动器层,没有侵入业务帧路由。
  • source_default_application_rootsource_default_launcher_executable 必须成对配置,避免产生“只信任目录”或“只信任解释器”的半授权状态。
  • 特殊通道仅对以下组合生效:
    • launcher_kind == PYTHON
    • manifest.app_id == watcher_default
    • Application 根目录精确匹配
    • Launcher 路径精确匹配
  • 第三方 Application 仍回到 managed_app_root 的原有校验流程,测试也覆盖了不能借用默认 Application 授权的情况。
  • 普通受管 Application 继续执行已经解析、校验后的稳定目标;只有明确授权的源码默认 Application 保留虚拟环境符号链接路径。这个区分符合本次问题的实际边界。
  • Desktop、SDK、默认 Application 三仓职责在 README 和运行时契约中得到了明确说明,没有出现功能蔓延。

需要注意的架构假设:

源码默认 Launcher 保留的是可变符号链接路径,而不是解析后的稳定文件。这是保持 POSIX venv 语义所必需的,但意味着安全模型依赖 Workspace 及其 .runtime/venv 由同一可信用户管理。如果未来 Daemon 跨用户或提权运行,需要重新评估符号链接在“校验后、启动前”被替换的风险。当前桌面源码联调场景下可以接受。


🎨 产品视角 — ✅

本次修改直接对应桌面源码联调中的三个高频问题:

  1. macOS/Homebrew Python 符号链接解析后丢失 venv 运行语义。
  2. 本机 WebSocket 被系统代理劫持。
  3. 默认 WatcheRobot_server 无法稳定作为 watcher_default 被 Daemon 管理。

处理方式比较克制:

  • 没有要求用户修改全局代理设置。
  • 没有依赖调用者 shell 中的 Conda 或系统 SDK。
  • 不改变发行包内 Daemon 的协议和业务行为。
  • 默认 Application 的源码授权由 Desktop 显式传入,不进行不可靠的目录猜测。
  • 同时设置 NO_PROXYno_proxy,兼顾不同平台和依赖对环境变量大小写的处理差异。
  • 在新版 websockets 支持时显式传入 proxy=None,比单纯依赖环境变量更稳定。

一个轻微体验问题:

--source-default-application-root--source-default-launcher 缺少 CLI 参数层的成对校验。当前只传其中一个时,会在 ApplicationLauncher 构造阶段抛出 ValueError,可能表现为内部堆栈而不是清晰的命令行错误。建议在 run_runtime()argparse 层转换成明确错误,例如:

--source-default-application-root and --source-default-launcher must be provided together

这不影响 Desktop 正常成对传参,但会改善诊断体验。


📏 规范视角 — ⚠️

命名、类型和文档整体规范:

  • 参数名称明确表达“仅用于源码默认 Application”,没有使用含义模糊的 trusted_root 等通用名称。
  • Path.resolve()os.path.abspath() 的区别是有意设计:
    • Application 根目录规范化为真实目录。
    • Launcher 只做绝对化,从而保留 venv 符号链接身份。
  • _require_executable_file() 抽取后复用了存在性、文件类型和执行权限检查,减少重复逻辑。
  • 中英文 README 与运行时契约同步更新。
  • 测试覆盖 POSIX symlink、Windows pythonw.exe、第三方授权隔离和 CLI 参数传递链路。

有两项测试覆盖仍可加强:

  1. test_local_connect_options_match_the_installed_websockets_api() 依赖当前测试环境安装的 websockets 版本。如果 CI 使用不支持 proxy 参数的旧版本,该测试只会验证“不传 proxy”,无法覆盖本次新增的核心分支。

    建议增加一个显式声明 proxy 参数的 fake factory:

    def fake_connect(url: str, *, max_size=None, proxy="auto"):
        ...

    并断言两个连接都收到 proxy=None

  2. 当前 symlink 测试主要验证最终生成的 spec.command,尚未通过真实子进程验证“使用保留的 venv 路径启动后,确实能够加载 venv 依赖”。由于本次修复的根因正是解释器启动语义,建议至少保留一个小型端到端测试:

    • 创建临时 venv;
    • 安装或写入仅在该 venv 可见的模块;
    • 通过生成的 command 启动;
    • 验证模块可以导入。

这些属于测试稳健性问题,目前没有发现实现与测试断言相矛盾的代码错误。


⚠️ 损伤视角 — ⚠️

未发现业务协议、业务帧路由或第三方 Application 隔离方面的明显回归。

正面因素:

  • 新 CLI 参数均为可选参数,未配置时继续走原有行为。
  • proxy=None 仅用于 Daemon/Application 的本机 channel,不影响外部设备连接。
  • NO_PROXY/no_proxy 只注入 Application 子进程环境,没有修改用户全局环境。
  • Bundled Application 仍受 bundled_resource_root 限制。
  • 普通 Python Application 仍受 managed_app_root 限制。
  • Windows pythonw.exe 仍要求位于受信 Launcher 的相邻目录,且解析后的目标不得逃逸。
  • 没有增加新的业务消息分支或 Desktop 直连设备旁路。

主要风险如下:

1. 完整 runtime 测试仍存在 6 项失败

PR 已说明这些失败能够在干净的 origin/main 上复现,因而不应直接归因于本次修改。但合并依据不能只依赖文字声明,建议在 CI 记录或 PR 评论中保留以下证据:

  • 同一台机器、同一 Python 环境;
  • origin/main 的失败用例列表;
  • 当前分支的失败用例列表;
  • 两边失败原因一致。

如果失败集合或异常栈不同,则不能按基线问题豁免。

2. 跨仓联调是本 PR 的核心验收点

SDK 侧只提供两个新的 CLI 参数,真正能否解决问题依赖 Desktop 和 Workspace 同时满足:

  • 两个参数必须原子地成对传递;
  • Application 根目录必须与 Daemon 规范化后的路径一致;
  • Launcher 必须传入 venv 原始路径,不能由 Desktop 预先 realpath
  • 模式切换或 Daemon 重启时不能丢失这两个参数;
  • yarn desktop:dev 必须在 Desktop 启动前完成 SDK editable install 和依赖验证。

仅凭当前 SDK 单仓测试,还不能完全证明这条跨仓链路已经闭环。

3. 受信符号链接存在明确的信任前提

_python_executable_for_trusted_source_default() 在 POSIX 上返回原始 symlink,这是正确实现 venv 语义的关键,但也允许该 symlink 在校验后发生变化。当前应明确保证:

  • Workspace 目录不是其他低权限用户可写目录;
  • Daemon 不以高于 Desktop/Workspace 所有者的权限运行;
  • .runtime/venv/bin/python 不来自不可信共享目录。

在当前同用户桌面源码开发场景中,该风险可接受,不构成阻断项。


维度二:意图分析

🎯 意图提炼

该 PR 的真实意图是建立一条可重复、显式授权的桌面源码联调运行链路:Daemon 保留 Workspace venv 的原始解释器路径,稳定管理官方源码默认 Application,并确保本机 Application WebSocket 不受系统代理影响。

它同时试图明确三仓边界:SDK 是 Daemon 唯一源码,Workspace 负责编排,Desktop 负责传入运行上下文,默认 Application 仓库负责业务逻辑。

🔀 偏离检测

意图与项目方向一致,没有明显功能蔓延。

本次没有:

  • 将 Desktop UI 或打包职责放入 SDK;
  • 将默认 Application 的 ASR/LLM/TTS 业务放入 Daemon;
  • 为业务消息增加特殊旁路;
  • 放宽所有第三方 Application 的可执行文件边界;
  • 修改发行协议或设备连接模型。

实现与描述基本一致。对 watcher_default 增加的特例不是基于业务消息类型的硬编码,而是一个由 Daemon 启动参数显式授予、根目录和 Launcher 双重绑定的运行时能力,边界足够明确。

需要防止后续把这套机制扩展为多个业务 Application 的任意外部路径白名单。若未来出现这种需求,应抽象为正式的、可审计的启动授权模型,而不是继续增加 source_xxx_root 参数。


Merge 建议

⚠️ 有条件合并

代码本身未发现阻断性缺陷,核心实现方向正确,权限边界也没有被普遍放宽。满足以下条件后建议合并:

  1. 确认 Desktop 与 Workspace 配套修改已经完成跨仓联调:

    • yarn desktop:dev 可正常启动;
    • Daemon 在线;
    • watcher_default 可启动并连接 Desktop/device 两个本机 channel;
    • macOS 系统代理开启时仍可连接;
    • Launcher 保持为 Workspace venv 原始路径。
  2. 对 6 项完整 runtime 失败保留可核查的基线对比,确认当前分支没有新增失败或改变失败原因。

  3. 建议补充一个显式支持 proxy 参数的 fake connect 测试,确保 ApplicationCommunicators.run() 实际传入 proxy=None。该项改动很小,并且直接覆盖本次代理修复的核心行为。

非阻断改进:

  • 在 CLI 层增加两个源码授权参数的成对校验和可读错误信息。
  • 后续补充真实临时 venv 子进程测试,验证保留 symlink 路径确实能够加载 venv 依赖。
  • 在开发文档中明确 Workspace 和 venv 必须与 Daemon 处于同一用户信任边界。

总结: 修复方向准确,架构边界清晰,源码默认 Application 的授权足够收敛;在完成跨仓 macOS 联调验证并核实 6 项失败确属基线问题后,可以合并。

@github-actions

Copy link
Copy Markdown

🤖 Luxiao PR 审查报告

🤖 PR 审查报告

PR: fix(daemon): 稳定桌面源码联调运行时
变更: 12 个文件,+595 / -41


关键审查发现

P1 — 完整 Runtime 测试仍未通过,当前不能直接视为回归验证完成

PR 说明中明确提到:完整 Runtime 测试在受管 Python 3.14 环境下仍存在子进程依赖缺失问题。

虽然该问题被解释为“既有测试依赖问题”,且聚焦测试已经覆盖官方源码默认 Application,但当前材料没有提供以下证据:

  • 相同失败在 PR 基线分支上也能稳定复现;
  • 失败用例与本次修改的普通 Application 执行路径完全无关;
  • CI 的必需测试集仍然通过。

本次恰好修改了普通 Python Application 的执行路径语义:

resolved_executable = _require_controlled_executable(...)
command_executable = _python_executable_for_application(
    resolved_executable,
    ...
)

因此,“普通 Application 使用解析后的基础 Python,导致丢失 venv 依赖”不能仅凭描述排除为本次变更造成的损伤。至少需要通过基线对照或明确的测试夹具修复来证明。

合并前要求:

  1. 在目标分支相同 Python 3.14 环境运行完整 Runtime 测试,证明失败为基线已有;或
  2. 修复测试,使完整 Runtime 测试通过;或
  3. 给出失败用例列表及逐项原因,并由 CI/维护者明确确认这些用例不是本 PR 的合并门禁。

P2 — Windows 缺少 pythonw.exe 时会静默退回 python.exe,与 PR 描述及测试覆盖不完全一致

位置:

def _python_executable_for_trusted_source_default(...):
    ...
    pythonw = executable.with_name("pythonw.exe")
    if not pythonw.is_file():
        return executable

PR 描述和风险说明强调:

  • Windows 官方源码 Application 使用 launcher 同目录内的 pythonw.exe
  • 只允许使用授权 launcher 同目录内的 pythonw.exe
  • 防止符号链接逃逸。

但当前实现中,如果 pythonw.exe 不存在,会直接执行 python.exe,并不会失败。这样会产生两个问题:

  1. 实际安全/启动合同与文档描述不一致;
  2. 桌面运行时可能重新出现控制台窗口,而不是及时暴露不完整的受管环境。

现有测试只覆盖:

  • 相邻 pythonw.exe 正常使用;
  • pythonw.exe 符号链接逃逸被拒绝。

没有覆盖 pythonw.exe 缺失时应该怎样处理。

建议明确合同并补充测试:

  • 如果 python.exe 是允许的兼容回退:更新 PR 描述和合同文档,并增加回退测试;
  • 如果官方 Windows 源码环境必须提供 pythonw.exe:此处应抛出 ApplicationLaunchError,不要静默回退。

P2 — Windows 只验证了相邻 pythonw.exe 的逃逸,没有验证授权 python.exe 本身的符号链接逃逸

当前可信启动器验证:

resolved = _require_executable_file(executable, is_windows=is_windows)
_require_platform_executable_name(
    resolved,
    kind=ApplicationLauncherKind.PYTHON,
    is_windows=is_windows,
)

它会解析 python.exe,但不会要求解析后的目标仍位于授权 launcher 目录内。若授权路径本身是指向外部 python.exe 的符号链接:

  • 精确字符串匹配仍然成立;
  • 平台名称检查仍可能通过;
  • 当相邻 pythonw.exe 不存在时,还会直接执行该符号链接路径。

这与“Windows 拒绝符号链接逃逸”的描述存在边界缺口。当前测试只验证了 pythonw.exe 逃逸,没有覆盖 source_default_launcher_executable 自身逃逸。

建议:

Windows 下对授权 python.exe 增加目录约束,或明确禁止其为符号链接/重解析点,并增加对应测试。POSIX 保留 venv 符号链接语义的例外不应自动扩展到 Windows。


维度一:代码质量

🏗️ 架构视角 — ✅

整体架构方向正确。

优点:

  • SDK 继续持有唯一 Daemon 实现,没有向 Desktop 或 Server 复制运行时逻辑;
  • 源码默认 Application 的特殊授权集中在 ApplicationLauncher,没有散落到业务帧路由层;
  • CLI 参数通过 run_runtime()DaemonRuntime 传递到 ApplicationLauncher,依赖链清楚;
  • 官方源码默认 Application 与普通第三方 Application 的权限路径被明确分开;
  • 普通 Application 恢复“校验对象等于执行对象”的解析路径,避免符号链接在校验后被替换;
  • 本机代理绕过分别在父进程连接参数和子进程环境中处理,没有侵入 WebSocket 路由逻辑。

值得肯定的是,本次审查修复没有简单地对所有 Python Application 保留 venv 符号链接,而是把例外限制在:

launcher_kind is ApplicationLauncherKind.PYTHON
and manifest.app_id == self._default_app_id
and self._source_default_application_root is not None

随后再校验源码根目录和 launcher 的精确组合。这一设计基本符合最小授权原则。

需要关注的架构债务是:watcher_default 在配置源码授权后,会天然进入可信源码分支,再通过根目录匹配决定是否允许。当前模型可行,但意味着同一 Daemon 实例中,其他 Python 形式的 watcher_default 无法退回普通受管 Application 路径。若未来存在默认 Application 多来源切换,需要重新定义来源优先级。


🎨 产品视角 — ⚠️

核心用户问题得到了针对性处理:

  • macOS venv launcher 不再因 resolve() 退化为 Homebrew Python;
  • 本地 Application channel 不再误走系统代理;
  • Workspace 的统一启动入口和仓库职责边界写入中英文文档;
  • CLI 成对参数缺失时能够给出明确错误。

代理兼容处理也较稳妥:

if "proxy" in parameters:
    options["proxy"] = None

这避免了在 websockets 14 中传入不支持的参数,同时在 websockets 15 明确禁用代理。

但产品层面还有两个未闭合点:

  1. Windows 缺少 pythonw.exe 时的行为没有明确合同;
  2. 完整 Runtime 测试失败意味着普通 Application 的实际运行体验尚未被完整验证。

因此产品视角暂不能评为完全通过。


📏 规范视角 — ⚠️

整体规范质量较好:

  • 参数命名清楚;
  • 成对配置在 CLI 和 ApplicationLauncher 两层都做了校验;
  • 类型标注完整;
  • 错误信息区分了 root 不匹配和 launcher 不匹配;
  • README、中文 README 和 Runtime 合同同步更新;
  • 测试覆盖了 CLI 到 Launcher 的完整参数传递链路;
  • NO_PROXYno_proxy 均被处理,兼顾不同库和操作系统行为。

但仍有以下规范问题:

1. Windows 行为与文档表述不一致

代码允许缺少 pythonw.exe 时回退 python.exe,文档却表达为使用同目录 pythonw.exe。安全和运行合同必须以代码、测试、文档三者一致为准。

2. 私有实现耦合较重

新增链路测试直接访问:

runtime.application._application_launcher
manager._build_environment(run)
runtime.control_server._bound_port
runtime.external_server._bound_port

作为白盒单元测试可以接受,但端到端链路测试依赖多个私有字段,后续内部重构容易产生与产品行为无关的测试破损。建议至少对 launcher spec 或环境构建提供更稳定的测试接口,或者把测试明确归类为组件白盒测试。

3. _validate_source_default_options() 被调用两次

main() 调用一次以转换为 argparse 错误,run_runtime() 又调用一次以保护编程式入口。这个重复有合理目的,但建议增加注释说明,否则后续维护者可能误删其中一层。


⚠️ 损伤视角 — ⚠️

安全边界相比原始方案已有明显改善:

  • 第三方 Application 不能借用官方源码授权;
  • 普通 Application 执行解析后的稳定目标;
  • 官方源码根目录和 launcher 分开报错;
  • pythonw.exe 被限制在授权 launcher 目录;
  • 本地代理绕过仅作用于 Application channel;
  • 原有代理环境内容得到保留,而不是被覆盖。

但仍存在以下回归风险:

  1. 普通 Application 的 venv 依赖可能丢失。
    PR 自身已经观察到相关完整测试失败,需要基线对照证明不是新增回归。

  2. Windows launcher 自身的符号链接边界未完整覆盖。
    当前只覆盖相邻 pythonw.exe,未覆盖被授权的 python.exe 本身。

  3. Windows 缺少 pythonw.exe 时静默改变运行方式。
    可能出现控制台窗口或与 Desktop 生命周期不同的行为。

  4. 可信 POSIX launcher 仍保留有意的 TOCTOU 风险。
    校验时解析符号链接确认目标有效,但最终执行原始 venv 符号链接路径。若该路径可被不可信主体修改,校验后仍可能被替换。考虑到该例外限定在 Desktop 显式授权的官方 Workspace 路径,此风险可以接受,但前提是 Workspace 自管 venv 目录不能由不可信用户或第三方 Application 写入。建议在合同文档中明确这一信任前提。


维度二:意图分析

🎯 意图提炼

本 PR 的真实意图是为官方 Workspace 源码联调建立一个受限的启动授权:允许 watcher_default 在 macOS 保留 venv 符号链接执行语义,同时保证普通第三方 Application 继续使用受管目录内经过解析和校验的稳定解释器。

此外,通过显式禁用本地 WebSocket 代理及补齐 NO_PROXY/no_proxy,消除系统代理对 Daemon 与本机 Application channel 的干扰。

🔀 偏离检测

整体没有偏离项目方向,也没有明显功能蔓延。

变更仍然集中在:

  • Daemon CLI;
  • Application launcher;
  • Application client;
  • 子进程环境;
  • Runtime 合同和测试。

没有增加第二份 Daemon、业务消息类型分支或设备旁路,也没有把 Desktop 编排职责放进 SDK。

新增的源码默认 Application 授权属于针对官方开发环境的必要例外,而不是对通用 Application 安全模型的扩权。审查修复后的实现已经将例外收窄到明确的 app ID、源码根目录和 launcher 组合,方向合理。

唯一需要防止的后续偏离是:不要继续通过扩大“可信源码 Application”范围来解决普通 Application 的 venv 依赖问题。普通 Application 的依赖隔离应由安装和受管环境模型解决,而不是复用本次官方源码授权。


Merge 建议

⚠️ 有条件合并

本 PR 的整体设计正确,且针对上一轮审查意见做了实质性修复;macOS venv 路径、系统代理干扰和官方源码 Application 识别三个问题均有对应实现和聚焦测试。

但合并前应满足以下条件:

  1. 必须闭合完整 Runtime 测试失败问题。
    修复测试,或提供目标分支同环境基线对照,证明失败不是本 PR 引入。

  2. 明确 Windows 缺少 pythonw.exe 时的合同。
    选择“拒绝启动”或“允许回退 python.exe”,并保证代码、文档和测试一致。

  3. 补充 Windows 授权 python.exe 自身的符号链接逃逸测试。
    如果 Windows 不允许该语义,应在实现中明确拒绝。

满足以上条件后,可以合并;不建议通过扩大第三方 Application launcher 权限来消除测试失败。


总结: 方案方向正确,官方源码授权边界已基本收紧,但完整测试未闭合且 Windows launcher 合同仍有缺口,建议修复并验证后合并。

@orulink-wugui

Copy link
Copy Markdown
Contributor Author

完整 Runtime 测试基线对照

为核对 Luxiao 审查提出的完整 Runtime 回归风险,使用完全相同的本机环境执行:

  • Python 3.14.2
  • pytest 8.4.2
  • websockets 15.0.1
  • 命令:python -m pytest tests/runtime -q --tb=no

结果:

  • 最新远端 main 30e53a6:32 项失败。
  • 当前 PR:31 项失败。
  • 当前 PR 没有新增失败,并额外修复了 tests/runtime/test_daemon_runtime_routing.py::test_same_daemon_switches_between_two_real_python_environments

两边共同失败集合一致分布在以下文件:

  • test_application_bridge.py:2 项
  • test_application_logging.py:1 项
  • test_application_runtime.py:7 项
  • test_connection_identity.py:1 项
  • test_control_rest.py:2 项
  • test_daemon_runtime_routing.py:基线 5 项,当前 4 项
  • test_external_websocket_routing.py:6 项
  • test_face_tracking_preview_broker.py:3 项
  • test_full_architecture_flow.py:1 项
  • test_pairing_websocket_gate.py:4 项

代表用例 test_local_bridge_keeps_channel_source_and_raw_frame_content 在两边均以相同异常结束:

websockets.asyncio.client.connect
→ asyncio.exceptions.CancelledError
→ TimeoutError: timed out during opening handshake

因此当前 PR 没有扩大完整 Runtime 的失败集合;相反,受管双 Python 环境切换链路由失败变为通过。共同失败仍需作为独立的本机代理/测试环境问题继续治理,但不应通过放宽第三方 Application launcher 权限绕过。

@github-actions

Copy link
Copy Markdown

🤖 Luxiao PR 审查报告

🤖 PR 审查报告

PR: fix(daemon): 稳定桌面源码联调运行时
变更: 12 个文件,+679 / -41


维度一:代码质量

🏗️ 架构视角 — ✅

整体改动遵守现有职责边界,没有引入第二套 Daemon 或 Desktop 到 Device 的旁路。

做得较好的地方:

  • Workspace 特例被限制在 ApplicationLauncher 内,没有把源码联调判断扩散到 Runtime 业务消息层。
  • 授权条件同时绑定:
    • launcher_kind == PYTHON
    • app_id == watcher_default
    • Application 根目录
    • launcher 原始路径
  • 普通 Application 继续执行解析后的稳定目标,没有因为解决 Homebrew venv 问题而整体放宽可执行文件边界。
  • CLI、DaemonRuntimeApplicationLauncher 的参数传递链路完整,且入口层和构造层都有成对配置校验。
  • 本地代理处理位于 Application channel 和子进程环境构建处,没有修改用户全局代理配置。
  • README 和 Runtime contract 已明确 SDK、Desktop、默认 Application 各自的所有权边界。

没有发现不合理的跨层耦合或功能蔓延。

需要保留的架构认知:

POSIX 官方源码模式现在有意执行 venv launcher 的原始符号链接路径,而不是已验证的解析目标。这是维持 venv 语义所必需的特例,但意味着该模式信任 Workspace 自管 venv 在构建启动规格到实际创建进程期间不会被恶意替换。这个信任边界目前足够窄,且没有扩展给第三方 Application,可以接受。


🎨 产品视角 — ✅

改动直接解决源码联调中的两个实际稳定性问题:

  1. macOS/Homebrew Python 符号链接解析后丢失虚拟环境语义。
  2. 系统代理错误接管本机 Application WebSocket。

用户侧行为合理:

  • 完整配置时维持正常源码联调流程。
  • 半配置会在 Daemon 生命周期开始前明确失败,而不是运行到 Application 启动阶段才暴露问题。
  • proxy=None 只在当前 websockets API 明确支持时传入,兼容最低依赖版本。
  • 同时补充 NO_PROXYno_proxy,兼顾不同代理库及操作系统环境。
  • 不覆盖已有代理例外,只追加缺失的 loopback 地址。
  • Windows 优先使用同目录 pythonw.exe,缺失时回退到已验证的 python.exe,符合桌面应用无控制台启动预期。

错误信息也足够明确,能够区分根目录不匹配、launcher 不匹配、受控目录逃逸和参数缺失。


📏 规范视角 — ✅

代码规范整体良好:

  • 参数和内部字段命名清楚,能够区分 Application 根目录和 launcher executable。
  • 类型标注完整,Callable[..., Any] 用于兼容不同 websockets 连接工厂签名是合理的。
  • _require_executable_file() 的拆分减少了受控 Application 与官方源码特例之间的重复校验。
  • _validate_trusted_source_default_executable()_python_executable_for_trusted_source_default() 的职责划分清晰。
  • 关键安全特例均有注释和 contract 文档说明。
  • 测试覆盖了正常路径、半配置、第三方隔离、符号链接替换、Windows launcher 逃逸和 CLI 完整传递链路。
  • WebSocket 测试不仅检查辅助函数返回值,还通过显式声明 proxy 参数的连接工厂验证了实际调用参数。

两点非阻塞建议:

  1. _local_connect_options(connect) 每次连接都会执行一次 inspect.signature()。当前每次 Application 启动仅调用两次,性能影响可以忽略,不需要为此增加缓存复杂度。
  2. Windows 安全测试是在 POSIX Path 和符号链接语义下模拟的,不能完全代表 Windows junction、reparse point 和 ACL 行为。这不是代码规范问题,但真实 Windows 验收仍不可省略。

⚠️ 损伤视角 — ⚠️

没有发现明确的现有功能破坏,但仍存在以下集成和平台风险。

1. POSIX 源码 launcher 保留了可变路径身份

官方源码模式下:

spec_executable = requested_executable
command_executable = requested_executable

校验阶段会解析符号链接并检查目标,但实际启动仍使用原始符号链接。如果另一个本地进程在校验后、进程创建前替换该链接,实际执行目标可能发生变化。

该风险目前受到以下条件约束:

  • 只适用于显式配置的官方 watcher_default
  • Application 根目录和 launcher 路径必须精确匹配。
  • 第三方 Application 无法借用该授权。
  • 场景限定于 Workspace 源码开发环境。

因此不构成本 PR 的阻塞问题,但应把“Workspace 自管 venv 目录属于可信本地资源”作为明确的安全假设保留在架构文档中。

2. Windows reparse point 仍需真实平台验证

实现正确检查了 launcher 和 pythonw.exe 解析后是否留在受信目录,但现有测试无法完整覆盖 Windows 的 junction/reparse point 行为。特别需要在真实 Windows 环境确认:

  • python.exe 文件符号链接逃逸被拒绝。
  • pythonw.exe 文件符号链接逃逸被拒绝。
  • Scripts 目录自身是 junction/reparse point 时,行为符合预期的授权边界。
  • 回退到 python.exe 时不会出现控制台或启动参数异常。

3. 三仓参数契约需要同步落地

SDK 新增了两个必须成对传入的 CLI 参数。代码本身采用 fail-closed 行为是正确的,但最终可用性依赖 Desktop PR #111 和 Workspace PR #98 使用完全一致的参数名称及规范化路径。

如果只合并 SDK 而配套仓库仍传递旧参数,现有非源码路径不受影响;但新的 desktop:dev 源码流程无法形成完整交付。

4. 代理行为需要系统环境验收

单元测试证明了:

  • websockets 支持时传入 proxy=None
  • 子进程环境包含 loopback 的 NO_PROXY/no_proxy

但 macOS 和 Windows 上实际代理行为还可能受到 PAC、系统代理、企业代理软件以及不同 websockets 版本影响。PR 描述已经正确将其列为联调验收项。


维度二:意图分析

🎯 意图提炼

该 PR 的真实意图是为官方 Workspace 源码联调建立一个严格受限的启动例外:保留官方默认 Application 的 venv launcher 路径语义,同时确保普通及第三方 Application 仍执行经过解析和边界校验的稳定目标。

此外,它通过显式绕过本机 WebSocket 代理和 CLI 成对参数门禁,降低 Desktop、Daemon、默认 Application 三仓联调过程中的环境不确定性。

🔀 偏离检测

没有发现偏离项目方向或功能蔓延。

实现与描述基本一致:

  • 没有复制 Daemon。
  • 没有将 Desktop 变成业务帧路由实现。
  • 没有增加按业务消息类型分支。
  • 没有修改用户全局代理配置。
  • 没有向第三方 Application 扩大源码 launcher 特权。
  • 测试台基线同步未混入额外功能修改。

679 行新增看起来较大,但主要来自跨平台安全边界和回归测试。考虑到该改动涉及可执行文件授权、符号链接及代理兼容,测试体量是合理的,不属于过度设计。


Merge 建议

⚠️ 有条件合并

代码层面没有发现必须退回修改的阻塞问题,授权收紧、第三方隔离、代理兼容和参数门禁均已形成完整实现与测试链路。

建议满足以下条件后合并:

  1. Desktop PR #111 和 Workspace PR #98 已确认使用相同的成对参数,并与本 PR 按兼容顺序落地。
  2. 在真实 macOS 环境完成一次 yarn desktop:dev 验收,确认 Homebrew Python venv 依赖语义得到保留。
  3. 在真实 Windows 环境验证 python.exe、相邻 pythonw.exe、符号链接或 reparse point 逃逸拒绝行为。
  4. 在至少一个启用了系统代理的环境中确认本机 Desktop/Application/Device WebSocket 不再经过代理。
  5. 将 POSIX 源码模式所依赖的“Workspace 自管 venv 为可信本地资源”保留为明确安全假设。

这些条件主要属于跨仓和真实平台验收,不要求继续扩大本 PR 的代码范围。


总结: 改动方向正确,源码特例边界收得足够窄,普通第三方 Application 的稳定执行语义得到保留;完成三仓联调及 macOS/Windows 真实环境验收后可以合并。

@github-actions

Copy link
Copy Markdown

🤖 Luxiao PR 审查报告

🤖 PR 审查报告

PR: fix(daemon): 稳定桌面源码联调运行时
变更: 28 个文件,+702 / -6102
审查范围: 本报告仅基于输入中可见的 Diff。Diff 已截断约 4927 行、197759 个字符,维护模块删除后的控制器清理、测试覆盖及其他文件未完成审查,不能视为全量代码审查。


关键发现

1. 🔴 删除维护 REST API,但控制协议版本仍保持 2

位置:src/watcherobot/runtime/daemon/control/rest.py

本次变更删除了完整的 /daemon/maintenance/* API,包括:

  • 固件与 SD 资源安装
  • Release 查询与下载
  • 串口和读卡器发现
  • 作品导入、导出、读取、删除和安装
  • 维护任务状态查询

但文件中的:

DAEMON_CONTROL_PROTOCOL_VERSION = 2

没有变化。

这是控制面兼容性问题,而不只是内部重构。旧版 Desktop 或其他客户端看到相同协议版本,会认为原来的 API 能力仍然存在,随后得到 404,而无法通过协议协商提前识别不兼容。

建议至少采取一种方案:

  1. 提升控制协议版本,并让 Desktop 明确拒绝不兼容版本;
  2. 在状态接口增加 capabilities,明确暴露 maintenance 是否由 Daemon 提供;
  3. 如果这些接口从未发布、从未被任何受支持客户端使用,需要在 PR 中提供可核验依据;
  4. 确认 Desktop PR #111 已完全移除对这些路由的调用,并验证新旧 Desktop/Daemon 组合的行为。

这是当前版本的合并阻塞项。


2. 🔴 PR 实际包含大规模维护功能迁移,与标题和主要意图不匹配

PR 标题和描述的主要目标是:

  • 保持 POSIX venv launcher 语义;
  • 收紧官方默认 Application 授权;
  • 绕过本机 WebSocket 系统代理;
  • 增加 CLI 成对配置门禁。

但实际 Diff 同时删除约 6000 行维护实现以及相关依赖:

pyserial
esptool
types-pyserial

并删除 Daemon 的整套固件、SD 卡和 portable work 维护 API。

这不是简单的“同步测试台基线”,而是一个独立的架构迁移和破坏性 API 变更。它显著扩大了:

  • 审查面;
  • 回滚面;
  • 跨仓库发布顺序要求;
  • Desktop/SDK 版本兼容风险;
  • 用户升级风险。

建议:

  • 如果这些删除只是错误地出现在当前 PR Diff 中,应先更新 base/rebase,清理无关变更;
  • 如果维护能力迁移确实属于本次交付,应拆成独立 PR,单独说明 API 生命周期、迁移方案、版本兼容和回滚路径;
  • 如果因跨仓原子落地无法拆分,至少应修改标题和 PR 描述,将“维护能力从 Daemon 迁移到 Desktop worker”列为主要变更,而不是作为基线同步处理。

在目前形态下,PR 的可审查性不足。


3. ⚠️ 核心目标场景仍未完成真实平台验收

PR 要解决的两个主要问题都高度依赖平台环境:

  • macOS Homebrew Python/venv 符号链接语义;
  • 系统代理对本机 Application WebSocket 的劫持;
  • Windows python.exepythonw.exe 和重解析点边界。

现有验证中,聚焦测试通过是积极信号,但完整 Runtime 基线仍有 31 项失败,且 PR 明确说明:

真实 GUI、设备连接和系统代理场景仍应由 macOS/Windows 联调验收确认。

“相较 main 没有新增失败”只能证明失败集合没有明显扩大,不能直接证明本 PR 的目标场景已经修复。尤其当前失败集中在真实网络和子进程 Runtime 测试,而这正是本 PR 修改的范围。

合并前至少需要记录以下验收结果:

  • macOS:
    • Homebrew Python venv 能启动默认 Application;
    • Application 实际从 Workspace venv 导入依赖;
    • 设置 HTTP_PROXY/HTTPS_PROXY/ALL_PROXY 后,本机两个 WebSocket channel 仍可连接;
  • Windows:
    • pythonw.exe 存在时被优先使用;
    • 缺失时安全回退 python.exe
    • launcher 或 pythonw.exe 重解析到 Scripts 外时被拒绝;
  • 第三方 Application:
    • 无法复用官方源码授权;
    • 仍执行受控目录内解析后的稳定目标。

这些可以是人工联调记录,不一定要求全部自动化,但不能只依赖 mock 或函数级测试。


4. ⚠️ POSIX launcher 保留原始符号链接路径,引入校验与执行之间的替换窗口

位置:application/launcher.py

官方源码默认 Application 在 POSIX 下返回原始 launcher:

if not is_windows:
    return executable

校验阶段验证的是当时的解析目标,但真正创建子进程时使用的是原始符号链接路径。如果该路径在校验完成后、进程启动前被替换,最终执行目标可能与校验目标不同。

这可能是维持 venv 语义所必须接受的权衡,但需要明确安全假设:

  • Workspace venv launcher 目录是否只有可信组件可写;
  • Application 选择和进程启动之间是否存在明显异步窗口;
  • 普通第三方 Application 是否完全不能影响该目录;
  • 测试是否覆盖“build_spec() 后、spawn 前替换 launcher”的情况,而不仅是校验前替换。

如果该目录与 Desktop/Daemon 同用户可写,路径精确匹配并不等于执行目标不可变。建议在代码注释或架构契约中记录这一信任边界,避免后续把它误认为强不可变授权。


维度一:代码质量

🏗️ 架构视角 — ⚠️

正向评价:

  • SDK 继续保持唯一 Daemon 来源,没有在 Desktop 或 Server 中复制第二套 Daemon;
  • 官方源码默认 Application 的例外集中在 ApplicationLauncher 内,没有污染普通第三方 Application 的启动路径;
  • 授权同时绑定:
    • 默认 Application ID;
    • Application 根目录;
    • launcher 精确路径;
    • launcher 类型;
  • CLI、Daemon 初始化和 launcher 构造器都执行成对配置检查,边界比较完整;
  • 将固件和 SD 维护移出 Daemon,从职责划分上可以降低 Runtime 控制面的业务负担。

主要问题:

  • “源码联调稳定性修复”和“维护能力迁出 Daemon”被放进同一个 PR,职责跨度过大;
  • 删除维护控制面属于跨仓架构迁移,但当前 PR 没有提供完整的兼容协议;
  • SDK、Desktop、Workspace 三个 PR 存在强发布顺序耦合,任何一个仓库单独落地都可能产生不可用组合;
  • 控制协议版本未反映 API 能力删除。

结论:局部 launcher 架构清晰,但 PR 级架构边界和跨仓兼容设计尚未闭合。


🎨 产品视角 — ⚠️

正向评价:

  • --source-default-application-root--source-default-launcher 半配置时立即失败,优于 Daemon 启动后才出现模糊故障;
  • Windows 缺少 pythonw.exe 时回退到已校验的 python.exe,兼顾安全和开发环境可用性;
  • 同时补齐 NO_PROXYno_proxy,且不修改用户全局代理设置,行为克制;
  • 本机 WebSocket 显式绕过代理,符合用户对本机 Runtime channel 的预期;
  • 错误信息基本能够说明根目录或 launcher 不匹配。

风险:

  • 维护 REST API 的直接删除可能导致旧版 Desktop 中固件升级、SD 资源和作品管理功能突然不可用;
  • 用户看到的表现可能只是功能入口报错或请求 404,缺少版本不兼容提示;
  • PR 目标场景尚未经过真实 GUI、系统代理和设备连接验收;
  • 三仓源码编排要求提高后,需要确保失败信息能明确指出是 SDK checkout、venv、launcher 还是 Workspace 配置错误。

产品行为本身合理,但版本组合和升级体验没有被完整证明。


📏 规范视角 — ✅

正向评价:

  • 新增参数命名清楚,CLI 和构造器字段一致;
  • 类型标注完整,辅助函数职责比较单一;
  • _local_connect_options()inspect.signature() 失败进行了兼容处理;
  • POSIX 和 Windows 的 launcher 策略被拆成独立函数;
  • 中英文 README 和 runtime contract 同步更新;
  • 注释解释了保留 venv 路径和 Windows 回退的原因;
  • 普通 Application 与官方源码默认 Application 的逻辑分支明确,没有通过模糊的路径白名单扩权。

可改进项:

  • _validate_source_default_options()main()run_runtime() 中执行两次。防御式校验可以接受,但最好通过注释明确:
    • main() 负责生成友好 argparse 错误;
    • run_runtime() 负责保护程序化调用;
  • _local_connect_options() 使用 Callable[..., Any] 是兼容性折中,但可考虑用一个局部 Protocol 描述连接工厂;
  • 对控制面 API 的大规模删除应有 changelog、迁移文档或协议版本说明,而不应只更新职责文档;
  • PR 标题、描述和实际 Diff 范围不一致,属于变更管理规范问题。

就可见新增代码而言,命名、类型和注释质量较好。


⚠️ 损伤视角 — 🔴

主要损伤风险:

  1. 删除 /daemon/maintenance/* 全部接口,但协议版本不变;
  2. 删除 pyserialesptool 等运行依赖,可能影响仍依赖 SDK 维护能力的下游调用方;
  3. SDK PR、Desktop PR #111、Workspace PR #98 必须协调落地,存在中间版本不可用窗口;
  4. 完整 Runtime 测试仍有 31 项失败,无法从全量测试证明网络和子进程链路稳定;
  5. Diff 截断,无法确认:
    • Runtime controller 中所有维护引用是否已删除;
    • 是否残留导入错误;
    • 原维护测试是否被删除或迁移;
    • Desktop worker 是否完整承接原有安全校验;
    • 包导出、公开 API 和文档是否全部同步;
  6. POSIX 原始符号链接路径存在校验后替换窗口,需要明确其信任模型。

值得肯定的是,可见代码没有通过放宽第三方 Application 的 launcher 边界来修复默认 Application,Windows 目录逃逸检查也较严谨。


维度二:意图分析

🎯 意图提炼

本 PR 的主要意图是:让 Desktop 源码联调能够可靠启动 Workspace 自管 venv 中的官方默认 Application,同时保持第三方 Application 的现有可执行文件隔离,并避免系统代理劫持本机 Application WebSocket。

此外,当前 Diff 实际还承担了第二个独立意图:将固件、SD 资源和 portable work 维护能力从 SDK Daemon 迁移到 Desktop 本地 worker。

🔀 偏离检测

源码默认 Application 的 launcher 授权、代理绕过和 CLI 门禁与 PR 标题一致,且实现没有明显功能蔓延。

但维护模块整体删除明显超出“稳定桌面源码联调运行时”的范围。无论这些提交来自同步 main 还是主动迁移,只要它们存在于最终合并 Diff,就必须作为本 PR 的正式内容接受审查。

因此当前存在明显范围偏离:

  • 标题没有体现破坏性控制面变更;
  • 描述没有完整说明协议兼容策略;
  • 测试结果主要围绕 launcher 和 WebSocket,无法覆盖被删除的维护能力;
  • 回滚说明称“成功配置路径保持不变”,但维护 API 调用路径实际上被移除;
  • 配套 PR 的存在不能替代 SDK 自身的版本与兼容设计。

Merge 建议

🔴 当前不建议合并

局部 launcher 和代理修复方向正确,安全边界也比简单放宽受控目录更可靠;但当前 PR 还不能作为一个可独立、安全合并的变更。

合并前必须满足以下条件:

  1. 处理控制协议兼容

    • 提升 DAEMON_CONTROL_PROTOCOL_VERSION,或增加明确的能力协商;
    • 验证旧 Desktop 对新 Daemon、新 Desktop 对旧 Daemon的行为;
    • 不允许相同协议版本静默删除整组 API。
  2. 处理 PR 范围

    • 将维护能力迁移拆分为独立 PR;
    • 修改标题和描述,将其列为主要架构变更,并提供完整迁移、发布顺序和回滚说明。
  3. 验证跨仓落地

    • Desktop PR #111 确认不再调用 /daemon/maintenance/*
    • Workspace PR #98 确认完整传递两个 source-default 参数;
    • 明确三仓合并和发布顺序,避免产生不可用版本组合。
  4. 完成目标平台验收

    • macOS Homebrew venv;
    • macOS 系统代理环境;
    • Windows pythonw.exe 存在、缺失和目录逃逸场景;
    • 第三方 Application 隔离回归。
  5. 提供未截断 Diff 或仓库上下文重新审查

    • 当前缺少约 4927 行,无法确认大规模删除是否完整、是否残留引用及测试是否充分迁移。
  6. 明确 POSIX symlink 信任模型

    • 说明为什么允许使用校验后的原始路径;
    • 确认 launcher 所在目录的写权限边界;
    • 覆盖校验与 spawn 之间替换路径的风险或明确接受该风险。

总结: 默认 Application 的 venv 保留、第三方隔离和本机代理绕过实现方向基本正确,但 PR 混入大规模维护控制面删除,且没有升级协议版本或完成跨仓兼容闭环;当前版本不应直接合并。

@Mr-KID-github
Mr-KID-github force-pushed the codex/sdk-desktop-runtime-main branch 4 times, most recently from abfc7d5 to 58ead6c Compare August 17, 2026 10:41
为源码默认 Application 增加根目录与启动器成对配置校验,并在 POSIX 保留受信任 venv 启动路径,在 Windows 校验 python.exe/pythonw.exe 不得通过重解析点逃出授权目录。

普通 Application 仅允许使用 Application Store 受管根目录内的 Python 启动路径,保留虚拟环境语义;包内可执行文件继续要求解析目标位于资源根目录。CLI 启动后台 Daemon 时不再将 venv Python 解析为基础解释器。

本机 Desktop/Device WebSocket channel 显式绕过系统代理,并兼容不同 websockets 版本。补齐 CLI 参数门禁、真实双 venv、代理、Windows 回退与逃逸、Daemon 路由及子进程回归测试,同时记录 POSIX 符号链接的本机信任边界。

本提交不删除维护 REST API、固件/SD 依赖,也不调整控制协议版本。
@Mr-KID-github
Mr-KID-github force-pushed the codex/sdk-desktop-runtime-main branch from 58ead6c to a41db0b Compare August 17, 2026 10:49
@orulink-wugui

Copy link
Copy Markdown
Contributor Author

已根据 Luxiao 审查报告完成整改并重新压缩为单提交:

  • 已移除与本 PR 无关的维护能力迁移;当前 Diff 不删除 /daemon/maintenance/*、不移除维护依赖、不修改 DAEMON_CONTROL_PROTOCOL_VERSION
  • 已补齐 macOS/POSIX venv、无效系统代理、Windows pythonw.exe 优先/回退及路径逃逸、第三方 Application 隔离等回归覆盖。
  • 已记录 POSIX launcher 符号链接的信任边界。
  • SDK CI 全绿:Python 3.10–3.12 最低/最新依赖、BLE、发行包构建、twine 检查及 wheel 安装均通过。
  • 当前仅 PR Review by Luxiao 显示失败;日志确认自托管 Runner 在读取 PR Diff 前因 gh: command not found 退出(exit 127),未执行新的代码审查。该失败属于 Runner 环境问题,不是本 PR 的测试或审查结论。

当前提交:a41db0b fix(daemon): 收紧桌面源码联调运行时边界

@Mr-KID-github
Mr-KID-github merged commit d7c0246 into main Aug 17, 2026
8 of 9 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.

2 participants