refactor(desktop): make the session UI store the only pending authority

[Astro-Han]

Summary

A simplification audit of the session-workspace slice, following the perf fix in #4110 (now merged; this branch is rebased onto it and the whole diff below is its own — 16 files, +360 / −440). Four concepts leave the renderer. Nothing is added that the code did not already imply.

One authority for "an action is in flight." Four facts each had two representations: a Set ref that the duplicate guard read, and a Record<string, boolean> in the session UI store that the disabled mask rendered — message retry, stop, permission mode, session model. Nothing read across the pair. They were kept aligned by hand at every claim, every finally, and two separate teardown paths that agreed only because one called the other in the right order.

The ref half is redundant: state replacement in the controller is synchronous, so a claim is visible to the next getState() in the same task, and the guard can read the map it already writes. createPendingClaim puts both halves behind one compare-and-set. Gone with it: addPendingSessionAction / clearPendingSessionAction and their optional-setter parameter that both call sites always passed, three private copies of omitSessionKey that existed only to maintain the map half, the four set*BySession setters those copies fed, and one of the two teardown paths.

The pending registry collapses onto the turn footer. useKeyedPendingRegistry was generic over trackState and autoClearMs because it served three instances. Two moved to the store above, and both options had only ever had one consumer — the turn footer needs the reactive snapshot for its disabled mask and the timers so a dropped sessions:changed cannot disable a button forever. useTurnActionRegistry says that directly. Separately, clearAll() had no caller: the unmount cleanup in app-shell-effects walked timersRef and keysRef itself, reaching around the method that exists to do it. It now calls clearAll(), which is why timersRef no longer needs to be public.

The store is reached through its controller. useAppShellSessionUiState returned the controller plus a member-by-member copy; useAppShellSessionWorkspace copied that copy; AppShell destructured the result. Adding a map meant editing three lists nothing keeps in agreement, and sessionUiController was already in the same destructure. The hook now returns the controller. 27 call sites name it — longer to read, shorter to keep true.

The delegating facade gets one home and a contract test. stable-actions.ts was split from its hook on the stated grounds that staying React-free made it testable from node:test. No such test existed and createDelegatingActions had one consumer. The claim is honoured the other way round: the facade moves in with useStableActions, becomes private, and the contract is asserted through the hook, where React's commit semantics are part of what is being promised.

Refs #4109

Verification

• tsc for the renderer, main, and preload configs — clean
• biome format across the repository, biome check on every touched file — clean
• 59 affected unit tests across session settings, stop, chat send, session navigation, workspace action identity, and the new facade contract — pass
• Playwright session-rail-render-contract — passes, which exercises this rewiring end to end in a real window

Both new assertions were checked for falsifiability. return actions in place of the facade fails with the facade itself is re-created; the pending-claim tests fail if the claim stops being compare-and-set.

Not run: the full repository suite.

Behaviour change

setPermissionMode now claims before its bypass confirmation rather than after, so a second click cannot open a second dialog. The cost is that the control reads as pending while the user decides, which is what is true. A cancelled confirmation releases the claim.

Review focus

The identity contract is the part worth arguing about. It constrains the mechanism — identities fixed across renders, calls delegating to the latest committed closures — rather than each of the nine useStableActions call sites, so it covers them by construction. That is deliberate: per-factory tests would need dependency graphs only AppShell can assemble, and they would assert the same thing nine times.

What no identity test can catch is a factory that goes through neither the facade nor a once-created object. A bare function declaration in a hook body — the actual #4109 defect — keeps its dependency arrays correct and has nothing for useExhaustiveDependencies to flag. That gap is why the Session rail also carries the outcome budget from #4110, and why the comment in session-workspace-actions.ts that used to argue one mechanism over the other now defers to the tests. Two mechanisms for stable identity remain, and that is intended: the facade exists for factories whose closures capture changing deps, and a factory whose deps are structurally constant should not pay for it.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code. It ran the audit that found these four, traced the demand chains behind each removal, wrote the change and the tests, and drafted this description. The human contributor reviewed the diff, the commit messages, and the behaviour change above. Generated-by trailers are on all four commits.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Behaviour change above
• No

[jackwener]

I found no P0–P3 issues on exact head f3904c852f90766e96b24511361f0232e10efae4.

Retry, stop, permission-mode, and session-model each kept “in flight” in two places: a Set the guard read and a store map the disabled mask rendered. Nothing read across the pair; they stayed aligned only by hand. createPendingClaim writes the map the UI already reads, and replacement is synchronous, so the next getState() in the same task sees the claim. That is a real dual-authority cut, not a file move.

The one behavior change is reachable: setPermissionMode claims before the bypass dialog, so a second click cannot open a second dialog, and a cancelled confirmation releases the claim. Turn-footer pending stays in useTurnActionRegistry because it still needs timers and a reactive snapshot. The workspace hook now returns the controller instead of a member-by-member copy. I did not treat issue #4109 as evidence.

I am not merging. This is a refactor; merge is a human call. Hosted test was still queued when I posted. This review does not claim CI is 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.

简体中文

精确 head f3904c852f90766e96b24511361f0232e10efae4 上我没有发现 P0–P3。

重试、停止、权限模式、会话模型各自把「进行中」存了两份：守卫读 Set，禁用态读 store map，两边没有交叉读取，只靠手写对齐。createPendingClaim 写 UI 已经在读的那份 map，替换是同步的，同一次任务里下一次 getState() 能看到。这是真的去掉双重权威，不是搬家。

行为变化走得到：setPermissionMode 在 bypass 确认框之前就占位，第二次点击打不开第二个对话框，取消会释放。回合页脚仍用 useTurnActionRegistry，因为它还需要定时器和响应式快照。workspace hook 现在直接返回 controller。我没有把 issue #4109 当证据。

这是重构，我不合入，合入由人类决定。发这条时 hosted test 还在排队，这次审查不表示 CI 已绿。

本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查，也不能替代人类审查。有人类对本条评论负责，如有错误请直接指出。

[M4n5ter]

I found no P0–P3 issues on exact head f3904c8.

The refactor preserves the retry, stop, permission-mode, model/thinking, and turn-footer pending guards while making each session-level pending flag use the same synchronous store entry that the UI renders. The turn-footer registry retains its timer and reactive snapshot, and the stable-action facade still delegates to the latest committed closures.

The hosted test check completed successfully, including build, typecheck, affected workspace tests, desktop end-to-end coverage, and Storybook smoke. I am not merging this refactor; merge remains a human decision.

---

Posted by an automated review agent operated by @Astro-Han. 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.

简体中文

精确 head f3904c8 上没有发现 P0–P3 问题。

这次重构保留了消息重试、停止、权限模式、模型/思考级别和回合页脚的进行中保护，并让每个会话级进行中标记都使用 UI 正在渲染的同一份同步 store 状态。回合页脚仍保留定时器和响应式快照，稳定 action facade 仍然转发到最近一次已提交 render 的闭包。

Hosted test 已成功完成，包括构建、类型检查、受影响 workspace 测试、桌面端到端测试和 Storybook smoke。本条不执行合并；合并仍由人类决定。

本条评论由 @Astro-Han 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查，也不能替代人类审查。有人类对本条评论负责，如有错误请直接指出。

[zhiiw]

Reviewed at exact head f3904c85 (16 files, +360/-440).

The four collapses all hold up under reading: the pending claim is a genuine compare-and-set (the store's state replacement is synchronous, so a claim is visible to the next read in the same task and the old ref half was redundant); the controller is returned directly instead of a member-by-member copy; the generic keyed registry folds into the one concrete turn-footer registry, with the auto-clear timer kept because a dropped sessions:changed must not disable a button forever; and the delegating facade moves in with its hook and gains a contract test that fails if you bypass it.

The one behavior change — claiming before the bypass confirmation rather than after — is correct: a second click while the dialog is open no longer opens a second one, and a cancelled confirmation releases the claim.

Verified locally on a real Windows machine at this head: clean desktop-main rebuild, and the affected suites (use-stable-actions, app-shell-session-settings-actions, app-shell-stop-action, session-workspace-action-identity) 15/15 pass. test is completed/success on the exact head.