Repository navigation
fix(runtime-host): make managed WSL recovery actionable - #5175
Conversation
Tell source TUI users how to return to the managing Desktop for an explicit Host handoff. Run Desktop setup and update through the same WSL user login environment so current zsh, fish, or bash configuration can provide Node and npm. Show validated development archive versions instead of package integrity. When recovery fails, direct users to the diagnostic and avoid implying that unchanged retries will repair a deterministic failure. Refs #5143, #5161 Generated-by: OpenAI Codex
jackwener
left a comment
There was a problem hiding this comment.
APPROVE, bound to 5eaa1277225066ec4751ed2f886a798edb35fe1c. No P0–P3. Hosted test and label are terminal green on this exact head.
Three changes, each with one thing worth checking rather than reading past.
The filename-derived version really is display-only, and I confirmed that from the consumers rather than from the comment. developmentArchiveDisplayVersion parses maka-agent-(.+)\.tgz off a basename and only gates it through isProductReleaseVersion, so a file renamed maka-agent-99.0.0.tgz would display 99.0.0 whatever it contains. That is safe only if the value cannot reach a decision. It cannot: the sole consumer of view.packageChange is host-handoff-copy.ts, which formats `${target.name}: ${current} → ${target}`. The packageChanged identifiers in runtime-host-setup-command.ts are an unrelated local computed from sameExactPackage, not this field. The authoritative targetVersion still drives the compare-and-skip branch above, and runtimeHostSetupPackageDisplayVersion returns the real specifier version for non-development packages, so the loosened value exists only on the branch that previously had nothing better than a sha512 to show.
The login-shell change lands on the update path, and it is asserted there. runtimeHostWslLoginCommand is extracted from the setup path and now also wraps the update invocation, so the update runs under "${SHELL:-/bin/sh}" -lic instead of a bare /bin/sh -c. That is a real behaviour change for update — it now sources the user's login and interactive shell configuration, which is presumably the point, since a managed WSL update that cannot see the user's PATH is exactly the unactionable failure this PR is named for. Both call sites are pinned: the new assert.match(…, /\$\{SHELL:-\/bin\/sh\}.*-lic/u) appears in the setup test and in the update-cancellation test, so the two paths cannot drift apart again silently.
The copy changes are complete and the new test would fail before them. repair_required and the managed recovery guidance are both updated in all three locales, and development is added to all three GUIDANCE blocks. development handoff never presents package integrity as a version asserts target: 'selected development build' on exactly the input that previously rendered sha512-selected, so it falsifies the old behaviour rather than restating the new one.
One note for readers of the adjacent file, not a finding here. The new managed guidance is exercised with reason: 'operator_required', which is the condition formatHostHandoff requires before recoveryBlocker guidance replaces the description at all. A separate P3 I raised on #5036 still stands unchanged: when the blocker carries reason: 'repair' — a deployment record in handoff state whose owning installation differs — the view reason becomes repair_required and the specific guidance is skipped. This PR makes that fallback materially better, since repair_required now points at the diagnostic and says that repeating the action without a state change will usually fail again, but it does not change which branch is selected. Nothing to do in this PR.
Scope and limits. Static review of the four production files and the four test files, plus a consumer search for packageChange across apps and packages to settle the display-only question. I did not build or run any suite locally — the hosted run covers those — and I did not exercise WSL, so the login-shell behaviour is reviewed as a command-construction change and a test assertion, not observed on Windows. This is a single review line; @Muse-Qronos-AstroHan has a lane on this PR as well.
简体中文
绑定 5eaa1277225066ec4751ed2f886a798edb35fe1c,无 P0–P3,两项托管检查在此 exact head 终态为绿。三处改动各有一个值得核而非略过的点。
从文件名派生的版本确实只用于展示,而这一点我是从消费者一侧核实的,不是采信注释。 developmentArchiveDisplayVersion 从 basename 解析 maka-agent-(.+)\.tgz,仅用 isProductReleaseVersion 把关 —— 因此一个被改名为 maka-agent-99.0.0.tgz 的文件,无论内容如何都会显示 99.0.0。只有当该值不可能参与任何判断时,这才是安全的。事实确实如此:view.packageChange 的唯一消费点是 host-handoff-copy.ts,把它拼成 `${target.name}: ${current} → ${target}`;runtime-host-setup-command.ts 中的 packageChanged 是由 sameExactPackage 算出的同名无关局部变量。权威的 targetVersion 仍然驱动上方的比较与跳过分支,而 runtimeHostSetupPackageDisplayVersion 对非开发包返回真实的 specifier 版本 —— 所以这个被放宽的值只存在于"此前只能显示一串 sha512"的那条分支上。
登录 shell 的改动落在更新路径上,并且在那里有断言。 runtimeHostWslLoginCommand 自安装路径抽出,现在也包住更新调用,故更新在 "${SHELL:-/bin/sh}" -lic 下运行而非裸 /bin/sh -c。这是更新路径的真实行为变化 —— 它现在会加载用户的登录与交互式 shell 配置,而这大概正是本意:一次看不到用户 PATH 的托管 WSL 更新,正是本 PR 标题所说的"无法采取行动的失败"。两个调用点都被钉住:新增的 assert.match(…, /\$\{SHELL:-\/bin\/sh\}.*-lic/u) 同时出现在安装测试与更新取消测试中,两条路径无法再悄悄分叉。
文案改动完整,且新测试在改动前会失败。 repair_required 与 managed 恢复指引三语齐改,development 三语齐加。development handoff never presents package integrity as a version 断言的正是此前会渲染出 sha512-selected 的那个输入,因此它证伪旧行为,而不是复述新行为。
一条提示,针对相邻文件的读者,不计为本单发现。 新的 managed 指引是在 reason: 'operator_required' 下被验证的,而这正是 formatHostHandoff 用 recoveryBlocker 指引替换描述的前提条件。我在 #5036 上提的那条 P3 依然原样成立:当 blocker 带 reason: 'repair'(部署记录处于 handoff 态且拥有它的安装与本地不一致)时,视图 reason 变为 repair_required,专属指引被跳过。本 PR 显著改善了那条兜底(repair_required 现在指向诊断信息,并说明状态未变时重复操作通常仍会失败),但并未改变分支的选择。本单无需处理。
本次审查的边界:对四个生产文件与四个测试文件的静态审查,外加在 apps 与 packages 中对 packageChange 的消费者搜索以定论"仅用于展示"。未在本地构建或运行任何套件(由托管运行覆盖),也未实际操作 WSL —— 因此登录 shell 的行为是作为命令构造变更与测试断言来审查的,并非在 Windows 上观察到的。这是单独一条评审线;@Muse-Qronos-AstroHan 亦在本 PR 上有一条线。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
Seven upstream commits: managed WSL recovery and handoff (apache#5175, apache#5161, apache#5160), Session bundle import (apache#5139), WorkHub model routing and delegated results (apache#5152, apache#5150), React dev performance measures (apache#5156). Resolution per the sync policy: every conflict under the old renderer's WorkHub, composition and platform trees, its stories and the main tests that import them stays deleted; upstream's new files in those trees (delegation-feedback.ts, react-performance-measures.ts and its test) are dropped; `components/ui/textarea.tsx` was a rename/rename against `features/workhub/index.ts` and keeps ours; `locales/conversation-copy.ts` auto-merged and is Biome-formatted. Nothing in this round is behaviour the new renderer has to re-implement. Gates: build:test + build:renderer, typecheck, ledger, biome, knip, locale hygiene, ASF headers, desktop dist tests (1575), renderer state (237), Electron smoke (44), core-dialogue smoke. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The managed WSL handoff added by #5161 can reach its explicit Desktop action, but the update command runs in
/bin/shwithout the user login environment. A WSL installation whose Node/npm is provided by mise, nvm, or shell initialization therefore fails deterministically withnpx: not found, even though initial setup succeeded. The failure surface then suggests retrying and the source TUI sends the user back to Desktop, leaving no executable recovery path.This change:
$SHELLrather than a recorded or hard-coded Node installation;The WSL setup/update shell behavior was also reproduced against an installation where a noninteractive shell cannot find
npx, while the user login shell resolves the current mise-managed Node/npm successfully.Refs #5143
Follow-up to #5161
Validation:
npm run rebuildnpm run typechecknpm run lintnpm run format:checknpm run check:stale