fix(desktop): repair imported sessions from workspace readiness notice

[me2seeks]

Summary

Refs #5551

• Reuse main's existing Host-authoritative bridge.sessions.moveToProject(sessionId, projectId) path for recovery; this PR adds no duplicate Runtime Host client, IPC channel, preload method, or bridge contract.
• Make the workspace-readiness action open a picker scoped to the active Session's Runtime Host.
• Relocate the existing Session after selecting an available Project; adding a local Project also repairs that same Session.
• Mount the recovery picker closed and open it on the following render so the controlled DropdownMenu can place keyboard focus correctly.
• Keep the regular new-task workspace picker behavior unchanged.

This is the already-imported-Session recovery half of #5551; PR #5552 covers import-time prevention.

Validation

• Rebased onto origin/main@1c18700a7.
• npm run check:architecture --workspace @maka/desktop: 112 fixture tests passed and the checker passed.
• Task Entry boundary test: 4 passed.
• Biome and diff checks passed for all changed files.
• Changed-file TypeScript checks passed after rebuilding the available workspace artifacts.

The complete local Desktop dependency build remains blocked by the worktree dependency snapshot missing ai/@ai-sdk/provider and virtua; the affected-source checks above reported no errors. Hosted CI for this new head is running.

[jackwener]

Seat: kabi-opus-review-orchestrator. Review of exact head 0ed0c8a49, against merge-base e2a7cf233 and main 1c18700a7. COMMENT only.

Verdict

1×P1, 3×P3. I don't recommend merging this head as it stands — not because the branch is wrong, but because main landed the same Host operation 46 minutes after this PR was opened, with a deliberately different concurrency contract. The recovery UI itself is the valuable and non-duplicated part.

I also closed both validation gaps you flagged in the description — see the bottom. Desktop main typecheck passes with 0 errors, and the provider test runs.

P1 — main already ships this Host operation, with retry semantics it explicitly rejected

apps/desktop/src/main/runtime-host-client.ts

Main commit 1c18700a7 ("name a project when creating it, and move tasks between projects", landed 2026-09-21T04:02Z; this PR was opened 03:16Z) added its own relocateSessionWorkspace to DesktopRuntimeHostClient, plus an IPC channel, preload method and bridge-contract entry for the same session.workspace.relocate operation. A test merge conflicts in two files:

CONFLICT (content): apps/desktop/src/main/runtime-host-client.ts
CONFLICT (content): .../task-entry/controller/use-task-entry-controller.ts

The controller conflict is trivial (a dependency-array collision with newProjectDialog/openNewProject). The client conflict is not, and this is the part I'd ask you to look at before rebasing:

• Main's version takes the revision and does not retry. Signature relocateSessionWorkspace(sessionId, expectedRevision, workspace); on a non-committed result it throws revisionConflict. Its comment is explicit: "Deliberately a single attempt… retrying against a fresher one would commit that stale directory under the new revision… A conflict is the answer, not a replay." Its caller moveSessionToProject needs that, because for the detach case it sends { kind: 'host_path', path: current.workspace.hostCwd } — a directory that is only meaningful paired with the revision it was read at.
• This PR's version retries. It routes through #updateSession, which loops to MAX_SESSION_REVISION_ATTEMPTS, re-reading the Session and re-committing against each fresher revision.

On this branch in isolation that is harmless: this PR always sends { kind: 'project', projectId }, which carries no revision-bound directory. The hazard is created by the merge. Whichever body survives becomes the single method on a shared class, and if it is the retrying one, main's host_path detach silently acquires exactly the replay its comment was written to prevent. The arity differs (2 args vs 3), so the compiler catches a straight substitution — but it will not catch a resolution that keeps main's signature around this body, and the conflict markers say nothing about retry-vs-conflict.

There is also a simpler consequence worth weighing. Main's bridge.sessions.moveToProject(sessionId, projectId | null) is already exposed through preload and the bridge contract, and for a non-null projectId it does precisely what this PR's new path does. So after a rebase, TaskEntrySessionService.relocateWorkspace could call that existing bridge method, and the new client method, the sessions:relocateWorkspace channel, the preload method and the bridge-contract entry could all be dropped. That removes about five files and leaves the part main does not have: the recovery-scoped picker. Two IPC channels for one Host operation, differing only in their concurrency contract, is the outcome I'd most like to avoid.

To be fair about attribution: the two landed 46 minutes apart. Nothing here is a mistake on your side — it is a collision, and it needs a decision rather than a fix.

P3 — the recovery menu opens without taking keyboard focus

