Open
Conversation
- Unrelated: fix local agent host being broken on main following some overnight cleanups - Register a JSON schema for agent session settings (only when they're open since it has a perf cost) - When there are multiple custom approval buttons, show them in a dropdown similar to other tool calls.
In advance of host-level settings, replaces our ad-hoc setting handling with a IAgentConfigurationService. This handles inheritance correctly. We also have a 'schema builder' now that allows for better type safety as we produce and consume schema. - New `IAgentConfigurationService` with `getEffectiveValue`, `getEffectiveWorkingDirectory`, and `updateSessionConfig`. Owns the inheritance chain so every consumer composes layers identically. - New schema builder (`schemaProperty<T>` / `createSchema`) with phantom TypeScript types + derived runtime validators. `assertValid` throws a `ProtocolError(InvalidParams)` annotated with the offending dotted path (e.g. `permissions.allow[2]`); `validate` is the boolean form; `values()` validates at write sites; `validateOrDefault()` sanitizes untrusted input at protocol boundaries. - `platformSessionSchema` centralizes the `autoApprove` + `permissions` property descriptors that `copilotAgent.resolveSessionConfig` previously duplicated; agents compose via `...platformSessionSchema .definition`. - `getEffectiveValue` validates each layer against a caller-supplied schema and falls through when a layer's value is malformed, logging the path-annotated reason. - Wire DI through `AgentService` (local `InstantiationService` scope) so `AgentSideEffects` and `SessionPermissionManager` resolve `ILogService` and `IAgentConfigurationService` via DI rather than plain-class plumbing. - `copilotAgent.resolveSessionConfig` collapses to a single schema construction + `validateOrDefault` call; duplicated schema literals removed. - Tests: 33 new unit tests covering schema validation, path-annotated errors, `validateOrDefault`, and the full precedence chain in `AgentConfigurationService`
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors agent-host session configuration handling by introducing a dedicated IAgentConfigurationService (with correct inheritance across session → parent subagent → host), adds a typed schema builder with runtime validation, and centralizes well-known session config keys to reduce duplication across platform + Copilot agent code.
Changes:
- Add
IAgentConfigurationService+ schema builder utilities (createSchema/schemaProperty) and centralize platform-owned config schema (platformSessionSchema). - Replace ad-hoc stringly-typed config keys (e.g. branchNameHint/autoApprove/permissions) with
SessionConfigKey. - Add JSON schema registration for
agent-session-settings://...files (lazy registration + refresh/dispose) and expand unit tests.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts | Update branch name hint expectation to use SessionConfigKey. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/abstractToolConfirmationSubPart.ts | Refactor tool confirmation custom option button building with protocol option kinds/groups. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts | Pass branch name hint using SessionConfigKey.BranchNameHint. |
| src/vs/sessions/contrib/chat/browser/agentHost/agentHostSessionConfigPicker.ts | Switch well-known config property comparisons to SessionConfigKey. |
| src/vs/sessions/contrib/chat/browser/agentHost/agentHostPermissionPickerDelegate.ts | Remove local auto-approve constants; use centralized keys/values. |
| src/vs/sessions/contrib/agentHost/test/browser/agentSessionSettingsFileSystemProvider.test.ts | Add schema registration tests and update harness to support provider/session events. |
| src/vs/sessions/contrib/agentHost/browser/localAgentHost.contribution.ts | Ensure agent-host chat + terminal contributions are registered in the Sessions app. |
| src/vs/sessions/contrib/agentHost/browser/agentSessionSettingsFileSystemProvider.ts | Add schema conversion + lazy per-session schema registration/refresh/disposal. |
| src/vs/sessions/contrib/agentHost/browser/agentSessionSettings.contribution.ts | Wire schema registrar into the settings FS provider registration. |
| src/vs/platform/agentHost/test/node/agentSideEffects.test.ts | Update tests to construct AgentSideEffects via DI scope including IAgentConfigurationService. |
| src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts | New tests for effective config inheritance and session config patching. |
| src/vs/platform/agentHost/test/common/agentHostSchema.test.ts | New tests for schema validation, path-annotated errors, and defaults sanitization. |
| src/vs/platform/agentHost/node/sessionPermissions.ts | Route permission/auto-approve logic through IAgentConfigurationService + platformSessionSchema. |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Replace duplicated schema literals with composed typed schema + validateOrDefault; use SessionConfigKey. |
| src/vs/platform/agentHost/node/agentSideEffects.ts | Create SessionPermissionManager via DI and stop manual plumbing of config/log dependencies. |
| src/vs/platform/agentHost/node/agentService.ts | Introduce local instantiation scope providing IAgentConfigurationService to downstream components. |
| src/vs/platform/agentHost/node/agentConfigurationService.ts | New IAgentConfigurationService implementation providing inheritance-aware reads and session patch writes. |
| src/vs/platform/agentHost/common/sessionConfigKeys.ts | New centralized constants for well-known session config keys + known auto-approve values. |
| src/vs/platform/agentHost/common/agentService.ts | Remove legacy AgentHostSessionConfigBranchNameHintKey constant. |
| src/vs/platform/agentHost/common/agentHostSchema.ts | New schema builder utilities + platform-owned session config schema definitions. |
Copilot's findings
- Files reviewed: 20/20 changed files
- Comments generated: 4
Contributor
Screenshot ChangesBase: Changed (2)
|
| Before | After |
|---|---|
 |
 |
agentSessionsViewer/CompletedUnread/Dark
| Before | After |
|---|---|
 |
 |
connor4312
force-pushed
the
connor4312/ah-session-settings-2
branch
from
3eb81bf to
9636727
Compare
roblourens
previously approved these changes
Apr 22, 2026
pwang347
approved these changes
Apr 22, 2026
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.
In advance of host-level settings, replaces our ad-hoc setting handling with a IAgentConfigurationService. This handles inheritance correctly. We also have a 'schema builder' now that allows for better type safety as we produce and consume schema.
IAgentConfigurationServicewithgetEffectiveValue,getEffectiveWorkingDirectory, andupdateSessionConfig. Owns the inheritance chain so every consumer composes layers identically.schemaProperty<T>/createSchema) with phantom TypeScript types + derived runtime validators.assertValidthrows aProtocolError(InvalidParams)annotated with the offending dotted path (e.g.permissions.allow[2]);validateis the boolean form;values()validates at write sites;validateOrDefault()sanitizes untrusted input at protocol boundaries.platformSessionSchemacentralizes theautoApprove+permissionsproperty descriptors thatcopilotAgent.resolveSessionConfigpreviously duplicated; agents compose via...platformSessionSchema .definition.getEffectiveValuevalidates each layer against a caller-supplied schema and falls through when a layer's value is malformed, logging the path-annotated reason.AgentService(localInstantiationServicescope) soAgentSideEffectsandSessionPermissionManagerresolveILogServiceandIAgentConfigurationServicevia DI rather than plain-class plumbing.copilotAgent.resolveSessionConfigcollapses to a single schema construction +validateOrDefaultcall; duplicated schema literals removed.validateOrDefault, and the full precedence chain inAgentConfigurationService