Skip to content

chat: support selective customization lockdown - #332071

Open
Harald Kirschner (digitarald) wants to merge 7 commits into
mainfrom
digitarald/agents/rubber-duck-review-fix-8826
Open

chat: support selective customization lockdown#332071
Harald Kirschner (digitarald) wants to merge 7 commits into
mainfrom
digitarald/agents/rubber-duck-review-fix-8826

Conversation

@digitarald

@digitarald Harald Kirschner (digitarald) commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Fixes microsoft/vscode-internalbacklog#8826

Dependency microsoft/vscode-policy-watcher#75 is merged and @vscode/policy-watcher@1.5.0 is published.

Summary

Adds the canonical selective form of strictPluginOnlyCustomization while preserving existing boolean behavior:

  • true continues to block standalone skills, agents, hooks, instructions, and MCP servers.
  • arrays containing skills, agents, hooks, and/or mcp block only those standalone surfaces.
  • plugin-provided customizations remain eligible.
  • malformed or partially unknown arrays fail closed instead of applying a weaker partial policy.
  • selective arrays intentionally do not map to VS Code instructions; boolean true remains the instruction lockdown.

Implementation

  • carries boolean or selective values through server, file, and native managed-settings projection
  • keeps the named VS Code policy boolean-only for existing administrative templates
  • uses the scalar-union managed-setting API from policy-watcher 1.5.0
  • remains compatible with watcher 1.4 by retrying only the first declared scalar type (boolean) when the legacy addon rejects a union
  • reuses prompt discovery gates for skills, agents, hooks, and embedded agent hooks
  • applies MCP selection at registry lookup/start and live-server policy boundaries
  • preserves managed-hook, managed-MCP allowlist, strict-marketplace, and plugin provenance behavior
  • documents selective semantics, rollout compatibility, and control composition

Review fixes

Addressed all automated review findings:

  • model scalar unions as non-empty tuples
  • make native watcher replacement transactional so failed replacement preserves the active watcher
  • reject same-ID MCP collection/definition swaps during async connection resolution

Validation

  • merged current main and resolved managed-settings changes from upstream
  • npm run export-policy-data (clean generated policy result; named policy remains boolean)
  • 271 targeted policy, account, prompt, hook, and MCP unit tests
  • 14 native managed-settings service tests, including legacy fallback and failed replacement
  • published policy-watcher 1.5.0 installed and built locally
  • policy-watcher Linux, macOS, and Windows Azure builds

Local typecheck-client after merging main is blocked by unavailable private SDK packages in this checkout; the pre-merge typecheck passed, and the fresh PR Compile & Hygiene job is the authoritative post-merge validation.

Preserve boolean-or-selective strictPluginOnlyCustomization values through managed settings and enforce individual skill, agent, hook, and MCP surfaces without weakening plugin provenance.\n\nAdd native scalar-union policy delivery, fail-closed validation, dynamic MCP revocation, focused tests, and enterprise documentation.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@digitarald

Copy link
Copy Markdown
Contributor Author

Dependency status: microsoft/vscode-policy-watcher#75 now has green GitHub and Azure compile/package jobs. Current CI failures are the expected npm 404 for unpublished @vscode/policy-watcher@1.5.0; I will regenerate the lockfile and rerun immediately after that package is published.

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Install dependencies

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds selective enterprise lockdown for standalone chat customizations while preserving plugin-provided customizations and boolean behavior.

Changes:

  • Supports validated selectors for skills, agents, hooks, and MCP.
  • Enforces MCP policy during discovery, resolution, and active connections.
  • Adds scalar-union managed-setting transport and coverage.
