microsoft / microsoft/vscode

agent host: collapse the client-state parameters of resolveCustomizationEnablement

Open
#330,914 0 comments 0 reactions 1 assignee Claimed by @connor4312 View on GitHub
agent-host debt
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

Follow-up cleanup from the scoped-enablement work (#330566, #330715).

## Problem

`resolveCustomizationEnablement` in `src/vs/platform/agentHost/node/shared/customizationEnablementGate.ts` now takes six parameters, the last three of which are all optional and all different views of the same client state:

```ts
export function resolveCustomizationEnablement(
service: IAgentHostCustomizationEnablementService,
session: URI,
customizations: readonly Customization[],
clientChildEnablement?: ReadonlyMap>>,
clientPlugins?: ReadonlyMap,
mcpServerOwners?: ReadonlyMap,
): IResolvedCustomizationEnablement
```

There are 14 call sites across 5 files, and because every extra argument is optional, **omitting one is silent**. That is not hypothetical:

- Codex never supplied the ownership map to its `McpCustomizationController`, so a promoted plugin-owned MCP server keyed as `mcpServers#` instead of `#mcp=` and forgot its global/workspace decision across sessions. Fixed in #330715, but only after it was found by hand.
- Auditing whether a given omission is a bug is genuinely hard. Codex's two `resolveCustomizationEnablement` call sites still omit `mcpServerOwners`, and that is *fine* — they only pass plugin candidates, so the nested branch derives ownership from the plugin in hand. But establishing that took several minutes of tracing. A reviewer cannot tell by looking.

## Suggestion

Collapse the three client-state parameters into a single explicit context object, e.g.

```ts
interface ICustomizationResolutionContext {
readonly clientChildEnablement?: ...;
readonly clientPlugins?: ...;
readonly mcpServerOwners?: ...;
}
```

Even keeping the members optional, a named object gives one documented place describing what a caller is expected to provide and why, and makes "which views of client state does this call site have?" reviewable at a glance. Better still if the shape can be made required-with-explicit-empty, so forgetting one is a type error.

Low risk, mechanical, and it would have made the Codex gap impossible to introduce.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.