fix(desktop): make model and thinking-level changes instant without success toasts

[Sun-GLiang]

Summary

• Active-task model, thinking-level, and permission-mode changes complete without success toasts or persistence-only loading presentation. Localized failure feedback remains.
• Model, thinking level, and permission mode use independent useSessionSettingIntent controllers. Controls show the latest requested value immediately, rapid changes converge on the latest intent, terminal failures roll back to the last committed value, and newer catalog revisions retire optimistic overlays.
• Full access confirmation happens before the permission intent starts, so the existing permission value remains visible until the user confirms.
• Optimistic model and thinking values are scoped to their controls. Session-health and execution-state decisions continue to use authoritative session data.
• A model becomes the saved composer default only after Runtime Host confirms the requested connection/model pair.
• Model and thinking menus expose the selected option with radio semantics, and each trigger's accessible name includes its current value.
• Send admission, Stop behavior, and active-turn follow-up ordering are unchanged. Configuration ordering before Host turn admission remains tracked by Allow Session configuration changes while a turn is running #3347.

Fixes #3745

Verification

• UI test suite: 229 passed.
• Desktop test suite: 1474 passed.
• Desktop typecheck passed.
• Biome passed for all 17 changed TypeScript/TSX files.
• Desktop production build passed, including the third-party notice check.
• Diff validation passed with no whitespace errors.
• Intent coverage verifies latest-wins coalescing, rollback to the last committed value, explicit default thinking level, catalog-revision retirement, and session teardown.
• UI coverage verifies cross-connection model changes when two connections expose the same model ID, selected radio state for model/thinking options, and accessible trigger labels containing the current value.
• Architecture coverage verifies that model, thinking level, and permission mode use the generic intent seam and that renderer-side setting state does not order send, Stop, or follow-up paths.
• No product documentation update is needed because the configuration contract is unchanged; this PR changes transient control feedback.

Visual verification

Before

Rapid thinking-level changes created stacked success toasts with obsolete intermediate values.

After — instant, no spinner

Model and thinking-level controls reflect each selection immediately without a spinner, disabled flicker, or success toast.

Static final state (GPT-5.6 Sol / Low):

AI use

• No generative or agentic tooling authored a material portion of this change.
• Generative or agentic tooling authored a material portion of this change.

Tool(s) and scope: OpenAI Codex implemented the session-setting intent integration, transient-feedback cleanup, regression and accessibility coverage, and verification. A Generated-by: OpenAI Codex trailer is present on each materially affected commit.

Checklist

• Tests cover the change or the PR explains why tests are not needed.
• Lint, formatting, and relevant test suites pass locally.
• User-facing changes update docs or the PR explains why docs are not needed.
• AI use is disclosed honestly, and any materially AI-authored commit includes the required Generated-by trailer.

Does this PR entail a user-facing behavior change?

• Yes, and the PR explains the change and why no user-facing docs are needed.
• No.

[Astro-Han]

I reviewed this head and found blocking issues.

[P2] Stop cannot cancel the pre-send model-setup flight

app-shell-chat-actions.ts:493-508 arms the turn before waiting for settleSessionModelSettings. Stop at that point only calls sessions.stop, which returns immediately when no Host turn exists, so the original send still proceeds after settings settle despite user cancellation.

[P2] Active-Turn follow-up bypasses the settlement barrier

app-shell.tsx:1940-1952 enqueues follow-ups directly without going through the settling actions.send path. A model mutation failing to settle still leaves the follow-up queued under the old model, violating the "latest mutation silently settles before immediate send" contract.

Checks on 3686d2aaf are test: success.

简体中文

存在预发送取消与跟随路径两项阻断。

[Sun-GLiang]

Addressed both blocking paths in ee34dd0.

• Stop now cancels and disarms a local pre-send settings flight before Runtime Host admission; when no local flight exists, it retains the existing sessions.stop behavior.
• Active-turn follow-ups now settle the latest model/thinking intent before enqueueing. A terminal settlement failure skips enqueue and leaves the composer draft available.

Added regression coverage for local cancellation, preserved Host stop behavior, successful follow-up settlement, and failed settlement. Desktop tests pass 1496/1496; typecheck, Biome, and the production build also pass.

[Astro-Han]

Update on ee34dd0026:

I found blocking issues that persist/drift in this head.

[P1] Follow-up can still start a new root after Stop

A follow-up submitted on an active turn waits for settings, then submitFollowUpAfterModelSettings has no cancellation identity. If the user presses Stop during the wait, preSendSettingsRef is cleared but the follow-up waiter is not cancelled — it still enqueues and the idle next_turn root starts.

[P2] Root/follow-up dispatches without re-checking surface owner

After the settings wait, the dispatch does not re-verify surface owner. Switching from surface A to B during the wait still sends with A's intent.

[P2] Thinking intent rolls back another client's new model

A stale thinking intent holding an old (model, thinking) tuple can overwrite a newer model B committed by another client.

Checks on ee34dd0026 are test: failure (slash-menu unrelated flake but still not green) — not green.

简体中文

存在跟随提交与所有者/模型回滚三项阻断。

[Sun-GLiang]

Addressed the latest settlement/readiness findings in b1ae2a7.

• Existing-session sends now capture the active settings settlement synchronously at send entry, before transcript or readiness awaits. A mutation failure remains observable after its intent is retired, so the send is skipped and the draft stays intact.
• Readiness now distinguishes an authoritative model overlay from the absence of a model override. A model adopted from a thinking-mutation response is checked before the first readiness probe, even when there was no local model selection.
• Mutation response fencing now uses a per-session catalog generation and only treats a response as stale when the session moved to a different model context. A same-model success is accepted even if an older catalog snapshot committed first, so it is not retried and cannot be negated by a later retry failure.

Added regression coverage for all three races. Desktop tests pass 1516/1516; production build, typecheck, Biome, and diff checks also pass.

[Astro-Han]

I reviewed this head and found blocking issues.

[P2] use-session-model-settings-intent.ts:118 — model equality used as ordering token

A thinking write M/high commits, another client then commits M/low and the new catalog snapshot arrives before the older IPC response. Because model is unchanged, the stale M/high is accepted and stamped at the current revision, leaving the overlay wrong if the best-effort refresh fails. Carry a causal token (host revision) through mutation result and catalog.

[P2] chat-model-switcher.tsx:231 — accessibility loses current/success state after toast removal

Fixed aria-label replaces visible label, current check is aria-hidden, no selected state, and status only announces warnings. After removing the toast, AT cannot read the current model or hear success.

[P3] app-shell.tsx:1773 — cancellation epoch tombstones accumulate

Every Stop inserts an epoch, but clearSessionRendererState never cleans it, leaving renderer-lifetime tombstones.

Checks on b1ae2a754f are test: success.

简体中文

存在两项正确性与一项可及性阻断，另有一项生命周期 P3。

[Astro-Han]

Thanks for the thorough work here — the test coverage in particular is careful. But I'd like to redirect the approach before this goes further.

The seam already exists. apps/desktop/src/renderer/use-session-setting-intent.ts is a 144-line generic hook that already does what the new 420-line use-session-model-settings-intent.ts does: optimistic overlay, latest-intent-wins coalescing, rollback to the committed value on failure, retirement against catalogRevision. Plan mode and orchestration mode already use it (app-shell.tsx:928, :940). Model and thinking level should join it rather than get a parallel implementation with its own version counters.

Two of the three reported problems need no new machinery. Stacked toasts and the wrong from model both disappear when the success toasts go. That from comes from latestAssistantModelId(messages) — the transcript, not the configuration before the switch — which is exactly why A → B → C can report A → C. It is also the only Desktop consumer of latestAssistantModelId, and the only reason the whole messages array is a dependency of the settings-action factory.

Part of this belongs to #3347, which we have not decided to do. Settle-before-send, Stop cancelling an in-flight settings write, and gating follow-ups on settlement all address "the send may use a configuration the Host has not committed yet". That is #3347's problem, and it puts renderer-side ordering in front of Host turn admission. Please drop those here.

Suggested shape — this should come out as a net deletion:

• route setModel, setThinkingLevel and setPermissionMode through the existing useSessionSettingIntent. For permission mode, move the bypass confirmation to the call site: the hook stays unaware of it, and the control keeps showing the old value until the user confirms;
• delete sessionModelChangeRegistry, permissionModeChangeRegistry, pendingSessionModelBySession, pendingPermissionModeBySession, the modelChangePending / permissionModePending prop chains, and pending / loading on the switchers;
• delete the success toasts and the copy, label helpers and transcript lookup that exist only for them. Failure toasts stay.

Two smaller things worth closing while you are here: pendingSessionModelChangesRef is keyed by session and shared between the model and thinking-level writes, so each blocks the other even though the Host exposes them as independent calls; and "why can't I change this now" is derived three times (composer.tsx:1345, app-shell.tsx:1242, app-shell.tsx:3095) with two separate sets of copy.

简体中文

感谢你细致的工作，测试尤其扎实。但在继续之前我想调整一下方向。

