diff --git a/.github/instructions/accessibility.instructions.md b/.github/instructions/accessibility.instructions.md new file mode 100644 index 0000000000000..7777574315a5d --- /dev/null +++ b/.github/instructions/accessibility.instructions.md @@ -0,0 +1,273 @@ +--- +description: Accessibility guidelines for VS Code features — covers accessibility help dialogs, accessible views, verbosity settings, accessibility signals, ARIA alerts/status announcements, keyboard navigation, and ARIA labels/roles. Applies to both new interactive UI surfaces and updates to existing features. +--- + +When adding a **new interactive UI surface** to VS Code — a panel, view, widget, editor overlay, dialog, or any rich focusable component the user interacts with — you **must** provide three accessibility components (if they do not already exist for the feature): + +1. **An Accessibility Help Dialog** — opened via the accessibility help keybinding when the feature has focus. +2. **An Accessible View** — a plain-text read-only editor that presents the feature's content to screen reader users (when the feature displays non-trivial visual content). +3. **An Accessibility Verbosity Setting** — a boolean setting that controls whether the "open accessibility help" hint is announced. + +Examples of existing features that have all three: the **terminal**, **chat panel**, **notebook**, **diff editor**, **inline completions**, **comments**, **debug REPL**, **hover**, and **notifications**. Features with only a help dialog (no accessible view) include **find widgets**, **source control input**, **keybindings editor**, **problems panel**, and **walkthroughs**. + +Sections 4–7 below (signals, ARIA announcements, keyboard navigation, ARIA labels) apply more broadly to **any UI change**, including modifications to existing features. + +When **updating an existing feature** — for example, adding new commands, keyboard shortcuts, or interactive capabilities — you must also update the feature's existing accessibility help dialog (`provideContent()`) to document the new functionality. Screen reader users rely on the help dialog as the primary way to discover available actions. + +--- + +## 1. Accessibility Help Dialog + +An accessibility help dialog tells the user what the feature does, which keyboard shortcuts are available, and how to interact with it via a screen reader. + +### Steps + +1. **Create a class implementing `IAccessibleViewImplementation`** with `type = AccessibleViewType.Help`. + - Set a `priority` (higher = shown first when multiple providers match). + - Set `when` to a `ContextKeyExpression` that matches when the feature is focused. + - `getProvider(accessor)` returns an `AccessibleContentProvider`. + +2. **Create a content-provider class** implementing `IAccessibleViewContentProvider`. + - `id` — add a new entry in the `AccessibleViewProviderId` enum in `src/vs/platform/accessibility/browser/accessibleView.ts`. + - `verbositySettingKey` — reference the new `AccessibilityVerbositySettingId` entry (see §3). + - `options` — `{ type: AccessibleViewType.Help }`. + - `provideContent()` — return localized, multi-line help text. + +3. **Implement `onClose()`** to restore focus to whatever element was focused before the help dialog opened. This ensures keyboard users and screen reader users return to their previous context. + +4. **Register** the implementation: + ```ts + AccessibleViewRegistry.register(new MyFeatureAccessibilityHelp()); + ``` + in the feature's `*.contribution.ts` file. + +### Example skeleton + +```ts +import { AccessibleViewType, AccessibleContentProvider, AccessibleViewProviderId, IAccessibleViewContentProvider, IAccessibleViewOptions } from '…/accessibleView.js'; +import { IAccessibleViewImplementation } from '…/accessibleViewRegistry.js'; +import { AccessibilityVerbositySettingId } from '…/accessibilityConfiguration.js'; + +export class MyFeatureAccessibilityHelp implements IAccessibleViewImplementation { + readonly priority = 100; + readonly name = 'my-feature'; + readonly type = AccessibleViewType.Help; + readonly when = MyFeatureContextKeys.isFocused; + + getProvider(accessor: ServicesAccessor) { + return new MyFeatureAccessibilityHelpProvider(); + } +} + +class MyFeatureAccessibilityHelpProvider extends Disposable implements IAccessibleViewContentProvider { + readonly id = AccessibleViewProviderId.MyFeature; + readonly verbositySettingKey = AccessibilityVerbositySettingId.MyFeature; + readonly options: IAccessibleViewOptions = { type: AccessibleViewType.Help }; + + provideContent(): string { + return [ + localize('myFeature.help.header', "Accessibility Help: My Feature"), + localize('myFeature.help.overview', "You are in My Feature. …"), + '', + localize('myFeature.help.keys', "Keyboard shortcuts:"), + localize('myFeature.help.key1', "- {0}: Do something", ''), + ].join('\n'); + } +} +``` + +--- + +## 2. Accessible View + +An accessible view presents the feature's visual content as plain text in a read-only editor. It is required when the feature renders rich or visual content that a screen reader cannot directly read (for example: chat responses, hover tooltips, notifications, terminal output, inline completions). + +If the feature is purely keyboard-driven with native text input/output (e.g., a simple input field), an accessible view is not needed — only an accessibility help dialog is required. + +### Steps + +1. **Create a class implementing `IAccessibleViewImplementation`** with `type = AccessibleViewType.View`. +2. **Create a content-provider** similar to the help dialog, but: + - `options` — `{ type: AccessibleViewType.View }`, optionally with a `language` for syntax highlighting. + - `provideContent()` — return the feature's current content as plain text. + - Optionally implement `provideNextContent()` / `providePreviousContent()` for item-by-item navigation. + - Implement `onClose()` to restore focus to whatever was focused before the accessible view was opened. + - Optionally provide `actions` for actions the user can take from the accessible view. +3. **Register** alongside the help dialog: + ```ts + AccessibleViewRegistry.register(new MyFeatureAccessibleView()); + ``` + +### Example skeleton + +```ts +export class MyFeatureAccessibleView implements IAccessibleViewImplementation { + readonly priority = 100; + readonly name = 'my-feature'; + readonly type = AccessibleViewType.View; + readonly when = MyFeatureContextKeys.isFocused; + + getProvider(accessor: ServicesAccessor) { + // Retrieve services, build content from the feature's current state + const content = getMyFeatureContent(); + if (!content) { + return undefined; + } + return new AccessibleContentProvider( + AccessibleViewProviderId.MyFeature, + { type: AccessibleViewType.View }, + () => content, + () => { /* onClose — refocus whatever was focused before the accessible view opened */ }, + AccessibilityVerbositySettingId.MyFeature, + ); + } +} +``` + +--- + +## 3. Accessibility Verbosity Setting + +A verbosity setting controls whether a hint such as "press Alt+F1 for accessibility help" is announced when the feature gains focus. Users who already know the shortcut can disable it. + +### Steps + +1. **Add an entry** to `AccessibilityVerbositySettingId` in + `src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts`: + ```ts + export const enum AccessibilityVerbositySettingId { + // … existing entries … + MyFeature = 'accessibility.verbosity.myFeature' + } + ``` + +2. **Register the configuration property** in the same file's `configuration.properties` object: + ```ts + [AccessibilityVerbositySettingId.MyFeature]: { + description: localize('verbosity.myFeature.description', + 'Provide information about how to access the My Feature accessibility help menu when My Feature is focused.'), + ...baseVerbosityProperty + }, + ``` + The `baseVerbosityProperty` gives it `type: 'boolean'`, `default: true`, and `tags: ['accessibility']`. + +3. **Reference the setting key** in both the help-dialog provider (`verbositySettingKey`) and the accessible-view provider so the runtime can check whether to show the hint. + +--- + +## 4. Accessibility Signals (Sounds & Announcements) + +Accessibility signals provide audible and spoken feedback for events that happen visually. Use `IAccessibilitySignalService` to play signals when something important occurs (e.g., an error appears, a task completes, content changes). + +### When to use + +- **Use an existing signal** when the event already has one defined (see `AccessibilitySignal.*` static members — e.g., `AccessibilitySignal.errorAtPosition`, `AccessibilitySignal.terminalQuickFix`, `AccessibilitySignal.clear`). +- **If no existing signal fits**, reach out to @meganrogge to discuss adding a new one. Do not register new signals without coordinating first. + +### How signals work + +Each signal has two modalities controlled by user settings: +- **Sound** — a short audio cue, configurable to `auto` (on when screen reader attached), `on`, or `off`. +- **Announcement** — a spoken message via `aria-live`, configurable to `auto` or `off`. + +### Usage + +```ts +// Inject the service via constructor parameter +constructor( + @IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService +) { } + +// Play a signal +this._accessibilitySignalService.playSignal(AccessibilitySignal.terminalQuickFix); + +// Play with options +this._accessibilitySignalService.playSignal(AccessibilitySignal.error, { userGesture: true }); +``` + +--- + +## 5. ARIA Alerts vs. Status Messages + +Use the `alert()` and `status()` functions from `src/vs/base/browser/ui/aria/aria.ts` to announce dynamic changes to screen readers. + +### `alert(msg)` — Assertive live region (`role="alert"`) +- **Use for**: Urgent, important information that the user must know immediately. +- **Examples**: Errors, warnings, critical state changes, results of a user-initiated action. +- **Behavior**: Interrupts the screen reader's current speech. + +### `status(msg)` — Polite live region (`aria-live="polite"`) +- **Use for**: Non-urgent, informational updates that should be spoken when the screen reader is idle. +- **Examples**: Progress updates, search result counts, background state changes. +- **Behavior**: Queued and spoken after the screen reader finishes its current output. + +### Guidelines + +- **Prefer `status()` over `alert()`** unless the information is time-sensitive or the result of a direct user action. Overusing `alert()` creates a noisy, disruptive experience. +- **Keep messages concise.** Screen readers read the entire message; long messages delay the user. +- **Do not duplicate** — if an accessibility signal already announces the event, do not also call `alert()` / `status()` for the same information. +- **Localize** all messages with `nls.localize()`. + +--- + +## 6. Keyboard Navigation + +Every interactive UI element must be fully operable via the keyboard. + +### Requirements + +- **Tab order**: All interactive elements must be reachable via `Tab` / `Shift+Tab` in a logical order. +- **Arrow key navigation**: Lists, trees, grids, and toolbars must support arrow key navigation following WAI-ARIA patterns. +- **Focus visibility**: Focused elements must have a visible focus indicator (VS Code's theme system provides this via `focusBorder`). +- **No mouse-only interactions**: Every action reachable by click or hover must also be reachable via keyboard (context menus, buttons, toggles, etc.). +- **Escape to dismiss**: Overlays, dialogs, and popups must be dismissable with `Escape`, returning focus to the previous element. +- **Focus trapping**: Modal dialogs must trap focus within the dialog until dismissed. + +--- + +## 7. ARIA Labels and Roles + +All interactive UI elements must have appropriate ARIA attributes so screen readers can identify and describe them. + +### Requirements + +- **`aria-label`**: Every interactive element without visible text (icon buttons, icon-only actions, custom widgets) must have a descriptive `aria-label`. Labels should be localized. +- **`aria-labelledby`** / **`aria-describedby`**: Use these to associate elements with existing visible text rather than duplicating strings. +- **`role`**: Custom widgets that do not use native HTML elements must declare the correct ARIA role (e.g., `role="button"`, `role="tree"`, `role="tablist"`). +- **`aria-expanded`**, **`aria-selected`**, **`aria-checked`**: Toggle and selection states must be communicated via the appropriate ARIA state attributes. +- **`aria-hidden="true"`**: Decorative or redundant elements (icons next to text labels, decorative separators) must be hidden from the accessibility tree. + +### Guidelines + +- Avoid generic labels like "button" or "icon" — describe the action: "Close panel", "Toggle sidebar", "Run task". +- Test with a screen reader (VoiceOver on macOS, NVDA on Windows) to verify labels are spoken correctly in context. +- Lists and trees should use `aria-setsize` and `aria-posinset` when virtualized so screen readers report the correct count. + +--- + +## Checklist for Every New Feature + +- [ ] New `AccessibleViewProviderId` entry added in `accessibleView.ts` +- [ ] New `AccessibilityVerbositySettingId` entry added in `accessibilityConfiguration.ts` +- [ ] Verbosity setting registered in the configuration properties with `...baseVerbosityProperty` +- [ ] `IAccessibleViewImplementation` with `type = Help` created and registered +- [ ] Content provider references the correct `verbositySettingKey` +- [ ] Help text is fully localized using `nls.localize()` +- [ ] Keybindings in help text use `` syntax for dynamic resolution +- [ ] `when` context key is set so the dialog only appears when the feature is focused +- [ ] If the feature has rich/visual content: `IAccessibleViewImplementation` with `type = View` created and registered +- [ ] Registration calls in the feature's `*.contribution.ts` file +- [ ] Accessibility signal played for important events (use existing `AccessibilitySignal.*` or register a new one) +- [ ] `aria.alert()` or `aria.status()` used appropriately for dynamic changes (prefer `status()` unless urgent) +- [ ] All interactive elements reachable and operable via keyboard +- [ ] All interactive elements without visible text have a localized `aria-label` +- [ ] Custom widgets declare the correct ARIA `role` and state attributes +- [ ] Decorative elements are hidden with `aria-hidden="true"` + +## Key Files + +- `src/vs/platform/accessibility/browser/accessibleView.ts` — `AccessibleViewProviderId`, `AccessibleContentProvider`, `IAccessibleViewContentProvider` +- `src/vs/platform/accessibility/browser/accessibleViewRegistry.ts` — `AccessibleViewRegistry`, `IAccessibleViewImplementation` +- `src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts` — `AccessibilityVerbositySettingId`, verbosity setting registration +- `src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts` — `IAccessibilitySignalService`, `AccessibilitySignal` +- `src/vs/base/browser/ui/aria/aria.ts` — `alert()`, `status()` for ARIA live region announcements diff --git a/.github/skills/accessibility.md b/.github/skills/accessibility.md new file mode 100644 index 0000000000000..f3177e1e59c85 --- /dev/null +++ b/.github/skills/accessibility.md @@ -0,0 +1,292 @@ +--- +name: vscode-accessibility +description: Use when creating new UI or updating existing UI features. Accessibility guidelines for VS Code features — covers accessibility help dialogs, accessible views, verbosity settings, accessibility signals, ARIA alerts/status announcements, keyboard navigation, and ARIA labels/roles. Applies to both new interactive UI surfaces and updates to existing features. +--- + +When adding a **new interactive UI surface** to VS Code — a panel, view, widget, editor overlay, dialog, or any rich focusable component the user interacts with — you **must** provide three accessibility components (if they do not already exist for the feature): + +1. **An Accessibility Help Dialog** — opened via the accessibility help keybinding when the feature has focus. +2. **An Accessible View** — a plain-text read-only editor that presents the feature's content to screen reader users (when the feature displays non-trivial visual content). +3. **An Accessibility Verbosity Setting** — a boolean setting that controls whether the "open accessibility help" hint is announced. + +Examples of existing features that have all three: the **terminal**, **chat panel**, **notebook**, **diff editor**, **inline completions**, **comments**, **debug REPL**, **hover**, and **notifications**. Features with only a help dialog (no accessible view) include **find widgets**, **source control input**, **keybindings editor**, **problems panel**, and **walkthroughs**. + +Sections 4–7 below (signals, ARIA announcements, keyboard navigation, ARIA labels) apply more broadly to **any UI change**, including modifications to existing features. + +When **updating an existing feature** — for example, adding new commands, keyboard shortcuts, or interactive capabilities — you must also update the feature's existing accessibility help dialog (`provideContent()`) to document the new functionality. Screen reader users rely on the help dialog as the primary way to discover available actions. + +--- + +## 1. Accessibility Help Dialog + +An accessibility help dialog tells the user what the feature does, which keyboard shortcuts are available, and how to interact with it via a screen reader. + +### Steps + +1. **Create a class implementing `IAccessibleViewImplementation`** with `type = AccessibleViewType.Help`. + - Set a `priority` (higher = shown first when multiple providers match). + - Set `when` to a `ContextKeyExpression` that matches when the feature is focused. + - `getProvider(accessor)` returns an `AccessibleContentProvider`. + +2. **Create a content-provider class** implementing `IAccessibleViewContentProvider`. + - `id` — add a new entry in the `AccessibleViewProviderId` enum in `src/vs/platform/accessibility/browser/accessibleView.ts`. + - `verbositySettingKey` — reference the new `AccessibilityVerbositySettingId` entry (see §3). + - `options` — `{ type: AccessibleViewType.Help }`. + - `provideContent()` — return localized, multi-line help text. + +3. **Implement `onClose()`** to restore focus to whatever element was focused before the help dialog opened. This ensures keyboard users and screen reader users return to their previous context. + +4. **Register** the implementation: + ```ts + AccessibleViewRegistry.register(new MyFeatureAccessibilityHelp()); + ``` + in the feature's `*.contribution.ts` file. + +### Example skeleton + +The simplest approach is to return an `AccessibleContentProvider` directly from `getProvider()`. This is the most common pattern in the codebase (used by chat, inline chat, quick chat, etc.): + +```ts +import { AccessibleViewType, AccessibleContentProvider, AccessibleViewProviderId } from '…/accessibleView.js'; +import { IAccessibleViewImplementation } from '…/accessibleViewRegistry.js'; +import { AccessibilityVerbositySettingId } from '…/accessibilityConfiguration.js'; +import { AccessibleViewType, AccessibleContentProvider, AccessibleViewProviderId, IAccessibleViewContentProvider, IAccessibleViewOptions } from '../../../../platform/accessibility/browser/accessibleView.js'; +import { IAccessibleViewImplementation } from '../../../../platform/accessibility/browser/accessibleViewRegistry.js'; +import { AccessibilityVerbositySettingId } from '../../../../platform/accessibility/common/accessibilityConfiguration.js'; + +export class MyFeatureAccessibilityHelp implements IAccessibleViewImplementation { + readonly priority = 100; + readonly name = 'my-feature'; + readonly type = AccessibleViewType.Help; + readonly when = MyFeatureContextKeys.isFocused; + + getProvider(accessor: ServicesAccessor) { + const helpText = [ + localize('myFeature.help.overview', "You are in My Feature. …"), + localize('myFeature.help.key1', "- {0}: Do something", ''), + ].join('\n'); + return new AccessibleContentProvider( + AccessibleViewProviderId.MyFeature, + { type: AccessibleViewType.Help }, + () => helpText, + () => { /* onClose — refocus whatever was focused before */ }, + AccessibilityVerbositySettingId.MyFeature, + ); + } +} +``` + +Alternatively, if the provider needs injected services or must track state (e.g., storing a reference to the previously focused element), create a custom class that extends `Disposable` and implements `IAccessibleViewContentProvider`, then instantiate it via `IInstantiationService` (see `CommentsAccessibilityHelpProvider` for an example): + +```ts +class MyFeatureAccessibilityHelpProvider extends Disposable implements IAccessibleViewContentProvider { + readonly id = AccessibleViewProviderId.MyFeature; + readonly verbositySettingKey = AccessibilityVerbositySettingId.MyFeature; + readonly options: IAccessibleViewOptions = { type: AccessibleViewType.Help }; + + provideContent(): string { /* … */ } + onClose(): void { /* … */ } +} + +// In getProvider(): +getProvider(accessor: ServicesAccessor) { + return accessor.get(IInstantiationService).createInstance(MyFeatureAccessibilityHelpProvider); +} +``` + +--- + +## 2. Accessible View + +An accessible view presents the feature's visual content as plain text in a read-only editor. It is required when the feature renders rich or visual content that a screen reader cannot directly read (for example: chat responses, hover tooltips, notifications, terminal output, inline completions). + +If the feature is purely keyboard-driven with native text input/output (e.g., a simple input field), an accessible view is not needed — only an accessibility help dialog is required. + +### Steps + +1. **Create a class implementing `IAccessibleViewImplementation`** with `type = AccessibleViewType.View`. +2. **Create a content-provider** similar to the help dialog, but: + - `options` — `{ type: AccessibleViewType.View }`, optionally with a `language` for syntax highlighting. + - `provideContent()` — return the feature's current content as plain text. + - Optionally implement `provideNextContent()` / `providePreviousContent()` for item-by-item navigation. + - Implement `onClose()` to restore focus to whatever was focused before the accessible view was opened. + - Optionally provide `actions` for actions the user can take from the accessible view. +3. **Register** alongside the help dialog: + ```ts + AccessibleViewRegistry.register(new MyFeatureAccessibleView()); + ``` + +### Example skeleton + +```ts +export class MyFeatureAccessibleView implements IAccessibleViewImplementation { + readonly priority = 100; + readonly name = 'my-feature'; + readonly type = AccessibleViewType.View; + readonly when = MyFeatureContextKeys.isFocused; + + getProvider(accessor: ServicesAccessor) { + // Retrieve services, build content from the feature's current state + const content = getMyFeatureContent(); + if (!content) { + return undefined; + } + return new AccessibleContentProvider( + AccessibleViewProviderId.MyFeature, + { type: AccessibleViewType.View }, + () => content, + () => { /* onClose — refocus whatever was focused before the accessible view opened */ }, + AccessibilityVerbositySettingId.MyFeature, + ); + } +} +``` + +--- + +## 3. Accessibility Verbosity Setting + +A verbosity setting controls whether a hint such as "press Alt+F1 for accessibility help" is announced when the feature gains focus. Users who already know the shortcut can disable it. + +### Steps + +1. **Add an entry** to `AccessibilityVerbositySettingId` in + `src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts`: + ```ts + export const enum AccessibilityVerbositySettingId { + // … existing entries … + MyFeature = 'accessibility.verbosity.myFeature' + } + ``` + +2. **Register the configuration property** in the same file's `configuration.properties` object: + ```ts + [AccessibilityVerbositySettingId.MyFeature]: { + description: localize('verbosity.myFeature.description', + 'Provide information about how to access the My Feature accessibility help menu when My Feature is focused.'), + ...baseVerbosityProperty + }, + ``` + The `baseVerbosityProperty` gives it `type: 'boolean'`, `default: true`, and `tags: ['accessibility']`. + +3. **Reference the setting key** in both the help-dialog provider (`verbositySettingKey`) and the accessible-view provider so the runtime can check whether to show the hint. + +--- + +## 4. Accessibility Signals (Sounds & Announcements) + +Accessibility signals provide audible and spoken feedback for events that happen visually. Use `IAccessibilitySignalService` to play signals when something important occurs (e.g., an error appears, a task completes, content changes). + +### When to use + +- **Use an existing signal** when the event already has one defined (see `AccessibilitySignal.*` static members — e.g., `AccessibilitySignal.error`, `AccessibilitySignal.terminalQuickFix`, `AccessibilitySignal.clear`). +- **If no existing signal fits**, reach out to @meganrogge to discuss adding a new one. Do not register new signals without coordinating first. + +### How signals work + +Each signal has two modalities controlled by user settings: +- **Sound** — a short audio cue, configurable to `auto` (on when screen reader attached), `on`, or `off`. +- **Announcement** — a spoken message via `aria-live`, configurable to `auto` or `off`. + +### Usage + +```ts +// Inject the service via constructor parameter +constructor( + @IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService +) { } + +// Play a signal +this._accessibilitySignalService.playSignal(AccessibilitySignal.terminalQuickFix); + +// Play with options +this._accessibilitySignalService.playSignal(AccessibilitySignal.error, { userGesture: true }); +``` + +--- + +## 5. ARIA Alerts vs. Status Messages + +Use the `alert()` and `status()` functions from `src/vs/base/browser/ui/aria/aria.ts` to announce dynamic changes to screen readers. + +### `alert(msg)` — Assertive live region (`role="alert"`) +- **Use for**: Urgent, important information that the user must know immediately. +- **Examples**: Errors, warnings, critical state changes, results of a user-initiated action. +- **Behavior**: Interrupts the screen reader's current speech. + +### `status(msg)` — Polite live region (`aria-live="polite"`) +- **Use for**: Non-urgent, informational updates that should be spoken when the screen reader is idle. +- **Examples**: Progress updates, search result counts, background state changes. +- **Behavior**: Queued and spoken after the screen reader finishes its current output. + +### Guidelines + +- **Prefer `status()` over `alert()`** unless the information is time-sensitive or the result of a direct user action. Overusing `alert()` creates a noisy, disruptive experience. +- **Keep messages concise.** Screen readers read the entire message; long messages delay the user. +- **Do not duplicate** — if an accessibility signal already announces the event, do not also call `alert()` / `status()` for the same information. +- **Localize** all messages with `nls.localize()`. + +--- + +## 6. Keyboard Navigation + +Every interactive UI element must be fully operable via the keyboard. + +### Requirements + +- **Tab order**: All interactive elements must be reachable via `Tab` / `Shift+Tab` in a logical order. +- **Arrow key navigation**: Lists, trees, grids, and toolbars must support arrow key navigation following WAI-ARIA patterns. +- **Focus visibility**: Focused elements must have a visible focus indicator (VS Code's theme system provides this via `focusBorder`). +- **No mouse-only interactions**: Every action reachable by click or hover must also be reachable via keyboard (context menus, buttons, toggles, etc.). +- **Escape to dismiss**: Overlays, dialogs, and popups must be dismissable with `Escape`, returning focus to the previous element. +- **Focus trapping**: Modal dialogs must trap focus within the dialog until dismissed. + +--- + +## 7. ARIA Labels and Roles + +All interactive UI elements must have appropriate ARIA attributes so screen readers can identify and describe them. + +### Requirements + +- **`aria-label`**: Every interactive element without visible text (icon buttons, icon-only actions, custom widgets) must have a descriptive `aria-label`. Labels should be localized. +- **`aria-labelledby`** / **`aria-describedby`**: Use these to associate elements with existing visible text rather than duplicating strings. +- **`role`**: Custom widgets that do not use native HTML elements must declare the correct ARIA role (e.g., `role="button"`, `role="tree"`, `role="tablist"`). +- **`aria-expanded`**, **`aria-selected`**, **`aria-checked`**: Toggle and selection states must be communicated via the appropriate ARIA state attributes. +- **`aria-hidden="true"`**: Decorative or redundant elements (icons next to text labels, decorative separators) must be hidden from the accessibility tree. + +### Guidelines + +- Avoid generic labels like "button" or "icon" — describe the action: "Close panel", "Toggle sidebar", "Run task". +- Test with a screen reader (VoiceOver on macOS, NVDA on Windows) to verify labels are spoken correctly in context. +- Lists and trees should use `aria-setsize` and `aria-posinset` when virtualized so screen readers report the correct count. + +--- + +## Checklist for Every New Feature + +- [ ] New `AccessibleViewProviderId` entry added in `accessibleView.ts` +- [ ] New `AccessibilityVerbositySettingId` entry added in `accessibilityConfiguration.ts` +- [ ] Verbosity setting registered in the configuration properties with `...baseVerbosityProperty` +- [ ] `IAccessibleViewImplementation` with `type = Help` created and registered +- [ ] Content provider references the correct `verbositySettingKey` +- [ ] Help text is fully localized using `nls.localize()` +- [ ] Keybindings in help text use `` syntax for dynamic resolution +- [ ] `when` context key is set so the dialog only appears when the feature is focused +- [ ] If the feature has rich/visual content: `IAccessibleViewImplementation` with `type = View` created and registered +- [ ] Registration calls in the feature's `*.contribution.ts` file +- [ ] Accessibility signal played for important events (use existing `AccessibilitySignal.*` or register a new one) +- [ ] `aria.alert()` or `aria.status()` used appropriately for dynamic changes (prefer `status()` unless urgent) +- [ ] All interactive elements reachable and operable via keyboard +- [ ] All interactive elements without visible text have a localized `aria-label` +- [ ] Custom widgets declare the correct ARIA `role` and state attributes +- [ ] Decorative elements are hidden with `aria-hidden="true"` + +## Key Files + +- `src/vs/platform/accessibility/browser/accessibleView.ts` — `AccessibleViewProviderId`, `AccessibleContentProvider`, `IAccessibleViewContentProvider` +- `src/vs/platform/accessibility/browser/accessibleViewRegistry.ts` — `AccessibleViewRegistry`, `IAccessibleViewImplementation` +- `src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts` — `AccessibilityVerbositySettingId`, verbosity setting registration +- `src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts` — `IAccessibilitySignalService`, `AccessibilitySignal` +- `src/vs/base/browser/ui/aria/aria.ts` — `alert()`, `status()` for ARIA live region announcements + diff --git a/src/vs/editor/contrib/find/browser/findController.ts b/src/vs/editor/contrib/find/browser/findController.ts index c7d731eaa3d1b..6459340af2472 100644 --- a/src/vs/editor/contrib/find/browser/findController.ts +++ b/src/vs/editor/contrib/find/browser/findController.ts @@ -34,6 +34,8 @@ import { Selection } from '../../../common/core/selection.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { FindWidgetSearchHistory } from './findWidgetSearchHistory.js'; import { ReplaceWidgetHistory } from './replaceWidgetHistory.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; const SEARCH_STRING_MAX_LENGTH = 524288; @@ -472,6 +474,8 @@ export class FindController extends CommonFindController implements IFindControl @IStorageService _storageService: IStorageService, @IClipboardService clipboardService: IClipboardService, @IHoverService hoverService: IHoverService, + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IAccessibilityService private readonly _accessibilityService: IAccessibilityService, ) { super(editor, _contextKeyService, _storageService, clipboardService, notificationService, hoverService); this._widget = null; @@ -529,7 +533,7 @@ export class FindController extends CommonFindController implements IFindControl } private _createFindWidget() { - this._widget = this._register(new FindWidget(this._editor, this, this._state, this._contextViewService, this._keybindingService, this._contextKeyService, this._hoverService, this._findWidgetSearchHistory, this._replaceWidgetHistory)); + this._widget = this._register(new FindWidget(this._editor, this, this._state, this._contextViewService, this._keybindingService, this._contextKeyService, this._hoverService, this._findWidgetSearchHistory, this._replaceWidgetHistory, this._configurationService, this._accessibilityService)); this._findOptionsWidget = this._register(new FindOptionsWidget(this._editor, this._state, this._keybindingService)); } diff --git a/src/vs/editor/contrib/find/browser/findWidget.ts b/src/vs/editor/contrib/find/browser/findWidget.ts index 2891f84836c14..03e698c3e8777 100644 --- a/src/vs/editor/contrib/find/browser/findWidget.ts +++ b/src/vs/editor/contrib/find/browser/findWidget.ts @@ -14,11 +14,11 @@ import { ReplaceInput } from '../../../../base/browser/ui/findinput/replaceInput import { IMessage as InputBoxMessage } from '../../../../base/browser/ui/inputbox/inputBox.js'; import { ISashEvent, IVerticalSashLayoutProvider, Orientation, Sash } from '../../../../base/browser/ui/sash/sash.js'; import { Widget } from '../../../../base/browser/ui/widget.js'; -import { Delayer } from '../../../../base/common/async.js'; +import { Delayer, disposableTimeout } from '../../../../base/common/async.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { onUnexpectedError } from '../../../../base/common/errors.js'; import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; -import { toDisposable } from '../../../../base/common/lifecycle.js'; +import { toDisposable, IDisposable } from '../../../../base/common/lifecycle.js'; import * as platform from '../../../../base/common/platform.js'; import * as strings from '../../../../base/common/strings.js'; import './findWidget.css'; @@ -28,7 +28,7 @@ import { Range } from '../../../common/core/range.js'; import { CONTEXT_FIND_INPUT_FOCUSED, CONTEXT_FIND_WIDGET_FOCUSED, CONTEXT_REPLACE_INPUT_FOCUSED, FIND_IDS, MATCHES_LIMIT } from './findModel.js'; import { FindReplaceState, FindReplaceStateChangedEvent } from './findState.js'; import * as nls from '../../../../nls.js'; -import { AccessibilitySupport } from '../../../../platform/accessibility/common/accessibility.js'; +import { AccessibilitySupport, IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; import { ContextScopedFindInput, ContextScopedReplaceInput } from '../../../../platform/history/browser/contextScopedHistoryWidget.js'; import { showHistoryKeybindingHint } from '../../../../platform/history/browser/historyWidgetKeybindingHint.js'; import { IContextKey, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; @@ -44,6 +44,7 @@ import { Selection } from '../../../common/core/selection.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { IHistory } from '../../../../base/common/history.js'; import { HoverStyle, type IHoverLifecycleOptions } from '../../../../base/browser/ui/hover/hover.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; const findCollapsedIcon = registerIcon('find-collapsed', Codicon.chevronRight, nls.localize('findCollapsedIcon', 'Icon to indicate that the editor find widget is collapsed.')); const findExpandedIcon = registerIcon('find-expanded', Codicon.chevronDown, nls.localize('findExpandedIcon', 'Icon to indicate that the editor find widget is expanded.')); @@ -144,6 +145,8 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL private _isVisible: boolean; private _isReplaceVisible: boolean; private _ignoreChangeEvent: boolean; + private _accessibilityHelpHintAnnounced: boolean; + private _labelResetTimeout: IDisposable | undefined; private _lastFocusedInputWasReplace: boolean = false; private readonly _findFocusTracker: dom.IFocusTracker; @@ -170,6 +173,8 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL private readonly _hoverService: IHoverService, private readonly _findWidgetSearchHistory: IHistory | undefined, private readonly _replaceWidgetHistory: IHistory | undefined, + private readonly _configurationService: IConfigurationService, + private readonly _accessibilityService: IAccessibilityService, ) { super(); this._codeEditor = codeEditor; @@ -182,6 +187,7 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL this._isVisible = false; this._isReplaceVisible = false; this._ignoreChangeEvent = false; + this._accessibilityHelpHintAnnounced = false; this._updateHistoryDelayer = new Delayer(500); this._register(toDisposable(() => this._updateHistoryDelayer.cancel())); @@ -483,23 +489,25 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL // ----- actions private _getAriaLabel(label: string, currentMatch: Range | null, searchString: string): string { + let result: string; if (label === NLS_NO_RESULTS) { - return searchString === '' + result = searchString === '' ? nls.localize('ariaSearchNoResultEmpty', "{0} found", label) : nls.localize('ariaSearchNoResult', "{0} found for '{1}'", label, searchString); - } - if (currentMatch) { + } else if (currentMatch) { const ariaLabel = nls.localize('ariaSearchNoResultWithLineNum', "{0} found for '{1}', at {2}", label, searchString, currentMatch.startLineNumber + ':' + currentMatch.startColumn); const model = this._codeEditor.getModel(); if (model && (currentMatch.startLineNumber <= model.getLineCount()) && (currentMatch.startLineNumber >= 1)) { const lineContent = model.getLineContent(currentMatch.startLineNumber); - return `${lineContent}, ${ariaLabel}`; + result = `${lineContent}, ${ariaLabel}`; + } else { + result = ariaLabel; } - - return ariaLabel; + } else { + result = nls.localize('ariaSearchNoResultWithLineNumNoCurrentMatch', "{0} found for '{1}'", label, searchString); } - return nls.localize('ariaSearchNoResultWithLineNumNoCurrentMatch', "{0} found for '{1}'", label, searchString); + return result; } /** @@ -574,6 +582,7 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL this._revealTimeouts.push(setTimeout(() => { this._domNode.classList.add('visible'); this._domNode.setAttribute('aria-hidden', 'false'); + this._updateFindInputAriaLabel(); }, 0)); // validate query again as it's being dismissed when we hide the find widget. @@ -622,6 +631,7 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL if (this._isVisible) { this._isVisible = false; + this._accessibilityHelpHintAnnounced = false; this._updateButtons(); @@ -1324,6 +1334,31 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL private updateAccessibilitySupport(): void { const value = this._codeEditor.getOption(EditorOption.accessibilitySupport); this._findInput.setFocusInputOnOptionClick(value !== AccessibilitySupport.Enabled); + this._updateFindInputAriaLabel(); + } + + private _updateFindInputAriaLabel(): void { + let findLabel = NLS_FIND_INPUT_LABEL; + let replaceLabel = NLS_REPLACE_INPUT_LABEL; + if (!this._accessibilityHelpHintAnnounced && this._configurationService.getValue('accessibility.verbosity.find') && this._accessibilityService.isScreenReaderOptimized()) { + const accessibilityHelpKeybinding = this._keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); + if (accessibilityHelpKeybinding) { + const hint = nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", accessibilityHelpKeybinding); + findLabel = nls.localize('findInputAriaLabelWithHint', "{0}, {1}", findLabel, hint); + replaceLabel = nls.localize('replaceInputAriaLabelWithHint', "{0}, {1}", replaceLabel, hint); + } + this._accessibilityHelpHintAnnounced = true; + // Schedule reset to plain labels after initial announcement + this._labelResetTimeout?.dispose(); + this._labelResetTimeout = disposableTimeout(() => { + if (this._isVisible) { + this._findInput.inputBox.setAriaLabel(NLS_FIND_INPUT_LABEL); + this._replaceInput.inputBox.setAriaLabel(NLS_REPLACE_INPUT_LABEL); + } + }, 1000); + } + this._findInput.inputBox.setAriaLabel(findLabel); + this._replaceInput.inputBox.setAriaLabel(replaceLabel); } getViewState() { diff --git a/src/vs/workbench/api/common/extHostLanguageModelTools.ts b/src/vs/workbench/api/common/extHostLanguageModelTools.ts index 84421c89cd32c..68b95881b32ae 100644 --- a/src/vs/workbench/api/common/extHostLanguageModelTools.ts +++ b/src/vs/workbench/api/common/extHostLanguageModelTools.ts @@ -290,8 +290,12 @@ export class ExtHostLanguageModelTools implements ExtHostLanguageModelToolsShape chatRequestId: context.chatRequestId, chatSessionId: context.chatSessionId, chatSessionResource: context.chatSessionResource, - chatInteractionId: context.chatInteractionId + chatInteractionId: context.chatInteractionId, + forceConfirmationReason: context.forceConfirmationReason }; + if (context.forceConfirmationReason) { + checkProposedApiEnabled(item.extension, 'chatParticipantPrivate'); + } if (item.tool.prepareInvocation) { const result = await item.tool.prepareInvocation(options, token); if (!result) { @@ -309,7 +313,7 @@ export class ExtHostLanguageModelTools implements ExtHostLanguageModelToolsShape } : undefined, invocationMessage: typeConvert.MarkdownString.fromStrict(result.invocationMessage), pastTenseMessage: typeConvert.MarkdownString.fromStrict(result.pastTenseMessage), - presentation: result.presentation as ToolInvocationPresentation | undefined + presentation: result.presentation as ToolInvocationPresentation | undefined, }; } diff --git a/src/vs/workbench/browser/parts/views/viewFilter.ts b/src/vs/workbench/browser/parts/views/viewFilter.ts index d6d5197adc2fa..8216d46c29c26 100644 --- a/src/vs/workbench/browser/parts/views/viewFilter.ts +++ b/src/vs/workbench/browser/parts/views/viewFilter.ts @@ -3,14 +3,14 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Delayer } from '../../../../base/common/async.js'; +import { Delayer, disposableTimeout } from '../../../../base/common/async.js'; import * as DOM from '../../../../base/browser/dom.js'; import { IAction } from '../../../../base/common/actions.js'; import { HistoryInputBox } from '../../../../base/browser/ui/inputbox/inputBox.js'; import { KeyCode } from '../../../../base/common/keyCodes.js'; import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js'; import { IContextViewService } from '../../../../platform/contextview/browser/contextView.js'; -import { toDisposable } from '../../../../base/common/lifecycle.js'; +import { toDisposable, IDisposable } from '../../../../base/common/lifecycle.js'; import { badgeBackground, badgeForeground, contrastBorder, asCssVariable } from '../../../../platform/theme/common/colorRegistry.js'; import { localize } from '../../../../nls.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; @@ -26,6 +26,8 @@ import { Widget } from '../../../../base/browser/ui/widget.js'; import { Emitter } from '../../../../base/common/event.js'; import { defaultInputBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { IActionViewItemOptions } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; const viewFilterMenu = new MenuId('menu.view.filter'); export const viewFilterSubmenu = new MenuId('submenu.view.filter'); @@ -85,6 +87,14 @@ export class FilterWidget extends Widget { private isMoreFiltersChecked: boolean = false; private lastWidth?: number; + /** + * Tracks whether the accessibility help hint has been announced in the ARIA label. + * Reset when the widget loses focus, allowing the hint to be announced again + * on the next focus. + */ + private _accessibilityHelpHintAnnounced: boolean = false; + private _labelResetTimeout: IDisposable | undefined; + private readonly focusTracker: DOM.IFocusTracker; get onDidFocus() { return this.focusTracker.onDidFocus; } get onDidBlur() { return this.focusTracker.onDidBlur; } @@ -94,7 +104,9 @@ export class FilterWidget extends Widget { @IInstantiationService private readonly instantiationService: IInstantiationService, @IContextViewService private readonly contextViewService: IContextViewService, @IContextKeyService contextKeyService: IContextKeyService, - @IKeybindingService private readonly keybindingService: IKeybindingService + @IKeybindingService private readonly keybindingService: IKeybindingService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IAccessibilityService private readonly accessibilityService: IAccessibilityService ) { super(); this.delayedFilterUpdate = new Delayer(300); @@ -121,9 +133,38 @@ export class FilterWidget extends Widget { } focus(): void { + this._updateFilterInputAriaLabel(); this.filterInputBox.focus(); } + /** + * Updates the ARIA label of the filter input box. + * When a screen reader is active and the accessibility verbosity setting is enabled, + * includes a hint about pressing Alt+F1 for accessibility help on first focus. + * The hint is only announced once per focus cycle to prevent double-speak. + */ + private _updateFilterInputAriaLabel(): void { + let ariaLabel = this.options.ariaLabel || localize('viewFilter', "Filter"); + + // Include accessibility help hint when screen reader is active and setting is enabled + // Note: Using string literal for setting ID to avoid layering violation (viewFilter.ts cannot import from contrib modules) + if (!this._accessibilityHelpHintAnnounced && this.configurationService.getValue('accessibility.verbosity.find') && this.accessibilityService.isScreenReaderOptimized()) { + const keybinding = this.keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); + if (keybinding) { + ariaLabel += ', ' + localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); + this._accessibilityHelpHintAnnounced = true; + + // Reset to plain label after delay to avoid repeated announcement on focus changes + this._labelResetTimeout?.dispose(); + this._labelResetTimeout = disposableTimeout(() => { + this.filterInputBox.setAriaLabel(this.options.ariaLabel || localize('viewFilter', "Filter")); + }, 1000); + } + } + + this.filterInputBox.setAriaLabel(ariaLabel); + } + blur(): void { this.filterInputBox.blur(); } diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserFindWidget.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserFindWidget.ts index f95b378d66b44..ea918e0428f7c 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserFindWidget.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserFindWidget.ts @@ -8,6 +8,8 @@ import { IContextViewService } from '../../../../platform/contextview/browser/co import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; import { IBrowserViewModel } from '../common/browserView.js'; import { localize } from '../../../../nls.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; @@ -33,7 +35,9 @@ export class BrowserFindWidget extends SimpleFindWidget { @IContextViewService contextViewService: IContextViewService, @IContextKeyService contextKeyService: IContextKeyService, @IHoverService hoverService: IHoverService, - @IKeybindingService keybindingService: IKeybindingService + @IKeybindingService keybindingService: IKeybindingService, + @IConfigurationService configurationService: IConfigurationService, + @IAccessibilityService accessibilityService: IAccessibilityService ) { super({ showCommonFindToggles: true, @@ -44,7 +48,7 @@ export class BrowserFindWidget extends SimpleFindWidget { previousMatchActionId: 'workbench.action.browser.findPrevious', nextMatchActionId: 'workbench.action.browser.findNext', closeWidgetActionId: 'workbench.action.browser.hideFind' - }, contextViewService, contextKeyService, hoverService, keybindingService); + }, contextViewService, contextKeyService, hoverService, keybindingService, configurationService, accessibilityService); this._findWidgetVisible = CONTEXT_BROWSER_FIND_WIDGET_VISIBLE.bindTo(contextKeyService); this._findWidgetFocused = CONTEXT_BROWSER_FIND_WIDGET_FOCUSED.bindTo(contextKeyService); diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts b/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts index 533761a59011a..50ae8b00a26ca 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts @@ -67,6 +67,10 @@ const TREE_END = '└─'; const ICON_ERROR = '❌'; // allow-any-unicode-next-line const ICON_WARN = '⚠️'; +// allow-any-unicode-next-line +const ICON_MANUAL = '🔧'; +// allow-any-unicode-next-line +const ICON_HIDDEN = '👁️‍🗨️'; /** * Information about a file that was loaded or skipped. @@ -81,6 +85,10 @@ export interface IFileStatusInfo { overwrittenBy?: string; /** Extension ID if this file comes from an extension */ extensionId?: string; + /** If true, hidden from / menu (user-invokable: false) */ + userInvokable?: boolean; + /** If true, won't be auto-loaded by agent (disable-model-invocation: true) */ + disableModelInvocation?: boolean; } /** @@ -440,7 +448,9 @@ function convertDiscoveryResultToFileStatus(result: IPromptFileDiscoveryResult): status: 'loaded', name: result.name, storage: result.storage, - extensionId: result.extensionId + extensionId: result.extensionId, + userInvokable: result.userInvokable, + disableModelInvocation: result.disableModelInvocation }; } @@ -607,7 +617,8 @@ export function formatStatusOutput( const prefix = isLast ? TREE_END : TREE_BRANCH; const filePath = getRelativePath(file.uri, workspaceFolders); if (file.status === 'loaded') { - lines.push(`${prefix} [\`${fileName}\`](${filePath})
`); + const flags = getSkillFlags(file, info.type); + lines.push(`${prefix} [\`${fileName}\`](${filePath})${flags}
`); } else if (file.status === 'overwritten') { lines.push(`${prefix} ${ICON_WARN} [\`${fileName}\`](${filePath}) - *${nls.localize('status.overwrittenByHigherPriority', 'Overwritten by higher priority file')}*
`); } else { @@ -648,7 +659,8 @@ export function formatStatusOutput( const prefix = isLast ? TREE_END : TREE_BRANCH; const filePath = getRelativePath(file.uri, workspaceFolders); if (file.status === 'loaded') { - lines.push(`${prefix} [\`${fileName}\`](${filePath})
`); + const flags = getSkillFlags(file, info.type); + lines.push(`${prefix} [\`${fileName}\`](${filePath})${flags}
`); } else if (file.status === 'overwritten') { lines.push(`${prefix} ${ICON_WARN} [\`${fileName}\`](${filePath}) - *${nls.localize('status.overwrittenByHigherPriority', 'Overwritten by higher priority file')}*
`); } else { @@ -738,6 +750,34 @@ export function formatStatusOutput( return lines.join('\n'); } +/** + * Gets flag annotations for skills based on their visibility settings. + * Returns an empty string for non-skill types or skills with default settings. + */ +function getSkillFlags(file: IFileStatusInfo, type: PromptsType): string { + if (type !== PromptsType.skill) { + return ''; + } + + const flags: string[] = []; + + // disableModelInvocation: true means agent won't auto-load, only manual /name trigger + if (file.disableModelInvocation) { + flags.push(`${ICON_MANUAL} *${nls.localize('status.skill.manualOnly', 'manual only')}*`); + } + + // userInvokable: false means hidden from / menu + if (file.userInvokable === false) { + flags.push(`${ICON_HIDDEN} *${nls.localize('status.skill.hiddenFromMenu', 'hidden from menu')}*`); + } + + if (flags.length === 0) { + return ''; + } + + return ` - ${flags.join(', ')}`; +} + /** * Checks if a file URI is under a given path URI. */ diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts index 7605b5b69fe72..8a363395dc855 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts @@ -178,7 +178,8 @@ abstract class SubmitAction extends Action2 { } } -const whenNotInProgress = ChatContextKeys.requestInProgress.negate(); +const requestInProgressOrPendingToolCall = ContextKeyExpr.or(ChatContextKeys.requestInProgress, ChatContextKeys.Editing.hasToolConfirmation); +const whenNotInProgress = ContextKeyExpr.and(ChatContextKeys.requestInProgress.negate(), ChatContextKeys.Editing.hasToolConfirmation.negate()); export class ChatSubmitAction extends SubmitAction { static readonly ID = 'workbench.action.chat.submit'; @@ -667,7 +668,7 @@ export class ChatEditingSessionSubmitAction extends SubmitAction { id: MenuId.ChatExecute, order: 4, when: ContextKeyExpr.and( - ChatContextKeys.requestInProgress.negate(), + whenNotInProgress, menuCondition), group: 'navigation', alt: { @@ -821,7 +822,7 @@ export class CancelAction extends Action2 { menu: [{ id: MenuId.ChatExecute, when: ContextKeyExpr.and( - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, ChatContextKeys.remoteJobCreating.negate(), ChatContextKeys.currentlyEditing.negate(), ), @@ -841,7 +842,7 @@ export class CancelAction extends Action2 { weight: KeybindingWeight.WorkbenchContrib, primary: KeyMod.CtrlCmd | KeyCode.Escape, when: ContextKeyExpr.and( - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, ChatContextKeys.remoteJobCreating.negate() ), win: { primary: KeyMod.Alt | KeyCode.Backspace }, diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts index bac6879be0dd4..b6627493b7c87 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts @@ -6,11 +6,11 @@ import { Codicon } from '../../../../../base/common/codicons.js'; import { KeyCode, KeyMod } from '../../../../../base/common/keyCodes.js'; import { URI } from '../../../../../base/common/uri.js'; +import { ServicesAccessor } from '../../../../../editor/browser/editorExtensions.js'; import { localize, localize2 } from '../../../../../nls.js'; import { Action2, MenuId, MenuRegistry, registerAction2 } from '../../../../../platform/actions/common/actions.js'; import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; import { KeybindingWeight } from '../../../../../platform/keybinding/common/keybindingsRegistry.js'; -import { ServicesAccessor } from '../../../../../editor/browser/editorExtensions.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { ChatRequestQueueKind, IChatService } from '../../common/chatService/chatService.js'; import { ChatConfiguration } from '../../common/constants.js'; @@ -19,6 +19,7 @@ import { IChatWidgetService } from '../chat.js'; import { CHAT_CATEGORY } from './chatActions.js'; const queueingEnabledCondition = ContextKeyExpr.equals(`config.${ChatConfiguration.RequestQueueingEnabled}`, true); +const requestInProgressOrPendingToolCall = ContextKeyExpr.or(ChatContextKeys.requestInProgress, ChatContextKeys.Editing.hasToolConfirmation); export interface IChatRemovePendingRequestContext { sessionResource: URI; @@ -47,23 +48,18 @@ export class ChatQueueMessageAction extends Action2 { category: CHAT_CATEGORY, precondition: ContextKeyExpr.and( queueingEnabledCondition, - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, ChatContextKeys.inputHasText ), keybinding: { when: ContextKeyExpr.and( ChatContextKeys.inChatInput, - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, queueingEnabledCondition ), primary: KeyCode.Enter, weight: KeybindingWeight.EditorContrib + 1 }, - menu: [{ - id: MenuId.ChatExecuteQueue, - group: 'navigation', - order: 1, - }] }); } @@ -96,23 +92,18 @@ export class ChatSteerWithMessageAction extends Action2 { category: CHAT_CATEGORY, precondition: ContextKeyExpr.and( queueingEnabledCondition, - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, ChatContextKeys.inputHasText ), keybinding: { when: ContextKeyExpr.and( ChatContextKeys.inChatInput, - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, queueingEnabledCondition ), primary: KeyMod.Alt | KeyCode.Enter, weight: KeybindingWeight.EditorContrib + 1 }, - menu: [{ - id: MenuId.ChatExecuteQueue, - group: 'navigation', - order: 2, - }] }); } @@ -277,19 +268,31 @@ export function registerChatQueueActions(): void { registerAction2(ChatSendPendingImmediatelyAction); registerAction2(ChatRemoveAllPendingRequestsAction); - // Register the queue submenu as a split button dropdown in the execute toolbar - // This shows "Add to Queue" / "Steer with Message" when a request is in progress and input has text + // Register the queue submenu in the execute toolbar. + // The custom ChatQueuePickerActionItem (registered via IActionViewItemService) + // replaces the default rendering with a dropdown that shows hover descriptions. + // We still need items in ChatExecuteQueue so the menu system treats it as non-empty. + MenuRegistry.appendMenuItem(MenuId.ChatExecuteQueue, { + command: { id: ChatQueueMessageAction.ID, title: localize2('chat.queueMessage', "Add to Queue"), icon: Codicon.add }, + group: 'navigation', + order: 1, + }); + MenuRegistry.appendMenuItem(MenuId.ChatExecuteQueue, { + command: { id: ChatSteerWithMessageAction.ID, title: localize2('chat.steerWithMessage', "Steer with Message"), icon: Codicon.arrowRight }, + group: 'navigation', + order: 2, + }); + MenuRegistry.appendMenuItem(MenuId.ChatExecute, { submenu: MenuId.ChatExecuteQueue, title: localize2('chat.queueSubmenu', "Queue"), icon: Codicon.listOrdered, when: ContextKeyExpr.and( queueingEnabledCondition, - ChatContextKeys.requestInProgress, + requestInProgressOrPendingToolCall, ChatContextKeys.inputHasText ), group: 'navigation', order: 4, - isSplitButton: { togglePrimaryAction: true } }); } diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index 2e05c9c728943..0cc2595296b8f 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -141,6 +141,7 @@ import { ChatWindowNotifier } from './chatWindowNotifier.js'; import { ChatRepoInfoContribution } from './chatRepoInfo.js'; import { VALID_PROMPT_FOLDER_PATTERN } from '../common/promptSyntax/utils/promptFilesLocator.js'; import { ChatTipService, IChatTipService } from './chatTipService.js'; +import { ChatQueuePickerRendering } from './widget/input/chatQueuePickerActionItem.js'; const toolReferenceNameEnumValues: string[] = []; const toolReferenceNameEnumDescriptions: string[] = []; @@ -613,6 +614,16 @@ configurationRegistry.registerConfiguration({ default: true, tags: ['experimental'], }, + [ChatConfiguration.RequestQueueingDefaultAction]: { + type: 'string', + enum: ['queue', 'steer'], + enumDescriptions: [ + nls.localize('chat.requestQueuing.defaultAction.queue', "Queue the message to send after the current request completes."), + nls.localize('chat.requestQueuing.defaultAction.steer', "Steer the current request by sending the message immediately, signaling the current request to yield."), + ], + description: nls.localize('chat.requestQueuing.defaultAction.description', "Controls which action is the default for the queue button when a request is in progress."), + default: 'steer', + }, [ChatConfiguration.EditModeHidden]: { type: 'boolean', description: nls.localize('chat.editMode.hidden', "When enabled, hides the Edit mode from the chat mode picker."), @@ -1438,6 +1449,7 @@ registerWorkbenchContribution2(ChatAgentActionsContribution.ID, ChatAgentActions registerWorkbenchContribution2(ToolReferenceNamesContribution.ID, ToolReferenceNamesContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(ChatAgentRecommendation.ID, ChatAgentRecommendation, WorkbenchPhase.Eventually); registerWorkbenchContribution2(ChatEditingEditorAccessibility.ID, ChatEditingEditorAccessibility, WorkbenchPhase.AfterRestored); +registerWorkbenchContribution2(ChatQueuePickerRendering.ID, ChatQueuePickerRendering, WorkbenchPhase.BlockRestore); registerWorkbenchContribution2(ChatEditingEditorOverlay.ID, ChatEditingEditorOverlay, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(SimpleBrowserOverlay.ID, SimpleBrowserOverlay, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(ChatEditingEditorContextKeys.ID, ChatEditingEditorContextKeys, WorkbenchPhase.AfterRestored); diff --git a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts index 21d55ce5f45c0..5536b0defbef9 100644 --- a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts @@ -34,7 +34,7 @@ import { Registry } from '../../../../../platform/registry/common/platform.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; -import { IPreToolUseCallerInput } from '../../common/hooks/hooksTypes.js'; +import { IPreToolUseCallerInput, IPreToolUseHookResult } from '../../common/hooks/hooksTypes.js'; import { IHooksExecutionService } from '../../common/hooks/hooksExecutionService.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { ChatRequestToolReferenceEntry, toToolSetVariableEntry, toToolVariableEntry } from '../../common/attachments/chatVariableEntries.js'; @@ -363,19 +363,21 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo /** * Execute the preToolUse hook and handle denial. - * Returns a tool result if the hook denied execution, or undefined to continue. + * Returns an object containing: + * - denialResult: A tool result if the hook denied execution (caller should return early) + * - hookResult: The full hook result for use in auto-approval logic (allow/ask decisions) * @param pendingInvocation If there's an existing streaming invocation from beginToolCall, pass it here to cancel it instead of creating a new one. */ - private async _executePreToolUseHookAndHandleDenial( + private async _executePreToolUseHook( dto: IToolInvocation, toolData: IToolData | undefined, request: IChatRequestModel | undefined, pendingInvocation: ChatToolInvocation | undefined, token: CancellationToken - ): Promise { + ): Promise<{ denialResult?: IToolResult; hookResult?: IPreToolUseHookResult }> { // Skip hook if no session context or tool doesn't exist if (!dto.context?.sessionResource || !toolData) { - return undefined; + return {}; } const hookInput: IPreToolUseCallerInput = { @@ -409,12 +411,15 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo const denialMessage = localize('toolExecutionDenied', "Tool execution denied: {0}", hookReason); return { - content: [{ kind: 'text', value: denialMessage }], - toolResultError: hookReason, + denialResult: { + content: [{ kind: 'text', value: denialMessage }], + toolResultError: hookReason, + }, + hookResult, }; } - return undefined; + return { hookResult }; } async invokeTool(dto: IToolInvocation, countTokens: CountTokensCallback, token: CancellationToken): Promise { @@ -465,7 +470,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } // Execute preToolUse hook - returns early if hook denies execution - const hookDenialResult = await this._executePreToolUseHookAndHandleDenial(dto, toolData, request, toolInvocation, token); + const { denialResult: hookDenialResult, hookResult: preToolUseHookResult } = await this._executePreToolUseHook(dto, toolData, request, toolInvocation, token); if (hookDenialResult) { // Clean up pending tool call if it exists if (pendingToolCallKey) { @@ -522,10 +527,11 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo dto.userSelectedTools = request.userSelectedTools && { ...request.userSelectedTools }; prepareTimeWatch = StopWatch.create(true); - preparedInvocation = await this.prepareToolInvocation(tool, dto, token); + preparedInvocation = await this.prepareToolInvocationWithHookResult(tool, dto, preToolUseHookResult, token); prepareTimeWatch.stop(); - const autoConfirmed = await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace, tool.data.source, dto.parameters, dto.context?.sessionResource); + const { autoConfirmed, preparedInvocation: updatedPreparedInvocation } = await this.resolveAutoConfirmFromHook(preToolUseHookResult, tool, dto, preparedInvocation, dto.context?.sessionResource); + preparedInvocation = updatedPreparedInvocation; // Important: a tool invocation that will be autoconfirmed should never @@ -570,9 +576,12 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } } else { prepareTimeWatch = StopWatch.create(true); - preparedInvocation = await this.prepareToolInvocation(tool, dto, token); + preparedInvocation = await this.prepareToolInvocationWithHookResult(tool, dto, preToolUseHookResult, token); prepareTimeWatch.stop(); - if (preparedInvocation?.confirmationMessages?.title && !(await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace, tool.data.source, dto.parameters, undefined))) { + + const { autoConfirmed: fallbackAutoConfirmed, preparedInvocation: updatedPreparedInvocation } = await this.resolveAutoConfirmFromHook(preToolUseHookResult, tool, dto, preparedInvocation, undefined); + preparedInvocation = updatedPreparedInvocation; + if (preparedInvocation?.confirmationMessages?.title && !fallbackAutoConfirmed) { const result = await this._dialogService.confirm({ message: renderAsPlaintext(preparedInvocation.confirmationMessages.title), detail: renderAsPlaintext(preparedInvocation.confirmationMessages.message!) }); if (!result.confirmed) { throw new CancellationError(); @@ -656,7 +665,68 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } } - private async prepareToolInvocation(tool: IToolEntry, dto: IToolInvocation, token: CancellationToken): Promise { + private async prepareToolInvocationWithHookResult(tool: IToolEntry, dto: IToolInvocation, hookResult: IPreToolUseHookResult | undefined, token: CancellationToken): Promise { + let forceConfirmationReason: string | undefined; + if (hookResult?.permissionDecision === 'ask') { + const hookMessage = localize('preToolUseHookRequiredConfirmation', "{0} required confirmation", HookType.PreToolUse); + forceConfirmationReason = hookResult.permissionDecisionReason + ? `${hookMessage}: ${hookResult.permissionDecisionReason}` + : hookMessage; + } + return this.prepareToolInvocation(tool, dto, forceConfirmationReason, token); + } + + /** + * Determines the auto-confirm decision based on a preToolUse hook result. + * If the hook returned 'allow', auto-approves. If 'ask', forces confirmation + * and ensures confirmation messages exist on `preparedInvocation`. Otherwise + * falls back to normal auto-confirm logic. + * + * Returns the possibly-updated preparedInvocation along with the auto-confirm decision, + * since when the hook returns 'ask' and preparedInvocation was undefined, we create one. + */ + private async resolveAutoConfirmFromHook( + hookResult: IPreToolUseHookResult | undefined, + tool: IToolEntry, + dto: IToolInvocation, + preparedInvocation: IPreparedToolInvocation | undefined, + sessionResource: URI | undefined, + ): Promise<{ autoConfirmed: ConfirmedReason | undefined; preparedInvocation: IPreparedToolInvocation | undefined }> { + if (hookResult?.permissionDecision === 'allow') { + this._logService.debug(`[LanguageModelToolsService#invokeTool] Tool ${dto.toolId} auto-approved by preToolUse hook`); + return { autoConfirmed: { type: ToolConfirmKind.ConfirmationNotNeeded, reason: localize('hookAllowed', "Allowed by hook") }, preparedInvocation }; + } + + if (hookResult?.permissionDecision === 'ask') { + this._logService.debug(`[LanguageModelToolsService#invokeTool] Tool ${dto.toolId} requires confirmation (preToolUse hook returned 'ask')`); + // Ensure confirmation messages exist when hook requires confirmation + if (!preparedInvocation?.confirmationMessages?.title) { + if (!preparedInvocation) { + preparedInvocation = {}; + } + const fullReferenceName = getToolFullReferenceName(tool.data); + const hookReason = hookResult.permissionDecisionReason; + const baseMessage = localize('hookRequiresConfirmation.message', "{0} hook confirmation required", HookType.PreToolUse); + preparedInvocation.confirmationMessages = { + ...preparedInvocation.confirmationMessages, + title: localize('hookRequiresConfirmation.title', "Use the '{0}' tool?", fullReferenceName), + message: new MarkdownString(hookReason ? `${baseMessage}\n\n${hookReason}` : baseMessage), + allowAutoConfirm: false, + }; + preparedInvocation.toolSpecificData = { + kind: 'input', + rawInput: dto.parameters, + }; + } + return { autoConfirmed: undefined, preparedInvocation }; + } + + // No hook decision - use normal auto-confirm logic + const autoConfirmed = await this.shouldAutoConfirm(tool.data.id, tool.data.runsInWorkspace, tool.data.source, dto.parameters, sessionResource); + return { autoConfirmed, preparedInvocation }; + } + + private async prepareToolInvocation(tool: IToolEntry, dto: IToolInvocation, forceConfirmationReason: string | undefined, token: CancellationToken): Promise { let prepared: IPreparedToolInvocation | undefined; if (tool.impl!.prepareToolInvocation) { const preparePromise = tool.impl!.prepareToolInvocation({ @@ -664,7 +734,8 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo chatRequestId: dto.chatRequestId, chatSessionId: dto.context?.sessionId, chatSessionResource: dto.context?.sessionResource, - chatInteractionId: dto.chatInteractionId + chatInteractionId: dto.chatInteractionId, + forceConfirmationReason: forceConfirmationReason }, token); const raceResult = await Promise.race([ diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatQueuePickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatQueuePickerActionItem.ts new file mode 100644 index 0000000000000..38809b39a137e --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatQueuePickerActionItem.ts @@ -0,0 +1,241 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { $, addDisposableListener, append, EventType } from '../../../../../../base/browser/dom.js'; +import { StandardKeyboardEvent } from '../../../../../../base/browser/keyboardEvent.js'; +import { ActionViewItem, BaseActionViewItem, IActionViewItemOptions } from '../../../../../../base/browser/ui/actionbar/actionViewItems.js'; +import { Action, IAction } from '../../../../../../base/common/actions.js'; +import { Codicon } from '../../../../../../base/common/codicons.js'; +import { KeyCode } from '../../../../../../base/common/keyCodes.js'; +import { Disposable, IDisposable } from '../../../../../../base/common/lifecycle.js'; +import { ThemeIcon } from '../../../../../../base/common/themables.js'; +import { localize } from '../../../../../../nls.js'; +import { IActionViewItemService } from '../../../../../../platform/actions/browser/actionViewItemService.js'; +import { ActionWidgetDropdownActionViewItem } from '../../../../../../platform/actions/browser/actionWidgetDropdownActionViewItem.js'; +import { MenuId, SubmenuItemAction } from '../../../../../../platform/actions/common/actions.js'; +import { IActionWidgetService } from '../../../../../../platform/actionWidget/browser/actionWidget.js'; +import { IActionWidgetDropdownAction } from '../../../../../../platform/actionWidget/browser/actionWidgetDropdown.js'; +import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; +import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; +import { IKeybindingService } from '../../../../../../platform/keybinding/common/keybinding.js'; +import { ITelemetryService } from '../../../../../../platform/telemetry/common/telemetry.js'; +import { IWorkbenchContribution } from '../../../../../common/contributions.js'; +import { ChatConfiguration } from '../../../common/constants.js'; +import { ChatSubmitAction } from '../../actions/chatExecuteActions.js'; +import { ChatQueueMessageAction, ChatSteerWithMessageAction } from '../../actions/chatQueueActions.js'; + +/** + * Split-button action view item for the queue/steer picker in the chat execute toolbar. + * The primary button runs the current default action (queue or steer). + * The dropdown arrow opens a custom action widget with hover descriptions. + * + * Follows the same split-button pattern as {@link DropdownWithDefaultActionViewItem}, + * but uses {@link ActionWidgetDropdownActionViewItem} for the dropdown to show + * an action widget with hover descriptions instead of a standard context menu. + */ +export class ChatQueuePickerActionItem extends BaseActionViewItem { + + private readonly _primaryActionAction: Action; + private readonly _primaryAction: ActionViewItem; + private readonly _dropdown: ActionWidgetDropdownActionViewItem; + + constructor( + action: IAction, + _options: IActionViewItemOptions, + @ICommandService private readonly commandService: ICommandService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IActionWidgetService actionWidgetService: IActionWidgetService, + @IKeybindingService keybindingService: IKeybindingService, + @IContextKeyService contextKeyService: IContextKeyService, + @ITelemetryService telemetryService: ITelemetryService, + ) { + super(undefined, action); + + const isSteerDefault = this._isSteerDefault(); + + // Primary action - runs the current default (queue or steer) + this._primaryActionAction = this._register(new Action( + 'chat.queuePickerPrimary', + isSteerDefault ? localize('chat.steerWithMessage', "Steer with Message") : localize('chat.queueMessage', "Add to Queue"), + ThemeIcon.asClassName(isSteerDefault ? Codicon.arrowRight : Codicon.add), + true, + () => this._runDefaultAction() + )); + this._primaryAction = this._register(new ActionViewItem(undefined, this._primaryActionAction, { icon: true, label: false })); + + // Dropdown - action widget with hover descriptions and chevron-down icon + const dropdownAction = this._register(new Action('chat.queuePickerDropdown', localize('chat.queuePicker.moreActions', "More Actions..."))); + this._dropdown = this._register(new ChevronActionWidgetDropdown( + dropdownAction, + { + actionProvider: { getActions: () => this._getDropdownActions() }, + showItemKeybindings: true, + }, + actionWidgetService, + keybindingService, + contextKeyService, + telemetryService, + )); + + // React to config changes + this._register(this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(ChatConfiguration.RequestQueueingDefaultAction)) { + this._updatePrimaryAction(); + } + })); + } + + private _isSteerDefault(): boolean { + return this.configurationService.getValue(ChatConfiguration.RequestQueueingDefaultAction) === 'steer'; + } + + private _updatePrimaryAction(): void { + const isSteerDefault = this._isSteerDefault(); + this._primaryActionAction.label = isSteerDefault + ? localize('chat.steerWithMessage', "Steer with Message") + : localize('chat.queueMessage', "Add to Queue"); + this._primaryActionAction.class = ThemeIcon.asClassName(isSteerDefault ? Codicon.arrowRight : Codicon.add); + } + + private _runDefaultAction(): void { + const actionId = this._isSteerDefault() + ? ChatSteerWithMessageAction.ID + : ChatQueueMessageAction.ID; + this.commandService.executeCommand(actionId); + } + + override render(container: HTMLElement): void { + super.render(container); + container.classList.add('monaco-dropdown-with-default'); + + // Primary action button + const primaryContainer = $('.action-container'); + this._primaryAction.render(append(container, primaryContainer)); + this._register(addDisposableListener(primaryContainer, EventType.KEY_DOWN, (e: KeyboardEvent) => { + const event = new StandardKeyboardEvent(e); + if (event.equals(KeyCode.RightArrow)) { + this._primaryAction.blur(); + this._dropdown.focus(); + event.stopPropagation(); + } + })); + + // Dropdown arrow button + const dropdownContainer = $('.dropdown-action-container'); + this._dropdown.render(append(container, dropdownContainer)); + this._register(addDisposableListener(dropdownContainer, EventType.KEY_DOWN, (e: KeyboardEvent) => { + const event = new StandardKeyboardEvent(e); + if (event.equals(KeyCode.LeftArrow)) { + this._dropdown.setFocusable(false); + this._primaryAction.focus(); + event.stopPropagation(); + } + })); + } + + override focus(fromRight?: boolean): void { + if (fromRight) { + this._dropdown.focus(); + } else { + this._primaryAction.focus(); + } + } + + override blur(): void { + this._primaryAction.blur(); + this._dropdown.blur(); + } + + override setFocusable(focusable: boolean): void { + this._primaryAction.setFocusable(focusable); + this._dropdown.setFocusable(focusable); + } + + private _getDropdownActions(): IActionWidgetDropdownAction[] { + const queueAction: IActionWidgetDropdownAction = { + id: ChatQueueMessageAction.ID, + label: localize('chat.queueMessage', "Add to Queue"), + tooltip: '', + enabled: true, + icon: Codicon.add, + class: undefined, + hover: { + content: localize('chat.queueMessage.hover', "Queue this message to send after the current request completes. The current response will finish uninterrupted before the queued message is sent."), + }, + run: () => { + this.commandService.executeCommand(ChatQueueMessageAction.ID); + } + }; + + const steerAction: IActionWidgetDropdownAction = { + id: ChatSteerWithMessageAction.ID, + label: localize('chat.steerWithMessage', "Steer with Message"), + tooltip: '', + enabled: true, + icon: Codicon.arrowRight, + class: undefined, + hover: { + content: localize('chat.steerWithMessage.hover', "Send this message at the next opportunity, signaling the current request to yield. The current response will stop and the new message will be sent immediately."), + }, + run: () => { + this.commandService.executeCommand(ChatSteerWithMessageAction.ID); + } + }; + + const sendAction: IActionWidgetDropdownAction = { + id: '_' + ChatSubmitAction.ID, // _ to avoid showing a keybinding which is not valid in this context + label: localize('chat.sendImmediately', "Send Immediately"), + tooltip: '', + enabled: true, + icon: Codicon.send, + class: undefined, + hover: { + content: localize('chat.sendImmediately.hover', "Cancel the current request and send this message immediately."), + }, + run: () => { + this.commandService.executeCommand(ChatSubmitAction.ID); + } + }; + + return [sendAction, queueAction, steerAction]; + } +} + +/** + * {@link ActionWidgetDropdownActionViewItem} that renders a chevron-down icon + * as its label, used as the dropdown arrow in the split button. + */ +class ChevronActionWidgetDropdown extends ActionWidgetDropdownActionViewItem { + protected override renderLabel(element: HTMLElement): IDisposable | null { + element.classList.add('codicon', 'codicon-chevron-down'); + return null; + } +} + + +/** + * Workbench contribution that registers a custom action view item for the + * queue/steer picker in the execute toolbar. This replaces the default split + * button with a custom dropdown similar to the model switcher. + */ +export class ChatQueuePickerRendering extends Disposable implements IWorkbenchContribution { + + static readonly ID = 'chat.queuePickerRendering'; + + constructor( + @IActionViewItemService actionViewItemService: IActionViewItemService, + @IInstantiationService instantiationService: IInstantiationService, + ) { + super(); + this._register(actionViewItemService.register(MenuId.ChatExecute, MenuId.ChatExecuteQueue, (action, options) => { + if (!(action instanceof SubmenuItemAction)) { + return undefined; + } + return instantiationService.createInstance(ChatQueuePickerActionItem, action, options); + })); + } +} diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputCompletions.ts b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputCompletions.ts index ecf2402d691cc..fdadd6673579c 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputCompletions.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/editor/chatInputCompletions.ts @@ -201,8 +201,14 @@ class SlashCommandCompletions extends Disposable { return null; } + // Filter out commands that are not user-invokable (hidden from / menu) + const userInvokableCommands = promptCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + if (userInvokableCommands.length === 0) { + return null; + } + return { - suggestions: promptCommands.map((c, i): CompletionItem => { + suggestions: userInvokableCommands.map((c, i): CompletionItem => { const label = `/${c.name}`; const description = c.description; return { diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index baa3145870c1e..8270d7cf9e497 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -12,6 +12,7 @@ export enum ChatConfiguration { AIDisabled = 'chat.disableAIFeatures', AgentEnabled = 'chat.agent.enabled', RequestQueueingEnabled = 'chat.requestQueuing.enabled', + RequestQueueingDefaultAction = 'chat.requestQueuing.defaultAction', AgentStatusEnabled = 'chat.agentsControl.enabled', EditorAssociations = 'chat.editorAssociations', UnifiedAgentsBar = 'chat.unifiedAgentsBar.enabled', diff --git a/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts b/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts index ac3f92cc4f274..20ec217b8827c 100644 --- a/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts +++ b/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts @@ -3,28 +3,28 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js'; -import { URI } from '../../../../../base/common/uri.js'; -import { HookType, HookTypeValue, IChatRequestHooks, IHookCommand } from '../promptSyntax/hookSchema.js'; -import { IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; -import { ILogService } from '../../../../../platform/log/common/log.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; +import { IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; import { StopWatch } from '../../../../../base/common/stopwatch.js'; -import { Extensions, IOutputChannelRegistry, IOutputService } from '../../../../services/output/common/output.js'; -import { Registry } from '../../../../../platform/registry/common/platform.js'; +import { URI } from '../../../../../base/common/uri.js'; import { localize } from '../../../../../nls.js'; +import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js'; +import { ILogService } from '../../../../../platform/log/common/log.js'; +import { Registry } from '../../../../../platform/registry/common/platform.js'; +import { Extensions, IOutputChannelRegistry, IOutputService } from '../../../../services/output/common/output.js'; +import { HookType, HookTypeValue, IChatRequestHooks, IHookCommand } from '../promptSyntax/hookSchema.js'; import { HookCommandResultKind, IHookCommandInput, IHookCommandResult, - IPreToolUseCommandInput, + IPreToolUseCommandInput } from './hooksCommandTypes.js'; import { commonHookOutputValidator, IHookResult, IPreToolUseCallerInput, IPreToolUseHookResult, - preToolUseOutputValidator, + preToolUseOutputValidator } from './hooksTypes.js'; export const hooksOutputChannelId = 'hooksExecution'; @@ -296,7 +296,8 @@ export class HooksExecutionService implements IHooksExecutionService { token: token ?? CancellationToken.None, }); - // Collect all valid outputs - "any deny wins" for security + // Collect all valid outputs - priority order: deny > ask > allow + let lastAskResult: IPreToolUseHookResult | undefined; let lastAllowResult: IPreToolUseHookResult | undefined; for (const result of results) { if (result.success && typeof result.output === 'object' && result.output !== null) { @@ -305,6 +306,12 @@ export class HooksExecutionService implements IHooksExecutionService { // Extract from hookSpecificOutput wrapper const hookSpecificOutput = validationResult.content.hookSpecificOutput; if (hookSpecificOutput) { + // Validate hookEventName if present - must match the hook type + if (hookSpecificOutput.hookEventName !== undefined && hookSpecificOutput.hookEventName !== HookType.PreToolUse) { + this._logService.warn(`[HooksExecutionService] preToolUse hook returned invalid hookEventName '${hookSpecificOutput.hookEventName}', expected '${HookType.PreToolUse}'`); + continue; + } + const preToolUseResult: IPreToolUseHookResult = { ...result, permissionDecision: hookSpecificOutput.permissionDecision, @@ -316,6 +323,10 @@ export class HooksExecutionService implements IHooksExecutionService { if (hookSpecificOutput.permissionDecision === 'deny') { return preToolUseResult; } + // Track 'ask' results (ask takes priority over allow) + if (hookSpecificOutput.permissionDecision === 'ask') { + lastAskResult = preToolUseResult; + } // Track the last allow in case we need to return it if (hookSpecificOutput.permissionDecision === 'allow') { lastAllowResult = preToolUseResult; @@ -328,7 +339,7 @@ export class HooksExecutionService implements IHooksExecutionService { } } - // Return the last allow result, or undefined if no valid outputs - return lastAllowResult; + // Return with priority: ask > allow > undefined + return lastAskResult ?? lastAllowResult; } } diff --git a/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts b/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts index b2e28e1ab7d0e..8a0fb32ab783e 100644 --- a/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts +++ b/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts @@ -67,7 +67,7 @@ export interface IPreToolUseCallerInput { export const preToolUseOutputValidator = vObj({ hookSpecificOutput: vOptionalProp(vObj({ hookEventName: vOptionalProp(vString()), - permissionDecision: vEnum('allow', 'deny'), + permissionDecision: vEnum('allow', 'deny', 'ask'), permissionDecisionReason: vOptionalProp(vString()), additionalContext: vOptionalProp(vString()), })), @@ -75,8 +75,11 @@ export const preToolUseOutputValidator = vObj({ /** * Valid permission decisions for preToolUse hooks. + * - 'allow': Auto-approve the tool execution (skip user confirmation) + * - 'deny': Deny the tool execution + * - 'ask': Always require user confirmation (never auto-approve) */ -export type PreToolUsePermissionDecision = 'allow' | 'deny'; +export type PreToolUsePermissionDecision = 'allow' | 'deny' | 'ask'; /** * Result from preToolUse hooks with permission decision fields. diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts index 1ec5dbb95b275..c56b279455dfb 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts @@ -314,7 +314,9 @@ export class ComputeAutomaticInstructions { } const agentSkills = await this._promptsService.findAgentSkills(token); - if (agentSkills && agentSkills.length > 0) { + // Filter out skills with disableModelInvocation=true (they can only be triggered manually via /name) + const modelInvokableSkills = agentSkills?.filter(skill => !skill.disableModelInvocation); + if (modelInvokableSkills && modelInvokableSkills.length > 0) { const useSkillAdherencePrompt = this._configurationService.getValue(PromptsConfig.USE_SKILL_ADHERENCE_PROMPT); entries.push(''); if (useSkillAdherencePrompt) { @@ -336,7 +338,7 @@ export class ComputeAutomaticInstructions { entries.push('Each skill comes with a description of the topic and a file path that contains the detailed instructions.'); entries.push(`When a user asks you to perform a task that falls within the domain of a skill, use the ${readTool.variable} tool to acquire the full instructions from the file URI.`); } - for (const skill of agentSkills) { + for (const skill of modelInvokableSkills) { entries.push(''); entries.push(`${skill.name}`); if (skill.description) { diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHeaderAutocompletion.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHeaderAutocompletion.ts index 64463f76fa6ab..56165a933ed32 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHeaderAutocompletion.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptHeaderAutocompletion.ts @@ -257,12 +257,12 @@ export class PromptHeaderAutocompletion implements CompletionItemProvider { } break; case PromptHeaderAttributes.userInvokable: - if (promptType === PromptsType.agent) { + if (promptType === PromptsType.agent || promptType === PromptsType.skill) { return ['true', 'false']; } break; case PromptHeaderAttributes.disableModelInvocation: - if (promptType === PromptsType.agent) { + if (promptType === PromptsType.agent || promptType === PromptsType.skill) { return ['true', 'false']; } break; diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts index 19550c51dafdf..33d61b4a2329b 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts @@ -195,7 +195,8 @@ export class PromptValidator { } case PromptsType.skill: - // Skill-specific validations (currently none beyond name/description) + this.validateUserInvokable(attributes, report); + this.validateDisableModelInvocation(attributes, report); break; } @@ -625,7 +626,7 @@ const allAttributeNames: Record = { [PromptsType.prompt]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.model, PromptHeaderAttributes.tools, PromptHeaderAttributes.mode, PromptHeaderAttributes.agent, PromptHeaderAttributes.argumentHint], [PromptsType.instructions]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.applyTo, PromptHeaderAttributes.excludeAgent], [PromptsType.agent]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.model, PromptHeaderAttributes.tools, PromptHeaderAttributes.advancedOptions, PromptHeaderAttributes.handOffs, PromptHeaderAttributes.argumentHint, PromptHeaderAttributes.target, PromptHeaderAttributes.infer, PromptHeaderAttributes.agents, PromptHeaderAttributes.userInvokable, PromptHeaderAttributes.disableModelInvocation], - [PromptsType.skill]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.license, PromptHeaderAttributes.compatibility, PromptHeaderAttributes.metadata], + [PromptsType.skill]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.license, PromptHeaderAttributes.compatibility, PromptHeaderAttributes.metadata, PromptHeaderAttributes.argumentHint, PromptHeaderAttributes.userInvokable, PromptHeaderAttributes.disableModelInvocation], [PromptsType.hook]: [], // hooks are JSON files, not markdown with YAML frontmatter }; const githubCopilotAgentAttributeNames = [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.tools, PromptHeaderAttributes.target, GithubPromptHeaderAttributes.mcpServers, PromptHeaderAttributes.infer]; @@ -666,6 +667,12 @@ export function getAttributeDescription(attributeName: string, promptType: Promp return localize('promptHeader.skill.name', 'The name of the skill.'); case PromptHeaderAttributes.description: return localize('promptHeader.skill.description', 'The description of the skill. The description is added to every request and will be used by the agent to decide when to load the skill.'); + case PromptHeaderAttributes.argumentHint: + return localize('promptHeader.skill.argumentHint', 'Hint shown during autocomplete to indicate expected arguments. Example: [issue-number] or [filename] [format]'); + case PromptHeaderAttributes.userInvokable: + return localize('promptHeader.skill.userInvokable', 'Set to false to hide from the / menu. Use for background knowledge users should not invoke directly. Default: true.'); + case PromptHeaderAttributes.disableModelInvocation: + return localize('promptHeader.skill.disableModelInvocation', 'Set to true to prevent the agent from automatically loading this skill. Use for workflows you want to trigger manually with /name. Default: false.'); } break; case PromptsType.agent: diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts index feabfd922fed1..d5f9eb6b4832c 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts @@ -219,6 +219,16 @@ export interface IAgentSkill { readonly storage: PromptsStorage; readonly name: string; readonly description: string | undefined; + /** + * If true, the skill should not be automatically loaded by the agent. + * Use for workflows you want to trigger manually with /name. + */ + readonly disableModelInvocation: boolean; + /** + * If false, the skill is hidden from the / menu. + * Use for background knowledge users shouldn't invoke directly. + */ + readonly userInvokable: boolean; } /** @@ -273,6 +283,10 @@ export interface IPromptFileDiscoveryResult { readonly duplicateOf?: URI; /** Extension ID if from extension */ readonly extensionId?: string; + /** If true, the skill is hidden from the / menu (user-invokable: false) */ + readonly userInvokable?: boolean; + /** If true, the skill won't be automatically loaded by the agent (disable-model-invocation: true) */ + readonly disableModelInvocation?: boolean; } /** diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index 43d627c1578f0..5db135db85841 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -893,7 +893,14 @@ export class PromptsService extends Disposable implements IPromptsService { for (const file of files) { if (file.status === 'loaded' && file.name) { const sanitizedDescription = this.truncateAgentSkillDescription(file.description, file.uri); - result.push({ uri: file.uri, storage: file.storage, name: file.name, description: sanitizedDescription }); + result.push({ + uri: file.uri, + storage: file.storage, + name: file.name, + description: sanitizedDescription, + disableModelInvocation: file.disableModelInvocation ?? false, + userInvokable: file.userInvokable ?? true + }); } } @@ -1088,10 +1095,10 @@ export class PromptsService extends Disposable implements IPromptsService { * Returns the discovery results and a map of skill counts by source type for telemetry. */ private async computeSkillDiscoveryInfo(token: CancellationToken): Promise<{ - files: (IPromptFileDiscoveryResult & { description?: string; source?: PromptFileSource })[]; + files: (IPromptFileDiscoveryResult & { description?: string; source?: PromptFileSource; disableModelInvocation?: boolean; userInvokable?: boolean })[]; skillsBySource: Map; }> { - const files: (IPromptFileDiscoveryResult & { description?: string; source?: PromptFileSource })[] = []; + const files: (IPromptFileDiscoveryResult & { description?: string; source?: PromptFileSource; disableModelInvocation?: boolean; userInvokable?: boolean })[] = []; const skillsBySource = new Map(); const seenNames = new Set(); const nameToUri = new Map(); @@ -1169,7 +1176,9 @@ export class PromptsService extends Disposable implements IPromptsService { seenNames.add(sanitizedName); nameToUri.set(sanitizedName, uri); - files.push({ uri, storage, status: 'loaded', name: sanitizedName, description, extensionId, source }); + const disableModelInvocation = parsedFile.header?.disableModelInvocation === true; + const userInvokable = parsedFile.header?.userInvokable !== false; + files.push({ uri, storage, status: 'loaded', name: sanitizedName, description, extensionId, source, disableModelInvocation, userInvokable }); // Track skill type skillsBySource.set(source, (skillsBySource.get(source) || 0) + 1); diff --git a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts index f04160b8efdac..d8631d62bbf61 100644 --- a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts @@ -203,6 +203,8 @@ export interface IToolInvocationPreparationContext { chatSessionId?: string; chatSessionResource: URI | undefined; chatInteractionId?: string; + /** If set, tells the tool that it should include confirmmation messages. */ + forceConfirmationReason?: string; } export type ToolInputOutputBase = { diff --git a/src/vs/workbench/contrib/chat/test/browser/promptSyntax/languageProviders/promptValidator.test.ts b/src/vs/workbench/contrib/chat/test/browser/promptSyntax/languageProviders/promptValidator.test.ts index e156499b3473d..fafa0b4fdb7e2 100644 --- a/src/vs/workbench/contrib/chat/test/browser/promptSyntax/languageProviders/promptValidator.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/promptSyntax/languageProviders/promptValidator.test.ts @@ -1673,6 +1673,184 @@ suite('PromptValidator', () => { assert.ok(markers.every(m => m.message.includes('Supported: '))); }); + test('skill with user-invokable: false is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Background knowledge skill', + 'user-invokable: false', + '---', + 'This skill provides background context.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'user-invokable: false should be valid for skills'); + }); + + test('skill with user-invokable: true is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: User-accessible skill', + 'user-invokable: true', + '---', + 'This skill can be invoked by users.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'user-invokable: true should be valid for skills'); + }); + + test('skill with invalid user-invokable value shows error', async () => { + // String value instead of boolean + { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'user-invokable: "false"', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'user-invokable' attribute must be a boolean.`); + } + + // Number value instead of boolean + { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'user-invokable: 0', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'user-invokable' attribute must be a boolean.`); + } + }); + + test('skill with disable-model-invocation: true is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Manual-only skill', + 'disable-model-invocation: true', + '---', + 'This skill must be triggered manually with /name.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'disable-model-invocation: true should be valid for skills'); + }); + + test('skill with disable-model-invocation: false is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Auto-loadable skill', + 'disable-model-invocation: false', + '---', + 'This skill can be loaded automatically by the agent.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'disable-model-invocation: false should be valid for skills'); + }); + + test('skill with invalid disable-model-invocation value shows error', async () => { + // String value instead of boolean + { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'disable-model-invocation: "true"', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'disable-model-invocation' attribute must be a boolean.`); + } + + // Number value instead of boolean + { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'disable-model-invocation: 1', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'disable-model-invocation' attribute must be a boolean.`); + } + }); + + test('skill with argument-hint is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Skill with argument hint', + 'argument-hint: "[issue-number]"', + '---', + 'This skill expects an issue number.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'argument-hint should be valid for skills'); + }); + + test('skill with empty argument-hint shows error', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'argument-hint: ""', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'argument-hint' attribute should not be empty.`); + }); + + test('skill with non-string argument-hint shows error', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Test Skill', + 'argument-hint: 123', + '---', + 'Body' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.strictEqual(markers.length, 1); + assert.strictEqual(markers[0].severity, MarkerSeverity.Error); + assert.strictEqual(markers[0].message, `The 'argument-hint' attribute must be a string.`); + }); + + test('skill with all visibility attributes combined is valid', async () => { + const content = [ + '---', + 'name: my-skill', + 'description: Complex visibility skill', + 'user-invokable: false', + 'disable-model-invocation: true', + 'argument-hint: "[optional-arg]"', + '---', + 'This skill has complex visibility settings.' + ].join('\n'); + const markers = await validate(content, PromptsType.skill, URI.parse('file:///.github/skills/my-skill/SKILL.md')); + assert.deepStrictEqual(markers, [], 'All visibility attributes combined should be valid'); + }); + }); }); diff --git a/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts b/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts index 452c1e5fe08c5..b212e9cb2552b 100644 --- a/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts @@ -3864,5 +3864,93 @@ suite('LanguageModelToolsService', () => { assert.strictEqual(invokeCalled, false, 'Tool invoke should not be called when hook denies'); }); + + test('when hook returns ask, tool is not auto-approved', async () => { + mockHooksService.preToolUseHookResult = { + output: undefined, + success: true, + permissionDecision: 'ask', + permissionDecisionReason: 'Requires user confirmation', + }; + + let invokeCompleted = false; + const tool = registerToolForTest(hookService, store, 'hookAskTool', { + invoke: async () => { + invokeCompleted = true; + return { content: [{ kind: 'text', value: 'success' }] }; + }, + prepareToolInvocation: async () => ({ + confirmationMessages: { + title: 'Confirm this action?', + message: 'This tool requires confirmation', + allowAutoConfirm: true + } + }) + }); + + const capture: { invocation?: ChatToolInvocation } = {}; + stubGetSession(hookChatService, 'hook-test-ask', { requestId: 'req1', capture }); + + // Start invocation - it should wait for confirmation + const invokePromise = hookService.invokeTool( + tool.makeDto({ test: 1 }, { sessionId: 'hook-test-ask' }), + async () => 0, + CancellationToken.None + ); + + // Wait for invocation to be captured + await new Promise(resolve => setTimeout(resolve, 50)); + const invocation = capture.invocation; + assert.ok(invocation, 'Tool invocation should be created'); + + // Check that the tool is waiting for confirmation (not auto-approved) + const state = invocation.state.get(); + assert.strictEqual(state.type, IChatToolInvocation.StateKind.WaitingForConfirmation, + 'Tool should be waiting for confirmation when hook returns ask'); + + // Confirm the tool to let the test complete + IChatToolInvocation.confirmWith(invocation, { type: ToolConfirmKind.UserAction }); + await invokePromise; + + assert.strictEqual(invokeCompleted, true, 'Tool should complete after confirmation'); + }); + + test('when hook returns allow, tool is auto-approved', async () => { + mockHooksService.preToolUseHookResult = { + output: undefined, + success: true, + permissionDecision: 'allow', + }; + + let invokeCompleted = false; + const tool = registerToolForTest(hookService, store, 'hookAutoApproveTool', { + invoke: async () => { + invokeCompleted = true; + return { content: [{ kind: 'text', value: 'success' }] }; + }, + prepareToolInvocation: async () => ({ + confirmationMessages: { + title: 'Confirm this action?', + message: 'This tool would normally require confirmation', + allowAutoConfirm: true + } + }) + }); + + const capture: { invocation?: ChatToolInvocation } = {}; + stubGetSession(hookChatService, 'hook-test-auto-approve', { requestId: 'req1', capture }); + + // Invoke the tool - it should auto-approve due to hook + const result = await hookService.invokeTool( + tool.makeDto({ test: 1 }, { sessionId: 'hook-test-auto-approve' }), + async () => 0, + CancellationToken.None + ); + + // Tool should have completed without waiting for confirmation + assert.strictEqual(invokeCompleted, true, 'Tool should complete immediately when hook allows'); + assert.strictEqual(result.content[0].kind, 'text'); + assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'success'); + }); }); }); diff --git a/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts b/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts index 0f36f339b8723..fa4aae5de8eb3 100644 --- a/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts @@ -308,6 +308,194 @@ suite('HooksExecutionService', () => { }); }); + suite('executePreToolUseHook', () => { + test('returns allow result when hook allows', async () => { + const proxy = createMockProxy(() => ({ + kind: HookCommandResultKind.Success, + result: { + hookSpecificOutput: { + permissionDecision: 'allow', + permissionDecisionReason: 'Tool is safe' + } + } + })); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'allow'); + assert.strictEqual(result.permissionDecisionReason, 'Tool is safe'); + }); + + test('returns ask result when hook requires confirmation', async () => { + const proxy = createMockProxy(() => ({ + kind: HookCommandResultKind.Success, + result: { + hookSpecificOutput: { + permissionDecision: 'ask', + permissionDecisionReason: 'Requires user approval' + } + } + })); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'ask'); + assert.strictEqual(result.permissionDecisionReason, 'Requires user approval'); + }); + + test('deny takes priority over ask and allow', async () => { + let callCount = 0; + const proxy = createMockProxy(() => { + callCount++; + // First hook returns allow, second returns ask, third returns deny + if (callCount === 1) { + return { + kind: HookCommandResultKind.Success, + result: { hookSpecificOutput: { permissionDecision: 'allow' } } + }; + } else if (callCount === 2) { + return { + kind: HookCommandResultKind.Success, + result: { hookSpecificOutput: { permissionDecision: 'ask' } } + }; + } else { + return { + kind: HookCommandResultKind.Success, + result: { hookSpecificOutput: { permissionDecision: 'deny', permissionDecisionReason: 'Blocked' } } + }; + } + }); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2'), cmd('hook3')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'deny'); + assert.strictEqual(result.permissionDecisionReason, 'Blocked'); + }); + + test('ask takes priority over allow', async () => { + let callCount = 0; + const proxy = createMockProxy(() => { + callCount++; + // First hook returns allow, second returns ask + if (callCount === 1) { + return { + kind: HookCommandResultKind.Success, + result: { hookSpecificOutput: { permissionDecision: 'allow' } } + }; + } else { + return { + kind: HookCommandResultKind.Success, + result: { hookSpecificOutput: { permissionDecision: 'ask', permissionDecisionReason: 'Need confirmation' } } + }; + } + }); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'ask'); + assert.strictEqual(result.permissionDecisionReason, 'Need confirmation'); + }); + + test('ignores results with wrong hookEventName', async () => { + let callCount = 0; + const proxy = createMockProxy(() => { + callCount++; + if (callCount === 1) { + // First hook returns allow but with wrong hookEventName + return { + kind: HookCommandResultKind.Success, + result: { + hookSpecificOutput: { + hookEventName: 'PostToolUse', // Wrong hook type + permissionDecision: 'deny' + } + } + }; + } else { + // Second hook returns allow with correct hookEventName + return { + kind: HookCommandResultKind.Success, + result: { + hookSpecificOutput: { + hookEventName: 'PreToolUse', + permissionDecision: 'allow' + } + } + }; + } + }); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + // The deny with wrong hookEventName should be ignored + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'allow'); + }); + + test('allows results without hookEventName (optional field)', async () => { + const proxy = createMockProxy(() => ({ + kind: HookCommandResultKind.Success, + result: { + hookSpecificOutput: { + // No hookEventName - should be accepted + permissionDecision: 'allow' + } + } + })); + service.setProxy(proxy); + + const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; + store.add(service.registerHooks(sessionUri, hooks)); + + const result = await service.executePreToolUseHook( + sessionUri, + { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } + ); + + assert.ok(result); + assert.strictEqual(result.permissionDecision, 'allow'); + }); + }); + function createMockProxy(handler?: (cmd: IHookCommand, input: unknown, token: CancellationToken) => IHookCommandResult): IHooksExecutionProxy { return { runHookCommand: async (hookCommand, input, token) => { diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts index 1727898d2468a..89c383218f083 100644 --- a/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts @@ -2982,4 +2982,274 @@ suite('PromptsService', () => { assert.strictEqual(skillCommand, undefined, 'Should not find skill command when skills are disabled'); }); }); + + suite('getPromptSlashCommands - userInvokable filtering', () => { + teardown(() => { + sinon.restore(); + }); + + test('should return correct userInvokable value for skills with user-invokable: false', async () => { + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + testConfigService.setUserConfiguration(PromptsConfig.SKILLS_LOCATION_KEY, {}); + + const rootFolderName = 'user-invokable-false'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a skill with user-invokable: false (should be hidden from / menu) + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/skills/hidden-skill/SKILL.md`, + contents: [ + '---', + 'name: "hidden-skill"', + 'description: "A skill hidden from the / menu"', + 'user-invokable: false', + '---', + 'Hidden skill content', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + const hiddenSkillCommand = slashCommands.find(cmd => cmd.name === 'hidden-skill'); + assert.ok(hiddenSkillCommand, 'Should find hidden skill in slash commands'); + assert.strictEqual(hiddenSkillCommand.parsedPromptFile?.header?.userInvokable, false, + 'Should have userInvokable=false in parsed header'); + + // Verify the filtering logic would correctly exclude this skill + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + const hiddenSkillInFiltered = filteredCommands.find(cmd => cmd.name === 'hidden-skill'); + assert.strictEqual(hiddenSkillInFiltered, undefined, + 'Hidden skill should be filtered out when applying userInvokable filter'); + }); + + test('should return correct userInvokable value for skills with user-invokable: true', async () => { + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + testConfigService.setUserConfiguration(PromptsConfig.SKILLS_LOCATION_KEY, {}); + + const rootFolderName = 'user-invokable-true'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a skill with explicit user-invokable: true + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/skills/visible-skill/SKILL.md`, + contents: [ + '---', + 'name: "visible-skill"', + 'description: "A skill visible in the / menu"', + 'user-invokable: true', + '---', + 'Visible skill content', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + const visibleSkillCommand = slashCommands.find(cmd => cmd.name === 'visible-skill'); + assert.ok(visibleSkillCommand, 'Should find visible skill in slash commands'); + assert.strictEqual(visibleSkillCommand.parsedPromptFile?.header?.userInvokable, true, + 'Should have userInvokable=true in parsed header'); + + // Verify the filtering logic would correctly include this skill + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + const visibleSkillInFiltered = filteredCommands.find(cmd => cmd.name === 'visible-skill'); + assert.ok(visibleSkillInFiltered, + 'Visible skill should be included when applying userInvokable filter'); + }); + + test('should default to true for skills without user-invokable attribute', async () => { + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + testConfigService.setUserConfiguration(PromptsConfig.SKILLS_LOCATION_KEY, {}); + + const rootFolderName = 'user-invokable-undefined'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a skill without user-invokable attribute (should default to true) + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/skills/default-skill/SKILL.md`, + contents: [ + '---', + 'name: "default-skill"', + 'description: "A skill without explicit user-invokable"', + '---', + 'Default skill content', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + const defaultSkillCommand = slashCommands.find(cmd => cmd.name === 'default-skill'); + assert.ok(defaultSkillCommand, 'Should find default skill in slash commands'); + assert.strictEqual(defaultSkillCommand.parsedPromptFile?.header?.userInvokable, undefined, + 'Should have userInvokable=undefined when attribute is not specified'); + + // Verify the filtering logic would correctly include this skill (undefined !== false is true) + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + const defaultSkillInFiltered = filteredCommands.find(cmd => cmd.name === 'default-skill'); + assert.ok(defaultSkillInFiltered, + 'Skill without user-invokable attribute should be included when applying userInvokable filter'); + }); + + test('should handle prompts with user-invokable: false', async () => { + const rootFolderName = 'prompt-user-invokable-false'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a prompt with user-invokable: false + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/prompts/hidden-prompt.prompt.md`, + contents: [ + '---', + 'name: "hidden-prompt"', + 'description: "A prompt hidden from the / menu"', + 'user-invokable: false', + '---', + 'Hidden prompt content', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + const hiddenPromptCommand = slashCommands.find(cmd => cmd.name === 'hidden-prompt'); + assert.ok(hiddenPromptCommand, 'Should find hidden prompt in slash commands'); + assert.strictEqual(hiddenPromptCommand.parsedPromptFile?.header?.userInvokable, false, + 'Should have userInvokable=false in parsed header'); + + // Verify the filtering logic would correctly exclude this prompt + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + const hiddenPromptInFiltered = filteredCommands.find(cmd => cmd.name === 'hidden-prompt'); + assert.strictEqual(hiddenPromptInFiltered, undefined, + 'Hidden prompt should be filtered out when applying userInvokable filter'); + }); + + test('should correctly filter mixed user-invokable values', async () => { + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + testConfigService.setUserConfiguration(PromptsConfig.SKILLS_LOCATION_KEY, {}); + + const rootFolderName = 'mixed-user-invokable'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a mix of skills and prompts with different user-invokable values + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/prompts/visible-prompt.prompt.md`, + contents: [ + '---', + 'name: "visible-prompt"', + 'description: "A visible prompt"', + '---', + 'Visible prompt content', + ], + }, + { + path: `${rootFolder}/.github/prompts/hidden-prompt.prompt.md`, + contents: [ + '---', + 'name: "hidden-prompt"', + 'description: "A hidden prompt"', + 'user-invokable: false', + '---', + 'Hidden prompt content', + ], + }, + { + path: `${rootFolder}/.github/skills/visible-skill/SKILL.md`, + contents: [ + '---', + 'name: "visible-skill"', + 'description: "A visible skill"', + 'user-invokable: true', + '---', + 'Visible skill content', + ], + }, + { + path: `${rootFolder}/.github/skills/hidden-skill/SKILL.md`, + contents: [ + '---', + 'name: "hidden-skill"', + 'description: "A hidden skill"', + 'user-invokable: false', + '---', + 'Hidden skill content', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + // All commands should be present in the raw list + assert.strictEqual(slashCommands.length, 4, 'Should find all 4 commands'); + + // Apply the same filtering logic as chatInputCompletions.ts + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + + assert.strictEqual(filteredCommands.length, 2, 'Should have 2 commands after filtering'); + assert.ok(filteredCommands.find(c => c.name === 'visible-prompt'), 'visible-prompt should be included'); + assert.ok(filteredCommands.find(c => c.name === 'visible-skill'), 'visible-skill should be included'); + assert.strictEqual(filteredCommands.find(c => c.name === 'hidden-prompt'), undefined, 'hidden-prompt should be excluded'); + assert.strictEqual(filteredCommands.find(c => c.name === 'hidden-skill'), undefined, 'hidden-skill should be excluded'); + }); + + test('should handle skills with missing header gracefully', async () => { + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + testConfigService.setUserConfiguration(PromptsConfig.SKILLS_LOCATION_KEY, {}); + + const rootFolderName = 'missing-header'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + + // Create a skill without any YAML header + await mockFiles(fileService, [ + { + path: `${rootFolder}/.github/skills/no-header-skill/SKILL.md`, + contents: [ + 'This skill has no YAML header at all.', + 'Just plain markdown content.', + ], + }, + ]); + + const slashCommands = await service.getPromptSlashCommands(CancellationToken.None); + + // Find the skill by checking all commands (name will be derived from filename) + const noHeaderSkill = slashCommands.find(cmd => + cmd.promptPath.uri.path.includes('no-header-skill')); + assert.ok(noHeaderSkill, 'Should find skill without header in slash commands'); + assert.strictEqual(noHeaderSkill.parsedPromptFile?.header, undefined, + 'Should have undefined header'); + + // Verify the filtering logic handles missing header correctly + // parsedPromptFile?.header?.userInvokable !== false + // When header is undefined: undefined !== false is true, so skill is included + const filteredCommands = slashCommands.filter(c => c.parsedPromptFile?.header?.userInvokable !== false); + const noHeaderSkillInFiltered = filteredCommands.find(cmd => + cmd.promptPath.uri.path.includes('no-header-skill')); + assert.ok(noHeaderSkillInFiltered, + 'Skill without header should be included when applying userInvokable filter (defaults to true)'); + }); + }); }); diff --git a/src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts b/src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts index a3c7b7d6ce500..9e1f0e696f3c4 100644 --- a/src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts +++ b/src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts @@ -8,8 +8,9 @@ import * as nls from '../../../../../nls.js'; import * as dom from '../../../../../base/browser/dom.js'; import { FindInput } from '../../../../../base/browser/ui/findinput/findInput.js'; import { Widget } from '../../../../../base/browser/ui/widget.js'; -import { Delayer } from '../../../../../base/common/async.js'; +import { Delayer, disposableTimeout } from '../../../../../base/common/async.js'; import { KeyCode } from '../../../../../base/common/keyCodes.js'; +import { IDisposable } from '../../../../../base/common/lifecycle.js'; import { FindReplaceState, INewFindReplaceState } from '../../../../../editor/contrib/find/browser/findState.js'; import { IMessage as InputBoxMessage } from '../../../../../base/browser/ui/inputbox/inputBox.js'; import { SimpleButton, findPreviousMatchIcon, findNextMatchIcon, NLS_NO_RESULTS, NLS_MATCHES_LOCATION } from '../../../../../editor/contrib/find/browser/findWidget.js'; @@ -27,6 +28,8 @@ import { ISashEvent, IVerticalSashLayoutProvider, Orientation, Sash } from '../. import { registerColor } from '../../../../../platform/theme/common/colorRegistry.js'; import type { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import type { IHoverLifecycleOptions } from '../../../../../base/browser/ui/hover/hover.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; const NLS_FIND_INPUT_LABEL = nls.localize('label.find', "Find"); const NLS_FIND_INPUT_PLACEHOLDER = nls.localize('placeholder.find', "Find"); @@ -69,6 +72,14 @@ export abstract class SimpleFindWidget extends Widget implements IVerticalSashLa private _foundMatch: boolean = false; private _width: number = 0; + /** + * Tracks whether the accessibility help hint has been announced in the ARIA label. + * Reset to false when the widget is hidden, allowing the hint to be announced again + * on the next reveal. + */ + private _accessibilityHelpHintAnnounced: boolean = false; + private _labelResetTimeout: IDisposable | undefined; + readonly state: FindReplaceState; constructor( @@ -77,6 +88,8 @@ export abstract class SimpleFindWidget extends Widget implements IVerticalSashLa contextKeyService: IContextKeyService, hoverService: IHoverService, private readonly _keybindingService: IKeybindingService, + private readonly _configurationService: IConfigurationService, + private readonly _accessibilityService: IAccessibilityService, ) { super(); @@ -307,6 +320,7 @@ export abstract class SimpleFindWidget extends Widget implements IVerticalSashLa } this._isVisible = true; + this._updateFindInputAriaLabel(); this.updateResultCount(); this.layout(); @@ -341,6 +355,8 @@ export abstract class SimpleFindWidget extends Widget implements IVerticalSashLa public hide(animated = true): void { if (this._isVisible) { + // Reset the accessibility help hint flag so it can be announced again on next reveal + this._accessibilityHelpHintAnnounced = false; this._innerDomNode.classList.toggle('suppress-transition', !animated); this._innerDomNode.classList.remove('visible-transition'); this._innerDomNode.setAttribute('aria-hidden', 'true'); @@ -436,6 +452,36 @@ export abstract class SimpleFindWidget extends Widget implements IVerticalSashLa this.state.change(state, false); } + /** + * Updates the ARIA label of the find input box. + * When a screen reader is active and the accessibility verbosity setting is enabled, + * includes a hint about pressing Alt+F1 for accessibility help on first reveal. + * The hint is only announced once per show/hide cycle to prevent double-speak. + */ + private _updateFindInputAriaLabel(): void { + let findLabel = NLS_FIND_INPUT_LABEL; + + // Include accessibility help hint on first reveal when screen reader is active + // Note: Using raw string for setting ID - this setting may not be registered yet + if (!this._accessibilityHelpHintAnnounced && this._configurationService.getValue('accessibility.verbosity.find') && this._accessibilityService.isScreenReaderOptimized()) { + const keybinding = this._keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); + if (keybinding) { + findLabel += ', ' + nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); + this._accessibilityHelpHintAnnounced = true; + + // Reset to plain label after delay to avoid repeated announcement on focus changes + this._labelResetTimeout?.dispose(); + this._labelResetTimeout = disposableTimeout(() => { + if (this._isVisible) { + this._findInput.inputBox.setAriaLabel(NLS_FIND_INPUT_LABEL); + } + }, 1000); + } + } + + this._findInput.inputBox.setAriaLabel(findLabel); + } + private _announceSearchResults(label: string, searchString?: string): string { if (!searchString) { return nls.localize('ariaSearchNoInput', "Enter search input"); diff --git a/src/vs/workbench/contrib/search/browser/searchWidget.ts b/src/vs/workbench/contrib/search/browser/searchWidget.ts index dc22006c34a0b..cfc48f507de64 100644 --- a/src/vs/workbench/contrib/search/browser/searchWidget.ts +++ b/src/vs/workbench/contrib/search/browser/searchWidget.ts @@ -13,7 +13,7 @@ import { ReplaceInput } from '../../../../base/browser/ui/findinput/replaceInput import { IInputBoxStyles, IMessage, InputBox } from '../../../../base/browser/ui/inputbox/inputBox.js'; import { Widget } from '../../../../base/browser/ui/widget.js'; import { Action } from '../../../../base/common/actions.js'; -import { Delayer } from '../../../../base/common/async.js'; +import { Delayer, disposableTimeout } from '../../../../base/common/async.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; import { CONTEXT_FIND_WIDGET_NOT_VISIBLE } from '../../../../editor/contrib/find/browser/findModel.js'; @@ -138,6 +138,14 @@ export class SearchWidget extends Widget { private ignoreGlobalFindBufferOnNextFocus = false; private previousGlobalFindBufferValue: string | null = null; + /** + * Tracks whether the accessibility help hint has been announced in the ARIA label. + * Reset when the widget loses focus, allowing the hint to be announced again + * on the next focus. + */ + private _accessibilityHelpHintAnnounced = false; + private _labelResetTimeout: IDisposable | undefined; + private _onSearchSubmit = this._register(new Emitter<{ triggeredOnType: boolean; delay: number }>()); readonly onSearchSubmit: Event<{ triggeredOnType: boolean; delay: number }> = this._onSearchSubmit.event; @@ -251,6 +259,7 @@ export class SearchWidget extends Widget { if (focusReplace && this.isReplaceShown()) { if (this.replaceInput) { + this._updateSearchInputAriaLabel(false); this.replaceInput.focus(); if (select) { this.replaceInput.select(); @@ -258,6 +267,7 @@ export class SearchWidget extends Widget { } } else { if (this.searchInput) { + this._updateSearchInputAriaLabel(true); this.searchInput.focus(); if (select) { this.searchInput.select(); @@ -266,6 +276,41 @@ export class SearchWidget extends Widget { } } + /** + * Updates the ARIA label of the search input box. + * When a screen reader is active and the accessibility verbosity setting is enabled, + * includes a hint about pressing Alt+F1 for accessibility help on first focus. + * The hint is only announced once per focus cycle to prevent double-speak. + * @param includeHint Whether to include the accessibility help hint in the label + */ + private _updateSearchInputAriaLabel(includeHint: boolean): void { + if (!this.searchInput) { + return; + } + + let searchLabel = nls.localize('label.Search', 'Search: Type Search Term and press Enter to search'); + + // Include accessibility help hint when requested, screen reader is active, and setting is enabled + // Note: Using raw string for setting ID - this setting may not be registered yet + if (includeHint && !this._accessibilityHelpHintAnnounced && this.configurationService.getValue('accessibility.verbosity.find') && this.accessibilityService.isScreenReaderOptimized()) { + const keybinding = this.keybindingService.lookupKeybinding('editor.action.accessibilityHelp')?.getAriaLabel(); + if (keybinding) { + searchLabel += ', ' + nls.localize('accessibilityHelpHintInLabel', "Press {0} for accessibility help", keybinding); + this._accessibilityHelpHintAnnounced = true; + + // Reset to plain label after delay to avoid repeated announcement on focus changes + this._labelResetTimeout?.dispose(); + this._labelResetTimeout = disposableTimeout(() => { + if (this.searchInput) { + this.searchInput.inputBox.setAriaLabel(nls.localize('label.Search', 'Search: Type Search Term and press Enter to search')); + } + }, 1000); + } + } + + this.searchInput.inputBox.setAriaLabel(searchLabel); + } + setWidth(width: number) { this.searchInput?.inputBox.layout(); if (this.replaceInput) { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index bb01def5f6030..8fad759ebe12e 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -638,12 +638,14 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { } } - const confirmationMessages = isFinalAutoApproved ? undefined : { + // If forceConfirmationReason is set, always show confirmation regardless of auto-approval + const shouldShowConfirmation = !isFinalAutoApproved || context.forceConfirmationReason !== undefined; + const confirmationMessages = shouldShowConfirmation ? { title: confirmationTitle, message: new MarkdownString(localize('runInTerminal.confirmationMessage', "Explanation: {0}\n\nGoal: {1}", args.explanation, args.goal)), disclaimer, terminalCustomActions: customActions, - }; + } : undefined; return { confirmationMessages, diff --git a/src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts b/src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts index d7af946592c16..285c2c026603f 100644 --- a/src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts +++ b/src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts @@ -22,6 +22,7 @@ import { TerminalClipboardContribution } from '../../clipboard/browser/terminal. import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js'; import { createTextInputActions } from '../../../../browser/actions/textInputActions.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; +import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; const TERMINAL_FIND_WIDGET_INITIAL_WIDTH = 419; @@ -43,7 +44,8 @@ export class TerminalFindWidget extends SimpleFindWidget { @IHoverService hoverService: IHoverService, @IKeybindingService keybindingService: IKeybindingService, @IThemeService themeService: IThemeService, - @ILogService logService: ILogService + @ILogService logService: ILogService, + @IAccessibilityService accessibilityService: IAccessibilityService ) { super({ showCommonFindToggles: true, @@ -59,7 +61,7 @@ export class TerminalFindWidget extends SimpleFindWidget { closeWidgetActionId: TerminalFindCommandId.FindHide, type: 'Terminal', matchesLimit: XtermTerminalConstants.SearchHighlightLimit - }, contextViewService, contextKeyService, hoverService, keybindingService); + }, contextViewService, contextKeyService, hoverService, keybindingService, configurationService, accessibilityService); this._register(this.state.onFindReplaceStateChange(() => { this.show(); diff --git a/src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts b/src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts index a52aedf2ec18c..c72a7bc0e96e5 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts @@ -8,6 +8,8 @@ import { IContextKey, IContextKeyService } from '../../../../platform/contextkey import { IContextViewService } from '../../../../platform/contextview/browser/contextView.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; import { SimpleFindWidget } from '../../codeEditor/browser/find/simpleFindWidget.js'; import { KEYBINDING_CONTEXT_WEBVIEW_FIND_WIDGET_FOCUSED } from './webview.js'; @@ -33,13 +35,15 @@ export class WebviewFindWidget extends SimpleFindWidget { @IContextViewService contextViewService: IContextViewService, @IContextKeyService contextKeyService: IContextKeyService, @IHoverService hoverService: IHoverService, - @IKeybindingService keybindingService: IKeybindingService + @IKeybindingService keybindingService: IKeybindingService, + @IConfigurationService configurationService: IConfigurationService, + @IAccessibilityService accessibilityService: IAccessibilityService ) { super({ showCommonFindToggles: false, checkImeCompletionState: _delegate.checkImeCompletionState, enableSash: true, - }, contextViewService, contextKeyService, hoverService, keybindingService); + }, contextViewService, contextKeyService, hoverService, keybindingService, configurationService, accessibilityService); this._findWidgetFocused = KEYBINDING_CONTEXT_WEBVIEW_FIND_WIDGET_FOCUSED.bindTo(contextKeyService); this._register(_delegate.hasFindResult(hasResult => { diff --git a/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts b/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts index 4b117fcb27536..9af69709a22ba 100644 --- a/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts +++ b/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts @@ -272,6 +272,10 @@ declare module 'vscode' { chatSessionId?: string; chatSessionResource?: Uri; chatInteractionId?: string; + /** + * If set, tells the tool that it should include confirmation messages. + */ + forceConfirmationReason?: string; } export interface PreparedToolInvocation {