policy: enterprise managed_settings for Copilot clients#318623
Merged
joshspicer merged 32 commits intoMay 29, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements ADR-002 enterprise-managed Copilot client settings by fetching /copilot_internal/managed_settings via the default account flow and surfacing the resulting values through the existing IPolicyService/AccountPolicyService policy pipeline, so enterprise .github/copilot/settings.json can influence chat plugin enablement and marketplace trust.
Changes:
- Add managed settings fetch + adaptation (
adaptManagedSettings) toDefaultAccountProvider, including sharedRetry-After-aware backoff for/copilot_internal/*calls. - Introduce three new policies wired to chat settings:
ChatEnabledPlugins→chat.pluginLocations,ChatPluginMarketplaces→chat.plugins.marketplaces, andChatStrictMarketplaces→chat.plugins.strictMarketplaces. - Update plugin marketplace + discovery consumers to merge
default + user + policylayers, add Policy Diagnostics output, and add unit tests for the managed settings adaptation.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/services/accounts/test/browser/defaultAccount.test.ts | Adds unit tests for adaptManagedSettings transformations and dedup/ref handling. |
| src/vs/workbench/services/accounts/browser/defaultAccount.ts | Fetches managed settings in parallel with token entitlements; adapts response into IPolicyData; adds Retry-After backoff logic. |
| src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts | Merges default/user/policy marketplaces for fetch; adds strict trust mode based on configured marketplaces. |
| src/vs/workbench/contrib/chat/common/plugins/agentPluginServiceImpl.ts | Merges default/user/policy for plugin locations via observable; adds plugin-id resolution support. |
| src/vs/workbench/contrib/chat/common/constants.ts | Adds ChatConfiguration.StrictMarketplaces key. |
| src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts | Registers new strict marketplaces setting and wires three new policies to existing/new settings. |
| src/vs/workbench/browser/actions/developerActions.ts | Extends Policy Diagnostics with an ADR-002 managed settings section. |
| src/vs/base/common/product.ts | Extends IDefaultChatAgent with managedSettingsUrl. |
| src/vs/base/common/defaultAccount.ts | Extends IPolicyData with managed settings-derived fields. |
| product.json | Adds default managedSettingsUrl pointing to GitHub.com API endpoint. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 5
Member
Author
|
Distro PR: microsoft/vscode-distro#1422 — adds |
22197b7 to
06cba15
Compare
…tMarketplaces settings Adds three new chat.plugins.* settings, each policy-backed: - chat.plugins.enabledPlugins (policy: objectChatEnabledPlugins) mapping plugin IDs (`<plugin>@<marketplace>`) to enable/disable. - chat.plugins.marketplaces (policy: array ofChatPluginMarketplaces) marketplace references (GitHub shorthand or Git URI). User entries survive alongside policy entries. - chat.plugins.strictMarketplaces (policy: ChatStrictMarketplaces) boolean restricting trust to listed marketplaces only. All three are gated on `tags: ['experimental']`. Consumers (plugin discovery, install, URL handler, marketplace service, quick-pick action) now read via `inspect()` so default + user + policy layers all flow through. A shared `readConfiguredMarketplaces` helper in marketplaceReference.ts dedups the inspect pattern across 5 sites. Adds three matching fields to IPolicyData so the policy framework has slots to fill in once the wiring lands; until then they're undefined and behave like an empty policy (no-op). Plugin discovery now distinguishes filesystem-path entries (removable from UI) from enterprise plugin IDs (non-removable) via a single shared loop; `IAgentPlugin.remove` is optional accordingly. build/lib/policies/policyData.jsonc regenerated for the new policy keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…wiring Wires the previously-added chat.plugins.* policy slots to the new `/copilot_internal/managed_settings` endpoint on the authenticated Copilot host. Core behavior in DefaultAccountProvider: - Fetches managed_settings alongside entitlements; shares the 1-hour cache used by other account-policy fetches. - Silent fallback to local-only policy on any non-2xx, network error, parse error, or missing managedSettingsUrl. - Rate-limit-aware: backs off all /copilot_internal/* calls when the endpoint signals 429, 403 + X-RateLimit-Remaining: 0, or any non-2xx with Retry-After. - adaptManagedSettings flattens the API's structured extraKnownMarketplaces map into the existing string-array shape that chat.plugins.marketplaces consumes; tolerates malformed entries and unknown response keys (forward-compatible). - Telemetry: emits `defaultaccount:managedSettings:fetch` (owner: joshspicer) with an `outcome` bucket (ok / no-response / parse-error / status:NNN) and a `rateLimitBackoffActive` flag. Surface area: - IDefaultAccountProvider/Service expose managedSettingsFetchStatus and managedSettingsFetchedAt; ManagedSettingsFetchStatus is a named union. - Developer: Policy Diagnostics shows a Managed Settings section with the URL status, last-fetched timestamp, and a JSON dump of the applied managed-settings policy slice. - product.json adds a managedSettingsUrl key (populated via distro). Refactor: `readHeader` and `retryAfterFromHeaders` are moved to `platform/request/common/request.ts` so githubRepoFetcher.ts and this new code share one implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
06cba15 to
145bcd1
Compare
Pulls in managedSettingsUrl from microsoft/vscode-distro#1422. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore historical default for chat.plugins.marketplaces (['github/copilot-plugins', 'github/awesome-copilot#marketplace']) so existing users don't lose the two built-in marketplaces on update. Regenerate policyData.jsonc accordingly. - Seed _managedSettingsFetchStatus = 'ok' on cache-hit so Policy Diagnostics reports the applied state after a process restart that warm-starts from cached policyData (instead of stuck at 'not yet fetched'). - Scope the <plugin>@<marketplace> ID-resolution rule to the enterprise ChatEnabledPlugins setting only. User-typed entries in chat.pluginLocations that happen to contain '@' are now treated as filesystem paths, as a user would expect, not silently rewritten to ~/.copilot/installed-plugins/<x>/<y>/. Split _resolvePluginPath into a path-only resolver and a dedicated _resolveEnterprisePluginId. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
chat.pluginLocations has no policy slot, so observableConfigValue (which uses getValue() under the hood) is functionally equivalent to the hand-rolled inspect() version. Reverting reduces diff thechurn inspect-based observable is now used only for _enterpriseEnabledPluginsConfig where the default+user+policy merge actually matters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds chat.plugins.extraMarketplaces (ChatExtraMarketplaces policy,
included: false so it's hidden from the Settings UI). This receives the
'extraKnownMarketplaces' payload from the managed_settings API.
Restores chat.plugins.marketplaces to its pre-PR shape: no policy slot,
no inspect()-juggling required in consumers, no risk of accidentally
clobbering user data. Users write to chat.plugins.marketplaces; the
enterprise writes to chat.plugins.extraMarketplaces; the effective set
is the union.
Consumer simplifications:
- readConfiguredMarketplaces returns { userValues, extraValues,
two getValue() reads, no inspect() needed.effectiveValues }
- Write-back is now just [...userValues, refValue] in all three sites.
- 'Manage Plugin Marketplaces' still surfaces the 'managed by enterprise
policy' badge by checking ref membership in extraValues.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- fetchMarketplacePlugins: drop the over-engineered pre-dedup-by-string; parseMarketplaceReferences already dedups by canonical id. - agentPluginServiceImpl: pass source.remove directly to _toPlugin instead of wrapping in a null-asserted closure. - adaptManagedSettings: use a Set for flatten-and-dedup (insertion order is preserved). - getDefaultAccountFromAuthenticatedSessions: spread merge instead of three explicit field assignments. - developerActions: collapse the 'ok' branch into the catch-all backtick wrap; same behavior, less code. - marketplaceReference.ts: tighter JSDoc on IConfiguredMarketplaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…scovery Previously the enterprise-managed policy values were delivered into the policy framework but not a plugin already installed locallyenforced (e.g. via the marketplace discovery path) would remain active even when the policy excluded it or strict-marketplace mode rejected its source. Adds policy enforcement on AgentPluginService.plugins, applied after discovery dedup/sort and gated by two observables: - ChatEnabledPlugins policy: when set, filters the surfaced plugin set to only those whose '<name>@<marketplace>' ID appears in the policy map with value true. Plugins without a marketplace provenance (filesystem entries from chat.pluginLocations) are unaffected. - ChatStrictMarketplaces: when on, filters out plugins whose source marketplace is not trusted. Trust is sourced ONLY from chat.plugins.extraMarketplaces (the policy-only user-setslot) entries in chat.plugins.marketplaces do NOT grant trust under strict mode. This matches the ADR-002 semantics: strict mode hands full marketplace control to the enterprise. Also updates the chat.plugins.strictMarketplaces description text to match the new behavior (was still pointing at the user setting). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Moves IManagedSettingsResponse and adaptManagedSettings out of defaultAccount.ts and into a new managedSettings.ts in the same folder. Adapter is a pure transformation function with no service dependencies, so it belongs in its own file alongside the HTTP/wiring code. Renames the test file to managedSettings.test.ts to match what it actually tests and tightens the suite name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…scription Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Blocked plugins (ChatEnabledPlugins / strict marketplaces) now stay visible but are forced disabled via their enablement observable, and the enable affordance notifies the user instead of re-enabling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…anges pluginMarketplaceService.onDidChangeMarketplaces only listened for PluginsEnabled and PluginMarketplaces config changes, so the ExtraMarketplaces values delivered by the ChatExtraMarketplaces policy never triggered a the union was stale until the next user editrefetch to chat.plugins.marketplaces or a workspace-trust change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tings Move the enterprise-managed marketplace entry type out of defaultAccount.ts into a dedicated managedSettings.ts so the type lives alongside other managed-settings-specific code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Sync policyData.jsonc ChatExtraMarketplaces description with the source declaration in chat.shared.contribution.ts (object-form entries were missing from the policy artifact). - Reorder Event import in agentPluginServiceImpl.ts to keep base/common imports alphabetical. - Fix stale doc reference (COPILOT_CLI_INSTALLED_PLUGINS_DIR -> the function it actually mirrors). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Base:
|
ADR-002 describes the `git` source `url` as a free-form `(string)` the example happens to be a full clone URL, but the schema doesn't require a repo path. Our marketplace-URI parser was rejecting host-only HTTPS endpoints (e.g. `https://plugins.internal.example.com`), so enterprise policy entries with marketplace-registry-style URLs were silently dropped before they ever reached the UI. Relax `parseUriMarketplaceReference` to accept host-only URLs and treat them as a marketplace endpoint identified by host alone. The canonical id becomes `git:<host>/` so distinct hosts still dedupe correctly. Existing path-aware behavior is preserved unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…; fix test cloneUrl expectation - Handle string-typed entries in extraKnownMarketplaces (IPolicyData allows string | IExtraKnownMarketplaceEntry) - Fix test expectation: URI.parse normalizes host-only URLs to include trailing slash Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The setting schema is now `{ [name]: url-or-shorthand }` (object), so
readConfiguredMarketplaces must convert each entry to the nested
IExtraMarketplaceObjectEntry shape that parseMarketplaceReferences expects.
Uses a regex to detect GitHub shorthand (owner/repo[#ref]) vs URI.
TypeError in CI:
'extraValues is not iterable' on [...userValues, ...extraValues].
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssion tests for Settings Editor display
Extract the policy.value conversion for ChatExtraMarketplaces out of
chat.shared.contribution.ts into a reusable, unit-testable helper. The
helper converts the IExtraKnownMarketplaceEntry[] policy payload into the
{ [name]: url-or-shorthand } dict that:
- the Settings Editor's ComplexObject renderer can display inline as
key/value rows (instead of just 'Edit in settings.json'), and
- readConfiguredMarketplaces reverses back into IExtraMarketplaceObjectEntry[]
so parseMarketplaceReferences preserves displayLabel = name.
Tests added:
undefined
owner/repo
owner/repo#ref
raw URL (+ optional #ref)
parseMarketplaceReferences flow (the regression test that catches the
'extraValues is not iterable' bug we just hit in CI)
- schema-shape: chat.plugins.extraMarketplaces is registered with
type=object + additionalProperties.type=['string'], the exact shape
the Settings Editor requires to render as ComplexObject
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-form entries url dict, policy entries always reach the marketplace fetcher as IExtraMarketplaceObjectEntry objects (not strings). The validation loop was only accepting strings, producing a 'Ignoring invalid marketplace entry: [object Object]' debug log for every valid policy entry. Validate using parseMarketplaceObjectEntry for object values so the warning fires only for genuinely-unparseable entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on commands The schema-shape test for chat.plugins.extraMarketplaces imported the full chat.shared.contribution module to populate the configuration registry. This re-registered commands (already registered by the workbench under test), producing 'Cannot register two commands with the same id: workbench.action.chat.markHelpful' and cascading disposable leaks in unrelated suites (EditorService, WorkingCopyBackupTracker). The other 5 tests (extraKnownMarketplacesToConfigDict + end-to-end round trip) cover the actual behavior that broke; the schema shape is exercised implicitly by the round-trip test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onical id Plugin marketplace trust under strict mode compares canonicalId. A plugin discovered from 'https://github.com/microsoft/vscode-team-kit.git' was being blocked even though 'microsoft/vscode-team-kit' was in the trusted list, because the URI parser produced 'git:github.com/microsoft/vscode-team-kit.git' while the shorthand parser produced 'github:microsoft/vscode-team-kit'. When parseUriMarketplaceReference / parseScpMarketplaceReference detect a github.com authority, emit the same canonical id form the shorthand parser uses so all three forms (shorthand, https URI, SCP) collapse to a single trusted reference. Existing dedup test now expects 1 entry instead of 2; ref-distinction test collapses the https+#ref entry with its shorthand sibling. Added a focused regression test asserting all four forms produce identical canonical ids. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
10 tasks
Yoyokrazy
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wires a new
/copilot_internal/managed_settingsendpoint into VS Code's existingAccountPolicyService/IPolicyDatastack so an enterprise admin's.github/copilot/settings.jsonpolicy applies to three new plugin/marketplace settings.enabledPluginschat.plugins.enabledPlugins(new)ChatEnabledPluginsextraKnownMarketplaceschat.plugins.extraMarketplaces(new, policy-only)ChatExtraMarketplacesstrictKnownMarketplaceschat.plugins.strictMarketplaces(new)ChatStrictMarketplacesAll three settings are tagged
experimental.chat.plugins.extraMarketplacesis policy-only (included: there's no user-writable surface for it.false)Architecture
AccountPolicyService's existingpolicy.value(policyData)callback flow handles everything natively.IPolicyServiceDefaultAccountProviderfetches in parallel withgetTokenEntitlements(shared sessions, sharedIRequestServiceplumbing, shared 1-hour cache). 5 s request timeout.managedSettingsUrl. An empty response ({}) is a successful "no policy file present" signal.adaptManagedSettings(services/accounts/browser/managedSettings.ts): the API'sRecord<id, { source }>forextraKnownMarketplacesbecomes anIExtraKnownMarketplaceEntry[], preserving the marketplacenamesoenabledPlugins["<plugin>@<marketplace>"]keys resolve consistently.chat.plugins.extraMarketplacesstored as{ [name]: url-or-shorthand }dict, which lets the Settings Editor's ComplexObject renderer display entries inline (read-only) when managed by policy.warn.parsingreadConfiguredMarketplaceshelper inmarketplaceReference.ts.chat.plugins.marketplaces(user) andchat.plugins.extraMarketplaces(policy) are merged;parseMarketplaceReferencesdedupes by canonical id.https://github.com/owner/repo.git), SSH form (git@github.com:owner/repo.git), and shorthand (owner/repo) all produce the same canonical id, so policy trust comparisons match regardless of which form the policy uses.chat.plugins.strictMarketplacesis on,isMarketplaceTrustedderives trust only fromchat.plugins.extraMarketplaces(policy slot). User-added marketplaces inchat.plugins.marketplacesdo not grant trust under strict mode.modeChatEnabledPluginsis an allowlist; combined with strict mode it gates both marketplace-discovered plugins and Copilot-CLI-installed plugins (matched by install-path<marketplace>/<plugin>identity).enforcementRate limiting
The shared
DefaultAccountProvider.request()applies aRetry-After-aware backoff that benefits every/copilot_internal/*call (entitlements, token entitlements, MCP registry, managed settings). Detects:429 Too Many Requests403withX-RateLimit-Remaining: 0(primary quota exhaustion)Retry-After(secondary throttling)Default 60 s when
Retry-Afteris absent. Mirrors the public-API pattern ingithubRepoFetcher.readHeaderandretryAfterFromHeadersare exported fromplatform/request/common/request.tsand shared by both call sites.tsTelemetry
New event
defaultaccount:managedSettings:fetch(owner:joshspicer):outcome:ok/no-response/parse-error/status:NNNrateLimitBackoffActive: whether the call was short-circuited by a priorRetry-AfterwindowNot fired for
no-url(would be noise from every non-Copilot environment).Diagnostics
Developer: Policy Diagnostics(Cmd+Shift+P) grows a## Managed Settingssection showing:ok/no-response/parse-error/no-url)Developer: Sync Account Policyinvalidates the cache and refetches.Tests
src/vs/workbench/services/accounts/test/browser/managedSettings.test.tscoversadaptManagedSettingsshape transformations (empty / partial / full / github vs git sources / ref handling) and resilience to malformed/unknown payloads.src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.tscovers the newreadConfiguredMarketplaces/extraKnownMarketplacesToConfigDicthelpers, host-only git URLs, and the canonical-id parity guarantee for GitHub URL vs shorthand.IDefaultAccountService/IDefaultAccountProviderupdated for the new readonly fields.Distro
Companion vscode-distro PR: microsoft/vscode-distro#1422 adds
managedSettingsUrltomixin/{stable,insider,exploration}/product.json.Out of scope
disableAllHooks.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com