这个 seam 已经存在。 apps/desktop/src/renderer/use-session-setting-intent.ts 是一个 144 行的泛型 hook，新增的 420 行 use-session-model-settings-intent.ts 做的是同一件事：乐观 overlay、最新意图胜出的合流、失败回滚到 committed 值、按 catalogRevision 退休。Plan 模式和编排模式已经在用它（app-shell.tsx:928、:940）。模型和思考级别应该接进去，而不是在旁边再实现一套带版本号的。

三个问题里有两个不需要新机制。 toast 堆叠和 from 取错模型，删掉成功 toast 就都没了。那个 from 取自 latestAssistantModelId(messages)——转录，而不是切换前的配置，这正是 A → B → C 会报成 A → C 的原因。它也是 Desktop 侧 latestAssistantModelId 的唯一消费者，以及整个 messages 数组成为 settings-action 工厂依赖的唯一原因。

有一部分属于 #3347，而我们还没有决定要做它。 发送前 settle、Stop 取消在途的设置写入、follow-up 进队列前先 settle，解决的都是「发送时用的配置可能还没提交到 Host」——那是 #3347 的问题，而且它让渲染层排在了 Host 的 Turn 准入之前。请从本 PR 移除。

建议的形状，结果应该是净删除：

• setModel、setThinkingLevel、setPermissionMode 全部走既有的 useSessionSettingIntent。权限模式的 bypass 确认弹窗提到调用点：hook 不需要知道它存在，用户确认之前控件继续显示旧值；
• 删除 sessionModelChangeRegistry、permissionModeChangeRegistry、pendingSessionModelBySession、pendingPermissionModeBySession、modelChangePending / permissionModePending 两条 props 链，以及切换器上的 pending / loading；
• 删除成功 toast，以及只为它们存在的文案、标签辅助函数和转录取源。失败 toast 保留。

另有两处顺手可以收掉：pendingSessionModelChangesRef 以 session 为 key，被模型写入和思考级别写入共用，于是两者互相阻塞，而 Host 侧本来就是两条独立调用；另外「为什么现在不能改」被推导了三遍（composer.tsx:1345、app-shell.tsx:1242、app-shell.tsx:3095），用了两套不同的文案。

[Astro-Han]

Following up with the full reasoning behind that suggestion — it is a fairly large redirect, so it seems only fair to show the whole picture rather than just the conclusion.

Two mechanisms for one fact

Desktop currently has two different answers to "a session setting write is in flight", and which one you get depends on which setting you touched.

	Plan / orchestration mode	Model / thinking / permission
Implementation	useSessionSettingIntent (144 lines, generic)	pending registry + React state + toast
Representation	one intent record	keysRef and a React state slot, kept in sync by hand
Rapid changes	latest intent wins	second click silently dropped (if (has) return)
During the write	control stays usable, shows the new value	disabled + spinner, still shows the old value
On success	nothing	toast
On failure	rolls back to the committed value	toast only, no rollback

Every one of these five settings goes through the same shape on the Host side — the same invokeSessionSummary call, the same SessionSummary return. The divergence is entirely in the renderer.

flowchart TB
  subgraph A["useSessionSettingIntent — 144 lines, generic"]
    A1["Plan mode"] --> AI["desired / committed<br/>committedAtCatalogRevision / inFlight"]
    A2["Orchestration mode"] --> AI
    AI --> AO["overlay: shows immediately, latest wins"]
    AI --> AW["write loop: re-reads desired<br/>rolls back to committed on failure"]
    AW --> AR["refreshCatalog, retire overlay by catalogRevision"]
  end
  subgraph B["pending registry + toast"]
    B1["Model"] --> BR["keysRef: Set of sessionId"]
    B2["Thinking level"] --> BR
    B3["Permission mode"] --> BR2["keysRef: Set of key"]
    BR --> BS["pendingSessionModelBySession"]
    BR2 --> BS2["pendingPermissionModeBySession"]
    BS --> BD["disabled + spinner"]
    BS2 --> BD
    BR --> BX["repeat click dropped"]
    BW["write succeeds"] --> BT["toast.success"] --> BF["refreshSessions"]
  end

Loading

What the difference looks like in time

sequenceDiagram
  autonumber
  participant U as User
  participant C as Control
  participant S as settings-actions
  participant R as pending registry
  participant H as Runtime Host
  Note over U,H: today, model / thinking / permission
  U->>C: pick B
  C->>S: setSessionModel(B)
  S->>R: keysRef.add(sessionId)
  S->>S: setPendingBySession(true)
  Note over C: disabled + spinner, still shows A
  U->>C: quickly pick C
  C->>S: setSessionModel(C)
  S-->>C: has(sessionId), returns early — C is lost
  S->>H: sessions:setModel(B)
  H-->>S: SessionSummary(B)
  S->>S: latestAssistantModelId(messages) for the "from" label
  S->>U: toast "A to B"
  S->>H: refreshSessions()
  S->>R: delete + setPendingBySession(false)

Loading

sequenceDiagram
  autonumber
  participant U as User
  participant C as Control
  participant I as useSessionSettingIntent
  participant H as Runtime Host
  Note over U,H: today, plan / orchestration
  U->>C: pick B
  C->>I: request(sessionId, B)
  I->>C: overlay = B, shown immediately, not disabled
  I->>H: write(B)
  U->>C: quickly pick C
  C->>I: request(sessionId, C)
  I->>I: desired = C, no second write started
  I->>C: overlay = C
  H-->>I: committed(B)
  I->>H: write(C)
  H-->>I: committed(C)
  I->>H: refreshCatalog()
  I->>I: newer catalogRevision retires the overlay

Loading

The second diagram is the behaviour your issue asks for. It is already in the tree.

What disappears if the three settings move over

• pendingSessionModelBySession and pendingPermissionModeBySession — two state slots, plus their selectors, setters, AppShellSessionUiState fields, and the two reads in use-app-shell-session-ui-reads
• sessionModelChangeRegistry and permissionModeChangeRegistry — two registry instances, four keysRef hand-offs, and the teardown in app-shell-effects
• three add/delete pairs in app-shell-session-settings-actions.ts, each written twice (once on the ref, once on the state slot), plus omitSessionKey
• the modelChangePending and permissionModePending prop chains, and pending / loading on both switchers
• the success toasts, their copy in both locales, modelLabel / modelEndpointLabel, and the latestAssistantModelId(messages) lookup — which also removes the entire messages array from the settings-action factory's dependencies, so the actions stop being rebuilt on every transcript change

Nothing new is added: useSessionSettingIntent is generic over the value, and the only adapter needed is SessionSummary to boolean in write.

For permission mode, the bypass confirmation moves to the call site — confirm first, then request. The hook never learns that a dialog exists, and the control keeps showing the old value until the user confirms, which is the behaviour we want anyway.

What we give up

The control is no longer disabled while a write is in flight. That is the point of your issue, so I think it is a gain — but it is a real behaviour change and worth stating plainly.

Two smaller things in the same area

pendingSessionModelChangesRef is keyed by session and shared between the model write and the thinking-level write, so changing one blocks the other. The Host exposes them as two independent calls; the coupling is an artifact of the shared key.

"Why can't I change this right now" is derived in three places — composer.tsx:1345, app-shell.tsx:1242, app-shell.tsx:3095 — from slightly different inputs, with two separate sets of copy. app-shell.tsx:3091 documents a race that came from reading the persisted status instead of the live turn witness; the copy in composer.tsx still takes activeSession.status as one of its inputs.

Neither is a blocker. They are cheap to close while the area is open, and both go away on their own once the three settings share one path.

Happy to talk through any of this if you see it differently — you have been in this code more recently than I have.

简体中文

补上建议背后的完整理由。这是一次幅度不小的方向调整，只给结论不太公平。

一个事实，两套机制

Desktop 现在对「会话设置写入在途」有两个不同的答案，取决于你动的是哪个设置。

	Plan / 编排模式	模型 / 思考级别 / 权限
实现	useSessionSettingIntent（144 行泛型）	pending 注册表 + React state + toast
表示	一条 intent 记录	keysRef 和 一个 state 槽，手工保持同步
快速连改	最新意图胜出	第二次点击被静默丢弃（if (has) return）
写入期间	控件可用，显示新值	禁用 + 转圈，仍显示旧值
成功时	无提示	toast
失败时	回滚到 committed 值	只弹 toast，不回滚

这五个设置在 Host 侧走的是完全相同的形状——同一个 invokeSessionSummary，同样返回 SessionSummary。分叉全部发生在渲染层。

上面两张时序图分别对应这两条路径。第二张就是你 issue 里想要的行为，它已经在仓库里了。

三个设置迁过去之后会消失的东西

• pendingSessionModelBySession 和 pendingPermissionModeBySession 两个 state 槽，及其 selector、setter、AppShellSessionUiState 字段和 use-app-shell-session-ui-reads 里的两处读取
• sessionModelChangeRegistry、permissionModeChangeRegistry 两个注册表实例、四处 keysRef 传参，以及 app-shell-effects 里的清理
• app-shell-session-settings-actions.ts 里三对 add/delete，每对都写两遍（ref 一次、state 槽一次），以及 omitSessionKey
• modelChangePending / permissionModePending 两条 props 链，和两个切换器上的 pending / loading
• 成功 toast、两种语言的文案、modelLabel / modelEndpointLabel，以及 latestAssistantModelId(messages)——这也顺带把整个 messages 数组从 settings-action 工厂的依赖里去掉，动作不再随转录每次变化而重建

不需要新增任何东西：useSessionSettingIntent 对值是泛型的，唯一需要的适配是 write 里把 SessionSummary 转成 boolean。

权限模式的 bypass 确认弹窗移到调用点——先确认，再 request。hook 完全不需要知道有弹窗，用户确认之前控件继续显示旧值，这本来也是我们想要的行为。

让出的东西

写入在途时控件不再被禁用。这正是你 issue 的目的，所以我认为是收益，但它确实是一次行为变化，应该明说。

同一片区域的两个小问题

pendingSessionModelChangesRef 以 session 为 key，被模型写入和思考级别写入共用，改一个会挡住另一个。Host 侧本来就是两条独立调用，这个耦合只是共用 key 的副产物。

「为什么现在不能改」在三个地方各推导了一遍——composer.tsx:1345、app-shell.tsx:1242、app-shell.tsx:3095——输入略有差异，还用了两套文案。app-shell.tsx:3091 记录过一次因为读持久化 status 而不是实时 turn witness 导致的竞态；composer.tsx 那份仍然把 activeSession.status 当作输入之一。

两个都不阻塞。趁这块打开时收掉很便宜，而且三个设置合并到同一条路径后它们会自行消失。

如果你有不同看法，随时聊——这块代码你比我更熟。

[Astro-Han]

Update on 3fdf5857a0:

[P2] Model intent fire-and-forget allows send on stale model

app-shell.tsx:3030-3032 fires modelIntent.request() without waiting, and app-shell-chat-actions.ts:349-350 send() no longer waits for settleSessionModelSettings. The Host setModel IPC can still be pending when sessions.send runs, so the turn can start with the old model A even though the UI shows B.

Fix: restore a per-session settlement barrier between model change and send, or carry expected model/version into send and retry on mismatch.

Checks on 3fdf5857a0 pending — code is NO-GO independent of CI.

简体中文

存在模型意图与发送竞态导致的错模型启动。

[Astro-Han]

I reviewed this head and found a blocking issue.

[P2] Model selection optimistic write races Session Send

app-shell.tsx:3080-3087 writes model setting optimistically and returns immediately; app-shell-chat-actions.ts:502-516 sends Session without waiting. turn.start may capture old model before session.configuration.update commits — user sees B but first turn runs with A. Shared admission only serializes enqueued ops, not cross-request ordering.

Fix: restore per-Session settlement barrier or add expected-revision fence to turn.start; add delayed-write regression test.

简体中文

模型切换与发送存在竞态。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sun-GLiang]

I reviewed this head and found a blocking issue.

[P2] Model selection optimistic write races Session Send

app-shell.tsx:3080-3087 writes model setting optimistically and returns immediately; app-shell-chat-actions.ts:502-516 sends Session without waiting. turn.start may capture old model before session.configuration.update commits — user sees B but first turn runs with A. Shared admission only serializes enqueued ops, not cross-request ordering.

Fix: restore per-Session settlement barrier or add expected-revision fence to turn.start; add delayed-write regression test.

简体中文

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

The race described here is a valid configuration-ordering concern, but it is explicitly out of scope for #3745 and tracked by #3347. Earlier review on this PR specifically asked to remove settle-before-send, Stop cancellation, and follow-up gating because renderer-side ordering should not own Host turn admission. The current head and its architecture test follow that direction.
Restoring the renderer settlement barrier here would reverse the agreed review direction. Unless the scope has changed, could this be treated as non-blocking and handled in #3347? If the scope has changed, I’d prefer an authoritative Host-side expected-revision/admission fence rather than reintroducing renderer-only ordering.

[Astro-Han]

Retracting this review. It contradicted my own earlier feedback on this PR: on 2026-08-25 I asked for a causal token carried through the mutation result and catalog, and flagged that a stale thinking intent holding an old (model, thinking) tuple can overwrite another client's newer model. The compound modelConfigurationIntent and the sessions:setModelConfiguration operation are the response to exactly that, so asking to split them back apart would reintroduce the bug I asked to have fixed. The inline comment about thinking writes carrying the model has been deleted for the same reason.

The remaining inline comments stand on their own; I will re-issue anything still outstanding as a fresh review.

Contradicted my own earlier guidance on this PR; re-reviewing.

[Astro-Han]

Re-reviewing after retracting my previous review, which contradicted my own earlier feedback here.

To be explicit so the retraction is not ambiguous: the compound modelConfigurationIntent and the sessions:setModelConfiguration operation are the right answer to what I raised on 2026-08-25 — a stale thinking intent carrying an old (model, thinking) tuple overwriting another client's newer model, and the need for a causal token through the mutation result and catalog. committedAtCatalogRevision is that token. Please disregard the earlier suggestion to split them back into independent controllers; #3745's "independent" wording predates that feedback.

The direction of the PR is right and the deletions are real: both change registries, both pending prop chains, the switcher's pending/loading, the success toasts and their copy in two languages are gone.

Two things on convergence, plus the inline notes.

Side chat still runs its own mechanism. use-quote-companion.ts:217,817-833 keeps a permissionModePending boolean with discard-on-reentry, and this PR adds a permissionChanging string to conversation-copy.ts — reviving in a new file the same copy just deleted from shell-copy.ts. That is the "one fact, two mechanisms" this PR sets out to remove. If side chat is deliberately out of scope, worth saying so in the body, and the new copy should not land.

One piece of scope is outside #3745. The catalog-diff teardown effect and session-catalog-removals.ts are not asked for by the issue, and they are where the widest inline note below sits. What this PR needs from that effect is the two new intent overlays cleared.

AI use: Claude Code assisted with source and issue investigation; the analysis and conclusions are my own.

[Astro-Han]

On the send-ordering race: you are right, and that automated finding is non-blocking here. Renderer-side settlement was removed at my request because ordering in front of Host turn admission is the wrong place for it, and #3347 is open for exactly this. If we fence it, I agree it should be a Host-side expected-revision check on turn.start, not a renderer barrier — nothing to change in this PR.

Separately, my earlier review today was retracted: it asked you to split the compound model/thinking intent back apart, which would have undone the stale-tuple fix I asked for on 08-25. Sorry for the churn — the re-review reflects the corrected position.

[Sun-GLiang]

Addressed the review summary in e7386e5. Side chat permission changes now use the same useSessionSettingIntent seam: the selected mode is projected immediately, rapid changes coalesce latest-wins, and a stale first response cannot replace the newer overlay. Removed permissionModePending and the newly added permissionChanging copy. The compound modelConfigurationIntent and sessions:setModelConfiguration remain intact. Local verification passes: Desktop 1532/1532, Desktop typecheck, production build, full-repo Biome, and diff checks.

[Astro-Han]

Thanks for continuing to converge this toward the existing useSessionSettingIntent seam. The core direction is right: remove the success-toast and pending-registry machinery, show the latest intent immediately, and retain failure rollback.

Before another review round, could you rebase onto current main and slim the cumulative diff back to the #3745 obligation? The current branch is conflicting and still carries +2016/-942 across 35 files. It includes about 1,355 lines of test additions, a 643-line session-intent test file, and deleted-session, purge, and quote-companion changes that need to be separated from or explicitly justified by the setting-feedback goal.

This issue should ideally land as a net simplification around the existing intent owner. Once the refreshed diff contains only that end state and the necessary behavioral coverage, we should be able to review and merge it quickly.

Please push back where an apparently adjacent path is required to preserve the same setting-intent invariant.

AI-assisted review disclosure: Codex assisted with the cumulative-diff inventory; Astro-Han reviewed the accepted direction and owns this recommendation.

[Sun-GLiang]

Hi! @Astro-Han

Thanks for the scope review. I rebuilt this PR on top of the latest main and force-pushed a single focused commit (d27f6fc37).

Kept because they are directly required by #3745 or by bugs identified during review:

• the shared useSessionSettingIntent flow for latest-wins, rollback, catalog retirement, and teardown;
• compound model/thinking configuration through sessions:setModelConfiguration, as the later review clarified this is required to avoid stale model/thinking tuples;
• optimistic presentation only for the affected controls, while session health and execution state remain authoritative;
• the same intent mechanism for Quote Companion permission changes;
• removal of success toasts, pending spinners/registries, and the related copy;
• accessible radio semantics and current-value announcements for model/thinking selectors.

Removed from this PR:

• the Conversation feature/ports/provider architecture migration;
• catalog-diff session purge handling;
• the new-task permission write helper;
• Plan/Swarm/Graph action relocation;
• unrelated architecture/inventory work and redundant test harnesses.

The diff is now 25 files (+759/-929), down from 45 files (+2809/-1123).

Verification:

• Desktop: 1726/1726 tests passed
• UI: 276/276 tests passed
• Desktop/UI typecheck passed
• Biome passed
• Renderer architecture: 60/60 passed
• AppShell hook-scope check passed

