Seed new chat-input sessions from the remembered session-config picks - #334814
Open
Ryan Ewen (RyanEwen) wants to merge 1 commit into
Open
Seed new chat-input sessions from the remembered session-config picks#334814Ryan Ewen (RyanEwen) wants to merge 1 commit into
Ryan Ewen (RyanEwen) wants to merge 1 commit into
Conversation
The Agents window and the chat input seed a new session's config from two different places. The Agents window replays the user's last picks from profile storage; the chat input reads only `chat.defaultConfiguration`, which covers the `autoApprove` and `mode` axes and nothing else. Any other property therefore falls back to its schema default in the chat input however many times the user changes it. Claude's `permissionMode` is the visible case: the Approvals chip reads that property, `chat.defaultConfiguration` does not write it, so every new chat starts on Default while the Agents window keeps whatever was last chosen. Move the storage key and the remembered-key predicate into `sessionConfigKeys.ts` so both surfaces share one definition rather than a copy, and seed the chat input's initial config from the same store. Isolation is spread last so it stays host-owned, and the existing `chat.defaultConfiguration` handling is left in place, so the two axes it already governs keep their current precedence in this path.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new storage dependency breaks the existing unit-test setup, and the new behavior lacks regression coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Seeds chat-input sessions from the Agents window’s remembered configuration.
Changes:
- Shares remembered-config storage helpers.
- Applies remembered values while preserving existing overrides.
File summaries
| File | Description |
|---|---|
agentHostUntitledProvisionalSessionService.ts |
Seeds chat sessions from stored picks. |
baseAgentHostSessionsProvider.ts |
Uses shared storage helpers. |
sessionConfigKeys.ts |
Defines shared key and validation helper. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @IAgentHostImportConversationStore private readonly _importConversationStore: IAgentHostImportConversationStore, | ||
| @IAgentHostActiveClientService private readonly _activeClientService: IAgentHostActiveClientService, | ||
| @IUriIdentityService private readonly _uriIdentityService: IUriIdentityService, | ||
| @IStorageService private readonly _storageService: IStorageService, |
Comment on lines
+1069
to
+1075
| const rememberedValues = this._storageService.getObject<Record<string, unknown>>(REMEMBERED_SESSION_CONFIG_STORAGE_KEY, StorageScope.PROFILE, {}); | ||
| for (const [property, value] of Object.entries(rememberedValues)) { | ||
| if (typeof value === 'string' && isRememberedSessionConfigKey(property)) { | ||
| remembered[property] = value; | ||
| } | ||
| } | ||
| const config: Record<string, unknown> = { ...remembered, [SessionConfigKey.Isolation]: 'folder' }; |
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
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.
A new session's config is seeded from two different places depending on which surface starts it.
The Agents window replays the user's last picks from profile storage:
The chat input does not.
AgentHostUntitledProvisionalSessionService._getInitialConfigreads onlychat.defaultConfiguration, which writes theautoApproveandmodeaxes. Every other session-config property falls back to its schema default, no matter how many times the user changes it.What a user sees
Claude's Approvals chip is the visible case. It reads
permissionMode(default/acceptEdits/plan/auto), andchat.defaultConfigurationnever writes that property. So the Agents window keeps whatever approvals level was last chosen, while every new chat in the panel opens on Default. Settingchat.defaultConfiguration.approvalsdoes not help, because it lands onautoApproveinstead.It is not specific to Claude. Any agent-advertised config property outside those two axes has the same asymmetry;
permissionModeis simply the one with a chip in front of it.What this changes
REMEMBERED_SESSION_CONFIG_STORAGE_KEYandisRememberedSessionConfigKeymove intosessionConfigKeys.ts, which both layers already import, so there is one definition instead of a copy in the sessions provider. The chat input then seeds its initial config from that same store.Deliberately conservative in two ways.
SessionConfigKey.Isolationis spread last so it stays host-owned rather than being restorable from a remembered pick. And the existingchat.defaultConfigurationhandling is untouched, soautoApproveandmodekeep the precedence they have in this path today; only properties the setting never wrote are affected. Bringing those two axes in line with the Agents window'spolicy > remembered > effectiveorder would be a separate, more visible change.AI disclosure: this comment and the related code were written with the assistance of AI.