Show a summary per file
File Description
src/vs/workbench/services/policies/test/browser/accountPolicyService.test.ts Updates managed-setting test types.
src/vs/workbench/services/accounts/test/browser/managedSettings.test.ts Tests selective response adaptation.
src/vs/workbench/services/accounts/browser/managedSettings.ts Types selective server responses.
src/vs/workbench/contrib/mcp/test/common/mcpRegistry.test.ts Tests selective MCP enforcement.
src/vs/workbench/contrib/mcp/common/mcpServer.ts Blocks unavailable policy-filtered collections.
src/vs/workbench/contrib/mcp/common/mcpRegistry.ts Applies MCP-specific lockdown checks.
src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts Tests selective agent and hook filtering.
src/vs/workbench/contrib/chat/test/common/customizationLockdown.test.ts Tests selector and fail-closed semantics.
src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md Documents selective lockdown behavior.
src/vs/workbench/contrib/chat/common/customizationLockdown.ts Implements per-surface policy evaluation.
src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts Registers union-typed managed policy transport.
src/vs/platform/policy/test/node/nativeManagedSettingsService.test.ts Tests native union definitions and watcher replacement.
src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts Tests selective normalization.
src/vs/platform/policy/test/common/copilotManagedSettings.test.ts Tests union projection and parsing.
src/vs/platform/policy/node/nativeManagedSettingsService.ts Passes unions to the native watcher.
src/vs/platform/policy/common/copilotManagedSettings.ts Defines selectors, normalization, and fail-closed parsing.
src/vs/platform/configuration/test/common/policyConfiguration.test.ts Tests structured internal boolean-policy values.
src/vs/base/common/policy.ts Adds managed-setting scalar unions.
package.json Requires policy-watcher 1.5.0.
package-lock.json Updates the locked native dependency.
.github/skills/policy-and-managed-settings/github-managed-settings.md Documents hybrid scalar/structured settings.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 20/21 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread src/vs/base/common/policy.ts Outdated
Comment thread src/vs/workbench/contrib/mcp/common/mcpRegistry.ts
Comment thread src/vs/platform/policy/node/nativeManagedSettingsService.ts Outdated
Tighten scalar-union typing, preserve active native policy delivery across failed watcher replacement, and reject same-ID MCP collection swaps during async resolution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@digitarald

Copy link
Copy Markdown
Contributor Author

Addressed all three Copilot review comments in 8267643. Focused native-policy and MCP suites pass (63 tests), and
pm run typecheck-client passes.

Use the reviewed watcher 1.5.0 commit so draft CI can install and validate the dependent change. This pin will be replaced with the published package before marking the PR ready.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@digitarald

Copy link
Copy Markdown
Contributor Author

Temporarily pinned the dependency to the reviewed policy-watcher #75 commit in 22d787d (will replace with the published 1.5.0 package before ready review). This removes the npm 404 so draft CI can validate the full VS Code change now.

@digitarald

Copy link
Copy Markdown
Contributor Author

Correction: the temporary dependency pin is commit 957df15.

@digitarald

Copy link
Copy Markdown
Contributor Author

Removed the temporary tarball pin because Azure's dependency-cleanliness gate rejects it. Draft CI now uses the current published 1.4.0 package so all source/tests can validate; the final 1.5.0 manifest+lock bump remains gated on policy-watcher #75 merge/publication.

Keep draft CI on the current published dependency. The final 1.5.0 version and lockfile update will land immediately after policy-watcher #75 is published.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retry scalar-union declarations with their first scalar type only when the installed native module reports the legacy type contract. This preserves existing boolean policy delivery until watcher 1.5.0 is installed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@digitarald

Copy link
Copy Markdown
Contributor Author

The macOS policy export timeout exposed watcher 1.4 rejecting union definitions. Added a narrowly detected compatibility fallback in d09d026: 1.4 continues watching the first type (�oolean), while 1.5 receives the full union. Added regression coverage; 14 native-policy tests and ypecheck-client pass. Windows/macOS smoke failures were unrelated workbench-startup flakes and will be rerun after this push.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid PR. Tests cover the important paths.

…ber-duck-review-fix-8826

# Conflicts:
#	src/vs/platform/policy/common/copilotManagedSettings.ts
#	src/vs/platform/policy/test/common/copilotManagedSettings.test.ts
#	src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts
#	src/vs/workbench/services/accounts/browser/managedSettings.ts
#	src/vs/workbench/services/policies/test/browser/accountPolicyService.test.ts
@digitarald

Copy link
Copy Markdown
Contributor Author

Dependency unblocked: policy-watcher #75 merged and 1.5.0 is published. Merged current main, resolved managed-settings conflicts, updated package/lock metadata to the real npm artifact, reran policy export (clean), and passed 271 targeted tests. Fresh CI is running; I will mark ready after it is green.

@digitarald

Copy link
Copy Markdown
Contributor Author

Fresh CI failures were the upstream unified-workspace-picker regression fixed by #334677. Merged that main commit in 6e94e7f; no feature code changes were needed. Fresh CI is running.

@digitarald
Harald Kirschner (digitarald) marked this pull request as ready for review September 5, 2026 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants