[pull] main from microsoft:main

.github/skills/chat-customizations-editor/SKILL.md

@@ -79,6 +79,49 @@ Each customization type requires its own mock path in `createMockPromptsService`
 
 All test data lives in `allFiles` (prompt-based items) and the `mcpWorkspace/UserServers` arrays. Add enough items per category (8+) to invoke scrolling.
 
+### Exercising built-in grouping
+
+The list widget regroups items from the default chat extension under a "Built-in" header. Three things must be in place for fixtures to exercise this:
+1. Include `BUILTIN_STORAGE` in the harness descriptor's visible sources
+2. Mock `IProductService.defaultChatAgent.chatExtensionId` (e.g., `'GitHub.copilot-chat'`)
+3. Give mock items extension provenance via `extensionId` / `extensionDisplayName` matching that ID
+
+Without all three, built-in regrouping silently doesn't run and the fixture only shows flat lists.
+
+### Editor contribution service mocks
+
+The management editor embeds a `CodeEditorWidget`. Electron-side editor contributions (e.g., `AgentFeedbackEditorWidgetContribution`) are instantiated automatically and crash if their injected services aren't registered. The fixture must mock at minimum:
+- `IAgentFeedbackService` — needs `onDidChangeFeedback`, `onDidChangeNavigation` as `Event.None`
+- `ICodeReviewService` — needs `getReviewState()` / `getPRReviewState()` returning idle observables
+- `IChatEditingService` — needs `editingSessionsObs` as empty observable
+- `IAgentSessionsService` — needs `model.sessions` as empty array
+
+These are cross-layer imports from `vs/sessions/` — use `// eslint-disable-next-line local/code-import-patterns` on the import lines.
+
+### CI regression gates
+
+Key fixtures have `blocksCi: true` in their labels. The `screenshot-test.yml` GitHub Action captures screenshots on every PR to `main` and **fails the CI status check** if any `blocks-ci`-labeled fixture's screenshot changes. This catches layout regressions automatically.
+
+Currently gated fixtures: `LocalHarness`, `McpServersTab`, `McpServersTabNarrow`, `AgentsTabNarrow`. When adding a new section or layout-critical fixture, add `blocksCi: true`:
+
+```typescript
+MyFixture: defineComponentFixture({
+    labels: { kind: 'screenshot', blocksCi: true },
+    render: ctx => renderEditor(ctx, { ... }),
+}),
+```
+
+Don't add `blocksCi` to every fixture — only ones that cover critical layout paths (default view, section with list + footer, narrow viewport). Too many gated fixtures creates noisy CI.
+
+### Screenshot stability
+
+Scrollbar fade transitions cause screenshot instability — the scrollbar shifts from `visible` to `invisible fade` class ~2 seconds after a programmatic scroll. After calling `revealLastItem()` or any scroll action, wait for the transition to complete before the fixture's render promise resolves:
+
+```typescript
+await new Promise(resolve => setTimeout(resolve, 2400));
+// Then optionally poll until .scrollbar.vertical loses the 'visible' class
+```
+
 ### Running unit tests
 
 ```bash
@@ -87,3 +130,53 @@ npm run compile-check-ts-native && npm run valid-layers-check
 ```
 
 See the `sessions` skill for sessions-window specific guidance.
+
+## Debugging Layout in the Real Product
+
+Component fixtures use mock data and a fixed container size. Layout bugs caused by reflow timing, real data shapes, or narrow window sizes often **don't reproduce in fixtures**. When a user reports a broken layout, debug in the live Code OSS product.
+
+For launching Code OSS with CDP and connecting `agent-browser`, see the **`launch` skill**. Use `--user-data-dir /tmp/code-oss-debug` to avoid colliding with an already-running instance from another worktree.
+
+### Navigating to the customizations editor
+
+After connecting, use `snapshot -i` to find the "Open Customizations" button (in the Chat panel header), then click it. To switch sections, use `eval` with a DOM click since sidebar items aren't interactive refs:
+
+```bash
+npx agent-browser eval "const items = [...document.querySelectorAll('.section-list-item')]; \
+  items.find(el => el.textContent?.includes('MCP'))?.click();"
+```
+
+### Inspecting widget layout
+
+`agent-browser eval` doesn't always print return values. Use `document.title` as a return channel:
+
+```bash
+npx agent-browser eval "const w = document.querySelector('.mcp-list-widget'); \
+  const lc = w?.querySelector('.mcp-list-container'); \
+  const rows = lc?.querySelectorAll('.monaco-list-row'); \
+  document.title = 'DBG:rows=' + (rows?.length ?? -1) \
+    + ',listH=' + (lc?.offsetHeight ?? -1) \
+    + ',seStH=' + (lc?.querySelector('.monaco-scrollable-element')?.style?.height ?? '') \
+    + ',wH=' + (w?.offsetHeight ?? -1);"
+npx agent-browser eval "document.title" 2>&1
+```
+
+Key diagnostics:
+- **`rows`** — fewer than expected means `list.layout()` never received the correct viewport height.
+- **`seStH`** — empty means the list was never properly laid out.
+- **`listH` vs `wH`** — list container height should be widget height minus search bar minus footer.
+
+### Common layout issues
+
+| Symptom | Root cause | Fix pattern |
+|---------|-----------|-------------|
+| List shows 0-1 rows in a tall container | `layout()` bailed out because `offsetHeight` returned 0 during `display:none → visible` transition | Defer layout via `DOM.getWindow(this.element).requestAnimationFrame(...)` |
+| Badge or row content clips at right edge | Widget container missing `overflow: hidden` | Add `overflow: hidden` to the widget's CSS class |
+| Items visible in fixture but not in product | Fixture uses many mock items; real product has few | Add fixture variants with fewer items or narrower dimensions (`width`/`height` options) |
+
+### Fixture vs real product gaps
+
+Fixtures render at a fixed size (default 900×600) with many mock items. They won't catch:
+- **Reflow timing** — the real product's `display:none → visible` transition may not have reflowed before `layout()` fires
+- **Narrow windows** — add narrow fixture variants (e.g., `width: 550, height: 400`)
+- **Real data counts** — a user with 1 MCP server sees very different layout than a fixture with 12

src/vs/platform/agentHost/node/copilot/copilotAgent.ts

