mirror of
https://github.com/microsoft/vscode.git
synced 2026-07-08 15:55:15 +01:00
Josh Spicer
2fcb6694a9
* config: allow one policy to govern multiple settings via policyReference Previously each enterprise policy mapped to exactly one configuration setting: `ConfigurationRegistry` enforced a strict 1:1 policy-name guard and rejected any second setting reusing a policy name. This made it impossible for a single admin policy to lock more than one setting (for example, gating the same agent in both the editor window and the Agents window). This introduces an explicit, subordinate attachment mechanism while keeping the strict owner guard: - Add `IPolicyReference` (name + optional runtime-only `value` / `managedSettings`). A setting may declare `policyReference` to be governed by a policy that is *owned* (fully declared via `policy`) by another setting. References deliberately carry no catalog metadata (category / minimumVersion / localization) so exactly one owner per policy name provides those, keeping the exported catalog and generated ADMX/plist unambiguous. - `ConfigurationRegistry` keeps `getPolicyConfigurations()` 1:1 (owners) and adds `getPolicyReferenceConfigurations()` (name -> set of subordinate settings). The duplicate-owner guard is retained; a setting declaring both `policy` and `policyReference` is rejected. - `PolicyConfiguration` resolves references: owners win when present, and a reference synthesizes a per-process definition from its own schema type so the policy still resolves in processes where the owner is not loaded. Policy changes now fan out to the owner and all references. - Developer "policy-controlled settings" diagnostics list references (tagged) alongside owners, and trace logging reports owner/reference registration and fan-out for debugging. Adds unit tests covering registration, the both-declared rejection, owner+reference value application, change propagation, and orphan references (owner registered elsewhere). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * policy: gate Claude/Codex 3P agents across editor and Agents windows Uses the new policyReference mechanism to extend enterprise policy control over the agent-host third-party agents. - Reuse the existing `Claude3PIntegration` policy (owned by the Copilot extension setting `github.copilot.chat.claudeAgent.enabled`) by adding `policyReference`s on the agent-host setting `chat.agentHost.claudeAgent.enabled` and the Agents window setting `sessions.chat.claudeAgent.enabled`. Disabling Claude via policy now applies across the editor window and the Agents window. - Add a new `Codex3PIntegration` policy, owned by `chat.agentHost.codexAgent.enabled` — the single runtime through which Codex is surfaced in both windows. - Both references and the Codex owner respect `chat_preview_features_enabled === false` so organizations that disable preview features automatically disable these agents. - Regenerate the policy catalog entry for Codex3PIntegration; Claude3PIntegration stays owned by the existing extension setting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * policy export: link referenced settings in the exported catalog The exported policy catalog (build/lib/policies/policyData.jsonc) drives enterprise docs. Now that one policy can govern multiple settings via policyReference, each catalog entry should list every setting the policy controls so docs can surface all of them. - Add `referencedSettings?: string[]` to PolicyDto. - During export, populate it for each policy: references registered in the workbench process are discovered from the configuration registry, and references that live in app surfaces not loaded during export (the Agents window, a separate layer the workbench cannot import) are supplemented from a small documented CROSS_SURFACE_POLICY_REFERENCES map. The union is sorted for stable output. - Regenerate the catalog: Claude3PIntegration now links both `chat.agentHost.claudeAgent.enabled` and `sessions.chat.claudeAgent.enabled`. Codex3PIntegration governs only its owning setting, so it has no referencedSettings. - Add trace diagnostics reporting how many referenced settings were linked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * config: make policy owners win over references regardless of registration order Council review found that a late-registering policy owner could not replace an earlier subordinate `policyReference`. `AbstractPolicyService.updatePolicyDefinitions` merged existing-wins and only re-ran when the definition *count* grew, while `PolicyConfiguration` only gave owners precedence within a single batch. So when a reference registered before its owner across separate update cycles — exactly the editor-window case, where the agent-host Claude reference loads eagerly via chat.shared.contribution while the Copilot extension owner loads later — the owner's `value`/`managedSettings`/`restrictedValue` were silently dropped. Fix: - `AbstractPolicyService.updatePolicyDefinitions` now replaces an existing definition for a policy name when a different definition object is submitted, and re-runs `_updatePolicyDefinitions` whenever anything changed (not only on count growth). - `PolicyConfiguration` resolves each policy name owner-first against the registry (not just the current batch) and caches the source schema object per name, so it submits a stable definition object and only re-submits on a real change (e.g. a reference being upgraded to its owner). This keeps owners authoritative regardless of registration order while avoiding redundant policy-service / watcher churn. Adds regression tests: owner definition wins when both are present, and a late-registering owner supersedes an earlier reference definition. Verified: node config/policy suites and browser AccountPolicyService / MultiplexPolicyService tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * policy: drop non-cloneable value callback from serialized policiesData CI's `policyExport.integrationTest` hung (60s timeout) on all Electron jobs. Root cause: `AbstractPolicyService.serialize()` included the full `PolicyDefinition` — including its `value` callback (a function) — in the `policiesData` that the main process sends to the renderer as part of the window configuration. Electron structure- clones that payload over IPC, and a function cannot be cloned ("An object could not be cloned"), so window-configuration resolution never completed and the export window never exited. This only surfaced now because the agent host settings contribution (`agentHostStarter.config.contribution.ts`) is imported in the main process, making it the first policy with a `value` callback registered in the main process policy service. Renderer-only policies reach the main service through the IPC channel, which already drops functions, so their callbacks never entered `policiesData`. Fix: `serialize()` now emits a structured-clone-safe definition (type, managedSettings, restrictedValue) via `toSerializablePolicyDefinition`, dropping the `value` callback. The callback is only evaluated by account-based policy services in the owning process and is never read by `PolicyChannelClient` consumers, so nothing depends on it being transported. Adds a regression test asserting `serialize()` output is structured-clone-safe. Verified: the local `--export-policy-data` run now completes and its output matches the checked-in build/lib/policies/policyData.jsonc under the integration test's normalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * config: simplify policyReference implementation Trim the policyReference change in response to review feedback that it was larger than necessary: - Drop the defensive policy type-consistency guard (`findPolicyTypeMismatch`) and its tests. The owner is authoritative for the resolved definition's type, so a mismatched reference type is harmless in practice; the strict owner guard and the both-declared guard remain. - Revert the developer "policy-controlled settings" diagnostics changes — that was a debug-only nicety, not needed for the feature. - Condense verbose comments and remove redundant trace logging in PolicyConfiguration and the IPolicyReference doc. No behavior change to policy resolution; the regression tests (owner-wins, late-owner, reference resolution, serialize clone-safety) are unchanged and still pass, and the policy export still matches the checked-in catalog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * config: merge reference runtime bits so a policy callback is never dropped Second council review found that owner-first resolution dropped a reference's account-policy `value` callback. Claude3PIntegration's owner is the Copilot extension setting (contributed via product.extensionConfigurationPolicy as JSON), which cannot carry a `value` function; the `chat_preview_features_enabled` callback lives only on the agent-host and sessions `policyReference`s. Because the owner won unconditionally, AccountPolicyService (which only evaluates gating when `policy.value` exists) never applied preview-feature gating to Claude in any process where the Copilot extension is loaded. Fix: `resolvePolicyDefinition` now merges — the owner provides the authoritative type plus any runtime bits it declares, and references fill `value` / `managedSettings` the owner does not provide (and still supply the whole definition when no owner is loaded). This keeps owners authoritative while letting a reference contribute the callback an extension/distro owner cannot declare. Also fixes a related gap (flagged in review): deregistering a policy owner now re-resolves the policy name so the definition falls back to a surviving reference, tracked via a key→policy-name map. Definition change detection now compares the resolved definition's fields (callbacks come from stable registry objects) instead of the source object identity. Adds regression tests: a value-less owner uses the reference's callback, and owner deregistration falls back to the reference. Export still matches the checked-in catalog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * config: make policyReference a pure pointer (drop value/managedSettings) A `policyReference` should not redefine policy semantics — the owner is the single source of truth. Allowing references to carry their own `value` callback recreated the owner-vs-reference divergence that required merge/precedence rules and produced the bug the previous review round fixed. `IPolicyReference` is now just `{ name }`. A reference contributes only the policy name so the setting is gated and the OS policy watcher observes the name in processes where the owner is not loaded; the owner provides the type, value callback and all other runtime behaviour. `resolvePolicyDefinition` is correspondingly simplified back to owner-authoritative (no value/managedSettings merge). Behavioural effect: the Claude agent-host and Agents-window settings are no longer auto-disabled by the `chat_preview_features_enabled` GitHub account policy — they are gated by the `Claude3PIntegration` policy itself (admin OS/MDM + account policy by name), exactly as the editor-window extension setting already behaves on main. That preview-features auto-disable was an artifact of the reference-value workaround, not a requirement; the extension-owned `Claude3PIntegration` cannot express a callback anyway. Codex keeps its preview-features auto-disable because its owner is core code. Tests updated: owner definition is authoritative, reference only contributes the name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * policy: enforce policyReference type match; diagnostics + cleanups - export throws if a policyReference setting's type differs from the owning policy's type - drop hardcoded CROSS_SURFACE_POLICY_REFERENCES (referencedSettings now registry-driven) - Developer: Policy Diagnostics lists policyReference settings - rename updateToPolicyDefinitionType -> toPolicyDefinitionType, fix 'proprety' typo, tighten comment - regenerate policyData.jsonc * policy: address PR review nits - make Codex3PIntegration policy description surface-agnostic (drop 'directly in the editor'); sync policyData.jsonc - fix 'acutal' -> 'actual' typo in the policyReference tests added by this PR * policy: trim verbose policyReference comments to one-liners * policy: trim verbose comments across the PR * policy: trim comments in policy.ts --------- Co-authored-by: Ubuntu <josh@ahp.4mywozgnka0etnlo23z031udwc.xx.internal.cloudapp.net> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>