fix(desktop): prepare peer support in development - #4134
Conversation
4c4086a to
a2666f1
Compare
jackwener
left a comment
There was a problem hiding this comment.
Approved at exact head a2666f1ac746eccdeffacebe15b95ef194233bee; I found no P0–P2 issues.
The supported Desktop development entry points now build the native peer addon before the app performs its startup capability probe, and the build output path is the exact path that the probe reads. The setup-package resolver also normalizes the development archive and computes its SHA-512 evidence once, then local, SSH, and WSL setup forward that same evidence to the Runtime Host setup command. Packaged exact-version setup remains unchanged.
Verification on this head passed the root build, 52 focused Desktop setup/management tests, Biome, git diff --check, and a direct script/path contract probe. The PR merges cleanly with current main at 82b36dc6d7480bb75d7fc511cbcbe0658f55cf9f, and the resulting full root build passes. This machine has Rust 1.95 while the new development prerequisite is Rust 1.98, so I could not execute the native preparation command locally. The exact-head hosted test and audit checks are successful. package is still running, and the author explicitly chose to wait for it after this approval; I am not counting that running check as green.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
7b99235 to
e981092
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for working through this—there is real value in fixing the mismatch between the startup capability probe and the missing development addon, and in preventing CLI packaging from overwriting Desktop’s live addon.
The resolver ownership also looks clean: it resolves one canonical archive and its integrity evidence, while local, SSH, and WSL paths only adapt and forward that result. I didn’t find unnecessary tests or a simpler way to preserve those cross-platform boundaries.
I found one merge-blocking behavior change in the normal Desktop development path, noted inline. Once ordinary non-peer development remains independent of the native peer toolchain—or the existing WSL contract is deliberately revised with a clear rationale—the rest of the approach looks good to me.
Reviewed with OpenAI Codex as an AI-assisted review; I verified the affected development entry points, setup-package ownership, WSL contract, and packaging output path.
中文对照
感谢你把这块梳理得这么完整。启动时会探测 peer addon、但开发入口没有准备它,确实存在不一致;CLI 打包不应该覆盖 Desktop 正在使用的 addon,这也是有实际价值的修复。
resolver 的职责划分也比较干净:统一解析规范化 archive 和完整性信息,本机、SSH 与 WSL 路径只负责转换和转交。我没有看到可以明显删除的低质量测试,也没有发现能在保留这些跨平台边界的同时大幅简化实现的方式。
我在行内标出了一个会阻塞合并的行为变化:当前实现会让普通 Desktop 开发也必须具备 native peer 工具链。只要普通的非 peer 开发重新保持独立,或者团队明确重新定义现有 WSL 契约并说明原因,其余实现方向我没有问题。
本次审查由 OpenAI Codex 辅助完成;我核对了受影响的开发启动入口、setup-package 职责、WSL 契约以及打包输出路径。
| "main": "dist/main/main.js", | ||
| "scripts": { | ||
| "start": "node scripts/start-dev-app.mjs", | ||
| "start": "npm run prepare:runtime-host-peer && node scripts/start-dev-app.mjs", |
There was a problem hiding this comment.
[P1] Keep ordinary Desktop development independent of Direct Peer
This makes every Desktop development launch fail before Electron starts when Rust, Cargo, or the platform linker is unavailable—even when the contributor only needs the ordinary WSL framed-stdio path. That conflicts with the contract established in #4085/#4087, where ordinary WSL setup remains independent of Direct Peer and does not require Rust or Zig.
Could we keep native preparation scoped to a peer-enabled development path, or otherwise preserve the existing graceful startup when the optional addon cannot be built? The archive/integrity resolver and isolated packaging output look sound; the blocking concern is making an optional capability a prerequisite for every development launch.
There was a problem hiding this comment.
English
Confirmed and addressed at head 27e3d332a.
The development contract now keeps the toolchain cost with the capability that needs it:
npm run devandnpm run dev:fulldo not build or enable the native peer addon. An addon left in the workspace by an earlier build is deliberately ignored, so ordinary Desktop development remains independent of Rust, Cargo, and the platform linker.npm run dev:peerandnpm run dev:full:peerstrictly build the addon and then reuse the same HMR and full-build launchers. Build failures remain fatal on these peer-enabled paths; there is no best-effort fallback that could hide a native regression.- The Runtime Host WSL setup still resolves the
nonetarget and produces the peer-less archive, preserving the dev(runtime-host): build target-aware archives for cross-platform hosts #4085/dev(runtime-host): build target-aware host packages #4087 WSL contract. - Packaged Desktop behavior and explicit native-path test overrides are unchanged.
- README and CONTRIBUTING now describe Rust 1.98 and the linker as additional prerequisites only for Direct Peer and Peer Mesh development.
A focused behavior test fixes the boundary: an existing workspace addon stays disabled for an ordinary development launch and is selected only for an explicit peer-enabled launch. The affected peer-client, setup-package, local, SSH, and WSL suites pass (29 tests), together with lint and formatting.
简体中文
已确认,并在 head 27e3d332a 处理。
开发契约现在把工具链成本限制在真正需要它的能力上:
npm run dev和npm run dev:full不构建、也不启用 native peer addon。即使工作区残留此前构建的 addon,普通 Desktop 开发也会明确忽略它,因此不依赖 Rust、Cargo 或平台 linker。npm run dev:peer和npm run dev:full:peer会严格构建 addon,再复用相同的 HMR 与完整构建 launcher。Peer 开发路径上的构建失败仍然是致命错误,不会通过 best-effort fallback 掩盖 native regression。- Runtime Host WSL setup 仍解析
nonetarget 并生成 peer-less archive,保留 dev(runtime-host): build target-aware archives for cross-platform hosts #4085/dev(runtime-host): build target-aware host packages #4087 的 WSL 契约。 - 正式打包 Desktop 的行为和显式 native-path 测试 override 均未改变。
- README 与 CONTRIBUTING 只把 Rust 1.98 和 linker 列为 Direct Peer、Peer Mesh 开发的额外前置条件。
一项聚焦行为测试固定了这个边界:普通开发不会因为工作区已有 addon 而启用 Peer,只有显式 Peer 开发入口会选择它。peer-client、setup-package、本机、SSH 与 WSL 受影响测试共 29 项通过,lint 与 format 也通过。
Prepare the native peer addon before every Desktop development launch. Keep live addon and package staging under separate owners, and let the setup resolver own immutable development archives with exact SHA-512 evidence. Generated-by: OpenAI Codex
e981092 to
27e3d33
Compare
Summary
English
Prepare the native Runtime Host peer addon for explicit peer-enabled Desktop development launches, so Direct Peer and Peer Mesh are available from both HMR and fully built starts without imposing the Rust toolchain on ordinary Desktop work.
npm run dev:peerandnpm run dev:full:peerbuild the addon strictly before reusing the normal launchers. Ordinary development remains peer-less even when a previous native artifact is still present in the workspace. The setup-package resolver owns one canonical development archive and its exact SHA-512 evidence. Explicit overrides are retained as immutable resolver snapshots, while CLI packaging writes native prebuilds to isolated staging instead of Desktop's live addon. Local, SSH, and WSL paths only adapt and forward the resolved value.中文
为显式启用 Peer 的 Desktop 开发入口准备 Runtime Host 原生 addon,使 Direct Peer 与 Peer Mesh 在 HMR 和完整构建启动中均可用,同时不要求普通 Desktop 开发安装 Rust 工具链。
npm run dev:peer与npm run dev:full:peer会严格构建 addon,再复用普通 launcher。即使工作区残留此前构建的 native artifact,普通开发仍保持 peer-less。setup-package resolver 统一持有规范化的开发版 archive 及其精确 SHA-512 evidence。显式 override 会成为 resolver 管理的不可变快照,CLI 打包则把 native prebuild 写入隔离 staging,不再覆盖 Desktop 正在使用的 addon;本机、SSH 与 WSL 路径只负责转换和转交最终值。Verification
English
npm run lintnpm run format:check中文
npm run lintnpm run format:checkAI use
Select exactly one:
Tool(s) and scope: OpenAI Codex implemented and verified the change under the contributor's direction and review
Checklist
Does this PR entail a change in behavior?