apps/desktop/src/renderer/features/task-entry/ui/task-entry-provider.tsx

With a Session active, the Composer does not render the picker at all (!props.activeSession || props.workspacePicker.showForActiveSession). Opening recovery flips showForActiveSession and isMenuOpen in the same render, so Astryx's DropdownMenu mounts for the first time already open. That shape is special-cased in the library:

// Mounting already open (`isMenuOpen` true on the first render) is not an
// open anyone asked for, so it must not move focus into the menu — that
// drops keyboard users mid-page (#5976).
const isMountedOpenRef = useRef(isControlled && controlledIsOpen === true);
…
shouldFocusOnOpenRef.current = !isMountedOpenRef.current;   // false here

The focus effect then returns early, so focus stays on the readiness notice. Here the mount-open is an open someone asked for, and the guard defeats it. The ref clears only when the consumer closes — but closing unmounts the picker, so the next recovery mounts fresh and suppresses focus again; this is every time, not just the first.

Mouse users are unaffected and keyboard users can Tab to it, which is why I've left it at P3 rather than P2 — but if keyboard parity on a new affordance is a release gate for you, it belongs one level up. Keeping the picker mounted with isMenuOpen starting false and flipping it after mount would let the library's normal open path run.

Source-traced, not reproduced in a real window. I read the installed @astryxdesign/core@0.6.2; I did not run Electron.

P3 — addSessionWorkspace has no test

"Adding a local Project also repairs that same Session" is one of the four stated behaviours, and addSessionWorkspace appears nowhere outside the controller and the provider — no test references it. The new provider test uses profileKind: 'remote', so it takes the branch where onAdd is undefined; the local branch that is the only route into addSessionWorkspace is never taken. That path also chains three steps (addProject → refreshAfterProjectMutation → relocateSessionWorkspace), which is where ordering mistakes live.

P3 — every relocate failure reports the same message

TaskEntrySessionWorkspaceResult distinguishes session_busy | operation_conflict | operation_unavailable | not_found, and the controller reports projectUpdateFailedTitle/projectUpdateFailedFallback for all four. session_busy is the one I'd separate: it is transient and retrying works, and a user repairing a workspace may well have a turn in flight. "Project update failed" tells them to stop trying.

What I ran

Your description lists two blockers; both clear with a fully built dependency chain.

• Desktop main typecheck: 0 errors. This covers runtime-host-client.ts and runtime-host-session-catalog-ipc-main.ts. The @maka/core/thread-search gap you hit is a stale-artifact effect — main renamed that export to transcript-search, so a dist built on an older branch resolves the old name. Rebuilding @maka/core clears it.
• Provider test runs: 3/3, including scopes active-session recovery to available projects on that Host. The virtua failure also came from an unbuilt @maka/ui dist.
• Full Desktop suite: 2799/2799, 0 failures. I first had to delete three orphaned compiled tests left in dist by a build on another branch (thread-search, multi-host-thread-search, runtime-host-search-ipc-main) whose sources no longer exist; without that they fail as phantoms.
• check:architecture: 112/112 and the checker passed — and separately with --base e2a7cf233 --strict-base, which is the run that enforces base monotonicity rather than only the ledger. Clean. The activeDesktopSession/activeMessageSubmitting inlining and sessionCount === 0 → !sessionCount in app-shell.tsx read as debt-shaving to stay under that gate; sessionCount is … ?? 0, so the rewrite is equivalent.

Checked and not reported. activeSession (passed to the picker consumer) and activeSessionForView (passed to the Composer) diverge only while a Session id is set and its summary has not loaded; there activeSession is undefined and the placeholder is truthy, so both sides agree the picker stays hidden. The recovery consumer is on the sharedSessionActive && activeId else branch, so it does render for an ordinary active Session. runtimeHostId is required and is the same host.hostId the group carries, so the group lookup cannot match on a shared undefined. And session.workspace.relocate already exists on main, so no protocol epoch bump is owed here.

未验证

No Electron run, no real Host, no Windows/Linux, no full-repository suite, no e2e. Hosted CI shows test green; that is the branch's own green against e2a7cf233, not a merge result — main has moved 2 commits since, and as shown above the merge is not clean.

Cross-PR note. #5552 is the import-time half and is disjoint from this one except apps/desktop/src/main/runtime-host-client.ts, where both add a type WorkspaceTarget import — a textual conflict, not a semantic one. Neither branch has been tested against the other. #5552's own review is posted separately; its only finding is a missing epoch changelog entry.

简体中文

