Skip to content

🤖 feat: remember last used model and mode per workspace across clients - #3968

Open
ibetitsmike wants to merge 35 commits into
mainfrom
mike/workspace-remember-model-mode
Open

🤖 feat: remember last used model and mode per workspace across clients#3968
ibetitsmike wants to merge 35 commits into
mainfrom
mike/workspace-remember-model-mode

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Xum now remembers the last used model and mode (agent) for every workspace on the backend, so any client — a fresh browser, another device, or a reinstalled app — restores them. Mode selection also live-syncs across connected clients, matching the existing behavior of model settings.

Background

Model/thinking settings were already client-independent: persisted per agent in config.json (aiSettingsByAgent) and re-seeded into localStorage from metadata. The active agent (mode) was not:

Net effect: a fresh client always fell back to exec, and consequently also seeded the wrong agent's model into the composer.

Consistency model

Selection state (agent + model/thinking) is optimistic client state with best-effort backend persistence:

  • A switch applies locally at once and fires one updateAgentAISettings write. A transport failure keeps the optimistic selection and surfaces a toast; the next send re-persists it. A typed rejection (e.g. the budgeted-goal pricing gate) instead reverts the switch locally alongside the toast: the backend refused the selection, and the same gate refuses sends before they can re-persist, so no self-heal is coming on that path. The shared revert (revertRejectedAgentSwitch) reconciles from workspace metadata read at settle time (via refs, so an accepted write's echo landing mid-flight is honored): it restores the backend-authoritative agent (legacy agentType compat included) and hydrates that agent's settings from its metadata bucket or the legacy blob, falling back to captured pre-switch values only when the target is the captured agent. It only undoes state the switch itself wrote — anything the user changed afterward wins — and the settings restore runs even when the backend already stores the rejected agent id (divergent carried-over settings). Propose-plan actions (Implement / Continue in auto) apply the same rule to typed send rejections.
  • Every send carries the current selection in its options and the backend re-persists it best-effort (maybePersistAISettingsFromOptions), so any client/backend divergence self-heals at the next send. Queued messages capture their options at enqueue time and dispatch with them.
  • Last writer wins across clients. There is deliberately no generation tracking or write serialization; the only compensation is the single-shot settle-time reconcile of a typed-rejected switch to the backend-authoritative (or pre-switch) selection.

Implementation

  • workspace.updateAgentAISettings accepts aiSettings: null to persist only the selected agent (persistSelectedAgentId), skipping settings normalization. When a budget-resumable goal is active, any selection change — with or without submitted settings — runs the pricing gate against the effective dispatch models (workspace bucket → configured defaults → agent-definition chain → legacy → default), probing both goal-continuation and heartbeat resolution (shared resolveHeartbeatAiSettings, overlaying the about-to-be-written bucket) so an unpriced mode can't silently disable cost tracking.
  • AgentContext.setAgentId persists workspace mode switches best-effort via that IPC, sending the switch's resolved model/thinking/reasoning (same resolution WorkspaceModeAISync applies locally) so fresh clients hydrate a bucket even for agents that had none. Project/global scopes and locked child workspaces are excluded; re-selecting the current agent is a no-op. A transport-failed write keeps the local selection (the next send re-persists it) while a typed rejection restores the pre-switch agent and settings, guarded so newer user changes are never clobbered; both surface the backend error as a toast (AGENT_SWITCH_ERROR_TOAST), and the pending-echo guard is released on every settled write. Switch resolution now gives the workspace's own saved bucket precedence over configured agent defaults (matching backend dispatch and ACP layering), so switching away and back cannot overwrite a workspace's last-used settings with a global default.
  • WorkspaceContext now seeds agentId from backend metadata for main workspaces too (child workspaces keep unconditional seeding). A new pending-echo guard (markPendingWorkspaceAgentId / shouldApplyWorkspaceAgentIdFromBackend, mirroring the existing AI-settings guard) prevents stale broadcasts from reverting an in-flight local switch — the failure mode that motivated the 🤖 fix: keep main workspace agent selection on metadata sync #3178 gate this change supersedes. Since agentId seeds before the model-key seeding, fresh clients now hydrate the correct agent's model. Legacy-only metadata (shared aiSettings, no per-agent buckets) synthesizes a bucket for the active agent as well, not just plan/exec; when a partial modern map coexists with the legacy blob, the legacy value overlays only a missing active-agent bucket (real per-agent entries always win) — both matching backend dispatch resolution, which treats the legacy blob as a fallback for whichever agent is selected, including custom agents.
  • ProposePlanToolCall marks the pending agent, applies the target agent's resolved settings locally, and sends "Implement the plan" with those options; the send itself persists the switch backend-side. There is no separate post-send settings write. A transport-failed send keeps the local switch (the next send re-persists it) while a typed send rejection reverts it via the shared helper; the echo guard is released once the send settles either way.
  • ACP: session/set_config_option mode switches persist the selected agent through the same IPC (built-in modes included), so ACP clients participate in cross-client mode memory. Child (subagent) workspaces stay session-local: their creation-time agent is a locked identity that scheduled dispatch resolves directly.

Validation

  • IPC test: agent-only persistence (aiSettings: null) updates agentId without touching stored per-agent settings.
  • WorkspaceContext tests: backend agentId seeds main workspaces; a pending local switch survives a stale broadcast, and the guard clears once the backend echoes it; legacy shared aiSettings hydrate a custom active agent's model/thinking.
  • AgentContext tests: a picker switch persists exactly once with persistSelectedAgentId: true; re-selection doesn't hit the backend; a typed rejection reverts to the previous agent (toast + echo-guard release), a rejection landing after a newer switch does not revert it, chained rejections restore the backend's authoritative agent instead of another rejected one, and the rollback reads settle-time metadata (a mid-flight acceptance echo wins over the render-time baseline).
  • workspaceAiSettingsSync unit tests: the revert hydrates the backend bucket (or legacy blob) even when the backend already stores the rejected agent id, and never clobbers newer user edits. workspaceModeAi: a saved workspace bucket beats configured defaults on explicit switches.
  • ProposePlan tests: Implement switches to exec with resolved settings and sends without issuing a separate settings write; a typed send rejection reverts the optimistic switch (agent + settings) while a transport-failed send keeps it, and both release the guard.
  • bun test on the three affected suites and make static-check pass locally.

Risks

Low, scoped to mode/model selection UX:

  • Mode becomes backend-synced across clients; two clients viewing one workspace converge (last writer wins) instead of diverging silently. This matches existing model-settings semantics.
  • Legacy workspaces whose persisted agentId/agentType predates this change will seed that value into clients once; it reflects the last send, and any switch/send immediately overwrites it.
  • If the persistence IPC fails in transport, the local selection stays and other clients may briefly see a stale backend value; the next send re-persists it and converges all clients. If the backend rejects the selection (pricing gate), the local switch reverts to the backend-retained agent.

Generated with xum • Model: anthropic:claude-fable-5 • Thinking: max • Cost: $149.44

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 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/contexts/AgentContext.tsx Outdated
Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx
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 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/utils/workspaceAiSettingsSync.ts Outdated
Comment thread src/browser/contexts/WorkspaceContext.tsx
Comment thread src/node/services/workspaceService.ts
- 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 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/workspaceService.ts Outdated
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 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/node/services/workspaceService.ts
- 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 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/node/services/workspaceService.ts Outdated
Comment thread src/browser/contexts/AgentContext.tsx Outdated
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_
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/browser/contexts/WorkspaceContext.tsx
Comment thread src/browser/contexts/AgentContext.tsx Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/node/services/workspaceService.ts Outdated
Comment thread src/node/acp/configOptions.ts
Generated with xum • Model: openai:gpt-5.6-sol • Thinking: high • Cost:

<!-- mux-attribution: model=openai:gpt-5.6-sol thinking=high costs=236.22 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc1f24152c

ℹ️ 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/node/orpc/router.ts Outdated
---

_Generated with `xum` • Model: `openai:gpt-5.6-sol` • Thinking: `high` • Cost: `$254.64`_

<!-- mux-attribution: model=openai:gpt-5.6-sol thinking=high costs=254.64 -->
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

// Normal sends/resumes also persist the selected agent so future backend heartbeat
// dispatches can reuse the same workspace default after reloads and reconnects.
persistSelectedAgentId: true,
...(options?.disableWorkspaceAgents === true ? { disableWorkspaceAgents: true } : {}),

P2 Badge Preserve locked child identity on ACP prompts

For a child workspace, ACP mode selection deliberately avoids persisting the selected agent, but the next prompt sends the session-local agent in src/node/acp/agent.ts:595-602 and this unconditional option then writes it as the workspace selection. The first prompt after a child ACP mode switch therefore overwrites the child's locked creation-time identity, causing later heartbeats, continuations, and renderer hydration to use the session-only mode that configOptions.ts:400-404 explicitly intended not to persist. Skip selected-agent persistence for locked child workspaces or mark these ACP sends as session-local.

ℹ️ 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".

Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/browser/features/Tools/ProposePlanToolCall.tsx Outdated
Comment thread src/browser/components/WorkspaceModeAISync/WorkspaceModeAISync.tsx Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 026e4eb600

ℹ️ 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".

Comment thread src/browser/features/ChatInput/index.tsx Outdated
Comment thread src/browser/utils/workspaceAiSettingsSync.ts Outdated
Comment thread src/node/services/workspaceService.ts Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/browser/contexts/AgentContext.tsx
Comment thread src/browser/contexts/AgentContext.tsx
Comment thread src/node/orpc/router.ts
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

export function clearPendingWorkspaceAiSettings(workspaceId: string, agentId: string): void {
if (!workspaceId || !agentId) {
return;
}
pendingAiSettingsByWorkspace.delete(getPendingKey(workspaceId, agentId));

P2 Badge Retain the newest settings guard across older failures

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".

Comment thread src/browser/utils/workspaceModeAi.ts
@chatgpt-codex-connector

This comment has been minimized.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

export function clearPendingWorkspaceAiSettings(workspaceId: string, agentId: string): void {
if (!workspaceId || !agentId) {
return;
}
pendingAiSettingsByWorkspace.delete(getPendingKey(workspaceId, agentId));

P2 Badge Clear only the matching pending settings write

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".

Comment on lines +146 to +148
// Definition defaults seed the initial selection and explicit switches only.
// Later descriptor arrival must preserve any model the user already selected.
mode: !hasPriorSelection || isExplicitAgentSwitch ? "creation-sync" : "background-sync",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply late-loaded defaults before the initial creation send

When the creation screen mounts before agents.list resolves, the first effect run records the selected agent with an empty descriptor list; the normal descriptor-arrival rerun is then classified as background-sync, which excludes definition defaults. If the user has not selected a model manually, a custom agent's frontmatter model/thinking defaults are therefore never applied, and the first send uses and persists the ambient project settings instead. Track whether a user actually changed the settings, or defer the initial creation sync until descriptors are available.

Useful? React with 👍 / 👎.

Comment on lines +517 to +518
agents,
mode: "explicit-switch",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard plan-action settings as well as the agent ID

When Implement/Continue starts and a stale metadata broadcast arrives during the send's preflight, this separate plan-action path guards only the optimistic agent ID. WorkspaceContext consequently retains the new exec/auto identity but can hydrate that agent's stale model/thinking bucket; a manual send initiated in this window captures those stale values and is serialized after the plan send, so it persists them last. Mark the resolved AI settings pending here as the picker switch path does, and clear that guard when the send settles.

AGENTS.md reference: AGENTS.md:L150-L150

Useful? React with 👍 / 👎.

Comment on lines +68 to +71
agents.map((agent) => [
agent.id,
{ base: agent.base, definitionAiDefaults: agent.ownAiDefaults },
])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use embedded ancestor layers in ACP resolution

When a selectable custom agent inherits from a disabled or otherwise omitted base agent, agents.list now embeds that complete chain in the child's aiAncestors, but this map still reconstructs ancestry only from top-level returned descriptors. Because the base descriptor is absent, ACP resolution loses its definition defaults and can persist a parent-runtime or system fallback into the child's workspace bucket, diverging from Node dispatch. Fresh evidence after the earlier ancestry fix is that resolveAcpAgentAiSettings still ignores agentDef.aiAncestors; pass those embedded layers to the shared resolver as the browser consumers do.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant