Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces ACP Session Config Options—a feature for managing configuration state across ACP agent processes and sessions. It upgrades the SDK dependency (0.5.1 → 0.16.1), adds config state normalization utilities, extends process and session managers to track and synchronize config state, introduces presenter APIs for reading/writing session options, updates the renderer to display inline config controls and a settings panel, and emits a new Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer<br/>(ChatStatusBar)
participant Presenter as Presenter<br/>(LLMProviderPresenter)
participant ProcMgr as ProcessManager
participant Provider as ACP Provider
participant SDK as ACP SDK<br/>(Agent)
Note over Renderer,SDK: Process Warmup & Config State Initialization
Renderer->>Presenter: warmupAcpProcess(agentId, workdir?)
Presenter->>Provider: warmupProcess(agentId, workdir?)
Provider->>SDK: initialize() / connect
SDK-->>Provider: configOptions, models, modes
Provider->>ProcMgr: Handle creation with configState
ProcMgr->>ProcMgr: normalizeAcpConfigState(configOptions)
ProcMgr->>ProcMgr: cache configState by agent
ProcMgr-->>Provider: handle.configState populated
Provider-->>Presenter: warmup complete
Presenter-->>Renderer: warmup done
Renderer->>Presenter: getAcpProcessConfigOptions(agentId, workdir?)
Presenter->>ProcMgr: getProcessConfigState(agentId, workdir?)
ProcMgr-->>Presenter: cached configState
Presenter-->>Renderer: configState displayed
sequenceDiagram
participant Renderer as Renderer<br/>(ChatStatusBar)
participant NewAgent as NewAgentPresenter
participant Presenter as LLMProviderPresenter
participant Provider as ACP Provider
participant SessMgr as SessionManager
participant SDK as ACP SDK<br/>(Session)
Note over Renderer,SDK: Session Creation & Config State Binding
Renderer->>NewAgent: setAcpSessionConfigOption(sessionId, configId, value)
NewAgent->>Presenter: setAcpSessionConfigOption(conversationId, configId, value)
Presenter->>Provider: setSessionConfigOption(conversationId, configId, value)
Provider->>SessMgr: session lookup
SessMgr->>SDK: connection.setSessionConfigOption({configId, value})
SDK-->>SessMgr: response with updated configOptions
SessMgr->>SessMgr: normalizeAcpConfigState(configOptions)
SessMgr->>SessMgr: session.configState = normalized
SessMgr->>SessMgr: update process.configState from session
SessMgr-->>Provider: session updated
Provider->>Provider: emit SESSION_CONFIG_OPTIONS_READY
Provider-->>Presenter: configState (normalized response)
Presenter-->>NewAgent: configState
NewAgent-->>Renderer: updated configState displayed
sequenceDiagram
participant SDK as ACP SDK<br/>(Agent)
participant Provider as ACP Provider
participant ProcMgr as ProcessManager
participant EventBus as EventBus
participant Renderer as Renderer<br/>(ChatStatusBar)
Note over SDK,Renderer: Config Option Update via Protocol
SDK->>Provider: emit sessionUpdate<br/>(config_option_update)
Provider->>Provider: mapper.map(sessionUpdate)
Provider->>Provider: handleConfigOptionUpdate()<br/>normalizeAcpConfigState()
Provider->>Provider: mapped.configState set
Provider->>ProcMgr: updateBoundProcessConfigState(configState)
ProcMgr->>ProcMgr: handle.configState = updated
ProcMgr->>EventBus: sendToRenderer<br/>(SESSION_CONFIG_OPTIONS_READY)
EventBus-->>Renderer: IPC event payload
Renderer->>Renderer: acpConfigState = payload.configState
Renderer->>Renderer: UI re-renders with new values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db913616ab
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/agentPresenter/acp/acpSessionManager.ts (1)
423-437:⚠️ Potential issue | 🔴 CriticalDon’t overwrite the server-reported mode before the session RPC runs.
Lines 423-437 replace the response/default mode with
handle.currentModeIdand immediately write that intoconfigState.createSession()later usescurrentModeId !== session.currentModeIdto decide whether it still needs to callsetSessionMode(), so this makes the comparison false and the RPC is skipped. The ACP session can stay on the old mode while local state says the preferred mode is active.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentPresenter/acp/acpSessionManager.ts` around lines 423 - 437, The code is prematurely persisting the local preferred mode (handle.currentModeId) into configState, which prevents createSession() from detecting a mode change and calling setSessionMode(); instead, compute currentModeId for UI selection only (using handle.currentModeId as preferredMode) but do not call updateAcpConfigStateValue with the preferred mode. Only update configState when the server/response/default mode (responseModeId or availableModes fallback) is chosen or after the session RPC confirms the mode change; keep handle.currentModeId (or a new preferredMode variable) separate from the value written via updateAcpConfigStateValue, and reference getAcpConfigOptionByCategory and updateAcpConfigStateValue when updating configState only with the server/response-determined mode.
🧹 Nitpick comments (2)
src/main/presenter/newAgentPresenter/index.ts (1)
1117-1128: Consider reusingisAcpBackedSessionin other ACP checks to avoid logic drift.
You now have a solid central predicate; applying it in other provider-detection branches (e.g., message/delete paths) would reduce duplication and future divergence risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/newAgentPresenter/index.ts` around lines 1117 - 1128, Centralize provider-detection by replacing duplicated ACP-check logic in other code paths (e.g., message/delete handlers) with calls to the existing isAcpBackedSession(sessionId, agentId). Locate places where code re-fetches agent state or queries configPresenter.getAcpAgents() to infer providerId and instead call this.isAcpBackedSession(sessionId, agentId) (after resolving the same agentId/ sessionId inputs there); ensure those branches remove their local providerId derivation and use the boolean result to decide ACP-specific behavior so all ACP-detection uses the single predicate isAcpBackedSession.src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (1)
929-945: Avoid double-emittingSESSION_CONFIG_OPTIONS_READY.
updateBoundProcessConfigState()andupdateBoundProcessMode()already notify the renderer inacpProcessManager.ts. Re-emitting here sends the same bound-session payload twice, which doubles IPC traffic and renderer refreshes for every config change. Keep a single emitter on the bound-session update path.Also applies to: 1289-1294, 1434-1449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/llmProviderPresenter/providers/acpProvider.ts` around lines 929 - 945, The code is double-emitting SESSION_CONFIG_OPTIONS_READY from acpProvider.ts after calling this.processManager.updateBoundProcessConfigState and updateBoundProcessMode; remove the local calls to emitSessionConfigOptionsReady (the ones immediately after updateBoundProcessConfigState/updateBoundProcessMode) so only the notifications originating from acpProcessManager.ts/processManager are sent; update the three occurrences referenced (around the blocks calling updateBoundProcessConfigState and updateBoundProcessMode at the shown ranges) to rely on the processManager's own emitter and keep the warning/log behavior but delete the duplicate emitSessionConfigOptionsReady calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/agentPresenter/acp/acpProcessManager.ts`:
- Around line 1114-1133: syncAgentCache currently keys caches by agentId only
which allows cross-workspace leakage; update it to use the same workspace-scoped
key used by warmups (e.g., `${handle.agentId}::${handle.workdir}` or similar)
when reading/writing latestConfigStates and latestModeSnapshots so
getProcessModes()/getProcessConfigState() return workspace-correct data, and
when both availableModes and currentModeId are empty remove (delete) the
corresponding entry from latestModeSnapshots to avoid returning stale fallback
snapshots; adjust any references in syncAgentCache, latestConfigStates, and
latestModeSnapshots to use that compound key.
In `@src/main/presenter/agentPresenter/acp/acpSessionManager.ts`:
- Around line 356-357: The code currently blindly replaces handle.configState
with normalizeAcpConfigState(...) and clears the warmup cache even when
loadSession()/newSession() return no config payload, which wipes warmup-only ACP
options; change the logic in acpSessionManager so after calling
normalizeAcpConfigState(...) you only assign to handle.configState (and clear
the warmup cache) if the normalized result contains actual config data (e.g.,
non-empty models, modes, or configOptions) — otherwise keep the existing
handle.configState and do not clear the warmup cache; use comparisons against
createEmptyAcpConfigState('legacy') or an explicit hasAnyConfig check to decide
whether to overwrite, and apply this guard wherever normalizeAcpConfigState(...)
is used (references: handle.configState, createEmptyAcpConfigState,
normalizeAcpConfigState, loadSession, newSession, warmup cache handling).
In `@src/main/presenter/llmProviderPresenter/providers/acpProvider.ts`:
- Around line 1398-1413: The current code replaces the entire session snapshot
by calling normalizeAcpConfigState({ configOptions: response.configOptions }),
which discards legacy sections like models/modes; instead, merge
response.configOptions into the existing session.configState prior to
normalizing so legacy entries are preserved. Update the block around
session.connection.setSessionConfigOption (the code that sets response and
computes nextConfigState) to compute a mergedConfigOptions = {
...session.configState.configOptions, ...response.configOptions } (or otherwise
merge deep if needed) and then call normalizeAcpConfigState({ configOptions:
mergedConfigOptions }) (or pass the merged snapshot) and assign that to
nextConfigState so you don't drop previously synthesized models/modes.
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 802-803: The panel is becoming writable as soon as
activeAcpSessionId is non-null while acpConfigState still holds the previous
session snapshot; update the readiness check and fetch flow so writes are
blocked until the session-scoped config finishes loading. Concretely, change the
computed acpConfigReadOnly to also require a "config loaded" condition (e.g., a
new acpConfigLoaded ref or compare acpConfigState.sessionId ===
activeAcpSessionId.value), set acpConfigLoaded = false (or clear acpConfigState)
immediately when activeAcpSessionId changes, then set acpConfigLoaded = true
only after getAcpSessionConfigOptions() resolves and updates acpConfigState;
ensure any submit logic checks the same loaded flag before allowing writes.
- Around line 761-799: The cache currently keys ACP config by agent only
(acpConfigCacheByAgent via acpConfigCacheKey) causing cross-workspace leaks;
change the cache key to include workspace (use acpWorkspacePath) so keys are
like `${agentId}::${workspace}` (update acpConfigCacheKey/acpConfigRequestKey
logic accordingly), update getCachedAcpConfigState and setCachedAcpConfigState
to accept/compute the combined agent+workspace key (or a single requestKey) and
store/lookup from acpConfigCacheByAgent using that composite key, and finally
update the SESSION_CONFIG_OPTIONS_READY event handler to verify the
workspace/workdir matches the cached requestKey before accepting/applying the
config to avoid applying a config warmed for a different folder.
---
Outside diff comments:
In `@src/main/presenter/agentPresenter/acp/acpSessionManager.ts`:
- Around line 423-437: The code is prematurely persisting the local preferred
mode (handle.currentModeId) into configState, which prevents createSession()
from detecting a mode change and calling setSessionMode(); instead, compute
currentModeId for UI selection only (using handle.currentModeId as
preferredMode) but do not call updateAcpConfigStateValue with the preferred
mode. Only update configState when the server/response/default mode
(responseModeId or availableModes fallback) is chosen or after the session RPC
confirms the mode change; keep handle.currentModeId (or a new preferredMode
variable) separate from the value written via updateAcpConfigStateValue, and
reference getAcpConfigOptionByCategory and updateAcpConfigStateValue when
updating configState only with the server/response-determined mode.
---
Nitpick comments:
In `@src/main/presenter/llmProviderPresenter/providers/acpProvider.ts`:
- Around line 929-945: The code is double-emitting SESSION_CONFIG_OPTIONS_READY
from acpProvider.ts after calling
this.processManager.updateBoundProcessConfigState and updateBoundProcessMode;
remove the local calls to emitSessionConfigOptionsReady (the ones immediately
after updateBoundProcessConfigState/updateBoundProcessMode) so only the
notifications originating from acpProcessManager.ts/processManager are sent;
update the three occurrences referenced (around the blocks calling
updateBoundProcessConfigState and updateBoundProcessMode at the shown ranges) to
rely on the processManager's own emitter and keep the warning/log behavior but
delete the duplicate emitSessionConfigOptionsReady calls.
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 1117-1128: Centralize provider-detection by replacing duplicated
ACP-check logic in other code paths (e.g., message/delete handlers) with calls
to the existing isAcpBackedSession(sessionId, agentId). Locate places where code
re-fetches agent state or queries configPresenter.getAcpAgents() to infer
providerId and instead call this.isAcpBackedSession(sessionId, agentId) (after
resolving the same agentId/ sessionId inputs there); ensure those branches
remove their local providerId derivation and use the boolean result to decide
ACP-specific behavior so all ACP-detection uses the single predicate
isAcpBackedSession.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e934f42f-67ff-4652-812d-bea1b7bf6484
📒 Files selected for processing (34)
docs/specs/acp-session-config-options/plan.mddocs/specs/acp-session-config-options/spec.mddocs/specs/acp-session-config-options/tasks.mdpackage.jsonsrc/main/events.tssrc/main/presenter/agentPresenter/acp/acpCapabilities.tssrc/main/presenter/agentPresenter/acp/acpConfigState.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/presenter/agentPresenter/acp/acpFsHandler.tssrc/main/presenter/agentPresenter/acp/acpMessageFormatter.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/agentPresenter/acp/acpTerminalManager.tssrc/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/agentPresenter/acp/mcpConfigConverter.tssrc/main/presenter/agentPresenter/acp/mcpTransportFilter.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/newAgentPresenter/index.tssrc/main/presenter/sessionPresenter/index.tssrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/events.tssrc/renderer/src/pages/NewThreadPage.vuesrc/shared/types/presenters/index.d.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/new-agent.presenter.d.tssrc/shared/types/presenters/session.presenter.d.tstest/main/presenter/acpMcpPassthrough.test.tstest/main/presenter/acpProvider.test.tstest/main/presenter/agentPresenter/acp/acpProcessManager.test.tstest/main/presenter/llmProviderPresenter/acpContentMapper.test.tstest/main/presenter/newAgentPresenter/newAgentPresenter.test.tstest/renderer/components/ChatStatusBar.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/agentPresenter/acp/acpSessionManager.ts (1)
430-444:⚠️ Potential issue | 🟠 MajorDon't collapse the preferred mode and the session's actual mode during initialization.
Lines 430-444 seed
currentModeIdfromhandle.currentModeId, write that value back intoconfigState, and return it assession.currentModeIdbefore the ACP session has actually been switched. WhencreateSession()reaches Line 231, the comparison is already false, sosetSessionMode()is skipped and local state can claim the preferred mode is active while the server is still on the response/default mode. Keep the response-reported mode in the returned session state, and only use the warmup preference as the desired mode to apply afterward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentPresenter/acp/acpSessionManager.ts` around lines 430 - 444, The code is seeding and persisting the preferred mode (handle.currentModeId) as the session's active mode before the server confirms the switch; instead, keep the server-reported mode (responseModeId / responseModeState) as the session's currentModeId during initialization and treat handle.currentModeId only as the desired/warmup mode to apply later. Concretely: set currentModeId = responseModeId ?? availableModes?.[0]?.id (do not default to handle.currentModeId), avoid calling updateAcpConfigStateValue(configState, modeOption.id, currentModeId) with the preferred mode during session creation, and only update configState / session.currentModeId after setSessionMode() succeeds (or when the server reports the new mode); use a separate variable like desiredMode = handle.currentModeId to carry the warmup preference until confirmed.
♻️ Duplicate comments (2)
src/main/presenter/agentPresenter/acp/acpConfigState.ts (1)
115-131:⚠️ Potential issue | 🟠 MajorDon't make
configOptionsmutually exclusive with legacymodels/modes.The early return on Line 116 drops the synthesized legacy model/mode options whenever a session payload also includes regular
configOptions.loadSession()/newSession()can therefore initializeconfigStatewithout ACP model/mode controls, even though the same response still carries legacymodels/modes. Merge the legacy options when those categories are missing instead of returning only the modern branch.💡 Suggested merge
export const normalizeAcpConfigState = (input: NormalizableConfigStateInput): AcpConfigState => { + const legacyOptions = [buildLegacyModelOption(input.models), buildLegacyModeOption(input.modes)].filter( + (option): option is AcpConfigOption => Boolean(option) + ) + if (input.configOptions !== undefined && input.configOptions !== null) { + const options = input.configOptions.map(normalizeConfigOption) + const categories = new Set( + options.map((option) => option.category).filter((category): category is string => Boolean(category)) + ) + return { source: 'configOptions', - options: input.configOptions.map(normalizeConfigOption) + options: [ + ...legacyOptions.filter((option) => !option.category || !categories.has(option.category)), + ...options + ] } } - const options = [buildLegacyModelOption(input.models), buildLegacyModeOption(input.modes)].filter( - (option): option is AcpConfigOption => Boolean(option) - ) + const options = legacyOptions return { source: 'legacy', options }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentPresenter/acp/acpConfigState.ts` around lines 115 - 131, normalizeAcpConfigState currently returns early when input.configOptions exists, which drops synthesized legacy options built by buildLegacyModelOption and buildLegacyModeOption and causes ACP model/mode controls to be lost; change normalizeAcpConfigState to always merge legacy options with normalized configOptions instead of returning immediately: call buildLegacyModelOption(input.models) and buildLegacyModeOption(input.modes), filter and include any resulting AcpConfigOption(s), then append or prepend the normalized input.configOptions.map(normalizeConfigOption) so the returned AcpConfigState.options contains both modern and synthesized legacy options (ensure the function still sets source appropriately, e.g., 'configOptions' when configOptions present but include legacy options too); update any callers such as loadSession/newSession if they rely on exclusive behavior.src/renderer/src/components/chat/ChatStatusBar.vue (1)
762-800:⚠️ Potential issue | 🟠 MajorScope the ACP config cache and ready events by request key, not just agent.
acpConfigRequestKeyis workspace-aware, but Lines 785-799 still read/writeacpConfigCacheByAgentwith onlyagentId, and Lines 1856-1864 still accept any process-levelSESSION_CONFIG_OPTIONS_READYfor that agent. Reusing the same ACP agent in two folders can therefore hydrate the second workspace with the first workspace's config, or keep stale options after a failed warmup. Use the same composite key/workdir check for cache lookups, writes, and event acceptance.Also applies to: 1834-1865
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 762 - 800, The ACP cache and event acceptance must be keyed by the composite request key (acpConfigRequestKey) not just agentId: update getCachedAcpConfigState and setCachedAcpConfigState to take a requestKey (string|null|undefined) and read/write acpConfigCacheByAgent using that composite key (use acpConfigRequestKey.value where caller previously passed activeAcpAgentId), and update the SESSION_CONFIG_OPTIONS_READY handler(s) (references around lines ~1834-1865) to validate the incoming event's requestKey equals the current acpConfigRequestKey.value before hydrating or accepting options so workspace-aware configs aren’t mixed across folders or stale after warmup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 46-66: The UI is rendering raw option IDs (entry.value and
option.currentValue) instead of their human labels; update the template in
ChatStatusBar.vue to display entry.label (and the selected option's label)
wherever values are shown while keeping comparisons and data attributes using
entry.value and option.currentValue; specifically, change the button text from
"{{ entry.value }}" to the label (use entry.label) and ensure any trigger/menu
places that currently render entry.value or option.currentValue use the
corresponding entry.label for display while leaving :data-value,
:data-option-id, disabled checks, class comparisons and the
onAcpSelectOption(option.id, entry.value) call unchanged so behavior still
matches by ID.
---
Outside diff comments:
In `@src/main/presenter/agentPresenter/acp/acpSessionManager.ts`:
- Around line 430-444: The code is seeding and persisting the preferred mode
(handle.currentModeId) as the session's active mode before the server confirms
the switch; instead, keep the server-reported mode (responseModeId /
responseModeState) as the session's currentModeId during initialization and
treat handle.currentModeId only as the desired/warmup mode to apply later.
Concretely: set currentModeId = responseModeId ?? availableModes?.[0]?.id (do
not default to handle.currentModeId), avoid calling
updateAcpConfigStateValue(configState, modeOption.id, currentModeId) with the
preferred mode during session creation, and only update configState /
session.currentModeId after setSessionMode() succeeds (or when the server
reports the new mode); use a separate variable like desiredMode =
handle.currentModeId to carry the warmup preference until confirmed.
---
Duplicate comments:
In `@src/main/presenter/agentPresenter/acp/acpConfigState.ts`:
- Around line 115-131: normalizeAcpConfigState currently returns early when
input.configOptions exists, which drops synthesized legacy options built by
buildLegacyModelOption and buildLegacyModeOption and causes ACP model/mode
controls to be lost; change normalizeAcpConfigState to always merge legacy
options with normalized configOptions instead of returning immediately: call
buildLegacyModelOption(input.models) and buildLegacyModeOption(input.modes),
filter and include any resulting AcpConfigOption(s), then append or prepend the
normalized input.configOptions.map(normalizeConfigOption) so the returned
AcpConfigState.options contains both modern and synthesized legacy options
(ensure the function still sets source appropriately, e.g., 'configOptions' when
configOptions present but include legacy options too); update any callers such
as loadSession/newSession if they rely on exclusive behavior.
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 762-800: The ACP cache and event acceptance must be keyed by the
composite request key (acpConfigRequestKey) not just agentId: update
getCachedAcpConfigState and setCachedAcpConfigState to take a requestKey
(string|null|undefined) and read/write acpConfigCacheByAgent using that
composite key (use acpConfigRequestKey.value where caller previously passed
activeAcpAgentId), and update the SESSION_CONFIG_OPTIONS_READY handler(s)
(references around lines ~1834-1865) to validate the incoming event's requestKey
equals the current acpConfigRequestKey.value before hydrating or accepting
options so workspace-aware configs aren’t mixed across folders or stale after
warmup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea227e75-7499-4417-91be-b9ea34ac2723
📒 Files selected for processing (8)
src/main/presenter/agentPresenter/acp/acpConfigState.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat/ChatStatusBar.vuetest/main/presenter/acpMcpPassthrough.test.tstest/main/presenter/acpProvider.test.tstest/renderer/components/ChatStatusBar.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/presenter/agentPresenter/acp/index.ts
- test/main/presenter/acpProvider.test.ts
- test/main/presenter/acpMcpPassthrough.test.ts
- test/renderer/components/ChatStatusBar.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Tests