Please review from d27f6fc37; the previous large implementation has been replaced.

[Astro-Han]

Thanks for carrying this through several rounds of review and simplification. On exact head d27f6fc, the model and thinking-level interaction now converges on one compound configuration path, cleanup follows explicit deletion facts, the dead model-only IPC and brittle source-text tests are removed, and the no-session persistence guard is preserved. The previous review findings are addressed, and I found no remaining P0–P3 issue.

The exact-head test check is currently queued, so that remains the normal merge-time gate.

This was an AI-assisted review using Codex across multiple review rounds. I independently verified the current head, the addressed findings, production ownership, and merge state, and I own this approval.

[Astro-Han]

Merging after the CI pass!

[Astro-Han]

Thanks — I was ready to merge once CI passed, but the exact-head test is failing the Renderer architecture check. Please fix the failure and rerun CI; once the required check is green, we can merge.

[Sun-GLiang]

CI update on exact head d27f6fc3: the test job failed in Check renderer architecture.

The checker’s 60 fixture tests pass; the repository check then rejects the PR’s renderer debt growth relative to base 29d02dc2. The failures form one architectural cluster:

• session-setting ownership moved back into app-shell.tsx, increasing bridge, dependency, hook, import, and token debt;
• side chat now imports the legacy root use-session-setting-intent.ts, creating a forbidden feature-to-legacy dependency;
• session-model-configuration-intent.ts is a new unclassified renderer-root source;
• the legacy use-session-setting-intent.ts also increases its stateful-hook debt.

Updating renderer-architecture.json does not admit these changes because the ledger is a monotonic ceiling compared against the base revision.

I will keep the reviewed behavior intact — the compound modelConfigurationIntent, the single sessions:setModelConfiguration production path, and convergence with side chat. The follow-up will refactor ownership and call sites so AppShell debt does not grow, remove the feature-to-legacy dependency, and place the shared intent mechanism behind a legal seam. It will not split the compound model/thinking controller or restore the old pending/toast mechanisms.

I’ll request another review after the architecture check and targeted regression tests are green.

[Sun-GLiang]

Hi @Astro-Han ,

I’ve pushed the latest updates, resolved the merge conflicts with apache/main, and verified that CI is now green.

Since these commits were added after the previous approval, could you please take another look at the updated diff and re-approve if everything looks good? Thanks!

[Astro-Han]

Re-reviewed the delta from d27f6fc3 (where I approved on Aug 31) to 96f1682a. Confirming first that all four of my open threads are genuinely closed in code, not by assertion — I've resolved them:

• sessions:setModel is gone; only sessions:setModelConfiguration remains (runtime-host-session-catalog-ipc-main.ts:209).
• model-settings-visual-contract.test.ts is deleted, so nothing asserts design intent by regex over source any more.
• session-catalog-removals.ts no longer exists, and clearSessionRendererState is now reached only from app-shell-effects.ts:190/250 and the explicit row action — absence and deletion are no longer the same fact.
• The no-session permission branch is no longer a hazard: setNewTaskPermissionMode resolves to useNewTaskChoice's synchronous setter, so there is no async persistence to reorder.

The delta itself holds up. The features/session-settings/ folder with ports.ts + services-context.tsx + a desktop adapter is the same shape features/goals already uses, and packages/ui already carries non-visual state hooks (use-composer-draft, use-transcript-projection, daily-review-view-state), so extracting the intent hook there follows precedent rather than inventing a home. Generalizing it to multi-channel is what lets the main chat and the side chat share one seam, and it is the replacement for the deleted app-shell-session-settings-actions.ts — one mechanism swap, so I'm not asking for a split.

Approving at 96f1682a. Two P2s and one P3.

P2-1 — the side chat gains a Full access confirmation it never had, and the description does not say so. On main, the main chat confirms bypass (app-shell-session-settings-actions.ts:122-127) while the side chat writes it straight through (use-quote-companion.ts:952). Routing both onto one intent seam makes closing that gap part of this change — that's the right call. But Summary line 5 reads as a sequencing note about an existing confirmation ("happens before the permission intent starts"), so a reader of the PR, the squash message, or the release notes cannot learn that unconfirmed Full access in the side chat was possible before and is not now. This is the only security-relevant change in the PR; please give it its own line.

P2-2 — the new Full access coverage tests a three-line helper instead of the path. declining Full access does not persist the side-chat permission mode calls requestPermissionModeWithConfirmation directly with two fake closures. It never mounts useQuoteCompanion, even though the same file already sets up createRoot, WorkbarServicesProvider, and createFakeWorkbarServices for the retry tests. The regression it exists to prevent — setPermissionMode (line 1004) calling requestPermissionMode(id, mode) directly, which is exactly what main does today — leaves this test green. Asserting through the hook is cheap here and is the difference between proving the helper's if and proving the confirmation is on the path.

P3 — the PR roughly doubled after approval and the Summary no longer describes it. 25 files / +759/-929 at d27f6fc3, 47 files / +1734/-1108 now. Added since: the session-settings feature folder and its services wiring, the packages/ui extraction, and the side-chat confirmation. Each is defensible individually, as above, but the Summary still describes only the instant-feedback and toast removal. Naming the structural moves would let the next reader see the same PR I just re-read.

AI use: re-reviewed with Claude Code (comparing each head's diff against its own merge-base to isolate PR-owned changes from what the merges pulled in). I verified each of the four resolved threads against the code at 96f1682a, the pre-existing side-chat gap on main, the confirmation's wiring through WorkbarHost → WorkbarSurface → QuoteCompanionPanel, and the new test's actual entry point myself.

[Astro-Han]

Follow-up to my re-review — I went back to answer why the diff doubled, and two of the three blocks are not what I first assumed. Neither of these is a merge condition; the second in particular is a direction, not a request.

The joining-the-DI-architecture block is not invention: composition/desktop-feature-services.tsx already wires seven feature service providers, platform/desktop/ already holds seven create*Services, and six of the eight have the same shape of adapter test. Session-settings is the eighth member, copied faithfully. No finding there.

The channels generalization is the part worth reconsidering. Reading all 253 lines of packages/ui/src/session-setting-intent.ts, there is no cross-channel coordination anywhere: every operation keys on (channel, sessionId), and channel is only an extra map key plus an index into options.channels. The three places channels meet — clear's loop, the catalog-revision effect's loop, and the single overlayByChannel state object — are all reproducible by holding N controllers and looping them.

At d27f6fc3 the hook was 198 lines and single-channel, instantiated four times in AppShell and once in the side chat. That gave per-channel isolation structurally. The generalization re-implements that isolation inside one controller and then needs a 128-line test whose only case is channels keep independent workers and overlays for the same session — asserting a property four separate controllers had by construction. And the generality has one consumer: the desktop wrapper passes four channels, the side chat passes one.

One consequence points the wrong way for where this codebase is heading. overlayByChannel is a single useState object, so a write on any channel produces a new top-level identity and wakes every channel's readers; four controllers keep four independent states. Invisible today because all the reads sit inside AppShell — but #4316 and #4315 are moving exactly these reads below AppShell.

Dropping the generalization removes the Values mapped types, the nested Map<keyof Values, Map<string, AnyIntent>>, the Reflect.ownKeys initializer, the channel parameter threaded through setOverlay/reconcile/request/isEqual, and that test — roughly 180 lines — and costs a loop in the wrapper's clear. The move into packages/ui still stands on its own; the package already carries non-visual state hooks like use-composer-draft and use-transcript-projection.

On the plan-mode split, I'd withdraw the way I framed it. I said three channels reach the Host through SessionSettingsServices while planMode goes through commitPlanMode directly, and implied the port should grow. Having read the surroundings, that is backwards. AppShell calls window.maka.* 33 times across ten distinct sessions.* methods, so commitPlanMode's three direct calls are ordinary there — the three channels that detour through a port are the unusual ones. And commitPlanMode is policy, not transport: it reads plan state, raises an error, confirms destructively, and branches between abandoning a proposal and writing collaborationMode. Putting that behind a port of three summary-returning setters would stop the port being one idea.

The more general representation is the injected closure: the port can be expressed as one, the reverse is not true, and plan mode proves it. So the lowest-entropy shape is for the wrapper to receive four writes of one shape — which is already the shared hook's own channels shape — and let the composer decide where each comes from. The split then does not need deciding; it stops existing. It also lets session-settings-controller.test.ts cover the plan channel, which it currently cannot fake.

The fair counter is that this is the one thing that would not match the other seven features, and that if session-settings later follows goals and module-hub below AppShell, the context port becomes load-bearing. I don't think that outweighs a four-channel boundary with one channel outside it today, but it is a real trade and it is yours to make.

Both of these are structural, and this PR has already doubled once. I would not do either here. They belong with the next round of controller-scoping work, alongside #4316 and #4315.

AI use: audited with Claude Code (full read of the shared hook, the desktop wrapper's four channels, the seven sibling feature ports and adapters, and AppShell's direct bridge usage). I verified the pre-PR hook's arity and instantiation sites, the absence of cross-channel coordination, and that sessions:setOrchestrationMode validates before applying — that last one I had suspected was a missing commit check and it is not.