diff --git a/.github/skills/component-fixtures/SKILL.md b/.github/skills/component-fixtures/SKILL.md index 5672a0a58110bb..53aa7f96a25a52 100644 --- a/.github/skills/component-fixtures/SKILL.md +++ b/.github/skills/component-fixtures/SKILL.md @@ -61,6 +61,26 @@ Key points: - Always register created widgets with `disposableStore.add(...)` to prevent leaks - Pass `colorTheme: theme` to `createEditorServices` so theme colors render correctly +### File icon themes + +Fixtures use Seti file icons by default. Select another built-in theme, or disable file icons, on the individual fixture: + +```typescript +defineComponentFixture({ fileIconTheme: 'vs-minimal', render: renderMyComponent }); +defineComponentFixture({ fileIconTheme: 'none', render: renderMyComponent }); +``` + +When the rendered component reads `IThemeService`, pass the selected theme from `ComponentFixtureContext` to `createEditorServices`: + +```typescript +function renderMyComponent({ disposableStore, theme, fileIconTheme }: ComponentFixtureContext): void { + const instantiationService = createEditorServices(disposableStore, { + colorTheme: theme, + fileIconTheme, + }); +} +``` + ## Utilities from fixtureUtils.ts | Export | Purpose | diff --git a/build/agent-sdk/agents/claude/package-lock.json b/build/agent-sdk/agents/claude/package-lock.json index 015e6be556186d..ad04e126c4c1b7 100644 --- a/build/agent-sdk/agents/claude/package-lock.json +++ b/build/agent-sdk/agents/claude/package-lock.json @@ -182,9 +182,9 @@ } }, "node_modules/@hono/node-server": { - "version": "1.19.14", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.14.tgz", - "integrity": "sha512-GwtvgtXxnWsucXvbQXkRgqksiH2Qed37H9xHZocE5sA3N8O8O8/8FA3uclQXxXVzc9XBZuEOMK7+r02FmSpHtw==", + "version": "1.19.17", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.17.tgz", + "integrity": "sha512-dSneS5qhiauZWGDCeK4o695Xd9nUNjviSZCMQrj10eetr8Uln1ucn6bbphOM6UynAMMtNIzZNSpL9vnASJwrPQ==", "license": "MIT", "peer": true, "engines": { @@ -292,21 +292,21 @@ } }, "node_modules/body-parser": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", - "integrity": "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.3.0.tgz", + "integrity": "sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==", "license": "MIT", "peer": true, "dependencies": { "bytes": "^3.1.2", - "content-type": "^1.0.5", + "content-type": "^2.0.0", "debug": "^4.4.3", - "http-errors": "^2.0.0", - "iconv-lite": "^0.7.0", + "http-errors": "^2.0.1", + "iconv-lite": "^0.7.2", "on-finished": "^2.4.1", - "qs": "^6.14.1", - "raw-body": "^3.0.1", - "type-is": "^2.0.1" + "qs": "^6.15.2", + "raw-body": "^3.0.2", + "type-is": "^2.1.0" }, "engines": { "node": ">=18" @@ -316,6 +316,20 @@ "url": "https://opencollective.com/express" } }, + "node_modules/body-parser/node_modules/content-type": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.1.0.tgz", + "integrity": "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==", + "license": "MIT", + "peer": true, + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/bytes": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", @@ -645,9 +659,9 @@ "peer": true }, "node_modules/fast-uri": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", - "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "version": "3.1.6", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.6.tgz", + "integrity": "sha512-7Ical1vFEMr0onbVzEDIreM22I4khW+fzyQPwvAFWBp1iwdshSZRsL4jjRvPG9JP1uiqMHRto+YU6R2/CzDz5Q==", "funding": [ { "type": "github", @@ -792,9 +806,9 @@ } }, "node_modules/hono": { - "version": "4.12.25", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.25.tgz", - "integrity": "sha512-2NFaIyNVgJmBs/ecmtGzlmluTFs5cHEWGTdu0t1HBwYzoGXOL5nUQBRMXsXWla5i4KkG//QMzVP88m1+I3fdAQ==", + "version": "4.13.4", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.4.tgz", + "integrity": "sha512-AGEwKIyRMHRv1t8Wjwa3LHxQ61X5CqrdFT+4BRNTpqS5aJNnpl5WLjADb7vFlJzI/8uK7T5QLVApCMQKNa3LgQ==", "license": "MIT", "peer": true, "engines": { @@ -847,9 +861,9 @@ "peer": true }, "node_modules/ip-address": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", - "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", + "version": "10.5.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.5.0.tgz", + "integrity": "sha512-R5SnVLJmgYYvf2F2ZgwSBnelz5G4q5AxIC277GDfUaNbrZKNANcBC7RHqYYePlszf4kBolVkJauG0ZjHHFh55g==", "license": "MIT", "peer": true, "engines": { diff --git a/build/azure-pipelines/dependencies-check.yml b/build/azure-pipelines/dependencies-check.yml index 2c81602695bb9b..af659e714bdc77 100644 --- a/build/azure-pipelines/dependencies-check.yml +++ b/build/azure-pipelines/dependencies-check.yml @@ -22,10 +22,13 @@ jobs: variables: VSCODE_ARCH: x64 steps: - - task: NodeTool@0 + - script: | + echo "##vso[task.setvariable variable=NODE_VERSION]$(cat .nvmrc)" + displayName: Read Node version from .nvmrc + + - task: UseNode@1 inputs: - versionSource: fromFile - versionFilePath: .nvmrc + version: $(NODE_VERSION) - task: AzureKeyVault@2 displayName: "Azure Key Vault: Get GitHub token" @@ -84,6 +87,30 @@ jobs: - script: | set -e + # Check if any build subfolders with package-lock.json were modified + git fetch origin main + CHANGED_FILES=$(git diff --name-only origin/main...HEAD) + MODIFIED_BUILD_FOLDERS="" + + # Check if build/package-lock.json itself was modified + if echo "$CHANGED_FILES" | grep -q "^build/package-lock\.json$"; then + MODIFIED_BUILD_FOLDERS="build" + echo "build/package-lock.json was modified" + fi + + # Find all modified package-lock.json files under build subfolders and extract their directories + for file in $CHANGED_FILES; do + if [[ $file =~ ^(build/.+)/package-lock\.json$ ]]; then + dir="${BASH_REMATCH[1]}" + if [[ ! " $MODIFIED_BUILD_FOLDERS " =~ " $dir " ]]; then + MODIFIED_BUILD_FOLDERS="$MODIFIED_BUILD_FOLDERS $dir" + echo "$dir was modified" + fi + fi + done + + echo "##vso[task.setvariable variable=MODIFIED_BUILD_FOLDERS]$MODIFIED_BUILD_FOLDERS" + for attempt in {1..120}; do if [ $attempt -gt 1 ]; then echo "Attempt $attempt: Waiting for 10 minutes before retrying..." @@ -92,6 +119,15 @@ jobs: echo "Attempt $attempt: Running npm ci" if npm i --ignore-scripts; then + # Rebuild each modified build subfolder separately + for folder in $MODIFIED_BUILD_FOLDERS; do + echo "Rebuilding modified $folder..." + if ! (cd "$folder" && npm i --ignore-scripts); then + echo "npm i failed for $folder on attempt $attempt" + continue 2 + fi + done + if node build/npm/postinstall.ts; then echo "npm i succeeded on attempt $attempt" exit 0 diff --git a/build/azure-pipelines/product-build-ado-ci.yml b/build/azure-pipelines/product-build-ado-ci.yml index b2c62d5a7154de..5fcf81dbe211c4 100644 --- a/build/azure-pipelines/product-build-ado-ci.yml +++ b/build/azure-pipelines/product-build-ado-ci.yml @@ -260,57 +260,63 @@ extends: - script: | set -euo pipefail - if [ "$(Build.Reason)" = "PullRequest" ]; then - # Azure normally checks out a synthetic merge commit for PR validation. Its first - # parent is the target and its second parent is the PR head. Fall back to finding - # the merge base when the provider checks out the PR head directly. - parent_count=$(git rev-list --parents -n 1 HEAD | awk '{ print NF - 1 }') - if [ "$parent_count" = "2" ]; then - base_sha=$(git rev-parse HEAD^1) - head_sha=$(git rev-parse HEAD^2) - else - target_branch="$(System.PullRequest.TargetBranch)" - target_branch="${target_branch#refs/heads/}" - git fetch origin "$target_branch:refs/remotes/origin/$target_branch" - head_sha=$(git rev-parse HEAD) - base_sha=$(git merge-base "origin/$target_branch" "$head_sha") - fi + # Only validate trailers for PR builds + if [ "$(Build.Reason)" != "PullRequest" ]; then + echo "Skipping trailer check for non-PR builds" + exit 0 + fi + + # Azure normally checks out a synthetic merge commit for PR validation. Its first + # parent is the target and its second parent is the PR head. Fall back to finding + # the merge base when the provider checks out the PR head directly. + parent_count=$(git rev-list --parents -n 1 HEAD | awk '{ print NF - 1 }') + if [ "$parent_count" = "2" ]; then + base_sha=$(git rev-parse HEAD^1) + head_sha=$(git rev-parse HEAD^2) else - # Manual builds validate the selected branch's latest commit. + target_branch="$(System.PullRequest.TargetBranch)" + target_branch="${target_branch#refs/heads/}" + git fetch origin "$target_branch:refs/remotes/origin/$target_branch" head_sha=$(git rev-parse HEAD) - base_sha=$(git rev-parse HEAD^1) + base_sha=$(git merge-base "origin/$target_branch" "$head_sha") fi - # MSRC pull requests must be squashed to one commit before validation succeeds. - commit_count=$(git rev-list --count "$base_sha..$head_sha") - if [ "$commit_count" != "1" ]; then - echo "##vso[task.logissue type=error]This PR has $commit_count commits. release/msrc/* PRs must contain exactly one commit. Please squash your commits." - exit 1 - fi - echo "PR has a single commit." + # Check each commit in the PR for the Msrc-Case-Id trailer + commits=$(git rev-list "$base_sha..$head_sha") + commits_with_trailer=0 + total_commits=0 - # Extract every Msrc-Case-Id trailer value from the PR head commit. - mapfile -t trailer_values < <( - git log -1 --pretty='format:%(trailers:key=Msrc-Case-Id,valueonly=true)' "$head_sha" | - sed '/^[[:space:]]*$/d' - ) + for commit in $commits; do + total_commits=$((total_commits + 1)) - if [ "${#trailer_values[@]}" = "0" ]; then - echo "##vso[task.logissue type=error]Commit $head_sha is missing the required 'Msrc-Case-Id' trailer." - printf "Add a trailer to the commit message, for example:\n\n Msrc-Case-Id: 12345\n\nIf there is no associated case ID, use N/A:\n\n Msrc-Case-Id: N/A\n\n" - exit 1 - fi + # Extract Msrc-Case-Id trailer values from this commit + mapfile -t trailer_values < <( + git log -1 --pretty='format:%(trailers:key=Msrc-Case-Id,valueonly=true)' "$commit" | + sed '/^[[:space:]]*$/d' + ) + + if [ "${#trailer_values[@]}" -gt 0 ]; then + commits_with_trailer=$((commits_with_trailer + 1)) - # Every supplied case ID must be numeric or explicitly marked not applicable. - for value in "${trailer_values[@]}"; do - if [[ ! "$value" =~ ^[0-9]+$ && "$value" != "N/A" ]]; then - echo "##vso[task.logissue type=error]Commit $head_sha has an invalid 'Msrc-Case-Id' trailer value. Expected a number or N/A." - printf "Use a numeric case id or N/A, for example:\n\n Msrc-Case-Id: 12345\n\n" - exit 1 + # Validate all trailer values are numeric or N/A + for value in "${trailer_values[@]}"; do + if [[ ! "$value" =~ ^[0-9]+$ && "$value" != "N/A" ]]; then + echo "##vso[task.logissue type=error]Commit $commit has an invalid 'Msrc-Case-Id' trailer value. Expected a number or N/A." + printf "Use a numeric case id or N/A, for example:\n\n Msrc-Case-Id: 12345\n\n" + exit 1 + fi + done + + echo "Commit $commit has the following 'Msrc-Case-Id' trailers: ${trailer_values[*]}" fi done - echo "Commit $head_sha has the required 'Msrc-Case-Id' trailer." + # At least one commit must have the Msrc-Case-Id trailer + if [ "$commits_with_trailer" -lt 1 ]; then + echo "##vso[task.logissue type=error]Expected at least one commit with 'Msrc-Case-Id' trailer, but found $commits_with_trailer out of $total_commits commits with a trailer." + printf "Add a trailer to at least one commit message, for example:\n\n Msrc-Case-Id: 12345\n\nIf there is no associated case ID, use N/A:\n\n Msrc-Case-Id: N/A\n\n" + exit 1 + fi displayName: Verify Commit and Trailer - stage: Quality diff --git a/build/npm/gyp/package-lock.json b/build/npm/gyp/package-lock.json index 887285c3d96c8b..828fa31cf4a071 100644 --- a/build/npm/gyp/package-lock.json +++ b/build/npm/gyp/package-lock.json @@ -138,9 +138,9 @@ "license": "MIT" }, "node_modules/brace-expansion": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.2.tgz", - "integrity": "sha512-w5JZcKgdhDOgOwm8H+KgbosopHMuGcl6qbulwjtz3SM7I7P3yW1eAjzMPLrIE+NQ9vjgANKHWeMHnrT0OXW1oA==", + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", + "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", "dev": true, "license": "MIT", "dependencies": { @@ -439,15 +439,11 @@ } }, "node_modules/ip-address": { - "version": "9.0.5", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-9.0.5.tgz", - "integrity": "sha512-zHtQzGojZXTwZTHQqra+ETKd4Sn3vgi7uBmlPoXVWZqYvuKmtI0l/VZTjqGmJY9x88GGOaZ9+G9ES8hC4T4X8g==", + "version": "10.5.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.5.0.tgz", + "integrity": "sha512-R5SnVLJmgYYvf2F2ZgwSBnelz5G4q5AxIC277GDfUaNbrZKNANcBC7RHqYYePlszf4kBolVkJauG0ZjHHFh55g==", "dev": true, "license": "MIT", - "dependencies": { - "jsbn": "1.1.0", - "sprintf-js": "^1.1.3" - }, "engines": { "node": ">= 12" } @@ -488,13 +484,6 @@ "@pkgjs/parseargs": "^0.11.0" } }, - "node_modules/jsbn": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/jsbn/-/jsbn-1.1.0.tgz", - "integrity": "sha512-4bYVV3aAMtDTTu4+xsDYa6sy9GyJ69/amsu9sYF2zqjiEoZA5xJi3BrfX3uY+/IekIu7MwdObdbDWpoZdBv3/A==", - "dev": true, - "license": "MIT" - }, "node_modules/lru-cache": { "version": "10.4.3", "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-10.4.3.tgz", @@ -915,13 +904,13 @@ } }, "node_modules/socks": { - "version": "2.8.4", - "resolved": "https://registry.npmjs.org/socks/-/socks-2.8.4.tgz", - "integrity": "sha512-D3YaD0aRxR3mEcqnidIs7ReYJFVzWdd6fXJYUM8ixcQcJRGTka/b3saV0KflYhyVJXKhb947GndU35SxYNResQ==", + "version": "2.8.9", + "resolved": "https://registry.npmjs.org/socks/-/socks-2.8.9.tgz", + "integrity": "sha512-LJhUYUvItdQ0LkJTmPeaEObWXAqFyfmP85x0tch/ez9cahmhlBBLbIqDFnvBnUJGagb0JbIQrkBs1wJ+yRYpEw==", "dev": true, "license": "MIT", "dependencies": { - "ip-address": "^9.0.5", + "ip-address": "^10.1.1", "smart-buffer": "^4.2.0" }, "engines": { @@ -944,13 +933,6 @@ "node": ">= 14" } }, - "node_modules/sprintf-js": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.1.3.tgz", - "integrity": "sha512-Oo+0REFV59/rz3gfJNKQiBlwfHaSESl1pcGyABQsnnIfWOFt6JNj5gCog2U6MLZ//IGYD+nA8nI+mTShREReaA==", - "dev": true, - "license": "BSD-3-Clause" - }, "node_modules/ssri": { "version": "12.0.0", "resolved": "https://registry.npmjs.org/ssri/-/ssri-12.0.0.tgz", @@ -1069,9 +1051,9 @@ } }, "node_modules/tar": { - "version": "7.5.19", - "resolved": "https://registry.npmjs.org/tar/-/tar-7.5.19.tgz", - "integrity": "sha512-4LeEWl96twnS2Q7Bz4MGqgazLqO+hJN63GZxXoIqh1T3VweYD997gbU1ItNsQafqqXTXd5WFyFdReLtwvRBNiw==", + "version": "7.5.22", + "resolved": "https://registry.npmjs.org/tar/-/tar-7.5.22.tgz", + "integrity": "sha512-MFO/QzvtAOmJbkhOaCTvbGcFN9L9b+JunIsDwaKljSOdcLMea3NJ1k9Usz/rjdfSXTq4dfzfeS7W4p4YOAAHeA==", "dev": true, "license": "BlueOak-1.0.0", "dependencies": { diff --git a/build/rspack/rspack.serve-out.config.mts b/build/rspack/rspack.serve-out.config.mts index 760c9880d12214..58b6a7beacce27 100644 --- a/build/rspack/rspack.serve-out.config.mts +++ b/build/rspack/rspack.serve-out.config.mts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { type Configuration, HtmlRspackPlugin, rspack } from '@rspack/core'; +import { type Configuration, CopyRspackPlugin, HtmlRspackPlugin, rspack } from '@rspack/core'; import { ComponentExplorerPlugin } from '@vscode/component-explorer-webpack-plugin'; import fs from 'fs'; import net from 'net'; @@ -12,6 +12,10 @@ import { fileURLToPath } from 'url'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(__dirname, '../..'); const isStaticComponentExplorerBuild = process.env['COMPONENT_EXPLORER_STATIC_BUILD'] === '1'; +const builtInFileIconThemeDirectories = [ + 'extensions/theme-defaults/fileicons', + 'extensions/theme-seti/icons', +]; function findFreePort(startPort: number): Promise { return new Promise(resolve => { @@ -103,7 +107,7 @@ export default { }, }, { - // Built-in theme JSON files use JSONC (comments / trailing + // Built-in color theme JSON files use JSONC (comments / trailing // commas), so import them as raw strings and let VS Code's // JSON parser handle them. test: /[\\/]extensions[\\/]theme-defaults[\\/]themes[\\/].*\.json$/, @@ -112,6 +116,12 @@ export default { ], }, plugins: [ + ...(isStaticComponentExplorerBuild ? [new CopyRspackPlugin({ + patterns: builtInFileIconThemeDirectories.map(directory => ({ + from: path.join(repoRoot, directory), + to: directory, + })), + })] : []), new ComponentExplorerPlugin({ include: 'src/**/*.fixture.ts', }), diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/vscode-node/copilotCLICustomizationProvider.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/vscode-node/copilotCLICustomizationProvider.ts index f2fcb50255496d..b58c056c6fffe3 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/vscode-node/copilotCLICustomizationProvider.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/vscode-node/copilotCLICustomizationProvider.ts @@ -93,7 +93,8 @@ export class CopilotCLICustomizationProvider extends Disposable implements vscod folders.push({ uri: URI.joinPath(folder, ...root.path), label: root.path[0], - source: 'local' + source: 'local', + destinationGroupId: URI.joinPath(folder, root.path[0]).toString(), }); } } @@ -103,7 +104,8 @@ export class CopilotCLICustomizationProvider extends Disposable implements vscod folders.push({ uri: URI.joinPath(this.envService.userHome, ...root.path), label: `~/${root.path[0]}`, - source: 'user' + source: 'user', + destinationGroupId: URI.joinPath(this.envService.userHome, root.path[0]).toString(), }); } } diff --git a/src/vs/platform/agentHost/common/agentHostChangesetOperationService.ts b/src/vs/platform/agentHost/common/agentHostChangesetOperationService.ts index 78bb549841ebe6..ae9a30f2bc5604 100644 --- a/src/vs/platform/agentHost/common/agentHostChangesetOperationService.ts +++ b/src/vs/platform/agentHost/common/agentHostChangesetOperationService.ts @@ -23,6 +23,7 @@ export const AGENT_HOST_SYNC_CHANGESET_OPERATION_ID = 'sync'; */ export const AgentHostPullRequestOperationId = { MarkReady: 'pr-mark-ready', + MarkReadyWithAgentMerge: 'pr-mark-ready-with-agent-merge', Merge: 'pr-merge', EnableAutoMerge: 'pr-enable-auto-merge', DisableAutoMerge: 'pr-disable-auto-merge', diff --git a/src/vs/platform/agentHost/common/agentMerge.ts b/src/vs/platform/agentHost/common/agentMerge.ts index bea5719dbf86ba..bcb870a235ec85 100644 --- a/src/vs/platform/agentHost/common/agentMerge.ts +++ b/src/vs/platform/agentHost/common/agentMerge.ts @@ -260,48 +260,48 @@ export interface AgentMergeDisableReason { export const agentMergeDisableReasons = { sessionArchived: (): AgentMergeDisableReason => ({ log: 'the session was archived', - notice: localize('agentMerge.disabled.sessionArchived', "Agent Merge was turned off because this session was archived."), + notice: localize('agentMerge.disabled.sessionArchived', "Agent Merge was disabled because this session was archived."), }), branchChanged: (from: string, to: string): AgentMergeDisableReason => ({ log: `branch changed from ${from} to ${to}`, notice: localize( 'agentMerge.disabled.branchChanged', - "Agent Merge was turned off because the checked-out branch changed from {0} to {1}.", + "Agent Merge was disabled because the checked-out branch changed from {0} to {1}.", appendEscapedMarkdownInlineCode(from), appendEscapedMarkdownInlineCode(to) ), }), branchChangedWhileRefreshing: (): AgentMergeDisableReason => ({ log: 'the checked-out branch changed while pull request state was refreshing', - notice: localize('agentMerge.disabled.branchChangedWhileRefreshing', "Agent Merge was turned off because the checked-out branch changed while its pull request state was refreshing."), + notice: localize('agentMerge.disabled.branchChangedWhileRefreshing', "Agent Merge was disabled because the checked-out branch changed while its pull request state was refreshing."), }), differentPullRequest: (): AgentMergeDisableReason => ({ log: 'the session became associated with a different pull request', - notice: localize('agentMerge.disabled.differentPullRequest', "Agent Merge was turned off because this session became associated with a different pull request."), + notice: localize('agentMerge.disabled.differentPullRequest', "Agent Merge was disabled because this session became associated with a different pull request."), }), invalidPullRequestUrl: (): AgentMergeDisableReason => ({ log: 'the associated pull request URL is invalid', - notice: localize('agentMerge.disabled.invalidPullRequestUrl', "Agent Merge was turned off because the associated pull request URL is invalid."), + notice: localize('agentMerge.disabled.invalidPullRequestUrl', "Agent Merge was disabled because the associated pull request URL is invalid."), }), differentGitHubHost: (): AgentMergeDisableReason => ({ log: 'the bound pull request belongs to a different GitHub host than the signed-in account', - notice: localize('agentMerge.disabled.differentGitHubHost', "Agent Merge was turned off because its pull request belongs to a different GitHub host than the signed-in account."), + notice: localize('agentMerge.disabled.differentGitHubHost', "Agent Merge was disabled because its pull request belongs to a different GitHub host than the signed-in account."), }), indeterminate: (minutes: number, reason: string): AgentMergeDisableReason => ({ log: `the pull request state could not be evaluated for ${minutes} minutes: ${reason}`, - notice: localize('agentMerge.disabled.indeterminate', "Agent Merge was turned off because its pull request state could not be evaluated for {0} minutes.", minutes), + notice: localize('agentMerge.disabled.indeterminate', "Agent Merge was disabled because its pull request state could not be evaluated for {0} minutes.", minutes), }), pullRequestClosed: (): AgentMergeDisableReason => ({ log: 'the pull request is closed or merged', - notice: localize('agentMerge.disabled.pullRequestClosed', "Agent Merge was turned off because its pull request is closed or merged."), + notice: localize('agentMerge.disabled.pullRequestClosed', "Agent Merge was disabled because its pull request is closed or merged."), }), repairBudgetExhausted: (): AgentMergeDisableReason => ({ log: 'the same pull request blockers remained after repeated repair attempts', - notice: localize('agentMerge.disabled.repairBudgetExhausted', "Agent Merge was turned off because the same pull request blockers remained after repeated repair attempts."), + notice: localize('agentMerge.disabled.repairBudgetExhausted', "Agent Merge was disabled because the same pull request blockers remained after repeated repair attempts."), }), - pullRequestMerged: (): AgentMergeDisableReason => ({ + pullRequestMerged: (pullRequestNumber: number, pullRequestUrl: string): AgentMergeDisableReason => ({ log: 'the pull request was merged', - notice: localize('agentMerge.disabled.pullRequestMerged', "Agent Merge merged its pull request and turned itself off."), + notice: localize('agentMerge.pullRequestMerged', "Agent Merge merged pull request [#{0}]({1}).", pullRequestNumber, pullRequestUrl), }), } as const; @@ -309,8 +309,8 @@ export const agentMergeDisableReasons = { export function agentMergeEnabledNotice(target: Pick, configuration: AgentMergeConfiguration): string { const lines = [ target.pullRequestUrl - ? localize('agentMerge.notice.enabled.withPullRequest', "Agent Merge is on for {0} and is monitoring its pull request.", appendEscapedMarkdownInlineCode(target.branchName)) - : localize('agentMerge.notice.enabled', "Agent Merge is on for {0}. It will wait for a pull request on this branch, then monitor it.", appendEscapedMarkdownInlineCode(target.branchName)), + ? localize('agentMerge.notice.enabled.withPullRequest', "Agent Merge is enabled for {0} and is monitoring its pull request.", appendEscapedMarkdownInlineCode(target.branchName)) + : localize('agentMerge.notice.enabled', "Agent Merge is enabled for {0}. It will wait for a pull request on this branch, then monitor it.", appendEscapedMarkdownInlineCode(target.branchName)), ]; if (configuration.addressReviews) { lines.push(localize('agentMerge.notice.enabled.addressReviews', "It will ask the agent to address new pull request review comments.")); @@ -424,9 +424,9 @@ function agentMergeMergeBehaviorChangedNotice(mergePullRequest: AgentMergeMergeP } } -/** The transcript notice shown when the user, rather than the controller, turns Agent Merge off. */ +/** The transcript notice shown when the user, rather than the controller, disables Agent Merge. */ export function agentMergeDisabledNotice(): string { - return localize('agentMerge.notice.disabled', "Agent Merge was turned off for this session."); + return localize('agentMerge.notice.disabled', "Agent Merge was disabled for this session."); } /** @@ -434,7 +434,7 @@ export function agentMergeDisabledNotice(): string { * because its own repair work changed the pull request. */ export function agentMergeMergePullRequestDemotedNotice(): string { - return localize('agentMerge.notice.mergeDemoted', "Agent Merge changed this pull request, so automatic merging was turned off for this session. Review the changes, then turn it back on if you want it merged automatically."); + return localize('agentMerge.notice.mergeDemoted', "Agent Merge changed this pull request, so automatic merging was disabled for this session. Review the changes, then enable it again if you want it merged automatically."); } export function readAgentMergeSessionState(values: Record | undefined): AgentMergeSessionState | undefined { @@ -628,16 +628,7 @@ export function evaluateAgentMerge(snapshot: PullRequestSnapshot, configuration: return { kind: 'indeterminate', reason: checks.reason, cause: `checks:${checks.reason}` }; } - const reviewThreads = snapshot.reviewThreads.value! - .filter(thread => !thread.isResolved && thread.comments.some(comment => isAgentMergeFeedbackAuthor(comment.author))); - const latestReviews = latestReviewsByAuthor(snapshot.submittedReviews.value!); - const changesRequested = latestReviews.filter(review => review.state.toUpperCase() === 'CHANGES_REQUESTED' && isAgentMergeFeedbackAuthor(review.author)); - const watermark = Date.parse(commentWatermark); - const newComments = snapshot.topLevelComments.value!.filter(comment => - isAgentMergeFeedbackAuthor(comment.author) - && comment.createdAt !== undefined - && Date.parse(comment.createdAt) > watermark - ); + const { reviewThreads, changesRequested, newComments } = getAgentMergeFeedback(snapshot, commentWatermark); const mergeability = snapshot.mergeability.value!; const behind = mergeability.mergeStateStatus?.toUpperCase() === 'BEHIND'; const conflicting = mergeability.mergeable === 'CONFLICTING'; @@ -762,6 +753,49 @@ function latestReviewsByAuthor(reviews: PullRequestSnapshot['submittedReviews'][ return [...latest.values()]; } +function getAgentMergeFeedback(snapshot: PullRequestSnapshot, commentWatermark: string) { + const reviewThreads = snapshot.reviewThreads.value! + .filter(thread => !thread.isResolved && thread.comments.some(comment => isAgentMergeFeedbackAuthor(comment.author))); + const latestReviews = latestReviewsByAuthor(snapshot.submittedReviews.value!); + const changesRequested = latestReviews.filter(review => review.state.toUpperCase() === 'CHANGES_REQUESTED' && isAgentMergeFeedbackAuthor(review.author)); + const watermark = Date.parse(commentWatermark); + const newComments = snapshot.topLevelComments.value!.filter(comment => + isAgentMergeFeedbackAuthor(comment.author) + && comment.createdAt !== undefined + && Date.parse(comment.createdAt) > watermark + ); + return { reviewThreads, changesRequested, newComments }; +} + +/** + * Returns whether a draft pull request has no pending or failed required checks + * and no actionable review feedback, or `undefined` while that state is incomplete. + */ +export function isAgentMergePullRequestReadyForReview(snapshot: PullRequestSnapshot, commentWatermark: string): boolean | undefined { + const core = snapshot.core; + if (core.status !== 'ready' || !core.complete || !core.value || core.value.state !== 'open' || !core.value.draft) { + return undefined; + } + for (const fragment of conversationFragments) { + if (!isCompleteFragment(snapshot, fragment)) { + return undefined; + } + } + if (!isCompleteHeadFragment(snapshot, 'checks', core.value.headSha)) { + return undefined; + } + const checks = classifyAgentMergeRequiredChecks(snapshot.checks.value!); + if (checks.kind === 'indeterminate') { + return undefined; + } + const { reviewThreads, changesRequested, newComments } = getAgentMergeFeedback(snapshot, commentWatermark); + return checks.failed.length === 0 + && !checks.pending + && reviewThreads.length === 0 + && changesRequested.length === 0 + && newComments.length === 0; +} + export function classifyAgentMergeRequiredChecks(checks: PullRequestChecks): AgentMergeRequiredChecks { if (!checks.requirednessComplete) { return { kind: 'indeterminate', reason: 'Required check classification is incomplete' }; diff --git a/src/vs/platform/agentHost/common/meta/agentSystemNotificationMeta.ts b/src/vs/platform/agentHost/common/meta/agentSystemNotificationMeta.ts index a96df3fe4b87fc..daf53e0da977ec 100644 --- a/src/vs/platform/agentHost/common/meta/agentSystemNotificationMeta.ts +++ b/src/vs/platform/agentHost/common/meta/agentSystemNotificationMeta.ts @@ -5,12 +5,20 @@ export const enum AgentSystemNotificationKind { WorktreeCreationFailure = 'worktreeCreationFailure', + /** An automatic approval review did not finish before its deadline. */ + AutomaticApprovalReviewTimedOut = 'automaticApprovalReviewTimedOut', + /** An automatic approval review stopped before reaching a decision. */ + AutomaticApprovalReviewAborted = 'automaticApprovalReviewAborted', + /** Automatic approval review denials triggered the turn circuit breaker. */ + AutomaticApprovalReviewInterrupted = 'automaticApprovalReviewInterrupted', /** Agent Merge started monitoring the session's branch. */ AgentMergeEnabled = 'agentMergeEnabled', /** Effective Agent Merge behavior changed while monitoring. */ AgentMergeConfigurationChanged = 'agentMergeConfigurationChanged', /** Agent Merge stopped monitoring the session, usually on its own. */ AgentMergeDisabled = 'agentMergeDisabled', + /** Agent Merge merged the pull request it was monitoring. */ + AgentMergePullRequestMerged = 'agentMergePullRequestMerged', } export const enum AgentSystemNotificationSeverity { @@ -19,9 +27,13 @@ export const enum AgentSystemNotificationSeverity { const knownKinds: ReadonlySet = new Set([ AgentSystemNotificationKind.WorktreeCreationFailure, + AgentSystemNotificationKind.AutomaticApprovalReviewTimedOut, + AgentSystemNotificationKind.AutomaticApprovalReviewAborted, + AgentSystemNotificationKind.AutomaticApprovalReviewInterrupted, AgentSystemNotificationKind.AgentMergeEnabled, AgentSystemNotificationKind.AgentMergeConfigurationChanged, AgentSystemNotificationKind.AgentMergeDisabled, + AgentSystemNotificationKind.AgentMergePullRequestMerged, ]); interface IHasSystemNotificationMeta { diff --git a/src/vs/platform/agentHost/node/agentHostPullRequestLifecycleOperationHandler.ts b/src/vs/platform/agentHost/node/agentHostPullRequestLifecycleOperationHandler.ts index c1602c25d3b51d..189f3377fbe9b6 100644 --- a/src/vs/platform/agentHost/node/agentHostPullRequestLifecycleOperationHandler.ts +++ b/src/vs/platform/agentHost/node/agentHostPullRequestLifecycleOperationHandler.ts @@ -38,6 +38,7 @@ export type PullRequestLifecycleAction = 'mark-ready' | 'merge' | 'enable-auto-m export class AgentHostPullRequestLifecycleOperationHandler implements IChangesetOperationHandler { public static readonly OPERATION_MARK_READY = AgentHostPullRequestOperationId.MarkReady; + public static readonly OPERATION_MARK_READY_WITH_AGENT_MERGE = AgentHostPullRequestOperationId.MarkReadyWithAgentMerge; public static readonly OPERATION_MERGE = AgentHostPullRequestOperationId.Merge; public static readonly OPERATION_ENABLE_AUTO_MERGE = AgentHostPullRequestOperationId.EnableAutoMerge; public static readonly OPERATION_DISABLE_AUTO_MERGE = AgentHostPullRequestOperationId.DisableAutoMerge; diff --git a/src/vs/platform/agentHost/node/agentHostPullRequestOperationHandler.ts b/src/vs/platform/agentHost/node/agentHostPullRequestOperationHandler.ts index 7685d9fea8c38a..9540d7a79e4e63 100644 --- a/src/vs/platform/agentHost/node/agentHostPullRequestOperationHandler.ts +++ b/src/vs/platform/agentHost/node/agentHostPullRequestOperationHandler.ts @@ -72,6 +72,7 @@ export class AgentHostPullRequestOperationHandler implements IChangesetOperation public static readonly OPERATION_CREATE_PR_AUTO_SQUASH = 'create-pr-auto-squash'; public static readonly OPERATION_CREATE_PR_AUTO_REBASE = 'create-pr-auto-rebase'; public static readonly OPERATION_CREATE_PR_AGENT_MERGE = 'create-pr-agent-merge'; + public static readonly OPERATION_CREATE_DRAFT_PR_AGENT_MERGE = 'create-draft-pr-agent-merge'; constructor( private readonly _draft: boolean, @@ -337,7 +338,9 @@ export class AgentHostPullRequestOperationHandler implements IChangesetOperation if (this._enableAgentMerge) { return isExisting ? localize('agentHost.changeset.pr.existing.agentMerge', "Pull request [#{0}]({1}) already exists; enabled Agent Merge.", pr.number, pr.url) - : localize('agentHost.changeset.pr.created.agentMerge', "Created pull request [#{0}]({1}) and enabled Agent Merge.", pr.number, pr.url); + : this._draft + ? localize('agentHost.changeset.pr.createdDraft.agentMerge', "Created draft pull request [#{0}]({1}) and enabled Agent Merge.", pr.number, pr.url) + : localize('agentHost.changeset.pr.created.agentMerge', "Created pull request [#{0}]({1}) and enabled Agent Merge.", pr.number, pr.url); } let mergeMethodLabel: string | undefined; diff --git a/src/vs/platform/agentHost/node/agentHostPullRequestOperationProvider.ts b/src/vs/platform/agentHost/node/agentHostPullRequestOperationProvider.ts index 8e4ee53a0880ff..20a2a86a7393e9 100644 --- a/src/vs/platform/agentHost/node/agentHostPullRequestOperationProvider.ts +++ b/src/vs/platform/agentHost/node/agentHostPullRequestOperationProvider.ts @@ -15,8 +15,9 @@ import { AgentHostPullRequestOperationHandler, type PullRequestCreatedEvent } fr import { AgentHostPullRequestLifecycleOperationHandler } from './agentHostPullRequestLifecycleOperationHandler.js'; import { IAgentHostPullRequestStatusService } from './agentHostPullRequestStatusService.js'; import { AgentHostStateManager, IAgentHostStateManager } from './agentHostStateManager.js'; -import { AgentMergeConfigKey, agentMergeRootConfigSchema } from '../common/agentMerge.js'; +import { AgentMergeConfigKey, agentMergeRootConfigSchema, readAgentMergeSessionState } from '../common/agentMerge.js'; import { IAgentConfigurationService } from './agentConfigurationService.js'; +import { ActionType } from '../common/state/sessionActions.js'; export class AgentHostPullRequestOperationContribution extends Disposable implements IChangesetOperationContribution { @@ -49,15 +50,18 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem const createAutoSquashPrHandler = this._instantiationService.createInstance(AgentHostPullRequestOperationHandler, false, 'SQUASH', false, getSessionState, resolveBaseBranchName, onCreated); const createAutoRebasePrHandler = this._instantiationService.createInstance(AgentHostPullRequestOperationHandler, false, 'REBASE', false, getSessionState, resolveBaseBranchName, onCreated); const createAgentMergePrHandler = this._instantiationService.createInstance(AgentHostPullRequestOperationHandler, false, undefined, true, getSessionState, resolveBaseBranchName, onCreated); + const createDraftAgentMergePrHandler = this._instantiationService.createInstance(AgentHostPullRequestOperationHandler, true, undefined, true, getSessionState, resolveBaseBranchName, onCreated); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR, createPrHandler)); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_DRAFT_PR, createDraftPrHandler)); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR_AUTO_MERGE, createAutoMergePrHandler)); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR_AUTO_SQUASH, createAutoSquashPrHandler)); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR_AUTO_REBASE, createAutoRebasePrHandler)); store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR_AGENT_MERGE, createAgentMergePrHandler)); + store.add(registry.registerChangesetOperationHandler(AgentHostPullRequestOperationHandler.OPERATION_CREATE_DRAFT_PR_AGENT_MERGE, createDraftAgentMergePrHandler)); for (const [operationId, action] of [ [AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY, 'mark-ready'], + [AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY_WITH_AGENT_MERGE, 'mark-ready'], [AgentHostPullRequestLifecycleOperationHandler.OPERATION_MERGE, 'merge'], [AgentHostPullRequestLifecycleOperationHandler.OPERATION_ENABLE_AUTO_MERGE, 'enable-auto-merge'], [AgentHostPullRequestLifecycleOperationHandler.OPERATION_DISABLE_AUTO_MERGE, 'disable-auto-merge'], @@ -66,6 +70,11 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem } store.add(this._pullRequestStatusService.onDidChangePullRequestStatus(sessionKey => registry.onDidChangeOperations(sessionKey))); + store.add(this._stateManager.onDidEmitEnvelope(envelope => { + if (envelope.action.type === ActionType.SessionConfigChanged) { + registry.onDidChangeOperations(envelope.channel); + } + })); let agentMergeEnabled = this._isAgentMergeEnabled(); store.add(this._configurationService.onDidRootConfigChange(() => { const nextAgentMergeEnabled = this._isAgentMergeEnabled(); @@ -112,6 +121,7 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem return undefined; } + const agentMergeEnabled = this._isAgentMergeEnabled(); return [{ id: 'create-pr', label: localize('agentHost.changeset.createPR', "Create PR"), @@ -144,9 +154,9 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem scopes: [ChangesetOperationScope.Changeset], status: ChangesetOperationStatus.Idle, }, - ...(this._isAgentMergeEnabled() ? [{ + ...(agentMergeEnabled ? [{ id: AgentHostPullRequestOperationHandler.OPERATION_CREATE_PR_AGENT_MERGE, - label: localize('agentHost.changeset.createPRAgentMerge', "Create PR & Enable Agent Merge"), + label: localize('agentHost.changeset.createPRAgentMerge', "Create PR & Agent Merge"), icon: 'git-merge', group: 'pull-request', scopes: [ChangesetOperationScope.Changeset], @@ -159,7 +169,16 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem group: 'pull-request_draft', scopes: [ChangesetOperationScope.Changeset], status: ChangesetOperationStatus.Idle, - }] satisfies ChangesetOperation[]; + }, + ...(agentMergeEnabled ? [{ + id: AgentHostPullRequestOperationHandler.OPERATION_CREATE_DRAFT_PR_AGENT_MERGE, + label: localize('agentHost.changeset.createDraftPRAgentMerge', "Create Draft PR & Agent Merge"), + icon: 'git-merge', + group: 'pull-request_draft', + scopes: [ChangesetOperationScope.Changeset], + status: ChangesetOperationStatus.Idle, + }] : []), + ] satisfies ChangesetOperation[]; } private _isAgentMergeEnabled(): boolean { @@ -193,8 +212,12 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem const operations: ChangesetOperation[] = []; if (status.draft) { + const agentMergeRunning = this._isAgentMergeRunning(sessionKey); + const operationId = agentMergeRunning && status.agentMergeReadyForReview !== true + ? AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY_WITH_AGENT_MERGE + : AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY; operations.push({ - id: AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY, + id: operationId, label: localize('agentHost.changeset.markReady', "Mark Ready"), description: localize('agentHost.changeset.markReady.description', "Take the pull request out of draft so it can be reviewed and merged."), icon: 'git-pull-request', @@ -243,6 +266,11 @@ export class AgentHostPullRequestOperationContribution extends Disposable implem return operations; } + private _isAgentMergeRunning(sessionKey: string): boolean { + return this._isAgentMergeEnabled() + && readAgentMergeSessionState(this._stateManager.getSessionState(sessionKey)?.config?.values)?.enabled === true; + } + /** * Logs the advertised operations whenever the set changes for a session. * `getOperations` is recomputed on every git/GitHub state change and once diff --git a/src/vs/platform/agentHost/node/agentHostPullRequestStatusService.ts b/src/vs/platform/agentHost/node/agentHostPullRequestStatusService.ts index e7a11c311b4c1a..b2f03010679d71 100644 --- a/src/vs/platform/agentHost/node/agentHostPullRequestStatusService.ts +++ b/src/vs/platform/agentHost/node/agentHostPullRequestStatusService.ts @@ -9,7 +9,7 @@ import { Disposable, DisposableStore, type IDisposable } from '../../../base/com import { autorun } from '../../../base/common/observable.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { ILogService } from '../../log/common/log.js'; -import type { PullRequestRef, PullRequestSnapshot, PullRequestSubscription } from '../../github/common/githubPullRequestService.js'; +import type { PullRequestRef, PullRequestSnapshot, PullRequestSubscription, PullRequestSubscriptionOptions } from '../../github/common/githubPullRequestService.js'; import type { GitHubCredentialInvalidation } from '../../github/common/githubCredentialService.js'; import type { GitHubAccountHandle } from '../../github/common/githubTypes.js'; import { IGitHubService } from '../../github/common/githubService.js'; @@ -19,6 +19,7 @@ import { getSessionRelatedPullRequestUrls, hasSessionPullRequestForBranch, isSes import { ActionType } from '../common/state/sessionActions.js'; import { AgentHostStateManager, IAgentHostStateManager } from './agentHostStateManager.js'; import { parsePullRequestUrl } from './agentMergeController.js'; +import { isAgentMergePullRequestReadyForReview, readAgentMergeSessionState } from '../common/agentMerge.js'; /** * Merge states GitHub reports for a pull request that can still be merged @@ -42,6 +43,8 @@ export interface IAgentHostPullRequestStatus { readonly draft: boolean; /** True once the pull request is open and can be merged as-is. */ readonly mergeReady: boolean; + /** Whether Agent Merge has observed all required checks and review feedback as ready. */ + readonly agentMergeReadyForReview?: boolean; readonly viewerCanEnableAutoMerge: boolean; readonly autoMergeEnabled: boolean; readonly allowedMergeMethods: readonly ('MERGE' | 'SQUASH' | 'REBASE')[]; @@ -55,10 +58,10 @@ export const IAgentHostPullRequestStatusService = createDecorator this._sync(session))); this._register(this._stateManager.onDidRemoveSession(session => this._stopWatch(session))); this._register(this._stateManager.onDidEmitEnvelope(envelope => { - if (envelope.action.type === ActionType.SessionIsArchivedChanged) { + if (envelope.action.type === ActionType.SessionIsArchivedChanged || envelope.action.type === ActionType.SessionConfigChanged) { this._sync(envelope.channel); } })); @@ -259,6 +262,8 @@ export class AgentHostPullRequestStatusService extends Disposable implements IAg const existing = this._watches.get(sessionKey); if (existing && sameRefAndHost(existing.ref, parsed)) { + existing.subscription.update(this._getSubscriptionOptions(sessionKey)); + this._updateStatus(sessionKey, existing, existing.subscription.resource.snapshot.get()); return; } @@ -288,11 +293,7 @@ export class AgentHostPullRequestStatusService extends Disposable implements IAg this._stopWatch(sessionKey, `replaced by ${describeRef(ref)}`); const store = new DisposableStore(); - const subscription = store.add(this._gitHubService.pullRequests.subscribePullRequest(ref, { - priority: 'visible', - core: true, - mergeability: true, - })); + const subscription = store.add(this._gitHubService.pullRequests.subscribePullRequest(ref, this._getSubscriptionOptions(sessionKey))); const watch: IWatch = { ref, subscription, @@ -313,6 +314,24 @@ export class AgentHostPullRequestStatusService extends Disposable implements IAg this._logService.debug(`[AgentHostPullRequestStatusService] Watching pull request: session=${sessionKey}, pr=${describeRef(ref)}`); } + private _getSubscriptionOptions(sessionKey: string): PullRequestSubscriptionOptions { + const agentMergeEnabled = readAgentMergeSessionState(this._stateManager.getSessionState(sessionKey)?.config?.values)?.enabled === true; + return { + priority: 'visible', + core: true, + mergeability: true, + ...(agentMergeEnabled ? { + conversation: { + topLevelComments: true, + submittedReviews: true, + reviewThreads: true, + includeBodies: true, + }, + checks: { required: true }, + } : {}), + }; + } + private _hasPersistedMergedState(sessionKey: string, ref: PullRequestRef): boolean { const gitHubState = readSessionGitHubState(this._stateManager.getSessionState(sessionKey)?._meta); const persistedPullRequest = gitHubState?.pullRequestStateUrl ? parsePullRequestUrl(gitHubState.pullRequestStateUrl) : undefined; @@ -371,7 +390,8 @@ export class AgentHostPullRequestStatusService extends Disposable implements IAg if (snapshot.core.status !== 'ready' && (watch.status?.state === 'merged' || persistedMergedStateApplies)) { return; } - this._setStatus(sessionKey, watch, toPullRequestStatus(snapshot)); + const agentMerge = readAgentMergeSessionState(this._stateManager.getSessionState(sessionKey)?.config?.values); + this._setStatus(sessionKey, watch, toPullRequestStatus(snapshot, agentMerge?.enabled ? agentMerge.target?.commentWatermark : undefined)); } private _setStatus(sessionKey: string, watch: IWatch, status: IAgentHostPullRequestStatus | undefined): void { @@ -429,6 +449,7 @@ function describeStatus(status: IAgentHostPullRequestStatus | undefined): string `state=${status.state}`, `draft=${status.draft}`, `mergeReady=${status.mergeReady}`, + `agentMergeReadyForReview=${status.agentMergeReadyForReview ?? 'unknown'}`, `autoMergeEnabled=${status.autoMergeEnabled}`, `canEnableAutoMerge=${status.viewerCanEnableAutoMerge}`, `allowedMergeMethods=${status.allowedMergeMethods.join('|') || 'none'}`, @@ -440,7 +461,7 @@ function describeStatus(status: IAgentHostPullRequestStatus | undefined): string * while either fragment the button bar depends on is still unresolved. Holding * back on partial data keeps the client from flashing a wrong primary button. */ -function toPullRequestStatus(snapshot: PullRequestSnapshot): IAgentHostPullRequestStatus | undefined { +function toPullRequestStatus(snapshot: PullRequestSnapshot, agentMergeCommentWatermark?: string): IAgentHostPullRequestStatus | undefined { const core = snapshot.core.value; if (!core) { return undefined; @@ -467,6 +488,9 @@ function toPullRequestStatus(snapshot: PullRequestSnapshot): IAgentHostPullReque if (!mergeability || snapshot.mergeability.headSha !== core.headSha) { return undefined; } + const agentMergeReadyForReview = agentMergeCommentWatermark !== undefined + ? isAgentMergePullRequestReadyForReview(snapshot, agentMergeCommentWatermark) + : undefined; return { ...(core.id ? { pullRequestId: core.id } : {}), @@ -479,6 +503,7 @@ function toPullRequestStatus(snapshot: PullRequestSnapshot): IAgentHostPullReque && mergeability.mergeable === 'MERGEABLE' && mergeability.viewerCanMerge && MERGEABLE_STATES.has(mergeability.mergeStateStatus?.toUpperCase() ?? 'CLEAN'), + ...(agentMergeReadyForReview !== undefined ? { agentMergeReadyForReview } : {}), viewerCanEnableAutoMerge: mergeability.viewerCanEnableAutoMerge, autoMergeEnabled: mergeability.autoMergeEnabled, allowedMergeMethods: mergeability.allowedMergeMethods, diff --git a/src/vs/platform/agentHost/node/agentMergeController.ts b/src/vs/platform/agentHost/node/agentMergeController.ts index 3545e22b34316b..8021c5efc0cacb 100644 --- a/src/vs/platform/agentHost/node/agentMergeController.ts +++ b/src/vs/platform/agentHost/node/agentMergeController.ts @@ -420,7 +420,11 @@ export class AgentMergeController extends Disposable { let target = agentMerge.target; if (!target) { const now = new Date().toISOString(); - target = { branchName, enabledAt: now, commentWatermark: now }; + const currentGitHubState = readSessionGitHubState(this._stateManager.getSessionState(session)?._meta); + const pullRequestUrl = currentGitHubState?.pullRequestBranchName === branchName + ? getSessionRelatedPullRequestUrls(currentGitHubState)[0] + : undefined; + target = { branchName, enabledAt: now, commentWatermark: now, ...(pullRequestUrl ? { pullRequestUrl } : {}) }; this._logService.info(`[AgentMergeController] Captured session branch and feedback watermark: session=${session}`); // Announce only on the first capture: a resumed session already has a // target, so restarting the host must not repeat the notice. @@ -811,7 +815,13 @@ export class AgentMergeController extends Disposable { } const result = await this._gitHubService.mutations.merge(preparation, { method, authorization }, runtime.abortController.signal); this._logService.info(`[AgentMergeController] Pull request merged natively: session=${session}, method=${method}, outcome=${result.outcome}`); - this._disable(session, currentState, agentMergeDisableReasons.pullRequestMerged()); + const mergedPullRequest = preparation.snapshot.core.value!; + this._disable( + session, + currentState, + agentMergeDisableReasons.pullRequestMerged(mergedPullRequest.number, mergedPullRequest.url), + AgentSystemNotificationKind.AgentMergePullRequestMerged, + ); } private async _completeTurn(session: string): Promise { @@ -946,14 +956,14 @@ export class AgentMergeController extends Disposable { }); } - private _disable(session: string, current: AgentMergeSessionState, reason: AgentMergeDisableReason): void { + private _disable(session: string, current: AgentMergeSessionState, reason: AgentMergeDisableReason, notificationKind = AgentSystemNotificationKind.AgentMergeDisabled): void { this._logService.info(`[AgentMergeController] Disabling Agent Merge for ${session}: ${reason.log}`); this._activeTurns.delete(session); // Claim the transition before the config write re-enters `_doSyncSession`, // so the reasoned notice below is the only one the user sees. this._monitoredSessions.delete(session); this._announcedConfigurations.delete(session); - this._postNotice(session, AgentSystemNotificationKind.AgentMergeDisabled, reason.notice); + this._postNotice(session, notificationKind, reason.notice); const patch: Record = { [SessionConfigKey.AgentMerge]: { enabled: false, diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 7a7a77019c5b6f..2143c0f4240a38 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import type { CCAModel } from '@vscode/copilot-api'; import { spawn, type ChildProcessWithoutNullStreams } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; @@ -24,6 +25,7 @@ import { IProductService } from '../../../product/common/productService.js'; import { createSchema, platformRootSchema, platformSessionSchema, schemaProperty, AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostCodexMultiRootEnabledConfigKey, AgentHostGitHubMcpServerEnabledConfigKey, AgentHostMcpServersConfigKey, type ISchemaProperty, type SessionMode } from '../../common/agentHostSchema.js'; import { createPricingMetaFromBilling, normalizeCAPIBilling } from '../../common/agentModelPricing.js'; import { CHATGPT_SUBSCRIPTION_MODEL_SOURCE_ID, createAgentModelGroupMeta, createAgentModelSourceMeta } from '../../common/agentModelSource.js'; +import { AgentSystemNotificationKind, toAgentSystemNotificationMeta } from '../../common/meta/agentSystemNotificationMeta.js'; import { AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../../common/agentHostCustomizationConfig.js'; import { AgentSdkSetupChannel } from '../agentSdkSetupChannel.js'; import { CODEX_ACCOUNT_META_KEY, CODEX_ACCOUNT_SIGN_IN_REQUEST_KEY, CODEX_ACCOUNT_SIGN_OUT_REQUEST_KEY, type ICodexAccountInfo } from '../../common/codexAccount.js'; @@ -145,7 +147,7 @@ import type { GuardianWarningNotification } from './protocol/generated/v2/Guardi import type { ThreadApproveGuardianDeniedActionResponse } from './protocol/generated/v2/ThreadApproveGuardianDeniedActionResponse.js'; import type { ConfigReadResponse } from './protocol/generated/v2/ConfigReadResponse.js'; import type { ConfigWriteResponse } from './protocol/generated/v2/ConfigWriteResponse.js'; -import { formatGuardianDenialNotification, summarizeGuardianReviewAction, toGuardianAssessmentEventJson } from './codexGuardianReview.js'; +import { formatGuardianDenialNotification, formatGuardianReviewStatusNotification, summarizeGuardianReviewAction, toGuardianAssessmentEventJson } from './codexGuardianReview.js'; import { CODEX_COMPACT_SLASH_COMMAND } from '../codexCompactCommand.js'; const CLIENT_INFO = { @@ -164,6 +166,7 @@ const CODEX_THREAD_TURNS_PAGE_SIZE = 100; const CODEX_STARTUP_ACCOUNT_PROBE_TIMEOUT_MS = 30_000; const CODEX_DESKTOP_WORKSPACE_DATE_PATTERN = /^\d{4}-\d{2}-\d{2}$/; const CODEX_DESKTOP_SESSION_META_PATTERN = /"type"\s*:\s*"session_meta".*"payload"\s*:\s*\{[^}]*"originator"\s*:\s*"Codex Desktop"/s; +const CODEX_GUARDIAN_TURN_INTERRUPTION_PREFIX = 'Automatic approval review rejected too many approval requests for this turn'; function isCodexDesktopGeneratedWorkspace(cwd: string, userHome: URI): boolean { const relativePath = extUriBiasedIgnorePathCase.relativePath(userHome, URI.file(cwd)); @@ -240,6 +243,17 @@ const CODEX_COPILOT_MODEL_GROUP = 'copilot'; const CODEX_OPENAI_MODEL_PROVIDER = 'openai'; const CODEX_MODEL_SELECTION_PREFIX = '@provider='; +/** + * The Codex harness relies on OpenAI Responses semantics beyond the endpoint + * shape. Other vendors can advertise `/responses` without supporting the full + * Codex request lifecycle, so only publish OpenAI's picker-eligible models. + */ +function isCodexCompatibleCopilotModel(model: CCAModel): boolean { + return model.vendor.toLowerCase() === CODEX_OPENAI_MODEL_PROVIDER + && !!model.model_picker_enabled + && !!model.supported_endpoints?.includes(CODEX_RESPONSES_ENDPOINT); +} + export function toCodexModelSelectionId(modelProvider: string, modelId: string): string { return `${CODEX_MODEL_SELECTION_PREFIX}${encodeURIComponent(modelProvider)}:${encodeURIComponent(modelId)}`; } @@ -591,9 +605,9 @@ interface ICodexSession { */ readonly acceptedForSession: Set; /** - * Guardian (auto-review) `reviewId`s that have already been surfaced to - * the user as a denied-action approval card. Guards against acting twice - * on the same review if the completed notification is redelivered. + * Guardian (auto-review) `reviewId`s whose terminal outcome has already + * been surfaced. Guards against acting twice on the same review if the + * completed notification is redelivered. */ readonly handledGuardianReviews: Set; /** @@ -1984,7 +1998,7 @@ export class CodexAgent extends Disposable implements IAgent { // OpenAI-shaped Responses endpoint. The chosen id is forwarded straight // through; CAPI remains the authority on what the token may actually use. const models = all - .filter(m => m.model_picker_enabled && m.supported_endpoints?.includes(CODEX_RESPONSES_ENDPOINT)) + .filter(isCodexCompatibleCopilotModel) .sort((a, b) => Number(b.is_chat_default) - Number(a.is_chat_default)) .map((m): IAgentModelInfo => ({ provider: CODEX_AGENT_PROVIDER_ID, @@ -2461,11 +2475,12 @@ export class CodexAgent extends Disposable implements IAgent { subscriptions.add(client.onNotification('thread/tokenUsage/updated', params => this._dispatchTokenUsageUpdated(params))); subscriptions.add(client.onNotification('item/completed', params => this._dispatchItemCompleted(params))); subscriptions.add(client.onNotification('turn/completed', params => this._dispatchTurnCompleted(params))); - // Auto-review (guardian) surfacing. The guardian warning is shown as a - // system notification; a completed *denied* review is turned into a + // Auto-review (guardian) surfacing. The guardian turn-interruption warning + // is shown as a system notification; terminal review failures are surfaced + // from the structured completion event, and a denied review also gets a // retroactive "Approve anyway" tool-call card. The review lifecycle is - // non-blocking (codex does not wait on us), so the completed handler is - // async and resolves its session directly rather than via _dispatchByThread. + // non-blocking (codex does not wait on us), so the completed handler is async + // and resolves its session directly rather than via _dispatchByThread. subscriptions.add(client.onNotification('guardianWarning', params => this._dispatchByThread(params.threadId, s => this._handleGuardianWarning(s, params)))); subscriptions.add(client.onNotification('item/autoApprovalReview/completed', params => { void this._handleGuardianReviewCompleted(client, params); })); @@ -3664,6 +3679,11 @@ export class CodexAgent extends Disposable implements IAgent { } private _handleGuardianWarning(session: ICodexSession, params: GuardianWarningNotification): ChatAction[] { + // Individual review outcomes are handled by the structured review event. The + // warning channel is only needed when the review circuit breaker ends a turn. + if (!params.message.startsWith(CODEX_GUARDIAN_TURN_INTERRUPTION_PREFIX)) { + return []; + } const turnId = session.currentTurnId; if (turnId === undefined) { this._logService.trace(`[Codex:${session.sessionId}] guardianWarning without active turn; ignoring`); @@ -3675,6 +3695,7 @@ export class CodexAgent extends Disposable implements IAgent { part: { kind: ResponsePartKind.SystemNotification, content: params.message, + _meta: toAgentSystemNotificationMeta({ kind: AgentSystemNotificationKind.AutomaticApprovalReviewInterrupted }), }, }]; } @@ -3686,18 +3707,18 @@ export class CodexAgent extends Disposable implements IAgent { this._logService.trace(`[Codex] autoApprovalReview/completed for unknown threadId=${params.threadId}; ignoring`); return; } - if (params.review.status !== 'denied') { + const status = params.review.status; + if (status === 'approved' || status === 'inProgress') { return; } if (session.handledGuardianReviews.has(params.reviewId)) { return; } - // Bind the denial surfacing to the review's OWN turn (mapped app→host), + // Bind review surfacing to the review's OWN turn (mapped app→host), // not whatever turn happens to be current. An `autoApprovalReview/completed` // that arrives out of order — after its turn ended, or once a later turn is - // active — must not mis-attribute the notice/card to a different turn, nor - // apply this review's stale action against it. When the review's turn is no - // longer the active turn there is nothing left to approve within it, so ignore. + // active — must not mis-attribute its notice to a different turn. A denied + // review's override also stops being actionable after its turn ends. const turnId = this._hostTurnId(session, params.turnId); if (session.currentTurnId !== turnId) { this._logService.trace(`[Codex:${sessionId}] autoApprovalReview/completed for non-current turn ${turnId} (current=${session.currentTurnId ?? '(none)'}); ignoring reviewId=${params.reviewId}`); @@ -3707,6 +3728,21 @@ export class CodexAgent extends Disposable implements IAgent { session.handledGuardianReviews.add(params.reviewId); const summary = summarizeGuardianReviewAction(params.action); + if (status === 'timedOut' || status === 'aborted') { + const kind = status === 'timedOut' + ? AgentSystemNotificationKind.AutomaticApprovalReviewTimedOut + : AgentSystemNotificationKind.AutomaticApprovalReviewAborted; + this._fire(session.sessionUri, { + type: ActionType.ChatResponsePart, + turnId, + part: { + kind: ResponsePartKind.SystemNotification, + content: formatGuardianReviewStatusNotification(summary, status, params.review.rationale), + _meta: toAgentSystemNotificationMeta({ kind }), + }, + }); + return; + } // Durable record: a Markdown response part survives turn completion AND is // rendered by the live streaming path (unlike a system-notification part, diff --git a/src/vs/platform/agentHost/node/codex/codexGuardianReview.ts b/src/vs/platform/agentHost/node/codex/codexGuardianReview.ts index d73e4ee94ed418..f5a736cbacf195 100644 --- a/src/vs/platform/agentHost/node/codex/codexGuardianReview.ts +++ b/src/vs/platform/agentHost/node/codex/codexGuardianReview.ts @@ -7,6 +7,7 @@ import type { GuardianApprovalReviewAction } from './protocol/generated/v2/Guard import type { ItemGuardianApprovalReviewCompletedNotification } from './protocol/generated/v2/ItemGuardianApprovalReviewCompletedNotification.js'; import type { RequestPermissionProfile } from './protocol/generated/v2/RequestPermissionProfile.js'; import type { JsonValue } from './protocol/generated/serde_json/JsonValue.js'; +import { localize } from '../../../../nls.js'; import { unwrapShellInvocation } from './codexShellCommand.js'; /** @@ -199,3 +200,21 @@ export function formatGuardianDenialNotification(summary: IGuardianActionSummary // part; trailing newline keeps subsequent model output on its own block. return `\n\n${quoted}\n`; } + +/** Compose a compact, collapsible notification for a review that did not decide. */ +export function formatGuardianReviewStatusNotification(summary: IGuardianActionSummary, status: 'timedOut' | 'aborted', rationale: string | null): string { + const title = status === 'timedOut' + ? localize('codex.guardianReview.timedOut', "Auto-review timed out") + : localize('codex.guardianReview.aborted', "Auto-review stopped"); + const detail = summary.detail?.trim(); + const action = detail ? `${summary.title} ${inlineCode(detail)}` : summary.title; + const lines = [ + title, + localize('codex.guardianReview.requestedAction', "Requested action: {0}", action), + ]; + const reason = rationale?.trim(); + if (reason) { + lines.push('', reason); + } + return lines.join('\n'); +} diff --git a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts index e3a13d572d71b2..2cddea6e377c72 100644 --- a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts @@ -407,6 +407,28 @@ function resolveWorkspace(workspace: string, sessions: readonly IAgentSessionMet return parsed; } +async function getCreateSessionCatalog(accessor: ISessionServerToolAccessor, rawArgs: unknown): Promise { + const args = (rawArgs ?? {}) as ICreateSessionArgs; + if (getCreateSessionRelationship(args) !== 'independent') { + return []; + } + const workspace = getOptionalString(args.workspace, 'workspace', SessionServerToolName.CreateSession); + if (workspace === undefined) { + return []; + } + try { + // Prefer the catalog because a project display name can also be a valid URI. + return await accessor.listSessions(); + } catch (error) { + // An explicit URI/path is self-contained, so it remains usable when a + // provider cannot enumerate its session catalog. + if (parseWorkspaceUri(workspace) !== undefined) { + return []; + } + throw error; + } +} + function resolveModel(modelName: string | undefined, models: readonly IAgentModelInfo[], provider?: AgentProvider): IAgentModelInfo | undefined { if (modelName === undefined) { return undefined; @@ -720,7 +742,7 @@ export interface ICreateSessionResult { * Creates work with the requested relationship and sends its initial prompt. */ export async function applyCreateSessionTool(accessor: ISessionServerToolAccessor, rawArgs: unknown, source?: URI, sourceTurnId?: string): Promise { - const sessions = await accessor.listSessions(); + const sessions = await getCreateSessionCatalog(accessor, rawArgs); const currentSession = source ? currentSessionUri(source.toString()) : undefined; const currentProvider = currentSession ? AgentSession.provider(currentSession) : undefined; const args = getCreateSessionArgs(rawArgs, sessions, accessor.getModels(), currentProvider); @@ -1391,7 +1413,11 @@ export function createSessionServerToolGroup(accessor?: ISessionServerToolAccess return serializeSessions(filterSessions(await accessor.listSessions(), getListSessionsArgs(rawArgs))); } case SessionServerToolName.GetCurrentSession: - return serializeCurrentSession(currentSessionUri(currentChannel), await accessor.listSessions()); + { + const currentSession = currentSessionUri(currentChannel); + const metadata = await accessor.getSession(currentSession); + return serializeCurrentSession(currentSession, metadata ? [metadata] : []); + } case SessionServerToolName.CreateSession: { const relationship = getCreateSessionRelationship(rawArgs); if (relationship === 'currentSession' && createdChatCount >= maxCreatedChats) { diff --git a/src/vs/platform/agentHost/test/common/agentMerge.test.ts b/src/vs/platform/agentHost/test/common/agentMerge.test.ts index dece3d12ed626c..50dbc8854e713e 100644 --- a/src/vs/platform/agentHost/test/common/agentMerge.test.ts +++ b/src/vs/platform/agentHost/test/common/agentMerge.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { AgentMergeConfiguration, AGENT_MERGE_UNKNOWN_COMMIT, agentMergeConfigurationChangedNotice, agentMergeEnabledNotice, evaluateAgentMerge, getNonMergeSessionConfigValues, readAgentMergeSessionState, shouldStopMergingAfterAgentChanges } from '../../common/agentMerge.js'; +import { AgentMergeConfiguration, AGENT_MERGE_UNKNOWN_COMMIT, agentMergeConfigurationChangedNotice, agentMergeDisableReasons, agentMergeEnabledNotice, evaluateAgentMerge, getNonMergeSessionConfigValues, isAgentMergePullRequestReadyForReview, readAgentMergeSessionState, shouldStopMergingAfterAgentChanges } from '../../common/agentMerge.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { PullRequestSnapshot } from '../../../github/common/githubPullRequestService.js'; @@ -75,6 +75,62 @@ suite('Agent Merge gate', () => { }); }); + test('repairs draft pull requests without merging them', () => { + const repair = evaluateAgentMerge(readySnapshot({ + draft: true, + reviewThreads: [{ + id: 'thread-1', + isResolved: false, + comments: [{ id: 'comment-1', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Please fix this' }], + }], + checks: [{ id: 'required', type: 'checkRun', name: 'Build', required: true, status: 'COMPLETED', conclusion: 'FAILURE' }], + }), configuration, '2026-08-02T00:00:00.000Z'); + const ready = evaluateAgentMerge(readySnapshot({ draft: true }), configuration, '2026-08-02T00:00:00.000Z'); + + assert.deepStrictEqual({ + repair: repair.kind === 'prompt' ? repair.actions : repair.kind, + ready: ready.kind, + }, { + repair: ['addressReviews', 'fixCI'], + ready: 'noWork', + }); + }); + + test('reports when required checks and review feedback are ready', () => { + const watermark = '2026-08-02T00:00:00.000Z'; + assert.deepStrictEqual({ + ready: isAgentMergePullRequestReadyForReview(readySnapshot({ draft: true }), watermark), + notDraft: isAgentMergePullRequestReadyForReview(readySnapshot(), watermark), + pendingChecks: isAgentMergePullRequestReadyForReview(readySnapshot({ + draft: true, + checks: [{ id: 'required', type: 'checkRun', name: 'Build', required: true, status: 'IN_PROGRESS' }], + }), watermark), + failingChecks: isAgentMergePullRequestReadyForReview(readySnapshot({ + draft: true, + checks: [{ id: 'required', type: 'checkRun', name: 'Build', required: true, status: 'COMPLETED', conclusion: 'FAILURE' }], + }), watermark), + reviewComments: isAgentMergePullRequestReadyForReview(readySnapshot({ + draft: true, + reviewThreads: [{ + id: 'thread-1', + isResolved: false, + comments: [{ id: 'comment-1', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Please fix this' }], + }], + }), watermark), + newComments: isAgentMergePullRequestReadyForReview(readySnapshot({ + draft: true, + topLevelComments: [{ id: 'comment-1', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Please fix this', createdAt: '2026-08-03T00:00:00.000Z' }], + }), watermark), + }, { + ready: true, + notDraft: undefined, + pendingChecks: false, + failingChecks: false, + reviewComments: false, + newComments: false, + }); + }); + test('merges only from complete ready state', () => { assert.deepStrictEqual(evaluateAgentMerge(readySnapshot(), configuration, '2026-08-02T00:00:00.000Z').kind, 'merge'); }); @@ -215,7 +271,7 @@ suite('Agent Merge gate', () => { ...configuration, mergePullRequest: 'never', }), [ - 'Agent Merge is on for `feature`. It will wait for a pull request on this branch, then monitor it.', + 'Agent Merge is enabled for `feature`. It will wait for a pull request on this branch, then monitor it.', 'It will ask the agent to address new pull request review comments.', 'It will ask the agent to fix failing CI checks.', 'It will ask the agent to resolve merge conflicts and update the branch when it falls behind.', @@ -225,6 +281,13 @@ suite('Agent Merge gate', () => { ].map((line, index) => index === 0 ? `${line}\n` : `- ${line}`).join('\n')); }); + test('reports when Agent Merge merges a pull request', () => { + assert.strictEqual( + agentMergeDisableReasons.pullRequestMerged(123, 'https://github.com/octo/repo/pull/123').notice, + 'Agent Merge merged pull request [#123](https://github.com/octo/repo/pull/123).', + ); + }); + test('describes effective Agent Merge configuration changes', () => { const previous: AgentMergeConfiguration = { ...configuration, @@ -262,7 +325,7 @@ suite('Agent Merge gate', () => { mergePullRequest: 'always', mergeMethod: 'squash', }), [ - 'Agent Merge is on for `feature` and is monitoring its pull request.', + 'Agent Merge is enabled for `feature` and is monitoring its pull request.', 'It will ask the agent to fix failing CI checks.', 'It will ask the agent to resolve merge conflicts and update the branch when it falls behind.', 'After each update, it will wait for new CI results.', @@ -372,6 +435,7 @@ suite('Agent Merge gate', () => { }); function readySnapshot(overrides?: { + readonly draft?: boolean; readonly topLevelComments?: NonNullable; readonly submittedReviews?: NonNullable; readonly reviewThreads?: NonNullable; @@ -390,7 +454,7 @@ function readySnapshot(overrides?: { title: 'Change', url: 'https://github.com/octo/repo/pull/1', state: 'open', - draft: false, + draft: overrides?.draft ?? false, headSha: 'head', headRef: 'feature', baseSha: 'base', diff --git a/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationHandler.test.ts b/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationHandler.test.ts index 0441df9a391c1f..6f800c856eefd7 100644 --- a/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationHandler.test.ts @@ -362,6 +362,35 @@ suite('AgentHostPullRequestOperationHandler', () => { }); }); + test('creates a draft pull request and enables Agent Merge', async () => { + const gitService = new TestGitService(); + const octoKitService = new TestOctoKitService(); + const { handler, session, sessionConfigUpdates } = setup(disposables, gitService, octoKitService, { + draft: true, + enableAgentMerge: true, + }); + + const result = await handler.invoke({ channel: buildSessionChangesetUri(session.toString()), operationId: AgentHostPullRequestOperationHandler.OPERATION_CREATE_DRAFT_PR_AGENT_MERGE }, CancellationToken.None); + + assert.deepStrictEqual({ + message: result.message, + octoCalls: octoKitService.calls, + sessionConfigUpdates, + }, { + message: { markdown: 'Created draft pull request [#123](https://github.com/microsoft/vscode/pull/123) and enabled Agent Merge.' }, + octoCalls: [ + 'findPullRequestByHeadBranch:feature/test', + 'createPullRequest:true', + ], + sessionConfigUpdates: [{ + [SessionConfigKey.AgentMerge]: { + enabled: true, + }, + [SessionConfigKey.AgentMergeController]: {}, + }], + }); + }); + test('creates a generated branch before committing when the current branch is the base branch', async () => { const gitService = new TestGitService(); gitService.uncommitted = true; diff --git a/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationProvider.test.ts b/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationProvider.test.ts index d25bea09e54de8..83d5f6ad19b4b2 100644 --- a/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationProvider.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostPullRequestOperationProvider.test.ts @@ -10,6 +10,7 @@ import { InstantiationService } from '../../../instantiation/common/instantiatio import { NullLogService } from '../../../log/common/log.js'; import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; import { AgentHostPullRequestOperationContribution } from '../../node/agentHostPullRequestOperationProvider.js'; +import { AgentHostPullRequestLifecycleOperationHandler } from '../../node/agentHostPullRequestLifecycleOperationHandler.js'; import type { IAgentHostPullRequestStatus, IAgentHostPullRequestStatusService } from '../../node/agentHostPullRequestStatusService.js'; import { SessionStatus, type ISessionGitHubState, type ISessionGitState } from '../../common/state/sessionState.js'; import type { IAgentHostGitStateService } from '../../common/agentHostGitStateService.js'; @@ -73,10 +74,10 @@ const pullRequestForBranch: ISessionGitHubState = { suite('AgentHostPullRequestOperationContribution', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - function createContribution(status?: IAgentHostPullRequestStatus, isolation?: 'folder' | 'worktree', onDidChangePullRequestStatus = Event.None, agentMergeEnabled = false): AgentHostPullRequestOperationContribution { + function createContribution(status?: IAgentHostPullRequestStatus, isolation?: 'folder' | 'worktree', onDidChangePullRequestStatus = Event.None, agentMergeEnabled = false, sessionAgentMergeEnabled = false): AgentHostPullRequestOperationContribution { const stateManager = disposables.add(new AgentHostStateManager(new NullLogService())); - if (isolation) { - stateManager.createSession({ + if (isolation || sessionAgentMergeEnabled) { + const session = { resource: 'agent:/session', provider: 'copilot', title: 'Session', @@ -84,10 +85,18 @@ suite('AgentHostPullRequestOperationContribution', () => { createdAt: new Date(1).toISOString(), modifiedAt: new Date(1).toISOString(), workingDirectories: ['file:///repo'], - }); + }; + if (sessionAgentMergeEnabled) { + stateManager.restoreSession(session, []); + } else { + stateManager.createSession(session); + } stateManager.setSessionConfig('agent:/session', { schema: { type: 'object', properties: {} }, - values: { [SessionConfigKey.Isolation]: isolation }, + values: { + ...(isolation ? { [SessionConfigKey.Isolation]: isolation } : {}), + ...(sessionAgentMergeEnabled ? { [SessionConfigKey.AgentMerge]: { enabled: true } } : {}), + }, }); } const configurationService = new class extends mock() { @@ -113,12 +122,20 @@ suite('AgentHostPullRequestOperationContribution', () => { assert.deepStrictEqual(operations?.map(op => op.id), ['create-pr', 'create-pr-auto-merge', 'create-pr-auto-squash', 'create-pr-auto-rebase', 'create-draft-pr']); }); - test('advertises Create PR and Enable Agent Merge as the last Create PR option when Agent Merge is enabled', () => { + test('advertises Agent Merge variants as the last ready and draft Create PR options when Agent Merge is enabled', () => { const provider = createContribution(undefined, undefined, Event.None, true); const operations = provider.getOperations({ sessionKey: 'agent:/session', gitState: githubBranchWithUncommittedChanges, changesetKind: ChangesetKind.Session, changesetUri: '' }); - assert.deepStrictEqual(operations?.map(op => op.id), ['create-pr', 'create-pr-auto-merge', 'create-pr-auto-squash', 'create-pr-auto-rebase', 'create-pr-agent-merge', 'create-draft-pr']); + assert.deepStrictEqual(operations?.map(({ id, label }) => ({ id, label })), [ + { id: 'create-pr', label: 'Create PR' }, + { id: 'create-pr-auto-merge', label: 'Create PR (Auto-Merge)' }, + { id: 'create-pr-auto-squash', label: 'Create PR (Auto-Squash)' }, + { id: 'create-pr-auto-rebase', label: 'Create PR (Auto-Rebase)' }, + { id: 'create-pr-agent-merge', label: 'Create PR & Agent Merge' }, + { id: 'create-draft-pr', label: 'Create Draft PR' }, + { id: 'create-draft-pr-agent-merge', label: 'Create Draft PR & Agent Merge' }, + ]); }); test('does not advertise PR operations for folder sessions with outgoing changes', () => { @@ -158,7 +175,7 @@ suite('AgentHostPullRequestOperationContribution', () => { }); test('advertises lifecycle operations for a pull request on the current branch', () => { - const operationsFor = (status?: IAgentHostPullRequestStatus) => createContribution(status) + const operationsFor = (status?: IAgentHostPullRequestStatus, agentMergeEnabled = false) => createContribution(status, undefined, Event.None, agentMergeEnabled, agentMergeEnabled) .getOperations({ sessionKey: 'agent:/session', gitState: githubBranchWithUncommittedChanges, gitHubState: pullRequestForBranch, changesetKind: ChangesetKind.Session, changesetUri: '' }) ?.map(op => op.id); @@ -166,6 +183,9 @@ suite('AgentHostPullRequestOperationContribution', () => { unresolved: operationsFor(undefined), merged: operationsFor(openPullRequest({ state: 'merged' })), draft: operationsFor(openPullRequest({ draft: true, viewerCanEnableAutoMerge: true })), + agentMergeDraftWaiting: operationsFor(openPullRequest({ draft: true, agentMergeReadyForReview: false }), true), + agentMergeDraftUnknown: operationsFor(openPullRequest({ draft: true }), true), + agentMergeDraftReady: operationsFor(openPullRequest({ draft: true, agentMergeReadyForReview: true }), true), mergeable: operationsFor(openPullRequest({ mergeReady: true })), blocked: operationsFor(openPullRequest({ viewerCanEnableAutoMerge: true })), autoMerging: operationsFor(openPullRequest({ autoMergeEnabled: true })), @@ -174,6 +194,9 @@ suite('AgentHostPullRequestOperationContribution', () => { unresolved: undefined, merged: undefined, draft: ['pr-mark-ready', 'pr-enable-auto-merge'], + agentMergeDraftWaiting: ['pr-mark-ready-with-agent-merge'], + agentMergeDraftUnknown: ['pr-mark-ready-with-agent-merge'], + agentMergeDraftReady: ['pr-mark-ready'], mergeable: ['pr-merge'], blocked: ['pr-enable-auto-merge'], autoMerging: ['pr-disable-auto-merge'], @@ -181,4 +204,14 @@ suite('AgentHostPullRequestOperationContribution', () => { }); }); + test('uses the same label for the Agent Merge Mark Ready operation', () => { + const operations = createContribution(openPullRequest({ draft: true, agentMergeReadyForReview: false }), undefined, Event.None, true, true) + .getOperations({ sessionKey: 'agent:/session', gitState: githubBranchWithUncommittedChanges, gitHubState: pullRequestForBranch, changesetKind: ChangesetKind.Session, changesetUri: '' }); + + assert.deepStrictEqual(operations?.map(({ id, label }) => ({ id, label })), [{ + id: AgentHostPullRequestLifecycleOperationHandler.OPERATION_MARK_READY_WITH_AGENT_MERGE, + label: 'Mark Ready', + }]); + }); + }); diff --git a/src/vs/platform/agentHost/test/node/agentHostPullRequestStatusService.test.ts b/src/vs/platform/agentHost/test/node/agentHostPullRequestStatusService.test.ts index 6d794cc64d4b81..587019287468e8 100644 --- a/src/vs/platform/agentHost/test/node/agentHostPullRequestStatusService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostPullRequestStatusService.test.ts @@ -10,13 +10,15 @@ import { observableValue } from '../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { NullLogService } from '../../../log/common/log.js'; import type { GitHubCredential, GitHubCredentialInvalidation, IGitHubCredentials } from '../../../github/common/githubCredentialService.js'; -import type { PullRequestRef, PullRequestSnapshot, PullRequestSubscription } from '../../../github/common/githubPullRequestService.js'; +import type { PullRequestRef, PullRequestSnapshot, PullRequestSubscription, PullRequestSubscriptionOptions } from '../../../github/common/githubPullRequestService.js'; import type { IGitHubService } from '../../../github/common/githubService.js'; import type { IPullRequestResources } from '../../../github/common/pullRequestResourceService.js'; import { mock } from '../../../../base/test/common/mock.js'; import { IAgentHostChangesetSubscriptionService } from '../../common/agentHostChangesetSubscriptionService.js'; import type { IAgentHostGitStateService } from '../../common/agentHostGitStateService.js'; import { readSessionGitHubState, SessionStatus, withSessionGitHubState, withSessionGitState, type ISessionGitHubState, type SessionSummary } from '../../common/state/sessionState.js'; +import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; +import { ActionType } from '../../common/state/sessionActions.js'; import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; import { AgentHostPullRequestStatusService } from '../../node/agentHostPullRequestStatusService.js'; @@ -77,10 +79,33 @@ function snapshot(ref: PullRequestRef, overrides?: { readonly draft?: boolean; r } as PullRequestSnapshot; } +function agentMergeReadySnapshot(ref: PullRequestRef): PullRequestSnapshot { + const result = snapshot(ref, { draft: true }); + const ready = { status: 'ready', complete: true } as const; + return { + ...result, + topLevelComments: { ...ready, value: [] }, + submittedReviews: { ...ready, value: [] }, + reviewThreads: { ...ready, headSha: 'sha1', value: [] }, + checks: { + ...ready, + headSha: 'sha1', + value: { + headSha: 'sha1', + requirednessComplete: true, + expectedSuites: [], + expectedSuitesComplete: true, + checks: [], + }, + }, + }; +} + /** Records subscription lifecycle so tests can assert nothing is leaked. */ class TestPullRequestResources implements IPullRequestResources { readonly subscribed: PullRequestRef[] = []; + readonly subscriptionOptions: PullRequestSubscriptionOptions[] = []; disposedCount = 0; private _snapshot = observableValue('snapshot', undefined); private _nextSubscriptionSnapshot: PullRequestSnapshot | undefined; @@ -88,13 +113,14 @@ class TestPullRequestResources implements IPullRequestResources { get liveSubscriptions(): number { return this.subscribed.length - this.disposedCount; } - subscribePullRequest(ref: PullRequestRef): PullRequestSubscription { + subscribePullRequest(ref: PullRequestRef, options: PullRequestSubscriptionOptions): PullRequestSubscription { this.subscribed.push(ref); + this.subscriptionOptions.push(options); this._snapshot.set(this._nextSubscriptionSnapshot ?? snapshot(ref), undefined); this._nextSubscriptionSnapshot = undefined; return { resource: { ref, snapshot: this._snapshot as never }, - update: () => { }, + update: next => this.subscriptionOptions.push(next), refresh: async () => this.refreshHandler?.(), dispose: () => { this.disposedCount++; }, } as PullRequestSubscription; @@ -251,6 +277,70 @@ suite('AgentHostPullRequestStatusService', () => { }); }); + test('tracks review readiness in the host only while Agent Merge is enabled', async () => { + const { service, stateManager, subscriptions, resources, session } = createHarness(); + stateManager.setSessionConfig(session, { + schema: { type: 'object', properties: {} }, + values: { + [SessionConfigKey.AgentMerge]: { enabled: true }, + [SessionConfigKey.AgentMergeController]: { + target: { + branchName: 'feature', + pullRequestUrl, + enabledAt: new Date(1).toISOString(), + commentWatermark: '', + }, + }, + }, + }); + resources.setNextSubscriptionSnapshot(agentMergeReadySnapshot({ ...account, owner: 'octo', repo: 'repo', number: 7 })); + subscriptions.addSubscription(session, `${session}/changes`); + await waitForWatch(resources); + const enabled = { + options: resources.subscriptionOptions.at(-1), + readyForReview: service.getPullRequestStatus(session)?.agentMergeReadyForReview, + }; + + stateManager.dispatchServerAction(session, { + type: ActionType.SessionConfigChanged, + config: { [SessionConfigKey.AgentMerge]: { enabled: false } }, + replace: true, + }); + await pump(); + + assert.deepStrictEqual({ + enabled, + disabled: { + options: resources.subscriptionOptions.at(-1), + readyForReview: service.getPullRequestStatus(session)?.agentMergeReadyForReview, + }, + }, { + enabled: { + options: { + priority: 'visible', + core: true, + mergeability: true, + conversation: { + topLevelComments: true, + submittedReviews: true, + reviewThreads: true, + includeBodies: true, + }, + checks: { required: true }, + }, + readyForReview: true, + }, + disabled: { + options: { + priority: 'visible', + core: true, + mergeability: true, + }, + readyForReview: undefined, + }, + }); + }); + test('optimistically records a successful merge in host pull request state', async () => { const { service, subscriptions, resources, gitHubStates, session } = createHarness(); subscriptions.addSubscription(session, `${session}/changes`); diff --git a/src/vs/platform/agentHost/test/node/agentMergeController.test.ts b/src/vs/platform/agentHost/test/node/agentMergeController.test.ts index 215e77e6d4dbf1..02c2bffdef5d75 100644 --- a/src/vs/platform/agentHost/test/node/agentMergeController.test.ts +++ b/src/vs/platform/agentHost/test/node/agentMergeController.test.ts @@ -17,7 +17,7 @@ import { URI } from '../../../../base/common/uri.js'; import { AgentSystemNotificationKind } from '../../common/meta/agentSystemNotificationMeta.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { ActionType } from '../../common/state/protocol/common/actions.js'; -import { SessionStatus, buildDefaultChatUri, MessageKind, withSessionGitState, type SessionSummary } from '../../common/state/sessionState.js'; +import { SessionStatus, buildDefaultChatUri, MessageKind, withSessionGitHubState, withSessionGitState, type SessionSummary } from '../../common/state/sessionState.js'; import { IGitHubService } from '../../../github/common/githubService.js'; import { PullRequestSnapshot } from '../../../github/common/githubPullRequestService.js'; import { AgentConfigurationService } from '../../node/agentConfigurationService.js'; @@ -435,6 +435,7 @@ suite('AgentMergeController', () => { const gitStateService = new class extends mock() { override readonly onDidRefreshSessionGitState = Event.None; override readonly onDidChangeSessionGitHubState = Event.None; + override async attachSessionGitHubPullRequest(): Promise { } }(); const endpointService = disposables.add(new AgentHostGitHubEndpointService(configurationService, logService)); const notices: { kind: AgentSystemNotificationKind; content: string }[] = []; @@ -500,7 +501,13 @@ suite('AgentMergeController', () => { schema: platformSessionSchema.toProtocol(), values: {}, }); - stateManager.setSessionMeta(session, withSessionGitState(undefined, { branchName: 'feature', baseBranchName: 'main' })); + stateManager.setSessionMeta(session, withSessionGitHubState( + withSessionGitState(undefined, { branchName: 'feature', baseBranchName: 'main' }), + { + pullRequestUrls: ['https://github.com/octo/repo/pull/1'], + pullRequestBranchName: 'other', + }, + )); const captured = new Promise(resolve => { disposables.add(stateManager.onDidChangeSessionConfig(event => { if (event.session.toString() === session && readAgentMergeSessionState(event.current?.values)?.target) { @@ -533,12 +540,57 @@ suite('AgentMergeController', () => { kind: AgentSystemNotificationKind.AgentMergeEnabled, content: agentMergeEnabledNotice({ branchName: 'feature' }, defaultAgentMergeConfiguration), }, - { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was turned off because the checked-out branch changed from `feature` to `main`.' }, + { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was disabled because the checked-out branch changed from `feature` to `main`.' }, ], enabled: false, }); }); + test('announces a known pull request when it captures the Agent Merge target', async () => { + const { stateManager, configurationService, session, notices } = createControllerHarness(disposables); + const pullRequestUrl = 'https://github.com/octo/repo/pull/1'; + stateManager.setSessionMeta(session, withSessionGitHubState( + withSessionGitState(undefined, { branchName: 'feature', baseBranchName: 'main' }), + { + pullRequestUrls: [pullRequestUrl], + pullRequestBranchName: 'feature', + }, + )); + const captured = new Promise(resolve => { + disposables.add(stateManager.onDidChangeSessionConfig(event => { + if (event.session.toString() === session && readAgentMergeSessionState(event.current?.values)?.target) { + resolve(); + } + })); + }); + + configurationService.updateSessionConfig(session, { [SessionConfigKey.AgentMerge]: { enabled: true } }); + stateManager.dispatchServerAction(session, { type: ActionType.SessionReady }); + await captured; + const target = readAgentMergeSessionState(configurationService.getSessionConfigValues(session))?.target; + + assert.deepStrictEqual({ + target: target ? { + branchName: target.branchName, + pullRequestUrl: target.pullRequestUrl, + hasEnabledAt: target.enabledAt.length > 0, + watermarkMatchesEnablement: target.commentWatermark === target.enabledAt, + } : undefined, + notices, + }, { + target: { + branchName: 'feature', + pullRequestUrl, + hasEnabledAt: true, + watermarkMatchesEnablement: true, + }, + notices: [{ + kind: AgentSystemNotificationKind.AgentMergeEnabled, + content: agentMergeEnabledNotice({ branchName: 'feature', pullRequestUrl }, defaultAgentMergeConfiguration), + }], + }); + }); + test('announces effective session and global configuration changes while monitoring', () => { const { stateManager, configurationService, session, notices } = createControllerHarness(disposables); const target = { @@ -622,10 +674,10 @@ suite('AgentMergeController', () => { configurationService.updateSessionConfig(session, { [SessionConfigKey.AgentMerge]: { enabled: false } }); assert.deepStrictEqual({ afterSelfDisable, notices }, { - afterSelfDisable: [{ kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was turned off because this session was archived.' }], + afterSelfDisable: [{ kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was disabled because this session was archived.' }], notices: [ - { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was turned off because this session was archived.' }, - { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was turned off for this session.' }, + { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was disabled because this session was archived.' }, + { kind: AgentSystemNotificationKind.AgentMergeDisabled, content: 'Agent Merge was disabled for this session.' }, ], }); }); @@ -657,7 +709,7 @@ suite('AgentMergeController', () => { assert.deepStrictEqual(notices, [{ kind: AgentSystemNotificationKind.AgentMergeDisabled, - content: 'Agent Merge was turned off for this session.', + content: 'Agent Merge was disabled for this session.', }, { kind: AgentSystemNotificationKind.AgentMergeEnabled, content: agentMergeEnabledNotice({ branchName: 'feature' }, defaultAgentMergeConfiguration), diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 5c920384c1963c..57b83d6f59e4ea 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -15128,7 +15128,7 @@ suite('AgentService (node dispatcher)', () => { state: TurnState.Complete, responseParts: [{ kind: ResponsePartKind.SystemNotification, - content: 'Agent Merge was turned off for this session.', + content: 'Agent Merge was disabled for this session.', _meta: { kind: 'agentMergeDisabled' }, }], sentToAgent: 0, @@ -15211,7 +15211,7 @@ suite('AgentService (node dispatcher)', () => { afterTurn: { responseParts: [{ kind: ResponsePartKind.SystemNotification, - content: 'Agent Merge was turned off for this session.', + content: 'Agent Merge was disabled for this session.', _meta: { kind: 'agentMergeDisabled' }, }], anchoredTo: ['agent-turn'], diff --git a/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts b/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts index be0f0153c0b4f1..6d4e7745a8a8bf 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts @@ -11,13 +11,17 @@ import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { NullLogService } from '../../../../../platform/log/common/log.js'; import { AgentChatMigrationDeferred, AgentSession, CODEX_AGENT_PROVIDER_ID, type AgentProvider, type IAgentChatContext, type IAgentDiscoveredChat } from '../../../common/agent.js'; +import { AgentSystemNotificationKind, toAgentSystemNotificationMeta } from '../../../common/meta/agentSystemNotificationMeta.js'; +import { ActionType, type ChatAction } from '../../../common/state/sessionActions.js'; import { CustomizationEnablementKind, CustomizationType, McpServerStatus, type McpServerCustomization } from '../../../common/state/protocol/channels-session/state.js'; -import { buildDefaultChatUri, parseRequiredSessionUriFromChatUri } from '../../../common/state/sessionState.js'; +import { buildDefaultChatUri, parseRequiredSessionUriFromChatUri, ResponsePartKind } from '../../../common/state/sessionState.js'; import { AgentHostStateManager } from '../../../node/agentHostStateManager.js'; import { getCustomizationEnablementKey, type CustomizationEnablementResolution, type ICustomizationEnablementTarget } from '../../../node/agentHostCustomizationEnablementService.js'; import { CodexAgent } from '../../../node/codex/codexAgent.js'; import { CodexClientCustomizationStore, type ICodexClientPlugin } from '../../../node/codex/codexClientCustomizations.js'; import type { ICodexMcpServerConfigJson, ICodexMcpServerEntry } from '../../../node/codex/codexMcpServers.js'; +import type { ItemGuardianApprovalReviewCompletedNotification } from '../../../node/codex/protocol/generated/v2/ItemGuardianApprovalReviewCompletedNotification.js'; +import type { GuardianWarningNotification } from '../../../node/codex/protocol/generated/v2/GuardianWarningNotification.js'; import { targetForMcpServer } from '../../../node/shared/customizationEnablementGate.js'; import { McpCustomizationController, type IMcpCustomizationControllerOptions } from '../../../node/shared/mcpCustomizationController.js'; import { createGitHubMcpServerConfiguration, getGitHubMcpTools } from '../../../node/shared/githubMcpServer.js'; @@ -79,6 +83,31 @@ interface ICodexAuthenticateHarness { authenticate(resource: string, token: string): Promise; } +interface ICodexGuardianWarningHarness { + readonly _logService: NullLogService; +} + +interface ICodexGuardianWarningSession { + readonly sessionId: string; + readonly currentTurnId: string | undefined; +} + +interface ICodexGuardianReviewSession { + readonly sessionId: string; + readonly sessionUri: URI; + readonly currentTurnId: string | undefined; + readonly hostTurnIdByAppTurnId: Map; + readonly handledGuardianReviews: Set; +} + +interface ICodexGuardianReviewHarness { + readonly _logService: NullLogService; + readonly _sessionIdByThreadId: Map; + readonly _sessions: Map; + _hostTurnId(session: ICodexGuardianReviewSession, appTurnId: string): string; + _fire(sessionUri: URI, action: ChatAction): void; +} + function resolveConversationSession(harness: ICodexConversationResolverHarness, address: URI, context?: URI | IAgentChatContext): URI | undefined { const resolver = (CodexAgent.prototype as unknown as { _resolveConversationSession(this: ICodexConversationResolverHarness, address: URI, context?: URI | IAgentChatContext): URI | undefined; @@ -100,6 +129,20 @@ function handleMcpRequest(harness: ICodexMcpRequestHarness, chat: URI): Promise< return handler.call(harness, chat, 'server', 'tools/list', undefined); } +function handleGuardianWarning(harness: ICodexGuardianWarningHarness, session: ICodexGuardianWarningSession, params: GuardianWarningNotification): ChatAction[] { + const handler = (CodexAgent.prototype as unknown as { + _handleGuardianWarning(this: ICodexGuardianWarningHarness, session: ICodexGuardianWarningSession, params: GuardianWarningNotification): ChatAction[]; + })._handleGuardianWarning; + return handler.call(harness, session, params); +} + +function handleGuardianReviewCompleted(harness: ICodexGuardianReviewHarness, params: ItemGuardianApprovalReviewCompletedNotification): Promise { + const handler = (CodexAgent.prototype as unknown as { + _handleGuardianReviewCompleted(this: ICodexGuardianReviewHarness, client: never, params: ItemGuardianApprovalReviewCompletedNotification): Promise; + })._handleGuardianReviewCompleted; + return handler.call(harness, undefined as never, params); +} + function emptyHarness(): ICodexConversationResolverHarness { return { id: CODEX_AGENT_PROVIDER_ID, _sessionIdByChatUri: new Map() }; } @@ -108,6 +151,104 @@ suite('CodexAgent', () => { ensureNoDisposablesAreLeakedInTestSuite(); + test('ignores guardian review outcome warnings handled by structured events', () => { + const harness: ICodexGuardianWarningHarness = { _logService: new NullLogService() }; + const session: ICodexGuardianWarningSession = { sessionId: 'session', currentTurnId: 'turn' }; + + for (const message of [ + 'Automatic approval review approved (risk: low, authorization: high): Safe read.', + 'Automatic approval review denied (risk: high, authorization: unknown): Unsafe action.', + 'Automatic approval review timed out while evaluating the requested approval.', + ]) { + assert.deepStrictEqual(handleGuardianWarning(harness, session, { threadId: 'thread', message }), []); + } + }); + + test('surfaces guardian turn-interruption warnings', () => { + const harness: ICodexGuardianWarningHarness = { _logService: new NullLogService() }; + const session: ICodexGuardianWarningSession = { sessionId: 'session', currentTurnId: 'turn' }; + const message = 'Automatic approval review rejected too many approval requests for this turn (5 consecutive, 5 in the last 10 reviews); interrupting the turn.'; + + assert.deepStrictEqual(handleGuardianWarning(harness, session, { threadId: 'thread', message }), [{ + type: ActionType.ChatResponsePart, + turnId: 'turn', + part: { + kind: ResponsePartKind.SystemNotification, + content: message, + _meta: toAgentSystemNotificationMeta({ kind: AgentSystemNotificationKind.AutomaticApprovalReviewInterrupted }), + }, + }]); + }); + + test('surfaces terminal guardian review failures once on their current turn', async () => { + const actions: ChatAction[] = []; + const session: ICodexGuardianReviewSession = { + sessionId: 'session', + sessionUri: URI.parse('codex:/session'), + currentTurnId: 'host-turn', + hostTurnIdByAppTurnId: new Map([ + ['app-turn', 'host-turn'], + ['stale-app-turn', 'stale-host-turn'], + ]), + handledGuardianReviews: new Set(), + }; + const harness: ICodexGuardianReviewHarness = { + _logService: new NullLogService(), + _sessionIdByThreadId: new Map([['thread', session.sessionId]]), + _sessions: new Map([[session.sessionId, session]]), + _hostTurnId: (reviewSession, appTurnId) => reviewSession.hostTurnIdByAppTurnId.get(appTurnId) ?? appTurnId, + _fire: (_sessionUri, action) => actions.push(action), + }; + const notification = (reviewId: string, status: ItemGuardianApprovalReviewCompletedNotification['review']['status'], turnId = 'app-turn', rationale: string | null = null): ItemGuardianApprovalReviewCompletedNotification => ({ + threadId: 'thread', + turnId, + startedAtMs: 10, + completedAtMs: 20, + reviewId, + targetItemId: null, + decisionSource: 'agent', + review: { status, riskLevel: null, userAuthorization: null, rationale }, + action: { + type: 'networkAccess', + target: 'https://example.com', + host: 'example.com', + protocol: 'https', + port: 443, + }, + }); + + await handleGuardianReviewCompleted(harness, notification('approved', 'approved')); + await handleGuardianReviewCompleted(harness, notification('in-progress', 'inProgress')); + await handleGuardianReviewCompleted(harness, notification('stale', 'timedOut', 'stale-app-turn')); + await handleGuardianReviewCompleted(harness, notification('timed-out', 'timedOut', 'app-turn', 'The reviewer did not respond in time.')); + await handleGuardianReviewCompleted(harness, notification('timed-out', 'timedOut', 'app-turn', 'The reviewer did not respond in time.')); + await handleGuardianReviewCompleted(harness, notification('aborted', 'aborted')); + + assert.deepStrictEqual({ actions, handledReviewIds: [...session.handledGuardianReviews] }, { + actions: [ + { + type: ActionType.ChatResponsePart, + turnId: 'host-turn', + part: { + kind: ResponsePartKind.SystemNotification, + content: 'Auto-review timed out\nRequested action: Network access `https://example.com`\n\nThe reviewer did not respond in time.', + _meta: toAgentSystemNotificationMeta({ kind: AgentSystemNotificationKind.AutomaticApprovalReviewTimedOut }), + }, + }, + { + type: ActionType.ChatResponsePart, + turnId: 'host-turn', + part: { + kind: ResponsePartKind.SystemNotification, + content: 'Auto-review stopped\nRequested action: Network access `https://example.com`', + _meta: toAgentSystemNotificationMeta({ kind: AgentSystemNotificationKind.AutomaticApprovalReviewAborted }), + }, + }, + ], + handledReviewIds: ['timed-out', 'aborted'], + }); + }); + test('GitHub MCP injection respects unowned server enablement', () => { const createHarness = (enabled: boolean, customizationEnabled: boolean, token: string | undefined): ICodexGitHubMcpHarness => Object.assign(Object.create(CodexAgent.prototype), { _configurationService: { getRootValue: () => undefined }, diff --git a/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts b/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts index e9756d5ceb5bb2..dc9f5abc7d256a 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts @@ -176,7 +176,7 @@ function createSessionDatabaseReference(database: ISessionDatabase) { } async function createAgent(disposables: Pick, options: ICreateAgentOptions = {}): Promise { - const models = [{ id: 'gpt-test', name: 'GPT Test', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const models = [{ id: 'gpt-test', name: 'GPT Test', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const instantiationService = new TestInstantiationService(); const logService = new NullLogService(); const fileService = disposables.add(new FileService(logService)); diff --git a/src/vs/platform/agentHost/test/node/codex/codexGuardianReview.test.ts b/src/vs/platform/agentHost/test/node/codex/codexGuardianReview.test.ts index b57533129dcaa2..206d0a56fe8c64 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexGuardianReview.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexGuardianReview.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { formatGuardianDenialNotification, summarizeGuardianReviewAction, toGuardianAssessmentEventJson } from '../../../node/codex/codexGuardianReview.js'; +import { formatGuardianDenialNotification, formatGuardianReviewStatusNotification, summarizeGuardianReviewAction, toGuardianAssessmentEventJson } from '../../../node/codex/codexGuardianReview.js'; import type { ItemGuardianApprovalReviewCompletedNotification } from '../../../node/codex/protocol/generated/v2/ItemGuardianApprovalReviewCompletedNotification.js'; suite('codexGuardianReview', () => { @@ -145,4 +145,17 @@ suite('codexGuardianReview', () => { ] ); }); + + test('formatGuardianReviewStatusNotification separates the compact title from review details', () => { + assert.deepStrictEqual( + [ + formatGuardianReviewStatusNotification({ title: 'Network access', detail: 'https://example.com' }, 'timedOut', 'The reviewer did not respond in time.'), + formatGuardianReviewStatusNotification({ title: 'Elevated permissions', detail: '' }, 'aborted', null), + ], + [ + 'Auto-review timed out\nRequested action: Network access `https://example.com`\n\nThe reviewer did not respond in time.', + 'Auto-review stopped\nRequested action: Elevated permissions', + ] + ); + }); }); diff --git a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts index 8555b28c3f71d8..cf746e617444d1 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts @@ -208,7 +208,7 @@ suite('CodexAgent model refresh', () => { }); test('restored model waits for an authentication refresh queued behind activation', async () => { - const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const firstRefreshStarted = new DeferredPromise(); const releaseFirstRefresh = new DeferredPromise(); const authenticatedRefreshStarted = new DeferredPromise(); @@ -260,7 +260,7 @@ suite('CodexAgent model refresh', () => { }); test('model resolution starts discovery when the catalog is empty', async () => { - const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const agent = createAgent(disposables, async () => copilotModels); agent['_githubToken'] = 'token'; agent['_isSdkResolvableWithoutDownload'] = async () => false; @@ -299,7 +299,7 @@ suite('CodexAgent model refresh', () => { }); test('queues a fresh model refresh when Codex activates during an ambient refresh', async () => { - const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const ambientRefreshStarted = new DeferredPromise(); const ambientCodexRefreshFinished = new DeferredPromise(); const releaseAmbientRefresh = new DeferredPromise(); @@ -838,8 +838,8 @@ suite('CodexAgent model refresh', () => { test('does not publish Copilot models disabled for the model picker', async () => { const models = [ - { id: 'picker-enabled', name: 'Picker Enabled', model_picker_enabled: true, supported_endpoints: ['/responses'] }, - { id: 'picker-disabled', name: 'Picker Disabled', model_picker_enabled: false, supported_endpoints: ['/responses'] }, + { id: 'picker-enabled', name: 'Picker Enabled', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }, + { id: 'picker-disabled', name: 'Picker Disabled', model_picker_enabled: false, supported_endpoints: ['/responses'], vendor: 'OpenAI' }, ] as CCAModel[]; const agent = createAgent(disposables, async () => models); agent['_isSdkResolvableWithoutDownload'] = async () => false; @@ -852,6 +852,24 @@ suite('CodexAgent model refresh', () => { ]); }); + test('publishes only OpenAI Copilot models', async () => { + const models = [ + { id: 'grok-4.5', name: 'Grok 4.5', model_picker_enabled: true, supported_endpoints: ['/responses'], capabilities: { family: 'grok-4.5' }, vendor: 'xAI' }, + { id: 'grok-4.6', name: 'Grok 4.6', model_picker_enabled: true, supported_endpoints: ['/responses'], capabilities: { family: 'grok-4.6' }, vendor: 'xAI' }, + { id: 'mai-code-1.1-flash', name: 'MAI-Code-1.1-Flash', model_picker_enabled: true, supported_endpoints: ['/responses'], capabilities: { family: 'oswe-vscode-modelD' }, vendor: 'Microsoft' }, + { id: 'gpt-5.6', name: 'GPT-5.6', model_picker_enabled: true, supported_endpoints: ['/responses'], capabilities: { family: 'gpt-5.6' }, vendor: 'OpenAI' }, + ] as CCAModel[]; + const agent = createAgent(disposables, async () => models); + agent['_isSdkResolvableWithoutDownload'] = async () => false; + + await agent.authenticate(agent.getProtectedResources()[0].resource, 'token'); + await agent.refreshModels(); + + assert.deepStrictEqual(agent.models.get().map(model => model.id), [ + toCodexModelSelectionId('vscode-proxy', 'gpt-5.6'), + ]); + }); + test('waits for an app-server already starting when signed-out use becomes enabled', async () => { const agent = createAgent(disposables, async () => [], {}); const connection = createChatGPTConnection(); @@ -873,7 +891,7 @@ suite('CodexAgent model refresh', () => { }); test('publishes no ChatGPT models when the app server reports no account', async () => { - const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const agent = createAgent(disposables, async () => copilotModels, { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); agent['_githubToken'] = 'token'; agent['_connection'] = createChatGPTConnection(null) as never; @@ -911,7 +929,7 @@ suite('CodexAgent model refresh', () => { test('keeps the last known-good models when a periodic refresh fails', async () => { let shouldFail = false; - const models = [{ id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const models = [{ id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const agent = createAgent(disposables, async () => { if (shouldFail) { throw new Error('transient failure'); @@ -931,7 +949,7 @@ suite('CodexAgent model refresh', () => { test('retries Copilot model discovery after a transient authentication refresh failure', async () => { let attempts = 0; - const models = [{ id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const models = [{ id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const agent = createAgent(disposables, async () => { attempts++; if (attempts === 1) { @@ -1007,7 +1025,7 @@ suite('CodexAgent model refresh', () => { }); test('omits the thinking level when a Copilot model advertises no reasoning efforts', async () => { - const model = { id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'] } as CCAModel; + const model = { id: 'gpt-5.5', name: 'GPT-5.5', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' } as CCAModel; const agent = createAgent(disposables, async () => [model]); await agent.authenticate(agent.getProtectedResources()[0].resource, 'token'); diff --git a/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts b/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts index 92947217be4463..bed7b96fff06f1 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts @@ -197,7 +197,7 @@ class TestCodexConfigurationService extends AgentConfigurationService { } async function createAgent(disposables: Pick, options: ICreateAgentOptions = {}): Promise { - const models = [{ id: 'gpt-test', name: 'GPT Test', model_picker_enabled: true, supported_endpoints: ['/responses'] }] as CCAModel[]; + const models = [{ id: 'gpt-test', name: 'GPT Test', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const instantiationService = new TestInstantiationService(); const logService = new TestCodexLogService(); const fileService = disposables.add(new TestCodexFileService(logService)); diff --git a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts index 6f4d8c1357fd5d..f4e3969ea7782f 100644 --- a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts +++ b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts @@ -82,9 +82,10 @@ suite('SessionServerTools', () => { if (!config) { return undefined; } - const { _meta, ...rest } = config; + const { _meta, workingDirectories, ...rest } = config; return { ...rest, + ...(workingDirectories !== undefined ? { workingDirectories: workingDirectories.map(directory => directory.toString()) } : {}), createdBySession: readSessionCreationReference(_meta), }; } @@ -606,7 +607,7 @@ suite('SessionServerTools', () => { const text = await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, { relationship: 'independent', workspace: workspace.toString(), prompt: 'do it', title: 'New Task', model: 'gpt-4o' }); assert.deepStrictEqual(createConfigSnapshot(created), { - workingDirectories: [workspace], + workingDirectories: [workspace.toString()], provider: 'copilot', model: { id: 'gpt-4o' }, createdBySession: { @@ -638,6 +639,64 @@ suite('SessionServerTools', () => { store.dispose(); }); + test('create_session falls back to an explicit workspace when listing sessions fails', async () => { + const store = new DisposableStore(); + const stateManager = store.add(new AgentHostStateManager(new NullLogService())); + let created: IAgentCreateSessionConfig | undefined; + let catalogRequests = 0; + const accessor = createAccessor({ + listSessions: async () => { + catalogRequests++; + throw new Error('Provider codex cannot enumerate its native session catalog yet'); + }, + onCreate: config => { created = config; }, + }); + const group = createSessionServerToolGroup(accessor); + + const text = await group.execute(stateManager, executionContext('codex:/caller'), SessionServerToolName.CreateSession, { + relationship: 'independent', + workspace: workspace.toString(), + prompt: 'do it', + title: 'New Task', + }); + + assert.deepStrictEqual({ + catalogRequests, + workingDirectories: created?.workingDirectories?.map(directory => directory.toString()), + result: text.startsWith('New session created'), + }, { + catalogRequests: 1, + workingDirectories: [workspace.toString()], + result: true, + }); + store.dispose(); + }); + + test('create_session prefers a URI-shaped project display name from the catalog', async () => { + const store = new DisposableStore(); + const stateManager = store.add(new AgentHostStateManager(new NullLogService())); + const project = URI.parse('file:///projects/repo-main'); + let created: IAgentCreateSessionConfig | undefined; + const accessor = createAccessor({ + listSessions: async () => [{ + ...sessionMeta('project', SessionStatus.Idle, URI.parse('file:///worktrees/repo-main')), + project: { uri: project, displayName: 'repo:main' }, + }], + onCreate: config => { created = config; }, + }); + const group = createSessionServerToolGroup(accessor); + + await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, { + relationship: 'independent', + workspace: 'repo:main', + prompt: 'do it', + title: 'New Task', + }); + + assert.deepStrictEqual(created?.workingDirectories?.map(directory => directory.toString()), [project.toString()]); + store.dispose(); + }); + test('create_session and send_message results are neutral, non-terminal statements (issue #330138)', async () => { const store = new DisposableStore(); const stateManager = store.add(new AgentHostStateManager(new NullLogService())); @@ -700,7 +759,7 @@ suite('SessionServerTools', () => { }, { creationSource: source.toString(), created: { - workingDirectories: [workspace], + workingDirectories: [workspace.toString()], provider: 'copilot', model: { id: 'gpt-inherited' }, createdBySession: { @@ -732,7 +791,7 @@ suite('SessionServerTools', () => { assert.deepStrictEqual(created.map(createConfigSnapshot), [ { - workingDirectories: [workspace], + workingDirectories: [workspace.toString()], provider: 'copilot', createdBySession: { session: 'copilot:/source', @@ -741,7 +800,7 @@ suite('SessionServerTools', () => { config: { [SessionConfigKey.Isolation]: 'worktree' }, }, { - workingDirectories: [workspace], + workingDirectories: [workspace.toString()], provider: 'copilot', createdBySession: { session: 'copilot:/quick-chat', @@ -765,7 +824,7 @@ suite('SessionServerTools', () => { await applyCreateSessionTool(accessor, { relationship: 'independent', workspace: workspace.toString(), prompt: 'do it', title: 'Provider Task' }, URI.parse('claude:/source')); assert.deepStrictEqual(createConfigSnapshot(created), { - workingDirectories: [workspace], + workingDirectories: [workspace.toString()], provider: 'claude', createdBySession: { session: 'claude:/source', @@ -794,7 +853,7 @@ suite('SessionServerTools', () => { }, URI.parse('copilot:/source')); assert.deepStrictEqual(createConfigSnapshot(created), { - workingDirectories: [gitWorkspace], + workingDirectories: [gitWorkspace.toString()], provider: 'copilot', createdBySession: { session: 'copilot:/source', @@ -828,7 +887,7 @@ suite('SessionServerTools', () => { }, URI.parse('copilot:/source')); assert.deepStrictEqual(createConfigSnapshot(created), { - workingDirectories: [remoteProject], + workingDirectories: [remoteProject.toString()], provider: 'claude', model: { id: 'claude-sonnet' }, config: { [SessionConfigKey.Isolation]: 'folder' }, @@ -1453,6 +1512,27 @@ suite('SessionServerTools', () => { store.dispose(); }); + test('get_current_session does not depend on listing sessions', async () => { + const store = new DisposableStore(); + const stateManager = store.add(new AgentHostStateManager(new NullLogService())); + const metadata = { ...sessionMeta('s1', SessionStatus.Idle, workspace), session: URI.parse('codex:/s1') }; + const group = createSessionServerToolGroup(createAccessor({ + listSessions: async () => { throw new Error('Provider codex cannot enumerate its native session catalog yet'); }, + getSession: async session => session.toString() === metadata.session.toString() ? metadata : undefined, + })); + + const text = await group.execute(stateManager, executionContext('codex:/s1'), SessionServerToolName.GetCurrentSession, {}); + + assert.deepStrictEqual(JSON.parse(text), { + session: 'codex:/s1', + openLink: 'agent-host-session://codex/s1', + title: 'title-s1', + status: 'idle', + workingDirectory: 'file:///workspace/app', + }); + store.dispose(); + }); + test('getDeleteSessionArgs validates and refuses the current session', () => { const sessions = [sessionMeta('s1', SessionStatus.Idle, workspace), sessionMeta('s2', SessionStatus.Idle, workspace)]; assert.strictEqual(getDeleteSessionArgs({ session: 'copilot:/s2' }, sessions).toString(), 'copilot:/s2'); diff --git a/src/vs/platform/mcp/common/nativeMcpDiscoveryHelper.ts b/src/vs/platform/mcp/common/nativeMcpDiscoveryHelper.ts index 31fc9e2604a651..71bab1a57bd5c7 100644 --- a/src/vs/platform/mcp/common/nativeMcpDiscoveryHelper.ts +++ b/src/vs/platform/mcp/common/nativeMcpDiscoveryHelper.ts @@ -15,6 +15,7 @@ export interface INativeMcpDiscoveryData { // platform and homedir are duplicated by the remote/native environment, but here for convenience platform: Platform; homedir: URI; + copilotHome?: URI; winAppData?: URI; xdgHome?: URI; } diff --git a/src/vs/platform/mcp/node/nativeMcpDiscoveryHelperService.ts b/src/vs/platform/mcp/node/nativeMcpDiscoveryHelperService.ts index 987f25f9a7cd33..71e4cca181566b 100644 --- a/src/vs/platform/mcp/node/nativeMcpDiscoveryHelperService.ts +++ b/src/vs/platform/mcp/node/nativeMcpDiscoveryHelperService.ts @@ -4,30 +4,30 @@ *--------------------------------------------------------------------------------------------*/ import { homedir } from 'os'; -import { platform } from '../../../base/common/platform.js'; +import { IProcessEnvironment, platform } from '../../../base/common/platform.js'; import { URI } from '../../../base/common/uri.js'; import { INativeMcpDiscoveryData, INativeMcpDiscoveryHelperService } from '../common/nativeMcpDiscoveryHelper.js'; export class NativeMcpDiscoveryHelperService implements INativeMcpDiscoveryHelperService { declare readonly _serviceBrand: undefined; - constructor() { } + constructor(private readonly environment: IProcessEnvironment = process.env) { } load(): Promise { return Promise.resolve({ platform, homedir: URI.file(homedir()), + copilotHome: this.uriFromEnvVariable('COPILOT_HOME'), winAppData: this.uriFromEnvVariable('APPDATA'), xdgHome: this.uriFromEnvVariable('XDG_CONFIG_HOME'), }); } private uriFromEnvVariable(varName: string) { - const envVar = process.env[varName]; + const envVar = this.environment[varName]; if (!envVar) { return undefined; } return URI.file(envVar); } } - diff --git a/src/vs/platform/mcp/test/node/nativeMcpDiscoveryHelperService.test.ts b/src/vs/platform/mcp/test/node/nativeMcpDiscoveryHelperService.test.ts new file mode 100644 index 00000000000000..4f1f2d350ce5d6 --- /dev/null +++ b/src/vs/platform/mcp/test/node/nativeMcpDiscoveryHelperService.test.ts @@ -0,0 +1,30 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { NativeMcpDiscoveryHelperService } from '../../node/nativeMcpDiscoveryHelperService.js'; + +suite('NativeMcpDiscoveryHelperService', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('reads native configuration roots from the process environment', async () => { + const data = await new NativeMcpDiscoveryHelperService({ + COPILOT_HOME: '/custom/copilot', + APPDATA: '/custom/app-data', + XDG_CONFIG_HOME: '/custom/config', + }).load(); + + assert.deepStrictEqual({ + copilotHome: data.copilotHome?.path, + winAppData: data.winAppData?.path, + xdgHome: data.xdgHome?.path, + }, { + copilotHome: '/custom/copilot', + winAppData: '/custom/app-data', + xdgHome: '/custom/config', + }); + }); +}); diff --git a/src/vs/platform/theme/test/common/testThemeService.ts b/src/vs/platform/theme/test/common/testThemeService.ts index 817d76b3e435fe..f7486a084a8697 100644 --- a/src/vs/platform/theme/test/common/testThemeService.ts +++ b/src/vs/platform/theme/test/common/testThemeService.ts @@ -93,6 +93,11 @@ export class TestThemeService implements IThemeService { return this._fileIconTheme; } + setFileIconTheme(theme: IFileIconTheme): void { + this._fileIconTheme = theme; + this._onFileIconThemeChange.fire(theme); + } + public get onDidFileIconThemeChange(): Event { return this._onFileIconThemeChange.event; } diff --git a/src/vs/sessions/browser/media/workbench.css b/src/vs/sessions/browser/media/workbench.css index 9a562e6a26ebb8..cd17ad8eec366d 100644 --- a/src/vs/sessions/browser/media/workbench.css +++ b/src/vs/sessions/browser/media/workbench.css @@ -27,10 +27,6 @@ height: calc(100% - var(--vscode-agents-layout-floatingPanelGap)); } -.monaco-workbench.agent-sessions-workbench.nosidebar > .monaco-grid-view { - margin-left: var(--vscode-agents-layout-floatingPanelGap); -} - .monaco-workbench.agent-sessions-workbench.shell-gradient-background { position: relative; isolation: isolate; diff --git a/src/vs/sessions/browser/workbench.ts b/src/vs/sessions/browser/workbench.ts index 228d91f26e6708..fa0dea88f8d383 100644 --- a/src/vs/sessions/browser/workbench.ts +++ b/src/vs/sessions/browser/workbench.ts @@ -1876,9 +1876,9 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic protected _layoutGrid(): void { const mobileTopBarHeight = this.mobileTopBarElement?.offsetHeight ?? 0; - // Keep in sync with the desktop grid margin in workbench.css. + // Keep the desktop grid margin stable when sidebar visibility changes. const isPhone = this.layoutPolicy.viewportClass.get() === 'phone'; - const gridGutterW = isPhone ? 0 : AGENTS_FLOATING_PANEL_GAP + (this.partVisibility.sidebar ? 0 : AGENTS_FLOATING_PANEL_GAP); + const gridGutterW = isPhone ? 0 : AGENTS_FLOATING_PANEL_GAP; const gridGutterH = isPhone ? 0 : AGENTS_FLOATING_PANEL_GAP; this.workbenchGrid.layout( this._mainContainerDimension.width - gridGutterW, diff --git a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts index addcd03f330ed2..6d1edb789dcb4b 100644 --- a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts +++ b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts @@ -266,8 +266,8 @@ class CollapseAllSessionChangesDiffsAction extends Action2 { icon: Codicon.collapseAll, f1: false, menu: { - id: Menus.SessionsEditorTitle, - group: '1_diff', + id: Menus.SessionsEditorHeaderLayout, + group: 'secondary/1_diff', order: 10, when: ContextKeyExpr.and( singlePaneChangesEditorTitleVisible, @@ -296,8 +296,8 @@ class ExpandAllSessionChangesDiffsAction extends Action2 { icon: Codicon.expandAll, f1: false, menu: { - id: Menus.SessionsEditorTitle, - group: '1_diff', + id: Menus.SessionsEditorHeaderLayout, + group: 'secondary/1_diff', order: 10, when: ContextKeyExpr.and( singlePaneChangesEditorActive, @@ -325,7 +325,7 @@ registerAction2(ExpandAllSessionChangesDiffsAction); // The action changes the preferred layout. Side by side still falls back to inline // when the editor is narrow, so the label must not promise an immediate layout. -MenuRegistry.appendMenuItem(Menus.SessionsEditorTitle, { +MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderLayout, { command: { id: TOGGLE_DIFF_SIDE_BY_SIDE, title: localize('alwaysShowInlineDiff', "Always Show Inline Diff"), @@ -333,7 +333,7 @@ MenuRegistry.appendMenuItem(Menus.SessionsEditorTitle, { icon: Codicon.diffSidebyside, toggled: SessionsDiffRenderSideBySideContext.negate(), }, - group: '1_diff', + group: 'secondary/1_diff', order: 20, when: singlePaneDiffEditorTitleVisible }); diff --git a/src/vs/sessions/contrib/changes/browser/media/sessionChangesEditor.css b/src/vs/sessions/contrib/changes/browser/media/sessionChangesEditor.css index abab6370bdc2ad..e0ef7f03e2b8d4 100644 --- a/src/vs/sessions/contrib/changes/browser/media/sessionChangesEditor.css +++ b/src/vs/sessions/contrib/changes/browser/media/sessionChangesEditor.css @@ -82,7 +82,7 @@ * `.editor-group-header-primary-actions`). It remains fully interactive/clickable, so no * hover-background override is needed here -- it keeps the normal toolbar hover. */ .changes-diff-stats-action-rich { - flex: 1 1 auto; + flex: 1 0 auto; min-width: 0; } @@ -135,6 +135,18 @@ /* Branch Changes picker (label + chevron) styling, keyed off the item's own * marker class so its chevron stays compact and vertically centered in both the * classic internal changes-editor header and the single-pane editor-group header. */ +.changes-picker-action-rich { + flex: 0 1 auto; + min-width: 0; + max-width: 100%; +} + +.changes-picker-action-rich > .monaco-dropdown, +.changes-picker-action-rich > .monaco-dropdown > .dropdown-label { + min-width: 0; + max-width: 100%; +} + .changes-picker-action-rich .action-label { display: inline-flex; align-items: center; @@ -145,10 +157,11 @@ } .changes-picker-action-rich .action-label > span:not(.codicon) { + flex: 1 1 auto; + min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; - min-width: 0; } .changes-picker-action-rich .action-label > .codicon { diff --git a/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts b/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts index 0edea0d059055f..bb1bd3d93546ff 100644 --- a/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts +++ b/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts @@ -36,7 +36,7 @@ export class SessionsChangesAccessibilityHelp implements IAccessibleViewImplemen content.push(localize('sessionsChanges.viewMode', "The Changes view can show files as a tree or a flat list. Use the view's toolbar actions to switch between Tree and List modes.")); content.push(localize('sessionsChanges.operations', "When available, the Changes toolbar or editor title bar also provides actions to commit, merge, sync, or create a pull request. When Agent Merge is the primary action, activate it to toggle Agent Merge and use its dropdown to configure it. Use Tab and Shift+Tab to move between the file list and toolbar actions.")); content.push(layoutService.isSinglePaneLayoutEnabled - ? localize('sessionsChanges.diffView.singlePane', "File diffs can prefer side-by-side or inline layout. Unless screen reader optimized mode is enabled, side-by-side diffs automatically use inline layout when space is limited. Use Always Show Inline Diff in the editor title bar's More Actions menu, or use the Toggle Preferred Diff View command to switch the preference{0}.", '') + ? localize('sessionsChanges.diffView.singlePane', "File diffs can prefer side-by-side or inline layout. Unless screen reader optimized mode is enabled, side-by-side diffs automatically use inline layout when space is limited. Use Always Show Inline Diff in the editor header's More Actions menu, or use the Toggle Preferred Diff View command to switch the preference{0}.", '') : localize('sessionsChanges.diffView.classic', "File diffs can use side-by-side or inline layout. Use Inline View in the editor title area's More Actions menu, or use the Toggle Inline View command to switch the layout{0}.", '')); return new AccessibleContentProvider( diff --git a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts index 8687a09cd6ca75..454f6d49b384c1 100644 --- a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts +++ b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts @@ -135,12 +135,12 @@ suite('Changes View Actions', () => { ]); }); - test('collapse all diffs is contributed to the editor title bar overflow menu', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) + test('collapse all diffs is contributed to the editor header layout overflow menu', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderLayout) .filter(isIMenuItem) .find(item => item.command.id === 'workbench.action.agentSessions.collapseAllDiffs'); - assert.ok(item, 'expected collapse all diffs action in the editor title bar overflow menu'); + assert.ok(item, 'expected collapse all diffs action in the editor header layout overflow menu'); const when = item.when?.serialize() ?? ''; assert.deepStrictEqual({ group: item.group, @@ -151,7 +151,7 @@ suite('Changes View Actions', () => { hasSinglePaneConfigGate: when.includes(SinglePaneLayoutEnabledContext.key), hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), }, { - group: '1_diff', + group: 'secondary/1_diff', order: 10, icon: Codicon.collapseAll.id, hasSessionsWindowGate: true, @@ -161,12 +161,12 @@ suite('Changes View Actions', () => { }); }); - test('expand all diffs is contributed to the editor title bar overflow menu', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) + test('expand all diffs is contributed to the editor header layout overflow menu', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderLayout) .filter(isIMenuItem) .find(item => item.command.id === 'workbench.action.agentSessions.expandAllDiffs'); - assert.ok(item, 'expected expand all diffs action in the editor title bar overflow menu'); + assert.ok(item, 'expected expand all diffs action in the editor header layout overflow menu'); const when = item.when?.serialize() ?? ''; assert.deepStrictEqual({ group: item.group, @@ -178,7 +178,7 @@ suite('Changes View Actions', () => { hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), hasAllCollapsedGate: when.includes(EditorContextKeys.multiDiffEditorAllCollapsed.key), }, { - group: '1_diff', + group: 'secondary/1_diff', order: 10, icon: Codicon.expandAll.id, hasSessionsWindowGate: true, @@ -189,12 +189,12 @@ suite('Changes View Actions', () => { }); }); - test('always show inline diff is contributed to the editor title bar overflow menu for multi-file and single-file diffs', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) + test('always show inline diff is contributed to the editor header layout overflow menu for multi-file and single-file diffs', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderLayout) .filter(isIMenuItem) .find(item => item.command.id === 'toggle.diff.renderSideBySide'); - assert.ok(item, 'expected the preferred diff view action in the editor title bar overflow menu'); + assert.ok(item, 'expected the preferred diff view action in the editor header layout overflow menu'); const when = item.when?.serialize() ?? ''; const toggled = item.command.toggled; const toggledCondition = isICommandActionToggleInfo(toggled) ? toggled.condition : toggled; @@ -228,7 +228,7 @@ suite('Changes View Actions', () => { }, { id: 'toggle.diff.renderSideBySide', title: 'Always Show Inline Diff', - group: '1_diff', + group: 'secondary/1_diff', order: 20, icon: Codicon.diffSidebyside.id, tooltip: 'Always uses inline layout.', @@ -286,7 +286,7 @@ suite('Changes View Actions', () => { } test('Changes accessibility help describes the single-pane diff action', () => { - assert.strictEqual(getChangesAccessibilityHelp(true).includes('Use Always Show Inline Diff in the editor title bar\'s More Actions menu'), true); + assert.strictEqual(getChangesAccessibilityHelp(true).includes('Use Always Show Inline Diff in the editor header\'s More Actions menu'), true); }); test('Changes accessibility help describes the classic diff action', () => { diff --git a/src/vs/sessions/contrib/chat/browser/chatView.ts b/src/vs/sessions/contrib/chat/browser/chatView.ts index 750e7aec47a58b..772a21f6d48e86 100644 --- a/src/vs/sessions/contrib/chat/browser/chatView.ts +++ b/src/vs/sessions/contrib/chat/browser/chatView.ts @@ -30,7 +30,7 @@ import { ServiceCollection } from '../../../../platform/instantiation/common/ser import { EDITOR_DRAG_AND_DROP_BACKGROUND } from '../../../../workbench/common/theme.js'; import { chatPersistentContentVisibleClass, ChatWidget } from '../../../../workbench/contrib/chat/browser/widget/chatWidget.js'; import { setModelPreservingInputTypedWhileLoading } from '../../../../workbench/contrib/chat/browser/chat.js'; -import { IChatModelReference, IChatService } from '../../../../workbench/contrib/chat/common/chatService/chatService.js'; +import { IChatModelReference, IChatService, ResponseModelState } from '../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { isChatTranscriptContextVariableEntry, IChatRequestTranscriptContextVariableEntry, IChatRequestVariableEntry } from '../../../../workbench/contrib/chat/common/attachments/chatVariableEntries.js'; import { IChatModel } from '../../../../workbench/contrib/chat/common/model/chatModel.js'; import { ChatAgentLocation, ChatModeKind } from '../../../../workbench/contrib/chat/common/constants.js'; @@ -338,6 +338,11 @@ export class ChatView extends AbstractChatView { const activity = typeof statusMessage === 'string' ? statusMessage : statusMessage ? renderAsPlaintext(statusMessage) : undefined; const model = chatModel.read(reader); let showProgress: boolean; + let requestCount = 0; + let visibleRequestCount = 0; + let hiddenRequestIncomplete: boolean | undefined; + let hiddenRequestState: ResponseModelState | undefined; + let readyMessage: string | undefined; if (!resource) { showProgress = false; } else if (!model) { @@ -345,14 +350,17 @@ export class ChatView extends AbstractChatView { } else { const requests = model.getRequests(); const lastRequest = model.lastRequestObs.read(reader); - const visibleRequestCount = requests.filter(request => !request.isRequestHiddenFromTranscript).length; - const hiddenRequestIncomplete = lastRequest?.isRequestHiddenFromTranscript - ? lastRequest.response?.isIncomplete.read(reader) - : undefined; - showProgress = shouldShowTranscriptPreparationProgress(requests.length, visibleRequestCount, hiddenRequestIncomplete); + requestCount = requests.length; + visibleRequestCount = requests.filter(request => !request.isRequestHiddenFromTranscript).length; + const hiddenResponse = lastRequest?.isRequestHiddenFromTranscript ? lastRequest.response : undefined; + hiddenRequestIncomplete = hiddenResponse?.isIncomplete.read(reader); + hiddenRequestState = hiddenResponse?.state; + readyMessage = findTranscriptContextEntry(requests.filter(request => request.isHiddenFromTranscript))?.readyMessage?.trim(); + showProgress = shouldShowTranscriptPreparationProgress(requestCount, visibleRequestCount, hiddenRequestIncomplete); } - const progress = getTranscriptProgress(showProgress, activity); - this._widget.setTranscriptProgress(progress, progress); + const showCompletion = shouldShowTranscriptPreparationCompletion(requestCount, visibleRequestCount, hiddenRequestState, readyMessage); + const progress = showCompletion ? readyMessage : getTranscriptProgress(showProgress, activity); + this._widget.setTranscriptProgress(progress, progress, showCompletion ? { complete: true } : undefined); })); } @@ -652,6 +660,10 @@ export function shouldShowTranscriptPreparationProgress(requestCount: number, vi return requestCount === 0 || (visibleRequestCount === 0 && hiddenRequestIncomplete !== false); } +export function shouldShowTranscriptPreparationCompletion(requestCount: number, visibleRequestCount: number, hiddenRequestState: ResponseModelState | undefined, readyMessage: string | undefined): boolean { + return requestCount > 0 && visibleRequestCount === 0 && hiddenRequestState === ResponseModelState.Complete && !!readyMessage; +} + export function getTranscriptProgress(showProgress: boolean, activity: string | undefined): string | undefined { if (!showProgress) { return undefined; diff --git a/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts b/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts index 70581bab8a8253..a161588cb102ae 100644 --- a/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts @@ -13,9 +13,10 @@ import { CHAT_WIDGET_VIEW_STATE_CACHE_LIMIT } from '../../../../../workbench/con import { IChatRequestTranscriptContextVariableEntry } from '../../../../../workbench/contrib/chat/common/attachments/chatVariableEntries.js'; import { ChatInputNoticeHost, ChatInputNoticeLane } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputNoticeHost.js'; import { isChatInputStackSlotShowing } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputStack.js'; +import { ResponseModelState } from '../../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { SessionStatus } from '../../../../services/sessions/common/session.js'; import { SessionsChatBackgroundRenderer } from '../../../../services/chatBackground/browser/chatBackgroundRenderer.js'; -import { findInitialTranscriptContextEntry, findTranscriptContextEntry, getTranscriptProgress, NewChatView, shouldShowSessionChatTip, shouldShowTranscriptPreparationProgress } from '../../browser/chatView.js'; +import { findInitialTranscriptContextEntry, findTranscriptContextEntry, getTranscriptProgress, NewChatView, shouldShowSessionChatTip, shouldShowTranscriptPreparationCompletion, shouldShowTranscriptPreparationProgress } from '../../browser/chatView.js'; import { SessionsChatViewStateService } from '../../browser/chatViewStateService.js'; import { NewChatInSessionWidget } from '../../browser/newChatInSessionWidget.js'; import { NewChatWidget } from '../../browser/newChatWidget.js'; @@ -627,6 +628,24 @@ suite('Sessions - Chat View', () => { }); }); + test('shows transcript preparation completion until visible content appears', () => { + assert.deepStrictEqual({ + hiddenComplete: shouldShowTranscriptPreparationCompletion(1, 0, ResponseModelState.Complete, 'Session ready'), + hiddenPending: shouldShowTranscriptPreparationCompletion(1, 0, ResponseModelState.Pending, 'Session ready'), + hiddenFailed: shouldShowTranscriptPreparationCompletion(1, 0, ResponseModelState.Failed, 'Session ready'), + hiddenCancelled: shouldShowTranscriptPreparationCompletion(1, 0, ResponseModelState.Cancelled, 'Session ready'), + visibleRequest: shouldShowTranscriptPreparationCompletion(2, 1, ResponseModelState.Complete, 'Session ready'), + noReadyMessage: shouldShowTranscriptPreparationCompletion(1, 0, ResponseModelState.Complete, undefined), + }, { + hiddenComplete: true, + hiddenPending: false, + hiddenFailed: false, + hiddenCancelled: false, + visibleRequest: false, + noReadyMessage: false, + }); + }); + test('shows the session-list status message in the pre-request progress surface', () => { assert.deepStrictEqual({ fallback: getTranscriptProgress(true, 'Working...'), diff --git a/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts b/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts index db89b36aced01b..840484f5a354bc 100644 --- a/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts +++ b/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts @@ -71,7 +71,7 @@ class RunSessionCodeReviewAction extends Action2 { when: codeReviewChangesToolbarWhen, }, { - id: Menus.SessionsEditorTitle, + id: Menus.SessionsEditorHeaderLayout, group: 'navigation', order: 10, when: singlePaneCodeReviewWhen, diff --git a/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts b/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts index 7f8f11c8b799e5..f5f6016555973a 100644 --- a/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts +++ b/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts @@ -415,24 +415,24 @@ suite('Code Review Contributions', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); - test('Run Code Review is contributed to the editor title bar', () => { - const titleItem = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) + test('Run Code Review is contributed to the editor header layout actions', () => { + const headerItem = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderLayout) .filter(isIMenuItem) .find(item => item.command.id === 'sessions.codeReview.run'); - assert.ok(titleItem, 'expected Run Code Review in the editor title bar'); - const when = titleItem.when?.serialize() ?? ''; + assert.ok(headerItem, 'expected Run Code Review in the editor header layout actions'); + const when = headerItem.when?.serialize() ?? ''; const enablementContext = new Context(1, null); enablementContext.setValue(ChatContextKeys.hasAgentSessionChanges.key, false); enablementContext.setValue(SessionHasChangesContext.key, true); - const enabledFromSessionChanges = titleItem.command.precondition?.evaluate(enablementContext); + const enabledFromSessionChanges = headerItem.command.precondition?.evaluate(enablementContext); enablementContext.setValue(ChatContextKeys.hasAgentSessionChanges.key, true); enablementContext.setValue(SessionHasChangesContext.key, false); assert.deepStrictEqual({ - group: titleItem.group, - order: titleItem.order, + group: headerItem.group, + order: headerItem.order, enabledFromSessionChanges, - enabledFromChatChanges: titleItem.command.precondition?.evaluate(enablementContext), + enabledFromChatChanges: headerItem.command.precondition?.evaluate(enablementContext), hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key), hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditorInput.EDITOR_ID), hasSinglePaneLayoutGate: when.includes(SinglePaneLayoutEnabledContext.key), diff --git a/src/vs/sessions/contrib/editor/browser/media/editorHeader.css b/src/vs/sessions/contrib/editor/browser/media/editorHeader.css index 6cb4d3ac65e254..9c40d41d995b9f 100644 --- a/src/vs/sessions/contrib/editor/browser/media/editorHeader.css +++ b/src/vs/sessions/contrib/editor/browser/media/editorHeader.css @@ -33,6 +33,7 @@ align-items: center; min-width: 0; flex: 1 1 auto; + gap: var(--vscode-spacing-size20, 2px); } .agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-primary-actions { @@ -63,6 +64,9 @@ flex-shrink: 0; } +.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-primary-actions > .monaco-toolbar, +.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-primary-actions > .monaco-toolbar > .monaco-action-bar, +.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-primary-actions > .monaco-toolbar > .monaco-action-bar > .actions-container, .agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar, .agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar > .monaco-action-bar, .agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar > .monaco-action-bar > .actions-container { diff --git a/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts b/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts index 9b380ba20de85c..69e038687094fb 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestPicker.ts @@ -185,6 +185,7 @@ export function createPullRequestContextAttachment(context: IGitHubPullRequestCo icon: Codicon.gitPullRequest, uri: URI.parse(context.url), tooltip: localize('pullRequest.context.tooltip', "Pull request #{0} by @{1}", context.number, context.author), + readyMessage: localize('pullRequest.sessionReady', "Session ready. Pull request #{0} is checked out and attached.", context.number), }; } diff --git a/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts b/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts index d4479d95506b33..9147bb8cd88341 100644 --- a/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts +++ b/src/vs/sessions/contrib/github/test/browser/pullRequestPicker.test.ts @@ -165,6 +165,7 @@ suite('Create Session from Pull Request', () => { fullName: attachment.fullName, icon: attachment.icon?.id, uri: attachment.uri.toString(), + readyMessage: attachment.readyMessage, value: JSON.parse(attachment.value ?? ''), }, { kind: 'transcriptContext', @@ -172,6 +173,7 @@ suite('Create Session from Pull Request', () => { fullName: '#42 Improve sessions', icon: 'git-pull-request', uri: 'https://github.com/owner/repo/pull/42', + readyMessage: 'Session ready. Pull request #42 is checked out and attached.', value: { usageInstructions: 'Use this snapshot as the primary source for questions about the pull request. Do not fetch pull request data or run tools unless the user explicitly asks for refreshed information or the requested information is absent from this snapshot.', owner: 'owner', diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts index a62240a1737306..9358174e3acbc6 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts @@ -25,6 +25,7 @@ import { ITelemetryService } from '../../../../../platform/telemetry/common/tele import type { AutomationRunTrigger, AutomationTarget, IAutomationDescriptor, IAutomationRun, IAutomationSchedule } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; import { AutomationActiveRunError, type AutomationMutationGuard, type IAutomationRunClaim, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, isAutomationActiveRunError, serializeAutomationEditableState, type IUpdateAutomationOptions, type IUpdateAutomationRunOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; import { publishAutomationMigration } from '../../../../../workbench/contrib/chat/common/automations/automationTelemetry.js'; +import { ChatPermissionLevel } from '../../../../../workbench/contrib/chat/common/constants.js'; import type { IAutomation, IAutomationSnapshotImportResult, IGuardedAutomationSnapshotRemovalResult, ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js'; import { IAutomationStorageService } from '../../../automations/common/automationStorageService.js'; @@ -833,7 +834,7 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro const config = { ...existing?.session.config }; const provider = descriptor.target.sessionTypeId ?? this._providerFromModelId(descriptor.modelId); setOptional(config, SessionConfigKey.Mode, descriptor.mode); - setOptional(config, SessionConfigKey.AutoApprove, descriptor.permissionLevel); + setOptional(config, SessionConfigKey.AutoApprove, descriptor.permissionLevel === ChatPermissionLevel.Autopilot ? ChatPermissionLevel.Assisted : descriptor.permissionLevel); if (descriptor.target.kind === 'workspace') { setOptional(config, SessionConfigKey.Isolation, descriptor.target.isolation.kind === 'default' ? undefined : descriptor.target.isolation.kind); setOptional(config, SessionConfigKey.Branch, descriptor.target.isolation.kind === 'worktree' ? descriptor.target.isolation.branch : undefined); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts index c59a143d263cbc..6e2d6378122d81 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts @@ -55,9 +55,10 @@ const agentMergeHasPullRequest = ContextKeyExpr.or( const agentMergeMenuPrecondition = ContextKeyExpr.and(agentMergeCommandPrecondition, agentMergeHasPullRequest); /** - * Agent Merge owns the primary button only when neither marking the pull - * request ready nor merging it applies — the states the user is otherwise left - * waiting in, including a blocked pull request that offers no operation at all. + * Agent Merge owns the primary button while an enabled draft is still waiting + * for CI or review comments. The host advertises a distinct Mark Ready + * operation in that state so it remains available in the dropdown; once the + * pull request is ready, the normal Mark Ready operation takes over. * * The auto-merge states are included because Agent Merge replaces them on the * button: it subsumes "let this merge on its own once it is ready", and the @@ -68,6 +69,10 @@ const agentMergeMenuPrecondition = ContextKeyExpr.and(agentMergeCommandPrecondit const agentMergeOwnsPrimaryButton = ContextKeyExpr.or( ContextKeyExpr.equals(SessionPrimaryPullRequestOperationContext.key, AgentHostPullRequestOperationId.EnableAutoMerge), ContextKeyExpr.equals(SessionPrimaryPullRequestOperationContext.key, AgentHostPullRequestOperationId.DisableAutoMerge), + ContextKeyExpr.and( + ContextKeyExpr.equals(SessionPrimaryPullRequestOperationContext.key, AgentHostPullRequestOperationId.MarkReadyWithAgentMerge), + SessionAgentMergeEnabledContext, + ), ContextKeyExpr.and(SessionHasOpenPullRequestContext, ContextKeyExpr.equals(SessionPrimaryPullRequestOperationContext.key, '')), ); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts index b355573ab8cd58..ef15dc40a8eb5d 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts @@ -405,6 +405,7 @@ suite('AgentHostAutomationStore', () => { prompt: 'Review the current changes.', schedule: { interval: 'daily', scheduleHour: 9, scheduleMinute: 30, scheduleDay: 0 }, target: { kind: 'quickChat', providerId: 'local-agent-host', sessionTypeId: 'mock' }, + permissionLevel: 'autopilot', }); const create = connection.dispatched[0].action; const trigger = create.type === ActionType.AutomationCreateRequested ? create.definition.triggers[0] : undefined; @@ -413,6 +414,7 @@ suite('AgentHostAutomationStore', () => { subscribedChannel: connection.subscribedChannel, dispatchChannel: connection.dispatched[0].channel, definitionMeta: create.type === ActionType.AutomationCreateRequested ? create.definition._meta : undefined, + sessionConfig: create.type === ActionType.AutomationCreateRequested ? create.definition.session.config : undefined, triggerExpression: trigger?.kind === AutomationTriggerKind.Schedule ? trigger.schedule.expression : undefined, automation: { name: automation.name, @@ -425,6 +427,7 @@ suite('AgentHostAutomationStore', () => { subscribedChannel: URI.parse(AUTOMATION_CATALOG_URI).toString(), dispatchChannel: AUTOMATION_CATALOG_URI, definitionMeta: undefined, + sessionConfig: { autoApprove: 'assisted' }, triggerExpression: '30 9 * * *', automation: { name: 'Review changes', diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentMergeActions.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentMergeActions.test.ts index 121cf5307e9e7a..82c2956d4881d2 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentMergeActions.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentMergeActions.test.ts @@ -59,6 +59,18 @@ suite('Agent Merge Actions', () => { }); }); + test('Agent Merge owns a draft pull request until CI and review comments are ready', () => { + assert.deepStrictEqual({ + agentMergeMarkReady: ownsPrimaryButton({ primaryOperation: AgentHostPullRequestOperationId.MarkReadyWithAgentMerge, agentMergeEnabled: true }), + staleAgentMergeMarkReady: ownsPrimaryButton({ primaryOperation: AgentHostPullRequestOperationId.MarkReadyWithAgentMerge, agentMergeEnabled: false }), + normalMarkReady: ownsPrimaryButton({ primaryOperation: AgentHostPullRequestOperationId.MarkReady, agentMergeEnabled: true }), + }, { + agentMergeMarkReady: true, + staleAgentMergeMarkReady: false, + normalMarkReady: false, + }); + }); + test('Agent Merge leaves the primary button to the operation that moves the pull request along', () => { assert.deepStrictEqual({ markReady: ownsPrimaryButton({ primaryOperation: AgentHostPullRequestOperationId.MarkReady, agentMergeEnabled: true }), diff --git a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css index facf3410541846..75e27e16a6b68e 100644 --- a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css +++ b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css @@ -193,6 +193,7 @@ box-sizing: border-box; position: relative; overflow: visible; + min-width: 0; padding: 8px 6px 8px 12px; &.archived { @@ -507,6 +508,7 @@ box-sizing: border-box; position: relative; overflow: visible; + min-width: 0; padding: 0 var(--vscode-spacing-size120) 0 var(--vscode-spacing-size360); color: var(--vscode-foreground); font-size: var(--vscode-fontSize-body1); diff --git a/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts b/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts index 9513284e6dde91..6d415191959cda 100644 --- a/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts @@ -44,6 +44,7 @@ import { IAction } from '../../../../base/common/actions.js'; import { OS } from '../../../../base/common/platform.js'; import { IEnvironmentService } from '../../../../platform/environment/common/environment.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; +import { IProductService } from '../../../../platform/product/common/productService.js'; import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { asCssVariable } from '../../../../platform/theme/common/colorRegistry.js'; import { defaultButtonStyles } from '../../../../platform/theme/browser/defaultStyles.js'; @@ -1329,6 +1330,7 @@ export class NewSessionActionViewItemContribution extends Disposable implements @IContextKeyService contextKeyService: IContextKeyService, @IWorkbenchAssignmentService private readonly assignmentService: IWorkbenchAssignmentService, @IEnvironmentService private readonly environmentService: IEnvironmentService, + @IProductService private readonly productService: IProductService, ) { super(); @@ -1359,7 +1361,7 @@ export class NewSessionActionViewItemContribution extends Disposable implements return; } const enabled = await this.assignmentService.getTreatment(NewSessionActionViewItemContribution.NEW_SESSION_TITLEBAR_TREATMENT); - this.titleBarEnabledContext.set(enabled === true); + this.titleBarEnabledContext.set(enabled ?? this.productService.quality === 'insider'); } } diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index fa3818ce480c37..109218084ae841 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -214,9 +214,16 @@ function getSessionListChats(session: ISession, reader?: IReader): readonly ICha ); } -/** Returns the main-chat status for trees with chat rows, otherwise the aggregate session status. */ +/** Returns in-progress when any chat is active, then the main-chat status for trees with chat rows. */ function getSessionRowStatus(session: ISession, reader: IReader | undefined, deriveFromMainChat: boolean): SessionStatus { - return deriveFromMainChat ? session.mainChat.read(reader).status.read(reader) : session.status.read(reader); + const sessionStatus = session.status.read(reader); + if (!deriveFromMainChat) { + return sessionStatus; + } + if (session.chats.read(reader).some(chat => chat.status.read(reader) === SessionStatus.InProgress)) { + return SessionStatus.InProgress; + } + return session.mainChat.read(reader).status.read(reader); } function isSessionGroupItem(item: SessionListItem): item is ISessionGroupItem { diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts index fa4a342a22fe38..526c469fc604b4 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts @@ -1263,6 +1263,30 @@ suite('Sessions - SessionsList', () => { }); }); + test('parent session row shows progress while one non-main chat needs input and another is in progress', () => { + const main = createChat('Main chat'); + const waiting = createChat('Waiting chat', ChatOriginKind.User, ChatInteractivity.Full, SessionStatus.NeedsInput); + const active = createChat('Active chat', ChatOriginKind.User, ChatInteractivity.Full, SessionStatus.InProgress); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + status: constObservable(SessionStatus.NeedsInput), + chats: constObservable([main, waiting, active]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + + const container = renderSessionChats(session, undefined, true); + + assert.deepStrictEqual({ + session: sessionRowSnapshot(container), + hasProgress: !!container.querySelector('.session-item .session-icon > .monaco-pixel-spinner'), + }, { + session: { inProgress: true, needsInput: false, ariaLabel: 'Session, updated now, State: In Progress' }, + hasProgress: true, + }); + }); + test('needs-input chat row gets the same accent-pulse feedback class as a needs-input session row', () => { const main = createChat('Main chat'); const waiting = createChat('Waiting chat', ChatOriginKind.User, ChatInteractivity.Full, SessionStatus.NeedsInput); diff --git a/src/vs/sessions/test/browser/workbench.test.ts b/src/vs/sessions/test/browser/workbench.test.ts index defff36019518b..ee013c65ef0247 100644 --- a/src/vs/sessions/test/browser/workbench.test.ts +++ b/src/vs/sessions/test/browser/workbench.test.ts @@ -70,6 +70,7 @@ suite('Sessions - Workbench', () => { const toggleSecondarySideBar = Workbench.prototype.toggleSecondarySideBar as (this: ITestWorkbench) => void; const restoreSessionsPartOnActivation = Reflect.get(Workbench.prototype, '_restoreSessionsPartOnActivation') as (this: ITestWorkbench) => void; const restoreEditorPartOnActivation = Reflect.get(Workbench.prototype, '_restoreEditorPartOnActivation') as (this: ITestWorkbench) => void; + const layoutGrid = Reflect.get(Workbench.prototype, '_layoutGrid') as (this: IContainerResizeTestHarness) => void; const layoutSinglePaneGrid = Reflect.get(SinglePaneWorkbench.prototype, '_layoutGrid') as (this: IContainerResizeTestHarness) => void; const preserveSessionsEditorRatio = Reflect.get(SinglePaneWorkbench.prototype, '_preserveSessionsEditorRatio') as (this: IProportionalResizeTestHarness, previousSessionsWidth: number, previousEditorWidth: number) => void; const registerNotificationRowHeight = Reflect.get(Workbench.prototype, 'registerNotificationRowHeight') as (this: { @@ -649,6 +650,35 @@ suite('Sessions - Workbench', () => { }); }); + test('sidebar visibility does not change the grid width passed to layout', () => { + const layoutCalls: IViewSize[] = []; + const host: IContainerResizeTestHarness = { + partVisibility: { sidebar: true, editor: false, auxiliaryBar: false }, + mobileTopBarElement: undefined, + layoutPolicy: { viewportClass: { get: () => 'desktop' } }, + _mainContainerDimension: { width: 1200, height: 800 }, + sessionsPartView: { minimumWidth: 300 }, + editorPartView: { minimumWidth: 300 }, + workbenchGrid: { + getViewSize: () => ({ width: 0, height: 0 }), + resizeView: () => { }, + isViewVisible: () => true, + layout: (width, height) => { layoutCalls.push({ width, height }); }, + }, + _runWithEditorResizeSyncSuspended: fn => fn(), + }; + Object.setPrototypeOf(host, Workbench.prototype); + + layoutGrid.call(host); + host.partVisibility.sidebar = false; + layoutGrid.call(host); + + assert.deepStrictEqual(layoutCalls, [ + { width: 1196, height: 796 }, + { width: 1196, height: 796 }, + ]); + }); + test('single-pane sidebar visibility leaves a detail-only pane width unchanged', () => { const host = createHost({ single: true, sideBarWidth: 280, editorWidth: 620, dockedWidth: 300, partVisibility: { sidebar: true, editor: false, auxiliaryBar: true } }); diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts index a8262bc807ae98..3eb9222a1c6777 100644 --- a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts +++ b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts @@ -812,6 +812,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA uri: URI.revive(folder.uri), label: folder.label, source: folder.source, + destinationGroupId: folder.destinationGroupId, })); }, }; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 7f06743cc9d092..f0ccbf4316925a 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1833,6 +1833,7 @@ export interface IChatSessionCustomizationSourceFolderDto { readonly uri: UriComponents; readonly label: string; readonly source: IChatResourceSourceDto; + readonly destinationGroupId?: string; } export interface IChatParticipantMetadata { participant: string; diff --git a/src/vs/workbench/api/common/extHostChatAgents2.ts b/src/vs/workbench/api/common/extHostChatAgents2.ts index 6b03bf70318d35..de2cdb35ebfc24 100644 --- a/src/vs/workbench/api/common/extHostChatAgents2.ts +++ b/src/vs/workbench/api/common/extHostChatAgents2.ts @@ -887,6 +887,7 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS uri: folder.uri, label: folder.label, source: folder.source, + destinationGroupId: folder.destinationGroupId, } satisfies IChatSessionCustomizationSourceFolderDto)); } catch (err) { return undefined; diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationItemProvider.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationItemProvider.ts index c0c6dc804d0ef3..eabc546178fc08 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationItemProvider.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationItemProvider.ts @@ -206,6 +206,7 @@ export class AgentCustomizationItemProvider extends Disposable implements ICusto uri: this.toRemoteUri(customization.uri), label: customization.name, source, + destinationGroupId: dirname(this.toRemoteUri(customization.uri)).toString(), }); } return folders; diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index 8b041af73c6bb1..44d02bf689e634 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -484,14 +484,22 @@ export function systemNotificationToChatPart(content: StringOrMarkdown | undefin return meta.severity === AgentSystemNotificationSeverity.Warning ? { kind: 'warning', content: markdown } : { kind: 'systemNotification', content: markdown }; - // Agent Merge reports a state change rather than a completed step, so the - // default check would misdescribe both of these. + case AgentSystemNotificationKind.AutomaticApprovalReviewTimedOut: + return { kind: 'systemNotification', content: markdown, icon: Codicon.clock, collapsible: true }; + case AgentSystemNotificationKind.AutomaticApprovalReviewAborted: + return { kind: 'systemNotification', content: markdown, icon: Codicon.circleSlash, collapsible: true }; + case AgentSystemNotificationKind.AutomaticApprovalReviewInterrupted: + return { kind: 'systemNotification', content: markdown, icon: Codicon.warning }; + // Agent Merge state changes use icons that describe the transition rather + // than the default completed-step check. case AgentSystemNotificationKind.AgentMergeEnabled: - return { kind: 'systemNotification', content: markdown, icon: Codicon.gitMerge, collapsible: true }; + return { kind: 'systemNotification', content: markdown, icon: Codicon.gitMerge, collapsible: true, renderInlineTiming: true }; case AgentSystemNotificationKind.AgentMergeConfigurationChanged: - return { kind: 'systemNotification', content: markdown, icon: Codicon.settingsGear, collapsible: true }; + return { kind: 'systemNotification', content: markdown, icon: Codicon.settingsGear, collapsible: true, renderInlineTiming: true }; case AgentSystemNotificationKind.AgentMergeDisabled: - return { kind: 'systemNotification', content: markdown, icon: Codicon.circleSlash }; + return { kind: 'systemNotification', content: markdown, icon: Codicon.circleSlash, renderInlineTiming: true }; + case AgentSystemNotificationKind.AgentMergePullRequestMerged: + return { kind: 'systemNotification', content: markdown, icon: Codicon.gitMerge, renderInlineTiming: true }; default: return { kind: 'systemNotification', content: markdown }; } @@ -2483,7 +2491,9 @@ export function toolCallStateToStreamingInvocation(tc: ToolCallState, subAgentIn } else if (isRenameChatTool(tc)) { invocation.presentation = ToolInvocationPresentation.Hidden; } - if (sessionResource && isSubagentTool(tc)) { + if (getToolKind(tc) === 'search') { + invocation.toolSpecificData = { kind: 'search' }; + } else if (sessionResource && isSubagentTool(tc)) { invocation.toolSpecificData = toolCallStateToInvocation(tc, subAgentInvocationId, sessionResource, connectionAuthority ?? '', mcpServerAuthority).toolSpecificData; } return invocation; diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts index e5874fbaf1428b..90e17c1db674bb 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts @@ -102,7 +102,7 @@ import { EmbeddedExtensionToolsDetail } from './embeddedExtensionToolsDetail.js' import { ICustomizationHarnessService, type ICustomizationSourceFolder } from '../../common/customizationHarnessService.js'; import { ChatConfiguration } from '../../common/constants.js'; import { AICustomizationWelcomePage, type ICustomizationMigrationCategorySummary } from './aiCustomizationWelcomePage.js'; -import { type CustomizationMigrationTargetFolders, migrateCustomizations } from './customizationMigration.js'; +import { type CustomizationMigrationTargetFolders, type IMigratedCustomizationsResult, migrateCustomizations } from './customizationMigration.js'; import { CUSTOMIZATION_MIGRATION_CATEGORIES, CustomizationMigrationCategoryId, getCustomizationMigrationCategory, type ICustomizationMigrationBanner, type ICustomizationMigrationCategory } from './customizationMigrationCategories.js'; import { IViewsService } from '../../../../services/views/common/viewsService.js'; import { ILabelService } from '../../../../../platform/label/common/label.js'; @@ -386,6 +386,8 @@ export class AICustomizationManagementEditor extends EditorPane { private customizationMigrationRefreshSequence = 0; private customizationMigrationLoading = false; private customizationMigrationLoadError: string | undefined; + private customizationMigrationInProgress = false; + private customizationMigrationWritesInProgress = false; private readonly editorDisposables = this._register(new DisposableStore()); private _editorContentChanged = false; @@ -934,9 +936,7 @@ export class AICustomizationManagementEditor extends EditorPane { this.promptsService.onDidChangeCustomAgents, this.promptsService.onDidChangeInstructions, this.promptsService.onDidChangeAgentInstructions, - )(() => { - void this.refreshCustomizationMigrationInfo(); - })); + )(() => this.refreshCustomizationMigrationInfoFromPromptChange())); this.registerCustomizationMigrationSessionRefresh(); // Container for prompts-based content (Agents, Skills, Instructions, Prompts) @@ -1111,6 +1111,12 @@ export class AICustomizationManagementEditor extends EditorPane { })); } + private refreshCustomizationMigrationInfoFromPromptChange(): void { + if (!this.customizationMigrationWritesInProgress) { + void this.refreshCustomizationMigrationInfo(); + } + } + private async refreshCustomizationMigrationInfo(): Promise { const activeHarnessId = this.harnessService.activeHarness.get(); const activeSessionResource = this.harnessService.activeSessionResource.get(); @@ -1291,67 +1297,89 @@ export class AICustomizationManagementEditor extends EditorPane { } private async migrateSelectedCustomizations(category: ICustomizationMigrationCategory, customizations: readonly MigratableConfiguration[]): Promise { - if (customizations.length === 0 || !this.isMigrationCategoryEnabled(category)) { + if (this.customizationMigrationInProgress || customizations.length === 0 || !this.isMigrationCategoryEnabled(category)) { return; } - const sessionResource = this.harnessService.activeSessionResource.get(); - const targetFolders = await this.resolveCustomizationMigrationTargetFolders(customizations, this.customizationMigrationTargetFoldersByType, sessionResource); - if (!targetFolders || !this.isCustomizationMigrationSessionActive(sessionResource)) { - return; - } + this.customizationMigrationInProgress = true; + this.updateCustomizationMigrationActionState(); + try { + const sessionResource = this.harnessService.activeSessionResource.get(); + const targetFolders = await this.resolveCustomizationMigrationTargetFolders(customizations, this.customizationMigrationTargetFoldersByType, sessionResource); + if (!targetFolders || !this.isCustomizationMigrationSessionActive(sessionResource)) { + return; + } - const confirmation = category.getConfirmation( - customizations, - this.getActiveHarnessLabel(), - this.getCustomizationMigrationDestinationLabel( - [...targetFolders.values()].flatMap(foldersByStorage => [...foldersByStorage.values()]), - ), - ); - const confirmResult = await this.dialogService.confirm({ - type: 'question', - message: confirmation.message, - detail: confirmation.detail, - checkbox: { - label: confirmation.deleteOriginalsLabel, - checked: true, - }, - primaryButton: confirmation.primaryButton, - }); - if (!confirmResult.confirmed || !this.isCustomizationMigrationSessionActive(sessionResource)) { - return; - } + const confirmation = category.getConfirmation( + customizations, + this.getActiveHarnessLabel(), + this.getCustomizationMigrationDestinationLabel( + [...targetFolders.values()].flatMap(foldersByStorage => [...foldersByStorage.values()]), + ), + ); + const confirmResult = await this.dialogService.confirm({ + type: 'question', + message: confirmation.message, + detail: confirmation.detail, + checkbox: { + label: confirmation.deleteOriginalsLabel, + checked: true, + }, + primaryButton: confirmation.primaryButton, + }); + if (!confirmResult.confirmed || !this.isCustomizationMigrationSessionActive(sessionResource)) { + return; + } - const migrationResult = await migrateCustomizations( - customizations, - targetFolders, - this.fileService, - onUnexpectedError, - { deleteOriginalFiles: confirmResult.checkboxChecked !== false }, - ); - const { migratedCount, failedCustomizationFileNames, unsupportedHeaderKeys, migratedCustomizations } = migrationResult; + const deleteOriginalFiles = confirmResult.checkboxChecked !== false; + const migrationResult = await this.runCustomizationMigration(customizations, targetFolders, deleteOriginalFiles); + const { migratedCount, failedCustomizationFileNames, unsupportedHeaderKeys, migratedCustomizations } = migrationResult; - if (failedCustomizationFileNames.length > 0) { - const displayedFileNames = failedCustomizationFileNames.slice(0, 3); - const hiddenFileCount = failedCustomizationFileNames.length - displayedFileNames.length; - this.notificationService.error(category.getFailedMessage(displayedFileNames, hiddenFileCount)); - } + if (failedCustomizationFileNames.length > 0) { + const displayedFileNames = failedCustomizationFileNames.slice(0, 3); + const hiddenFileCount = failedCustomizationFileNames.length - displayedFileNames.length; + this.notificationService.error(category.getFailedMessage(displayedFileNames, hiddenFileCount)); + } - if (migratedCount === 0) { - if (failedCustomizationFileNames.length === 0) { - this.notificationService.warn(category.noFilesMigratedMessage); + if (migratedCount === 0) { + if (failedCustomizationFileNames.length === 0) { + this.notificationService.warn(category.noFilesMigratedMessage); + } + return; + } + + if (deleteOriginalFiles) { + await this.refreshCustomizationMigrationInfo(); } - return; - } - await this.refreshCustomizationMigrationInfo(); + const unsupportedKeysLabel = unsupportedHeaderKeys.join(', '); + this.notificationService.info(unsupportedKeysLabel.length > 0 && category.getMigratedWithReviewMessage + ? category.getMigratedWithReviewMessage(migratedCount, unsupportedKeysLabel) + : category.getMigratedMessage(migratedCount)); - const unsupportedKeysLabel = unsupportedHeaderKeys.join(', '); - this.notificationService.info(unsupportedKeysLabel.length > 0 && category.getMigratedWithReviewMessage - ? category.getMigratedWithReviewMessage(migratedCount, unsupportedKeysLabel) - : category.getMigratedMessage(migratedCount)); + if (deleteOriginalFiles) { + void this.revealMigratedCustomizations(migratedCustomizations); + } + } finally { + this.customizationMigrationInProgress = false; + this.updateCustomizationMigrationActionState(); + } + } - void this.revealMigratedCustomizations(migratedCustomizations); + private async runCustomizationMigration(customizations: readonly MigratableConfiguration[], targetFolders: CustomizationMigrationTargetFolders, deleteOriginalFiles: boolean): Promise { + this.customizationMigrationWritesInProgress = true; + try { + return await migrateCustomizations( + customizations, + targetFolders, + this.fileService, + onUnexpectedError, + { deleteOriginalFiles }, + ); + } finally { + await timeout(0); + this.customizationMigrationWritesInProgress = false; + } } private renderCustomizationMigrationPage(): void { @@ -1628,7 +1656,7 @@ export class AICustomizationManagementEditor extends EditorPane { } const category = this.getActiveMigrationCategory() ?? CUSTOMIZATION_MIGRATION_CATEGORIES[0]; const selectedCount = this.getMigrationCandidates(category).filter(customization => this.isCustomizationSelectedForMigration(customization)).length; - this.migrationMigrateButton.enabled = selectedCount > 0; + this.migrationMigrateButton.enabled = selectedCount > 0 && !this.customizationMigrationInProgress; if (this.migrationSelectedCountElement) { this.migrationSelectedCountElement.textContent = selectedCount === 1 ? localize('customizationMigrationOneSelected', "1 selected") @@ -1696,6 +1724,7 @@ export class AICustomizationManagementEditor extends EditorPane { } const targetFolders = new Map>(); + const selectedDestinationGroupIds = new Map(); for (const [targetType, requiredStorages] of requiredStorageByTargetType) { const availableFolders = availableSourceFolders.get(targetType) ?? []; if (!this.isCustomizationMigrationSessionActive(sessionResource)) { @@ -1709,9 +1738,21 @@ export class AICustomizationManagementEditor extends EditorPane { return undefined; } - const targetFolder = matchingFolders.length === 1 - ? matchingFolders[0] - : await this.pickCustomizationMigrationTargetFolder(matchingFolders, targetType); + const selectedDestinationGroupId = selectedDestinationGroupIds.get(storage); + const foldersAtSelectedDestination = selectedDestinationGroupId + ? matchingFolders.filter(folder => folder.destinationGroupId === selectedDestinationGroupId) + : []; + let targetFolder: ICustomizationSourceFolder | undefined; + if (foldersAtSelectedDestination.length === 1) { + targetFolder = foldersAtSelectedDestination[0]; + } else if (matchingFolders.length === 1) { + targetFolder = matchingFolders[0]; + } else { + targetFolder = await this.pickCustomizationMigrationTargetFolder(matchingFolders, targetType, requiredStorageByTargetType.size > 1); + if (targetFolder?.destinationGroupId) { + selectedDestinationGroupIds.set(storage, targetFolder.destinationGroupId); + } + } if (!targetFolder || !this.isCustomizationMigrationSessionActive(sessionResource)) { return undefined; } @@ -1747,7 +1788,7 @@ export class AICustomizationManagementEditor extends EditorPane { } } - private async pickCustomizationMigrationTargetFolder(sourceFolders: readonly ICustomizationSourceFolder[], targetType: PromptsType): Promise { + private async pickCustomizationMigrationTargetFolder(sourceFolders: readonly ICustomizationSourceFolder[], targetType: PromptsType, selectsMultipleTypes: boolean): Promise { const picks: IMigrationTargetQuickPickItem[] = sourceFolders.map(folder => ({ label: folder.label, description: this.labelService.getUriLabel(folder.uri, { relative: true }), @@ -1756,13 +1797,16 @@ export class AICustomizationManagementEditor extends EditorPane { const selected = await this.quickInputService.pick(picks, { canPickMany: false, - placeHolder: this.getMigrationTargetFolderPlaceholder(targetType), + placeHolder: this.getMigrationTargetFolderPlaceholder(targetType, selectsMultipleTypes), matchOnDescription: true, }); return selected?.folder; } - private getMigrationTargetFolderPlaceholder(targetType: PromptsType): string { + private getMigrationTargetFolderPlaceholder(targetType: PromptsType, selectsMultipleTypes: boolean): string { + if (selectsMultipleTypes) { + return localize('migrationPickCustomizationFolder', "Select a destination for the migrated customizations"); + } switch (targetType) { case PromptsType.skill: return localize('migrationPickSkillFolder', "Select a destination folder for migrated skills"); diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts index 5a5ae3ab13eda2..28aec0ee2b9923 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts @@ -987,15 +987,17 @@ function createBuiltinEntry(server: IMcpServer, activeSessionServer?: AgentHostM }; } -function createInstalledMcpServerDetailInput(entry: IMcpInstalledEntry): IMcpServerDetailInput { +export function createInstalledMcpServerDetailInput(entry: IMcpInstalledEntry): IMcpServerDetailInput { if (entry.type === 'server-item') { return createWorkbenchMcpServerDetailInput(entry.server); } const activeSessionServer = getActiveSessionServer(entry); const localServer = entry.type === 'session-server-item' ? undefined : entry.localServer; - const localDefinition = localServer?.readDefinitions().get().server; - const localSource = localDefinition?.presentation?.origin; + const localDefinitions = localServer?.readDefinitions().get(); + const localDefinition = localDefinitions?.server; + const collectionOrigin = localDefinitions?.collection?.presentation?.origin; + const localSource = localDefinition?.presentation?.origin ?? (collectionOrigin ? { uri: collectionOrigin } : undefined); const activeSessionSource = activeSessionServer?.sourceUri ? { uri: activeSessionServer.sourceUri, diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css index 8b5104713385df..766e6d6603db51 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css @@ -961,6 +961,9 @@ per-word capitalization does not survive translation. */ position: relative; flex: 1; min-height: 0; + width: 100%; + max-width: calc(840px + var(--vscode-spacing-size160)); + margin-inline: auto; } .ai-customization-management-editor .prompt-migration-footer { @@ -1068,7 +1071,8 @@ per-word capitalization does not survive translation. */ display: flex; flex-direction: column; overflow: hidden; - padding-bottom: var(--vscode-spacing-size60); + padding: 0 var(--vscode-spacing-size160) var(--vscode-spacing-size60) 0; + box-sizing: border-box; } .ai-customization-management-editor .prompt-migration-group { diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts index 57c594184b4f19..bf704c8e9dd397 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts @@ -71,7 +71,8 @@ export class PromptsServiceCustomizationItemProvider implements ICustomizationIt // folders like ~/.copilot/skills read naturally. Only folders that // carry a source (currently skills) use this; others fall back. label: (folder.source !== undefined ? getSourceDescription(folder.source) : undefined) ?? this.promptsService.getPromptLocationLabel(folder), - source: folder.storage + source: folder.storage, + destinationGroupId: dirname(folder.uri).toString(), })); } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSystemNotificationContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSystemNotificationContentPart.ts index 7ed591689cded6..fec0c170d5f2c1 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSystemNotificationContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSystemNotificationContentPart.ts @@ -35,6 +35,7 @@ const transparentButtonStyles: IButtonStyles = { export class ChatSystemNotificationContentPart extends Disposable implements IChatContentPart { readonly domNode: HTMLElement; + readonly inlineTimingContainer: HTMLElement | undefined; constructor( private readonly notification: IChatSystemNotificationPart, @@ -43,16 +44,32 @@ export class ChatSystemNotificationContentPart extends Disposable implements ICh ) { super(); + let notificationNode: HTMLElement; if (notification.collapsible) { const firstLineBreak = notification.content.value.indexOf('\n'); const detailsValue = firstLineBreak === -1 ? '' : notification.content.value.slice(firstLineBreak).trim(); if (detailsValue) { - this.domNode = this._renderCollapsibleNotification(notification, renderer, firstLineBreak, detailsValue); - return; + notificationNode = this._renderCollapsibleNotification(notification, renderer, firstLineBreak, detailsValue); + } else { + notificationNode = this._renderNotification(notification, renderer, instantiationService); } + } else { + notificationNode = this._renderNotification(notification, renderer, instantiationService); } + + if (notification.renderInlineTiming) { + this.domNode = dom.$('.chat-system-notification-layout'); + this.domNode.appendChild(notificationNode); + this.inlineTimingContainer = dom.append(this.domNode, dom.$('span.chat-system-notification-timing')); + } else { + this.domNode = notificationNode; + this.inlineTimingContainer = undefined; + } + } + + private _renderNotification(notification: IChatSystemNotificationPart, renderer: IMarkdownRenderer, instantiationService: IInstantiationService): HTMLElement { const rendered = this._register(renderer.render(notification.content)); - this.domNode = this._register(instantiationService.createInstance(ChatProgressSubPart, rendered.element, notification.icon ?? Codicon.check, undefined)).domNode; + return this._register(instantiationService.createInstance(ChatProgressSubPart, rendered.element, notification.icon ?? Codicon.check, undefined)).domNode; } private _renderCollapsibleNotification(notification: IChatSystemNotificationPart, renderer: IMarkdownRenderer, firstLineBreak: number, detailsValue: string): HTMLElement { @@ -101,6 +118,7 @@ export class ChatSystemNotificationContentPart extends Disposable implements ICh return other.kind === 'systemNotification' && other.content.value === this.notification.content.value && ThemeIcon.isEqual(other.icon ?? Codicon.check, this.notification.icon ?? Codicon.check) - && !!other.collapsible === !!this.notification.collapsible; + && !!other.collapsible === !!this.notification.collapsible + && !!other.renderInlineTiming === !!this.notification.renderInlineTiming; } } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatSystemNotificationContentPart.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatSystemNotificationContentPart.css index 811e8ed97f159a..92178273304792 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatSystemNotificationContentPart.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatSystemNotificationContentPart.css @@ -60,3 +60,47 @@ .chat-system-notification-disclosure.collapsed > .chat-system-notification-disclosure-body { display: none; } + +.chat-system-notification-layout { + display: flex; + align-items: last baseline; + gap: var(--vscode-spacing-size80); + margin-bottom: var(--vscode-spacing-size160); +} + +.chat-system-notification-layout > .progress-container, +.chat-system-notification-layout > .chat-system-notification-disclosure { + flex: 1; + min-width: 0; + margin-bottom: 0; +} + +.chat-system-notification-timing { + display: flex; + flex-shrink: 0; + opacity: 0; + color: var(--vscode-descriptionForeground); + font-size: var(--vscode-chat-font-size-body-xs); + line-height: 16px; + transition: opacity 0.1s ease-in-out; +} + +.chat-system-notification-timing.hidden { + display: none; +} + +.interactive-item-container.chat-system-notification-response.group-hovered .chat-system-notification-timing, +.interactive-item-container.chat-system-notification-response:focus-within .chat-system-notification-timing { + opacity: 0.7; +} + +.chat-system-notification-timing:focus-visible { + outline: var(--vscode-strokeThickness) solid var(--vscode-focusBorder); + outline-offset: var(--vscode-spacing-size20); +} + +@media (prefers-reduced-motion: reduce) { + .chat-system-notification-timing { + transition: none; + } +} diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 2674e66f786ac9..b43b6c624c57b7 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -1342,7 +1342,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer 0 + && element.response.value.every(part => part.kind === 'systemNotification' && part.renderInlineTiming); const responseTimingListeners = templateData.elementDisposables.add(new MutableDisposable()); const updateResponseDetails = () => { - const details = isResponseVM(element) ? element.result?.details : undefined; + const inlineTimingContainer = rendersInlineSystemNotificationTiming + ? templateData.renderedParts?.findLast(part => part instanceof ChatSystemNotificationContentPart)?.inlineTimingContainer + : undefined; + const responseDetailsContainer = inlineTimingContainer ?? templateData.footerDetailsContainer; + if (rendersInlineSystemNotificationTiming) { + renderChatResponseDetails(templateData.footerDetailsContainer, undefined, undefined, undefined, false); + } + const details = isResponseVM(element) && !rendersInlineSystemNotificationTiming ? element.result?.details : undefined; // Providers report usage asynchronously, often after the footer has already // rendered, so the breakdown is recomputed on every render pass. Sessions // whose provider reports no totals get no hover rather than an empty one. const tokenStats = isResponseVM(element) + && !rendersInlineSystemNotificationTiming ? formatResponseTokenStats(element.model.usage?.modelTotals, element.model.completionTimestamp) : undefined; const completedAtElement = renderChatResponseDetails( - templateData.footerDetailsContainer, + responseDetailsContainer, details, isResponseVM(element) ? element.model.completionTimestamp : undefined, - isResponseVM(element) ? element.model.elapsedMs : undefined, + isResponseVM(element) && !rendersInlineSystemNotificationTiming ? element.model.elapsedMs : undefined, isResponseVM(element) && this.configService.getValue(ChatConfiguration.Verbose), tokenStats?.footerAriaLabel, ); @@ -1422,29 +1433,28 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { const bounds = completedAtElement.getBoundingClientRect(); responseTimingBounds = bounds; - templateData.footerDetailsContainer.classList.add('chat-response-flip-reset'); - templateData.footerDetailsContainer.classList.remove('chat-response-flip-active'); - templateData.footerDetailsContainer.classList.toggle('chat-response-flip-down', e.clientY < bounds.top + bounds.height / 2); - void templateData.footerDetailsContainer.offsetWidth; - templateData.footerDetailsContainer.classList.remove('chat-response-flip-reset'); - void templateData.footerDetailsContainer.offsetWidth; - templateData.footerDetailsContainer.classList.add('chat-response-flip-active'); + responseDetailsContainer.classList.add('chat-response-flip-reset'); + responseDetailsContainer.classList.remove('chat-response-flip-active'); + responseDetailsContainer.classList.toggle('chat-response-flip-down', e.clientY < bounds.top + bounds.height / 2); + void responseDetailsContainer.offsetWidth; + responseDetailsContainer.classList.remove('chat-response-flip-reset'); + void responseDetailsContainer.offsetWidth; + responseDetailsContainer.classList.add('chat-response-flip-active'); })); - listeners.add(dom.addDisposableListener(templateData.footerDetailsContainer, dom.EventType.MOUSE_MOVE, e => { + listeners.add(dom.addDisposableListener(responseDetailsContainer, dom.EventType.MOUSE_MOVE, e => { if (responseTimingBounds && (e.clientX < responseTimingBounds.left || e.clientX > responseTimingBounds.right || e.clientY < responseTimingBounds.top || e.clientY > responseTimingBounds.bottom)) { responseTimingBounds = undefined; - templateData.footerDetailsContainer.classList.remove('chat-response-flip-active'); + responseDetailsContainer.classList.remove('chat-response-flip-active'); } })); - listeners.add(dom.addDisposableListener(templateData.footerDetailsContainer, dom.EventType.MOUSE_LEAVE, () => { + listeners.add(dom.addDisposableListener(responseDetailsContainer, dom.EventType.MOUSE_LEAVE, () => { responseTimingBounds = undefined; - templateData.footerDetailsContainer.classList.remove('chat-response-flip-active'); + responseDetailsContainer.classList.remove('chat-response-flip-active'); })); - listeners.add(dom.addDisposableListener(templateData.footerDetailsContainer, dom.EventType.FOCUS, () => { - templateData.footerDetailsContainer.classList.remove('chat-response-flip-active', 'chat-response-flip-down'); + listeners.add(dom.addDisposableListener(responseDetailsContainer, dom.EventType.FOCUS, () => { + responseDetailsContainer.classList.remove('chat-response-flip-active', 'chat-response-flip-down'); })); }; - updateResponseDetails(); ChatContextKeys.responseHasError.bindTo(templateData.contextKeyService).set(isResponseVM(element) && !!element.errorDetails); const isFiltered = !!(isResponseVM(element) && element.errorDetails?.responseIsFiltered); @@ -1454,6 +1464,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer