diff --git a/.github/skills/vscode-dev-workbench/SKILL.md b/.github/skills/vscode-dev-workbench/SKILL.md index 01589c1716cb1..93716893ca0d9 100644 --- a/.github/skills/vscode-dev-workbench/SKILL.md +++ b/.github/skills/vscode-dev-workbench/SKILL.md @@ -21,12 +21,20 @@ If your paths differ, check `server/` in `vscode-dev` for the source root resolu ## Start the dev server +**Critical:** Run `npm run dev` from the **`vscode-dev`** folder, NOT from `vscode`. The `vscode` repo has no `dev` script and will fail with `npm error Missing script: "dev"`. Terminal tools that simplify/strip leading `cd` into separate commands will silently keep the cwd of a previous terminal — always use an absolute `pushd` or verify with `pwd` before `npm run dev`. + ```bash -cd vscode-dev -npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 +cd /path/to/vscode-dev # NOT /path/to/vscode +npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 ``` -On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. +If you're driving this through an agent/terminal tool, prefer: + +```bash +pushd /absolute/path/to/vscode-dev >/dev/null && pwd && npm run dev +``` + +On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. The server is ready when `curl -sk -o /dev/null -w "%{http_code}" https://127.0.0.1:3000/` returns `200`. ## URLs @@ -97,15 +105,19 @@ For a true mobile viewport, drive a standalone Playwright script with `devices[' ## Testing the Agents window against a local mock agent host +If the scenario touches the Agents window (`/agents` route), you almost always need the mock agent host running. Without it, the Agents window will sit on the sign-in / tunnel-discovery screen and block any real interaction. Start it **in addition to** the dev server — it's a second terminal, not a replacement. + `vscode-dev` supports a `?mock-agent-host=ws://…` URL parameter that short-circuits tunnel discovery and wires the Agents window to a raw WebSocket. Pair it with the mock agent host binary from `microsoft/vscode`: ```bash -cd vscode +cd /path/to/vscode node out/vs/platform/agentHost/node/agentHostServerMain.js \ --enable-mock-agent --quiet --without-connection-token --port 8765 # Listens on ws://localhost:8765 ``` +Prerequisite: `out/` in the `vscode` repo must be populated by the `VS Code - Build` task (or `npm run watch`). If `out/vs/platform/agentHost/node/agentHostServerMain.js` is missing, start that task first. + `--enable-mock-agent` registers the `ScriptedMockAgent` from `src/vs/platform/agentHost/test/node/mockAgent.ts` with one pre-existing session. Seed additional sessions via the `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS` env var, using a comma-separated list of session URIs (for example, `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS=mock://pre-1,mock://pre-2`). Scripted prompts include `hello`, `use-tool`, `error`, `permission`, `write-file`, `run-safe-command`, `slow`, `client-tool`, `subagent`, etc. (see `mockAgent.ts` for the full list). Then open: diff --git a/src/vs/sessions/browser/parts/media/sidebarPart.css b/src/vs/sessions/browser/parts/media/sidebarPart.css index 43e3e446665cc..28f63b4d66872 100644 --- a/src/vs/sessions/browser/parts/media/sidebarPart.css +++ b/src/vs/sessions/browser/parts/media/sidebarPart.css @@ -70,18 +70,42 @@ /* ---- Phone Layout: Sidebar Drawer Overlay ---- */ -/* On phone, the sidebar is a drawer that slides over the chat. - It takes 85% width (max 360px) and sits on top of everything. */ +/* On phone, the sidebar is a full-width drawer that slides over the chat. + It covers the full viewport below the mobile top bar; the top bar's + sidebar toggle button remains visible and is used to dismiss it. + + The drawer slides in/out with a transition (not a keyframe animation) so + that interrupted toggles retarget smoothly from the current position + rather than restarting. The split-view wrapper toggles `display: none` + via a `.visible` class; `transition-behavior: allow-discrete` defers + the discrete `display` change until the slide-out completes, and + `@starting-style` provides the offscreen origin for the slide-in. */ .agent-sessions-workbench.phone-layout .split-view-view:has(> .part.sidebar) { position: absolute !important; top: 0 !important; left: 0 !important; bottom: 0 !important; - width: 85% !important; - max-width: 360px !important; + width: 100% !important; height: 100% !important; z-index: 250; - animation: sidebar-slide-in 200ms ease-out; + transform: translateX(0); + transition: + transform 260ms cubic-bezier(0.32, 0.72, 0, 1), + display 260ms allow-discrete; +} + +/* Slide-in starting state (applies on each transition into .visible) */ +@starting-style { + .agent-sessions-workbench.phone-layout .split-view-view.visible:has(> .part.sidebar) { + transform: translateX(-100%); + } +} + +/* Slide-out target: when `.visible` is removed, splitview's own rule sets + `display: none`. With `allow-discrete` above, the transform animates first + and the discrete `display` swap happens at the end of the transition. */ +.agent-sessions-workbench.phone-layout .split-view-view:not(.visible):has(> .part.sidebar) { + transform: translateX(-100%); } /* The sidebar Part inside fills its container */ @@ -90,32 +114,12 @@ height: 100%; } -@keyframes sidebar-slide-in { - from { - transform: translateX(-100%); - } - to { - transform: translateX(0); +@media (prefers-reduced-motion: reduce) { + .agent-sessions-workbench.phone-layout .split-view-view:has(> .part.sidebar) { + transition: none; } } -/* Sidebar backdrop — applied via JS when sidebar is open on phone */ -.mobile-sidebar-backdrop { - position: absolute; - top: 0; - left: 0; - right: 0; - bottom: 0; - background: rgba(0, 0, 0, 0.5); - z-index: 240; - animation: backdrop-fade-in 200ms ease-out; -} - -@keyframes backdrop-fade-in { - from { opacity: 0; } - to { opacity: 1; } -} - /* Increase sidebar footer action button height for touch */ .agent-sessions-workbench.phone-layout .part.sidebar > .sidebar-footer .sidebar-action-button { min-height: 44px; diff --git a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css index 72eddfcfe05a4..2f79a84a73e25 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css +++ b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css @@ -3,6 +3,21 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +/* + * `!important` policy for this file: + * + * Only use `!important` when fighting one of: + * 1. SplitView.View.layoutContainer — sets inline position/top/left/width/height + * on every `.split-view-view` (src/vs/base/browser/ui/splitview/splitview.ts). + * 2. Part.layoutContents — inlines width/height on `.part > .content` via size(). + * 3. An equal-specificity desktop rule in the main workbench stylesheet. + * + * Rules that only face lower-specificity desktop CSS (e.g. a single `.part.X` + * selector) do NOT need `!important` — the `.phone-layout` class on the + * workbench root already wins. When adding a new rule, omit `!important` + * first and only add it if DevTools shows an actual override. + */ + /* ---- Mobile Top Bar ---- */ .mobile-top-bar { @@ -114,52 +129,21 @@ /* On phone, stack the mobile top bar and grid vertically */ .agent-sessions-workbench.phone-layout { - display: flex !important; - flex-direction: column !important; - overflow: hidden !important; -} - -/* On phone, split-view-views that directly contain a Part fill the full - grid area. Uses :has(> .part) to target only part containers — NOT - nested split-views inside parts' own content. */ -.agent-sessions-workbench.phone-layout .split-view-view:has(> .part) { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* The grid's own branch nodes (NOT those inside parts) need full sizing. - Target only direct children of the grid root. */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* Split-view-views inside the grid root that contain branch nodes */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view:has(> .monaco-grid-branch-node) { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* Second-level grid branch nodes */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; + display: flex; + flex-direction: column; + overflow: hidden; } -/* Third-level (top-right section) */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view:has(> .monaco-grid-branch-node) { +/* On phone, all split-view-view wrappers inside the grid — both those + wrapping parts AND those wrapping nested grid branch nodes — fill the + full grid area. This collapses the multi-axis grid into a single + full-screen viewport so parts overlap rather than share horizontal + space. The sidebar uses its own z-index + transform to slide over + the chat (see sidebarPart.css). The :has() conditions scope strictly + to grid wrappers so splitviews used inside individual parts' content + (e.g. a sidebar's view list) are not affected. */ +.agent-sessions-workbench.phone-layout .split-view-view:has(> .part), +.agent-sessions-workbench.phone-layout .split-view-view:has(> .monaco-grid-branch-node) { position: absolute !important; top: 0 !important; left: 0 !important; @@ -167,42 +151,52 @@ height: 100% !important; } -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; +/* All grid branch nodes fill their parent. `.monaco-grid-branch-node` is + specific to the grid widget, so this descendant selector won't hit + splitviews used inside individual parts' content. The grid widget + does not write inline geometry to branch nodes (only to the wrapping + `.split-view-view`), so plain CSS suffices here. */ +.agent-sessions-workbench.phone-layout .monaco-grid-branch-node { + position: absolute; + top: 0; + left: 0; + width: 100%; + height: 100%; } -/* Remove card appearance from ALL parts on phone */ +/* Remove card appearance from ALL parts on phone. + Specificity wins over the desktop card rule in style.css without !important; + width/height match what the mobile Part.layout() already inlines. */ .agent-sessions-workbench.phone-layout .part.chatbar, .agent-sessions-workbench.phone-layout .part.sidebar, .agent-sessions-workbench.phone-layout .part.auxiliarybar, .agent-sessions-workbench.phone-layout .part.panel { - margin: 0 !important; - border: none !important; - border-radius: 0 !important; - box-shadow: none !important; - --part-border-color: transparent !important; - width: 100% !important; - height: 100% !important; + margin: 0; + border: none; + border-radius: 0; + box-shadow: none; + --part-border-color: transparent; + width: 100%; + height: 100%; } -/* Force content div inside parts to fill the part on phone. - Part.layoutContents() sets inline width/height via size(), which - may use the grid-allocated dimensions rather than the CSS-overridden - 100% dimensions. Override with !important. */ +/* Pin Part content to the wrapper's full width on phone. + `!important` is required because `Part.layoutContents()` inlines the + width on `.part > .content` based on the splitview size (rule #2 in the + policy above). Without this, opening the sidebar — which makes the + splitview share space between sidebar and chatbar — would shrink the + chatbar's content during the drawer slide animation. */ .agent-sessions-workbench.phone-layout .part.chatbar > .content, .agent-sessions-workbench.phone-layout .part.sidebar > .content, .agent-sessions-workbench.phone-layout .part.auxiliarybar > .content, .agent-sessions-workbench.phone-layout .part.panel > .content { width: 100% !important; + height: 100% !important; } /* Hide the session composite bar (Copilot CLI / Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .session-composite-bar { - display: none !important; + display: none; } /* Ensure the grid view element doesn't overflow — flex child must shrink */ @@ -224,8 +218,8 @@ } .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-part { - max-width: none !important; - padding-bottom: calc(10px + env(safe-area-inset-bottom)) !important; + max-width: none !important; /* fights equal-specificity rule in style.css */ + padding-bottom: calc(10px + env(safe-area-inset-bottom)); } /* Chat input minimum font size to prevent iOS auto-zoom */ @@ -247,7 +241,7 @@ } .agent-sessions-workbench.phone-layout .part.sidebar > .composite.title { - display: none !important; + display: none; } .agent-sessions-workbench.phone-layout .part.sidebar > .content { @@ -260,7 +254,7 @@ /* Customization toolbar: hidden on phone (opens editors, not mobile-compatible) */ .agent-sessions-workbench.phone-layout .part.sidebar .ai-customization-toolbar { - display: none !important; + display: none; } /* Make sidebar footer touch-friendly */ @@ -271,7 +265,7 @@ /* Hide the "+ Session" button in the sidebar on phone — replaced by top bar + button */ .agent-sessions-workbench.phone-layout .agent-sessions-new-button-container { - display: none !important; + display: none; } /* Hide sashes on phone */ @@ -291,7 +285,7 @@ /* On phone, push the chat input to the bottom of the chat area */ .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-and-execute-toolbar { - margin-top: auto !important; + margin-top: auto; } /* ---- Phone Layout: Chat Welcome Page ---- */ @@ -351,7 +345,7 @@ } .agent-sessions-workbench.phone-layout .session-workspace-picker-label { - font-size: 18px !important; + font-size: 18px; opacity: 0.6; } @@ -370,12 +364,12 @@ /* Hide the local mode bar (Copilot CLI / Default Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .new-chat-bottom-container { - display: none !important; + display: none; } /* Also hide the sessions-chat-widget's DnD overlay on phone */ .agent-sessions-workbench.phone-layout .sessions-chat-dnd-overlay { - display: none !important; + display: none; } /* Chat widget fills full width on phone */ diff --git a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts index 550ac0944a315..38c4cb598138f 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts +++ b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts @@ -16,6 +16,7 @@ import { IInstantiationService } from '../../../../platform/instantiation/common import { HiddenItemStrategy, MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js'; import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; import { IsNewChatSessionContext } from '../../../common/contextkeys.js'; +import { SideBarVisibleContext } from '../../../../workbench/common/contextkeys.js'; import { Menus } from '../../menus.js'; /** @@ -72,13 +73,28 @@ export class MobileTitlebarPart extends Disposable { this._register(toDisposable(() => this.element.remove())); parent.prepend(this.element); - // Hamburger button + // Sidebar toggle button. Uses the same icon as the desktop/web + // agents-app sidebar toggle and reflects open/closed state via the + // SideBarVisibleContext key. const hamburger = append(this.element, $('button.mobile-top-bar-button')); hamburger.setAttribute('aria-label', localize('mobileTopBar.openSessions', "Open sessions")); const hamburgerIcon = append(hamburger, $('span')); - hamburgerIcon.classList.add(...ThemeIcon.asClassNameArray(Codicon.menu)); + const closedIconClasses = ThemeIcon.asClassNameArray(Codicon.layoutSidebarLeftOff); + const openIconClasses = ThemeIcon.asClassNameArray(Codicon.layoutSidebarLeft); + hamburgerIcon.classList.add(...closedIconClasses); this._register(addDisposableListener(hamburger, EventType.CLICK, () => this._onDidClickHamburger.fire())); + const sidebarVisibleKeySet = new Set([SideBarVisibleContext.key]); + const updateSidebarIcon = () => { + const isOpen = !!SideBarVisibleContext.getValue(contextKeyService); + hamburgerIcon.classList.remove(...closedIconClasses, ...openIconClasses); + hamburgerIcon.classList.add(...(isOpen ? openIconClasses : closedIconClasses)); + hamburger.setAttribute('aria-label', isOpen + ? localize('mobileTopBar.closeSessions', "Close sessions") + : localize('mobileTopBar.openSessions', "Open sessions")); + }; + updateSidebarIcon(); + // Center slot: title and/or actions container (mutually exclusive) const center = append(this.element, $('div.mobile-top-bar-center')); @@ -126,6 +142,9 @@ export class MobileTitlebarPart extends Disposable { if (e.affectsSome(newChatKeySet)) { updateCenterMode(); } + if (e.affectsSome(sidebarVisibleKeySet)) { + updateSidebarIcon(); + } })); this._register(toolbar.onDidChangeMenuItems(() => updateCenterMode())); } diff --git a/src/vs/sessions/browser/workbench.ts b/src/vs/sessions/browser/workbench.ts index de3c9c25d0f43..01bd31f30a891 100644 --- a/src/vs/sessions/browser/workbench.ts +++ b/src/vs/sessions/browser/workbench.ts @@ -7,7 +7,7 @@ import '../../workbench/browser/style.js'; import './media/style.css'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../base/common/lifecycle.js'; import { Emitter, Event, setGlobalLeakWarningThreshold } from '../../base/common/event.js'; -import { getActiveDocument, getActiveElement, getClientArea, getWindowId, getWindows, IDimension, isAncestorUsingFlowTo, isHTMLElement, size, Dimension, runWhenWindowIdle, addDisposableListener, EventType } from '../../base/browser/dom.js'; +import { getActiveDocument, getActiveElement, getClientArea, getWindowId, getWindows, IDimension, isAncestorUsingFlowTo, isHTMLElement, size, Dimension, runWhenWindowIdle } from '../../base/browser/dom.js'; import { DeferredPromise, RunOnceScheduler } from '../../base/common/async.js'; import { isFullscreen, onDidChangeFullscreen, isChrome, isFirefox, isSafari } from '../../base/browser/browser.js'; import { mark } from '../../base/common/performance.js'; @@ -697,9 +697,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic })); } - private sidebarDrawerBackdrop: HTMLElement | undefined; - private readonly sidebarDrawerBackdropDisposables = this._register(new DisposableStore()); - private toggleMobileSidebarDrawer(): void { const isOpen = this.partVisibility.sidebar; if (isOpen) { @@ -710,17 +707,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic } private openMobileSidebarDrawer(): void { - // Show backdrop — created fresh each open so its click listener is - // tracked by a DisposableStore and cleaned up on close. - if (!this.sidebarDrawerBackdrop) { - const backdrop = document.createElement('div'); - backdrop.className = 'mobile-sidebar-backdrop'; - this.sidebarDrawerBackdropDisposables.add(addDisposableListener(backdrop, EventType.CLICK, () => this.closeMobileSidebarDrawer())); - this.sidebarDrawerBackdropDisposables.add(toDisposable(() => backdrop.remove())); - this.sidebarDrawerBackdrop = backdrop; - } - this.mainContainer.appendChild(this.sidebarDrawerBackdrop); - // Push a history entry so the Android back button dismisses the drawer. // Must come before setSideBarHidden(false) so layoutMobileSidebar() sees // the drawer state. @@ -729,16 +715,13 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic } // Show sidebar in grid — the actual drawer dimensions are applied by - // layoutMobileSidebar() from within layout(), which respects the - // "drawer" shape on phone (85% width, below the mobile top bar). + // layoutMobileSidebar() from within layout(), which uses the full + // viewport width below the mobile top bar on phone. The toggle button + // in the top bar remains visible and is used to close the drawer. this.setSideBarHidden(false); } private closeMobileSidebarDrawer(): void { - // Remove backdrop and dispose its listener. - this.sidebarDrawerBackdropDisposables.clear(); - this.sidebarDrawerBackdrop = undefined; - // Hide sidebar in grid this.setSideBarHidden(true); @@ -1272,32 +1255,27 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic return; } - // Only phone uses the overlay drawer shape. Tablet/desktop let the - // grid position the sidebar normally, so clear any inline styles. + // On phone the sidebar renders as a full-viewport overlay drawer. + // Geometry is fully expressed in CSS — see + // `mobileChatShell.css` (split-view-view fills the grid) and + // `sidebarPart.css` (drawer animation, z-index). We avoid setting + // inline position/size styles here because writing them after the + // grid has already laid out and painted the sidebar causes a + // visible one-frame snap on toggle. const isPhone = this.layoutPolicy.viewportClass.get() === 'phone'; if (!isPhone || !this.partVisibility.sidebar) { sidebarContainer.classList.remove('mobile-overlay-sidebar'); - sidebarContainer.style.position = ''; - sidebarContainer.style.top = ''; - sidebarContainer.style.left = ''; - sidebarContainer.style.width = ''; - sidebarContainer.style.height = ''; - sidebarContainer.style.zIndex = ''; return; } - // Phone drawer: 85% width (capped at 360px), positioned below the - // mobile top bar (the grid titlebar is hidden on phone). + sidebarContainer.classList.add('mobile-overlay-sidebar'); + + // Re-layout the sidebar Part with the drawer's content dimensions + // so its internal composite/list sizing matches the CSS-positioned + // drawer (grid area minus the mobile top bar). const topBarHeight = this.mobileTopBarElement?.offsetHeight ?? 48; - const drawerWidth = Math.min(Math.floor(this._mainContainerDimension.width * 0.85), 360); + const drawerWidth = this._mainContainerDimension.width; const drawerHeight = Math.max(0, this._mainContainerDimension.height - topBarHeight); - sidebarContainer.classList.add('mobile-overlay-sidebar'); - sidebarContainer.style.position = 'fixed'; - sidebarContainer.style.top = `${topBarHeight}px`; - sidebarContainer.style.left = '0'; - sidebarContainer.style.width = `${drawerWidth}px`; - sidebarContainer.style.height = `${drawerHeight}px`; - sidebarContainer.style.zIndex = '30'; sidebarPart.layout(drawerWidth, drawerHeight, topBarHeight, 0); }