结论:1×P1、3×P3,不建议按当前 head 合并。 原因不是这条分支写错了,而是本 PR 开出 46 分钟后,main 落地了同一个 Host 操作,而且并发语义是刻意不同的。恢复用的 picker UI 本身才是有价值、且未被重复的部分。描述里你标注的两处验证阻塞我都已打通(见末尾):Desktop main typecheck 0 错误,provider 测试可以跑。

P1 —— main 已经有这个 Host 操作,且明确拒绝了重试语义。 main 提交 1c18700a7(04:02Z;本 PR 开于 03:16Z)自己加了 relocateSessionWorkspace,并配了 IPC 通道、preload 方法与 bridge 契约,指向同一个 session.workspace.relocate。试合并在两个文件冲突:runtime-host-client.ts 与 use-task-entry-controller.ts。后者是琐碎的(依赖数组撞上 newProjectDialog/openNewProject);前者不是:

• main 版接收 revision 且不重试:relocateSessionWorkspace(sessionId, expectedRevision, workspace),非 committed 即抛 revisionConflict。注释写明:「刻意只尝试一次……对更新的 revision 重放会把那个陈旧目录提交进去……冲突就是答案,而不是重放。」它的调用方 moveSessionToProject 在 detach 分支发送 { kind: 'host_path', path: current.workspace.hostCwd },该目录只有与读它时的 revision 配对才有意义。
• 本 PR 版会重试:走 #updateSession,循环至 MAX_SESSION_REVISION_ATTEMPTS,每轮重读 Session 并对更新的 revision 重新提交。

单看这条分支无害:本 PR 只发 { kind: 'project', projectId },不携带与 revision 绑定的目录。危险是合并造出来的:活下来的那份会成为共享类上唯一的方法;若是重试版胜出,main 的 host_path detach 就会悄悄获得其注释专门要防的重放。两者参数个数不同(2 vs 3),直接替换编译器会拦;但「保留 main 的签名、套上这份实现」它拦不住,而冲突标记本身不会告诉你这是重试与冲突之别。

还有一个更简单的后果:main 的 bridge.sessions.moveToProject(sessionId, projectId | null) 已经通过 preload 和 bridge 契约暴露,projectId 非空时做的正是本 PR 新路径做的事。所以 rebase 之后,TaskEntrySessionService.relocateWorkspace 可以直接调用它,新增的 client 方法、sessions:relocateWorkspace 通道、preload 方法与 bridge 条目都可以去掉 —— 大约少五个文件,只留下 main 没有的那部分:作用域化的恢复 picker。一个 Host 操作配两条 IPC 通道、仅在并发契约上不同,是我最希望避免的结果。

需要讲清楚归属:两者相隔 46 分钟落地,这不是你的失误,是一次撞车;它需要的是决定,而不是修复。

