microsoft / microsoft/vscode

agent host: stop deleting enablement decisions against a mutable client base

Open
#330,917 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

The overlay model in `AgentHostCustomizationEnablementService` is correct but is the subtlest thing in the subsystem, and its failure mode was a **write that silently erased itself**.

The model: the client's global value is the *base*, host decisions layer on top, and "store only what differs" compares against that base rather than a hardcoded default. `_setPersistentDecision` deletes a decision that matches its inherited value, so the UI can restore inheritance without a third "Inherit" action.

The bug it produced: `replaceEnablement` wrote a decision, which synchronously triggered a republish, which fed the client's *stale* global back in, which made the just-written decision look redundant, which deleted it. The user's disable silently did nothing. Diagnosing it required a call stack showing the loop.

The residual hazard is that the comparison base is **mutable and asynchronous**. `_clientGlobalEnablement` can change after a host decision is stored, at which point a previously-stored decision may become "redundant" — or, worse, a decision that should exist was never stored because it matched a base that has since changed.

## Suggestion

Stop using the client global as a *deletion trigger*. Always persist explicit host decisions, and use the client global purely as the inheritance root when resolving. Deletion then happens only on an explicit user action (clear/reset), never as a side effect of a base value moving.

Cost is a few more stored entries. Storage was never the constraint — there is already a 512-entry LRU with eviction — and the reduction in reasoning burden is significant: the invariant becomes "we store what the user told us" rather than "we store what differs from a value that can change underneath us."

## Guard

Keep the regression coverage around `replaceEnablement` → republish → `applyClientGlobalEnablement`, which is where the self-defeating write happened.

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.