agentHost: address agent merge review feedback - #331158
Merged
Benjamin Christopher Simmonds (benibenj) merged 3 commits intoAug 17, 2026
Merged
agentHost: address agent merge review feedback#331158Benjamin Christopher Simmonds (benibenj) merged 3 commits into
Benjamin Christopher Simmonds (benibenj) merged 3 commits into
Conversation
Follow-ups to PR #331010 review comments: - Rename settings from `chat.agentHost.agentMerge.*` to `chat.agentMerge.*`. - Reject client writes to host-owned `agentMerge.controller` session config so a forged controller state cannot drive a native merge. - Reconcile injected autonomy configuration every cycle and roll back keys that policy later revokes; never widen configuration while a turn is active. - Revalidate the merge target before starting a turn and before merging, and refresh live state, config and top-level comments inside the merge step. - Split `AgentMergeRepairAction` out of `AgentMergeAction` so repair paths cannot express a merge. - Carry all feedback comments per review thread and bound the prompt with explicit caps and an aggregate character budget. - Fail closed when fork head provenance is missing, and ignore refs from a different GitHub host. - Cancel the controller turn when the runtime stops. - Split the enable/disable command preconditions with a dedicated context key and make the configure quick pick's reset a title button. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Benjamin Christopher Simmonds (benibenj)
August 17, 2026 07:57
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens Agent Merge security, correctness, configuration handling, and command UX following the initial controller rollout.
Changes:
- Protects host-owned merge state and revalidates merge authorization.
- Reconciles autonomy policy, cancels stopped repair turns, and bounds prompts.
- Renames settings and improves Agent Merge commands.
Show a summary per file
| File | Description |
|---|---|
agentMergeActions.ts |
Adds command state and reset button. |
AGENT_HOST_SESSIONS_PROVIDER.md |
Updates Agent Merge documentation. |
agentService.test.ts |
Tests client configuration authorization. |
agentMergeController.test.ts |
Tests autonomy-policy reconciliation. |
agentMerge.test.ts |
Tests feedback collection and bounds. |
agentService.ts |
Guards host state and supports cancellation. |
agentMergeController.ts |
Revalidates targets, merges, and policy. |
agentMerge.ts |
Renames settings and bounds prompt context. |
agentHostStarter.config.contribution.ts |
Registers renamed settings. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 9/9 changed files
- Comments generated: 4
- Review effort level: Balanced
…-merge-review-followups # Conflicts: # src/vs/platform/agentHost/node/agentService.ts
Benjamin Christopher Simmonds (benibenj)
marked this pull request as ready for review
August 17, 2026 08:12
Benjamin Christopher Simmonds (benibenj)
enabled auto-merge (squash)
August 17, 2026 08:12
- Preserve host-written session config across a client `SessionConfigChanged` with `replace: true`. Omitting `agentMerge.controller` previously cleared the bound target, comment watermark and attempt budgets, which bypassed the authorization boundary that explicit-write rejection was meant to enforce. - Canonicalize pull request web hosts to their API host when checking that the credential matches. GitHub Enterprise Cloud serves `tenant.ghe.com` from `api.tenant.ghe.com`, so comparing the web host rejected every GHE Cloud pull request. The derivation now reuses `deriveGitHubEndpoints`. - Refresh top-level comments inside `prepareMerge`, last, before the snapshot and token are captured. Refreshing them in the controller beforehand left a window across `prepareMerge`'s own authoritative refreshes in which a new maintainer comment could be missed; refreshing afterwards would invalidate the preparation generation. A comment landing after capture now invalidates the preparation. - Migrate the legacy `chat.agentHost.agentMerge.*` setting ids to their new `chat.agentMerge.*` names so an explicit opt-out is not silently discarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ulugbek Abdullaev (ulugbekna)
approved these changes
Aug 17, 2026
Benjamin Christopher Simmonds (benibenj)
merged commit Aug 17, 2026
ea32f80
into
main
44 of 45 checks passed
Benjamin Christopher Simmonds (benibenj)
deleted the
benibenj/agents/agent-merge-review-followups
branch
August 17, 2026 11:56
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.
Follow-up to #331010, addressing the review comments left on that PR after it was merged.
Setting rename
The Agent Merge settings no longer carry
agentHostin their ID:chat.agentHost.agentMerge.*->chat.agentMerge.*Security
agentMerge.controllersession config.SessionConfigChangedactions coming from a client that target host-written keys are rejected, so a client cannot forge a controller state (target, enabledAt, watermark, budgets) that later makes the gate returnconfirmed: trueand triggers a native merge. Covered by two regression tests._canRepairForkfails closed when head repository provenance is missing.Correctness
AgentMergeRepairActionis split out ofAgentMergeActionso repair paths cannot express a merge.Prompt bounding
Review threads now carry all of their feedback comments rather than only the first, and the prompt is bounded by explicit per-section caps plus a 20k character aggregate budget.
Commands
Enable/disable commands have separate preconditions driven by a dedicated context key, and the configure quick pick exposes reset as a title button rather than a list entry.
Validation
npm run typecheck-clientnpm run eslintnpm run valid-layers-check