feat(agent): add startTurn functionality and related validation#314899
Draft
DonJayamanne wants to merge 3 commits intomainfrom
Draft
feat(agent): add startTurn functionality and related validation#314899DonJayamanne wants to merge 3 commits intomainfrom
DonJayamanne wants to merge 3 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a rejectable startTurn command to the Agent Host protocol and threads it through client/server implementations, while enhancing session config handling to support server-pushed schema updates and post-mutation re-resolution.
Changes:
- Introduces
startTurnacross protocol, services, and agent implementations, plus logging and mocks. - Extends
SessionConfigChangedto optionally carry schema updates; updates reducers/providers to handle schema-only pushes and atomic schema+values updates. - Adds/updates protocol integration tests and helpers to validate
startTurnsuccess + error paths and config/schema race scenarios.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminal/test/browser/agentHostPty.test.ts | Updates test mock connection to implement startTurn. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/loggingAgentConnection.ts | Adds logging wrapper for startTurn on agent connections. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts | Adds TODO/commentary to migrate legacy action dispatch to startTurn. |
| src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts | Updates session config cache handling to accept schema pushes and optional config payloads. |
| src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts | Extends mock agent service for new startTurn method. |
| src/vs/platform/agentHost/test/node/protocol/testHelpers.ts | Enhances JSON-RPC test client to surface error codes/data via JsonRpcCallError. |
| src/vs/platform/agentHost/test/node/protocol/startTurn.integrationTest.ts | New integration test suite covering startTurn success + rejection cases. |
| src/vs/platform/agentHost/test/node/protocol/sessionConfig.integrationTest.ts | Updates expectations around mock provider’s config normalization behavior. |
| src/vs/platform/agentHost/test/node/mockAgent.ts | Adds startTurn to mock agents; extends resolver schema with a test-only required-key hook. |
| src/vs/platform/agentHost/node/protocolServerHandler.ts | Adds JSON-RPC command handler for startTurn forwarding into IAgentService. |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Implements startTurn on Copilot agent by delegating to sendMessage. |
| src/vs/platform/agentHost/node/claude/claudeAgent.ts | Implements startTurn on Claude agent by delegating to sendMessage. |
| src/vs/platform/agentHost/node/agentSideEffects.ts | Triggers server-side re-resolve/push of session config schema/values after config mutations. |
| src/vs/platform/agentHost/node/agentService.ts | Implements IAgentService.startTurn with config validation + turn-in-progress/session-not-found errors. |
| src/vs/platform/agentHost/electron-browser/agentHostService.ts | Adds startTurn to the Electron client proxy (IAgentHostService). |
| src/vs/platform/agentHost/common/state/protocol/reducers.ts | Updates reducer to handle schema-only and schema+values SessionConfigChanged shapes. |
| src/vs/platform/agentHost/common/state/protocol/messages.ts | Adds startTurn to the generated command map. |
| src/vs/platform/agentHost/common/state/protocol/commands.ts | Adds startTurn command types/docs; deprecates resolveSessionConfig in favor of schema-push. |
| src/vs/platform/agentHost/common/state/protocol/actions.ts | Deprecates client-dispatch SessionTurnStarted; extends SessionConfigChanged to carry optional schema. |
| src/vs/platform/agentHost/common/state/protocol/.ahp-version | Bumps synced protocol version hash. |
| src/vs/platform/agentHost/common/agentService.ts | Adds IAgentStartTurnParams and plumbs startTurn through service/connection interfaces. |
| src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts | Adds remote startTurn request implementation (currently drops attachments). |
| src/vs/platform/agentHost/browser/nullAgentHostService.ts | Adds startTurn stub to null service implementation. |
Copilot's findings
- Files reviewed: 23/23 changed files
- Comments generated: 5
Comment on lines
+549
to
+553
| // Prefer the provider's `startTurn` when implemented. The provider is | ||
| // responsible for emitting `SessionTurnStartedAction` exactly once | ||
| // (typically by dispatching it), so the service does not emit again | ||
| // on this path. | ||
| if (provider.startTurn) { |
Comment on lines
+448
to
+450
| * Optional. When implemented, the agent provider is responsible for | ||
| * validating the current session config against its schema and rejecting | ||
| * (by throwing) when required keys are missing. The |
Comment on lines
+588
to
+596
| // The wire-form `userMessage.attachments` use the protocol's | ||
| // `MessageAttachment` shape, whereas `IAgentStartTurnParams` | ||
| // expects `IAgentAttachment` (URI-typed). Pass the wire form | ||
| // through verbatim — the existing `dispatchAction` path stores | ||
| // it as-is, and side-effect consumers tolerate the looser shape. | ||
| await this._agentService.startTurn({ | ||
| session: URI.parse(params.session), | ||
| turnId: params.turnId, | ||
| userMessage: params.userMessage as unknown as { text: string }, |
Comment on lines
+251
to
+256
| // TODO: convert IAgentAttachment[] -> MessageAttachment[] (the | ||
| // wire-side attachment shape requires a `label` field and uses | ||
| // a tagged union the in-process IAgentAttachment does not yet | ||
| // match). Drop attachments on the wire for the initial | ||
| // iteration; the integration test agent will surface gaps. | ||
| attachments: undefined, |
Comment on lines
+843
to
+847
| // Re-resolve via the provider so dependent schema fields (e.g. | ||
| // `enum`/`readOnly` that depend on other values) and any | ||
| // server-resolved default values are pushed back to clients | ||
| // without each client having to call `resolveSessionConfig`. | ||
| void this._reresolveAndPushSessionConfig(action.session); |
- Remove queuedMessageId from IAgentStartTurnParams (queued turns are server-initiated, not client) - Remove @deprecated block from SessionTurnStartedAction (synced from protocol) - Sync protocol: StartTurnResult as empty object, StartTurnInvalidConfigErrorData without invalidValues - Update remoteAgentHostProtocolClient to not pass queuedMessageId Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DonJayamanne
force-pushed
the
don/copilot/agent-host-config-schema-update-15ec8272
branch
from
102f0cb to
1be7e0b
Compare
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.
startTurnmethod inIAgentHostServiceand implemented it inAgentHostServiceClient,AgentService, and agent classes (ClaudeAgent, CopilotAgent).startTurn, throwing appropriate errors for missing required fields.startTurninProtocolServerHandler, including error handling for session not found and turn already in progress scenarios.startTurn, covering success cases and various error scenarios, including missing required parameters and session not found.