P3 —— 恢复菜单打开了,但不拿键盘焦点。 有活跃 Session 时 Composer 根本不渲染 picker;打开恢复会在同一次渲染里同时翻转 showForActiveSession 与 isMenuOpen,于是 Astryx 的 DropdownMenu 首次挂载时就已是打开状态。该形状在库里被特判:注释写「挂载即打开不是任何人请求的打开,所以不得把焦点移入菜单 —— 那会把键盘用户丢在页面中间(#5976)」,isMountedOpenRef 于挂载时定为 true,shouldFocusOnOpenRef 随即为 false,焦点 effect 提前返回。这里的挂载即打开恰恰是有人请求的打开,守卫把它一并挡了。该 ref 只在消费方关闭时清除,而关闭会卸载 picker,所以下一次恢复又是全新挂载、又被抑制 —— 每次都如此,不只第一次。鼠标不受影响、键盘可 Tab 到达,故定 P3;若键盘等价性对新增交互是发布门槛,应上调一级。做法:保持 picker 挂载、isMenuOpen 初始为 false,挂载后再翻转。此条为读源码所得(已安装的 @astryxdesign/core@0.6.2),未在真实窗口复现。

P3 —— addSessionWorkspace 没有测试。 「新增本地 Project 同时修复该 Session」是四条声明行为之一,而 addSessionWorkspace 除控制器与 provider 外无任何引用。新增的 provider 测试用 profileKind: 'remote',走的是 onAdd 为 undefined 的分支;通往 addSessionWorkspace 的唯一入口 local 分支从未被走到。该路径还串了三步(addProject → refreshAfterProjectMutation → relocateSessionWorkspace),正是顺序类错误的栖身处。

P3 —— 所有 relocate 失败都报同一句话。 结果类型区分 session_busy | operation_conflict | operation_unavailable | not_found,控制器四种一律报 projectUpdateFailedTitle/projectUpdateFailedFallback。我最建议单独处理 session_busy:它是瞬时的、重试即可,而正在修复 workspace 的用户很可能有一轮对话在飞;「项目更新失败」会让他们放弃重试。

我跑了什么。 你列的两处阻塞,在依赖链完整构建后都能通过:①Desktop main typecheck 0 错误(覆盖本 PR 改动的 runtime-host-client.ts 与 runtime-host-session-catalog-ipc-main.ts);你遇到的 @maka/core/thread-search 是陈旧产物效应 —— main 已把该导出改名为 transcript-search,在旧分支上构建出的 dist 仍解析旧名,重建 @maka/core 即消失。②provider 测试 3/3 通过,含新增的 scopes active-session recovery to available projects on that Host;virtua 同样源于 @maka/ui dist 未构建。③Desktop 全量 2799/2799、0 失败;但需先删掉另一分支构建遗留在 dist 中的三个孤儿编译测试(thread-search、multi-host-thread-search、runtime-host-search-ipc-main,源文件已不存在),否则它们会以幻影失败出现。④**check:architecture 112/112 且检查器通过**,并另跑了 --base e2a7cf233 --strict-base(这一档才强制基线单调性,而非只比对账本)—— 干净。app-shell.tsx 中 activeDesktopSession/activeMessageSubmitting 的内联与 sessionCount === 0 → !sessionCount 读起来像是为压住该门槛而削减债务;sessionCount 是 … ?? 0,故改写等价。

已查但不作为发现。 activeSession(传给 picker consumer)与 activeSessionForView(传给 Composer)仅在「有 Session id 而摘要未加载」时分叉,此时前者 undefined、后者为占位符,两侧都判定 picker 隐藏,一致。恢复 consumer 位于 sharedSessionActive && activeId 的 else 分支,普通活跃 Session 确实会渲染。runtimeHostId 为必填且就是 group 携带的 host.hostId,不会在共同的 undefined 上误匹配。session.workspace.relocate 在 main 上已存在,故此处不欠协议 epoch 提升。

未验证:未跑 Electron、无真实 Host、无 Windows/Linux、无全仓库套件、无 e2e。CI 的 test 是绿的,但那是本分支对 e2a7cf233 自己的绿,不是合并结果的绿 —— main 此后已前进 2 个提交,且如上所示合并并不干净。

跨 PR 提示:#5552 是导入时预防的那一半,与本单除 runtime-host-client.ts 外互不相交(两者都在其中新增 type WorkspaceTarget 导入,属文本冲突而非语义冲突)。两条分支没有相互测试过。 #5552 的评审另行发布,其唯一发现是缺少 epoch 变更说明条目。

---

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.

[jackwener]

Seat: kabi-opus-review-orchestrator. Inline anchor for the P1 in the review comment.

[jackwener]

[P1] Main already added this method, and deliberately without retry

Main 1c18700a7 (landed 46 minutes after this PR was opened) added its own relocateSessionWorkspace(sessionId, expectedRevision, workspace) to this class, throwing revisionConflict on a non-committed result, with the comment: "Deliberately a single attempt... retrying against a fresher one would commit that stale directory under the new revision... A conflict is the answer, not a replay." Its caller moveSessionToProject needs that, because for detach it sends { kind: 'host_path', path: current.workspace.hostCwd } — a directory only meaningful at the revision it was read at.

This version routes through #updateSession, which loops to MAX_SESSION_REVISION_ATTEMPTS, re-reading and re-committing against each fresher revision. On this branch that is harmless (the target is always { kind: 'project', projectId }, which carries no revision-bound directory) — the hazard is created by the merge. A test merge conflicts here; whichever body survives becomes the only method on a shared class, and if it is this one, main's host_path detach silently acquires the replay its comment exists to prevent. The arity differs, so a straight substitution fails to compile, but a resolution keeping main's signature around this body does not — and the conflict markers say nothing about retry-vs-conflict.

Also worth weighing: main already exposes bridge.sessions.moveToProject(sessionId, projectId | null) through preload and the bridge contract, and for a non-null projectId it does exactly what this path does. After a rebase, TaskEntrySessionService.relocateWorkspace could call it, and this method, the sessions:relocateWorkspace channel, the preload method and the bridge-contract entry could all be dropped — leaving the part main does not have, the recovery-scoped picker. Two IPC channels for one Host operation differing only in concurrency contract is the outcome to avoid.

This is a collision, not a mistake on your side; it needs a decision rather than a fix.