Fix incorrect inspect property usage in scope detection

[ShehabSherif0]

Fixes #301471

Bug

Two IConfigurationValue property accesses introduced in bb18007 use the IInspectValue<T> object property instead of the scalar value property, causing incorrect scope detection.

1. themeConfiguration.ts - findAutoConfigurationTarget()

settings.userRemote is an IInspectValue<T> object ({value?, override?, overrides?}), not the scalar T value. It can be defined when only language-specific overrides exist in the remote config, even with no actual remote value set. This causes the function to incorrectly return USER_REMOTE, making theme settings target the wrong configuration scope.

Fix: settings.userRemote -> settings.userRemoteValue

2. commandLineAutoApprover.ts - source target detection

configInspectValue.workspaceFolder is an IInspectValue<T> object whose keys are {value, override, overrides}. The checkTarget function looks for tool-pattern keys (e.g., "git*") via hasOwnProperty, which never matches on IInspectValue. Workspace-folder-scoped auto-approve rules are never detected and fall through to a lower-priority scope.

Fix: configInspectValue.workspaceFolder -> configInspectValue.workspaceFolderValue

Root Cause

Both uses are from the same commit (bb18007). The IConfigurationValue<T> interface exposes both scalar values (userRemoteValue: T) and inspect objects (userRemote: IInspectValue<T>). The similar naming makes it easy to use the wrong one.

Testing

Added a regression test that stubs configurationService.inspect to return a workspaceFolderValue containing a rule key and asserts the matched rule reports sourceTarget as ConfigurationTarget.WORKSPACE_FOLDER.

[Copilot]

Pull request overview

Fixes incorrect use of IInspectValue<T> (inspect metadata objects) where scalar *Value properties should be used for configuration scope detection, preventing misclassification of configuration targets.

Changes:

• Use settings.userRemoteValue (scalar) instead of settings.userRemote (inspect object) when auto-selecting theme configuration target.
• Use configInspectValue.workspaceFolderValue (scalar) instead of configInspectValue.workspaceFolder (inspect object) when determining the source target for auto-approve rules.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/vs/workbench/services/themes/common/themeConfiguration.ts	Fixes auto-target selection to correctly detect USER_REMOTE based on the scalar remote value.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/autoApprove/commandLineAutoApprover.ts	Fixes workspace-folder scope detection for auto-approve rule source targeting by using the scalar workspace-folder value.

---

You can also share your feedback on Copilot code review. Take the survey.

[ShehabSherif0]

@microsoft-github-policy-service agree

[ShehabSherif0]

@sandy081 I have posted detailed reproduction steps and type-level analysis on the linked issue in response to your question.

Quick summary of the two fixes in this PR:

1. commandLineAutoApprover.ts (active bug): configInspectValue.workspaceFolder is an IInspectValue object with keys {value, override, overrides}. The checkTarget function does hasOwnProperty(obj, "git*") on it, which always returns false. Changing to configInspectValue.workspaceFolderValue passes the scalar config object {"git*": true}, where hasOwnProperty works correctly. The other scopes in the same chain already use the correct *Value properties.

2. themeConfiguration.ts (dead code): findAutoConfigurationTarget currently has zero callers, so no runtime impact. But settings.userRemote is IInspectValue<T> (can be non-undefined from language overrides alone) while settings.userRemoteValue is the scalar T value. Fixed for correctness.

A regression test is included that verifies workspace-folder-scoped rules get ConfigurationTarget.WORKSPACE_FOLDER as their sourceTarget.

[meganrogge]

Thanks! LGTM!

[ShehabSherif0]

Hi @dmitrivMS @meganrogge thank you both for the approvals!

The CI is failing because of an issue in the regression test I added. The test config uses 'git*': true as the auto-approve pattern, but since git* isn't wrapped in /.../, the auto-approver treats it as a plain string. The * gets escaped to \* by escapeRegExpCharacters, producing the regex /^git\*\b/ - which only matches the literal text git*, not git status.

The fix is a one-character change: 'git*' -> 'git'. The pattern 'git' becomes /^git\b/, which correctly matches git status (space is a word boundary). The production code fix itself is unaffected.

I'm pushing this now - apologies that it'll reset the approval status. The diff should be trivially reviewable (single character removal in test data).

[ShehabSherif0]

Please approve this @meganrogge it was supposed to be approved and merged before. Thank you!