feat: remember workspace model and mode on send - #3968
Conversation
Model settings already synced through backend metadata, but the active agent (mode) was client-local: sends persisted workspaceEntry.agentId, yet main workspaces never re-seeded it and picker switches never wrote it back, so a fresh client always fell back to exec and seeded the wrong agent's model. - AgentContext now persists workspace mode switches via updateAgentAISettings(persistSelectedAgentId), with aiSettings allowed to be null so a mode switch cannot clobber stored settings. - WorkspaceContext seeds agentId from metadata for main workspaces too, guarded by a pending-echo check (same pattern as model settings) so stale broadcasts cannot revert an in-flight local switch. This supersedes the #3178 gate, which existed because local switches were never written back. - ProposePlanToolCall marks the pending agent before its follow-up send persists the switch. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfbedb12c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review feedback on #3968: - AgentContext rolls the optimistic local mode switch back when the backend write fails (unless the user re-switched meanwhile), so a client cannot silently diverge from the backend-authoritative agent. - ProposePlan handlers clear the pending agent guard when the follow-up send fails (failed Result or throw); a stuck guard would block all backend agent seeds for the workspace. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4103151aff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Release the pending agent guard deterministically once persistence or the ProposePlan follow-up send settles: a successful no-op write emits no metadata echo, which previously stranded the guard and blocked all later cross-client agent updates. Real echoes are ordered after stale broadcasts, so releasing on the response cannot strand a stale value. - Never hydrate another agent's settings: workspace seeding now only applies model/thinking from the ACTIVE agent's own bucket instead of falling back to exec/plan, which overwrote locally resolved settings for bucket-less agents after an agent-only switch. - Apply the budgeted-goal pricing gate to agent-only switches using the target agent's stored model (bucket, then legacy settings), matching what heartbeat/goal-continuation dispatch would resolve. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf11f76b9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Round-3 Codex finding: the pricing gate only looked at the stored workspace bucket/legacy settings, but goal-continuation kickoff also resolves configured and definition defaults. Extract the kickoff resolution into resolveContinuationKickoffSendOptionsForAgent (shared by getGoalContinuationKickoffSendOptions) and gate the agent-only switch on the exact model that resolution selects, including the plan/compact -> exec dispatch remap. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd2228a734
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- ProposePlan explicitly persists the agent selection after a successful send (send-side persistence only logs failures), releasing the pending guard afterwards either way so clients converge on the backend agent. - The budgeted-goal pricing gate probes both dispatch surfaces: the continuation kickoff (plan/compact remapped to exec) and the heartbeat surface, which resolves the persisted agent without the remap. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: badc083c61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex round 5 follow-ups: - ProposePlan now honors the explicit selection persistence Result: a failed or thrown updateAgentAISettings (and a failed send) rolls the optimistic plan→exec/auto switch back — agent id plus the pre-switch model/thinking/reasoning settings — unless the user already moved on. - AgentContext rollback restores the pre-switch model settings alongside the agent id, so WorkspaceModeAISync's fallback cannot leave the restored agent on the target agent's just-applied settings. - The budgeted-goal pricing gate probes the heartbeat surface through the real heartbeat resolution (extracted resolveHeartbeatAiSettings, shared with buildHeartbeatSendOptions), which includes the activity snapshot's last-used model fallback the previous probe missed. _Generated with `mux` • Model: claude-x-large-4.6 • Thinking: max_
|
@codex review |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d9d468281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…es, surface switch errors
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c53bc5bc8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25acf9250e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
xum/src/browser/utils/workspaceAiSettingsSync.ts
Lines 112 to 116 in 2fd2af9
When two same-agent changes are queued, markPendingWorkspaceAiSettings replaces the first snapshot with the second, but an unsuccessful first RPC still calls this agent-key-only clear and deletes the second change's guard while its serialized write is in preflight. A stale metadata broadcast can then overwrite the newer local settings; if the user sends in that window, the captured stale options queue behind and ultimately overwrite the newer backend bucket. Fresh evidence beyond the earlier stale-metadata guard thread is this overlapping failure ordering; track pending generations/counts and clear only the matching request.
AGENTS.md reference: AGENTS.md:L150-L150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
xum/src/browser/utils/workspaceAiSettingsSync.ts
Lines 112 to 116 in 9cd6bf6
When two writes for the same workspace and agent overlap, this unconditional delete lets an older failed or settled operation erase the newer operation's pending snapshot. For example, after two rapid model choices, a transport failure or typed rejection of the first write invokes this helper while the second serialized write is still running; stale backend metadata is then accepted and can overwrite the latest local choice before the second write completes. Associate clears with the expected snapshot or a generation/count so an older operation cannot release a newer guard.
AGENTS.md reference: AGENTS.md:L150-L150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96209923fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 866d4c7d41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
|
The reduced implementation has a clean Codex code/security verdict on c841243, zero unresolved review threads, passing local static checks, and 590 passing affected tests. The full CI unit job crashed inside Bun 1.3.5 twice (exit 132, native segmentation fault, no failing test assertions); the retry reached a different test before the same native crash. The first crash-site suite passes locally with coverage. I am leaving this runtime/CI blocker separate rather than expanding the persistence PR with unrelated runtime workarounds. |
Summary
Remember each workspace's model and mode when the user sends a message, using the existing send-time persistence. Picker changes stay local. Synthetic turns and stream resumes do not replace the saved selection.
Fresh or reloaded clients restore the last submitted choices. Already-open composers deliberately do not live-sync later metadata, so delayed send echoes cannot overwrite unsent choices. ACP keeps one active draft; switching to another mode restores that mode's saved settings, without retaining a history of unsent per-mode drafts.
Removed picker-time writes, pending guards, rollback/serialization machinery, and the unrelated descriptor expansion from the previous implementation. The full PR is 304 additions and 515 deletions across 16 files, down from 3,279 additions across 36 files.
Validation
make static-checkpassed using pinned Bun 1.3.5.