@@ -136,8 +136,11 @@ export class CopilotAgent extends Disposable implements IAgent {
 			// If @vscode/ripgrep is in an .asar file, the binary is unpacked.
 			const rgDiskPath = rgPath.replace(/\bnode_modules\.asar\b/, 'node_modules.asar.unpacked');
 			const rgDir = dirname(rgDiskPath);
-			const currentPath = env['PATH'];
-			env['PATH'] = currentPath ? `${currentPath}${delimiter}${rgDir}` : rgDir;
+			// On Windows the env key is typically "Path" (not "PATH"). Since we copied
+			// process.env into a plain (case-sensitive) object, we must find the actual key.
+			const pathKey = Object.keys(env).find(k => k.toUpperCase() === 'PATH') ?? 'PATH';
+			const currentPath = env[pathKey];
+			env[pathKey] = currentPath ? `${currentPath}${delimiter}${rgDir}` : rgDir;
 			this._logService.info(`[Copilot] Resolved CLI path: ${cliPath}`);
 
 			const client = new CopilotClient({

src/vs/platform/browserElements/electron-main/nativeBrowserElementsMainService.ts

@@ -127,7 +127,6 @@ export class NativeBrowserElementsMainService extends Disposable implements INat
 		targetWebContents.on('console-message', onConsoleMessage);
 		targetWebContents.on('destroyed', onTargetDestroyed);
 		windowWebContents.on('ipc-message', onIpcMessage);
-		token.onCancellationRequested(cleanupListeners);
 	}
 
 	/**

src/vs/platform/browserView/common/browserView.ts

@@ -304,13 +304,6 @@ export interface IBrowserViewService {
 	 */
 	captureScreenshot(id: string, options?: IBrowserViewCaptureScreenshotOptions): Promise<VSBuffer>;
 
-	/**
-	 * Dispatch a key event to the browser view
-	 * @param id The browser view identifier
-	 * @param keyEvent The key event data
-	 */
-	dispatchKeyEvent(id: string, keyEvent: IBrowserViewKeyDownEvent): Promise<void>;
-
 	/**
 	 * Focus the browser view
 	 * @param id The browser view identifier

src/vs/platform/browserView/electron-browser/preload-browserView.ts

@@ -17,7 +17,7 @@
  */
 (function () {
 
-	const { contextBridge } = require('electron');
+	const { contextBridge, ipcRenderer } = require('electron');
 
 	// #######################################################################
 	// ###                                                                 ###
@@ -26,6 +26,37 @@
 	// ###       (https://github.com/electron/electron/issues/25516)       ###
 	// ###                                                                 ###
 	// #######################################################################
+
+	// Listen for keydown events that the page did not handle and forward them for shortcut handling.
+	window.addEventListener('keydown', (event) => {
+		// Require that the event is trusted -- i.e. user-initiated.
+		// eslint-disable-next-line no-restricted-syntax
+		if (!(event instanceof KeyboardEvent) || !event.isTrusted) {
+			return;
+		}
+
+		// If the event was already handled by the page, do not forward it.
+		if (event.defaultPrevented) {
+			return;
+		}
+
+		// filter to events that either have modifiers or do not have a character representation.
+		if (!(event.ctrlKey || event.altKey || event.metaKey) && event.key.length === 1) {
+			return;
+		}
+
+		ipcRenderer.send('vscode:browserView:keydown', {
+			key: event.key,
+			keyCode: event.keyCode,
+			code: event.code,
+			ctrlKey: event.ctrlKey,
+			shiftKey: event.shiftKey,
+			altKey: event.altKey,
+			metaKey: event.metaKey,
+			repeat: event.repeat
+		});
+	});
+
 	const globals = {
 		/**
 		 * Get the currently selected text in the page.

src/vs/platform/browserView/electron-main/browserSession.ts

@@ -9,6 +9,7 @@ import { URI } from '../../../base/common/uri.js';
 import { IApplicationStorageMainService } from '../../storage/electron-main/storageMainService.js';
 import { BrowserViewStorageScope } from '../common/browserView.js';
 import { BrowserSessionTrust, IBrowserSessionTrust } from './browserSessionTrust.js';
+import { FileAccess } from '../../../base/common/network.js';
 
 // Same as webviews, minus clipboard-read
 const allowedPermissions = new Set([
@@ -195,7 +196,7 @@ export class BrowserSession {
 		readonly storageScope: BrowserViewStorageScope,
 	) {
 		this._trust = new BrowserSessionTrust(this);
-		this.configurePermissions();
+		this.configure();
 		BrowserSession.knownSessions.add(electronSession);
 		BrowserSession._bySession.set(electronSession, this);
 		BrowserSession._byId.set(id, new WeakRef(this));
@@ -218,15 +219,19 @@ export class BrowserSession {
 	}
 
 	/**
-	 * Apply the standard permission policy to the session.
+	 * Apply the permission policy and preload scripts to the session.
 	 */
-	private configurePermissions(): void {
+	private configure(): void {
 		this.electronSession.setPermissionRequestHandler((_webContents, permission, callback) => {
 			return callback(allowedPermissions.has(permission));
 		});
 		this.electronSession.setPermissionCheckHandler((_webContents, permission, _origin) => {
 			return allowedPermissions.has(permission);
 		});
+		this.electronSession.registerPreloadScript({
+			type: 'frame',
+			filePath: FileAccess.asFileUri('vs/platform/browserView/electron-browser/preload-browserView.js').fsPath
+		});
 	}
 
 	/**