fix(vscode): getConfiguration handles object input for Service Provider connections with MSI#8820
fix(vscode): getConfiguration handles object input for Service Provider connections with MSI#8820samikay101 wants to merge 1 commit intomainfrom
Conversation
…connections with MSI
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
✅ Risk Level
✅ What & Why
✅ Impact of Change
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | No change needed |
| Commit Type | ✅ | No change needed |
| Risk Level | ✅ | Matches label risk:low |
| What & Why | ✅ | Consider referencing the changed file directly |
| Impact of Change | ✅ | Add a follow-up issue link for upstream fix |
| Test Plan | Add a unit test for object input | |
| Contributors | Add credits if applicable | |
| Screenshots/Videos | ✅ | N/A |
Final message
This PR passes the PR-title-and-body checks. The advised risk level based on the small, localized code change is risk:low, which matches the label and PR body. Please consider adding a minimal unit test for the object-input case and optionally add a follow-up issue link to fix the upstream caller (you already note this is a workaround — linking/tracking it helps reviewers). Also consider adding a Contributors line if others helped.
Thank you — this is a clear, small, well-documented fix. Happy to re-check after you add the suggested unit test or follow-up issue link if you want.
Last updated: Tue, 17 Feb 2026 23:06:32 GMT
📊 Coverage CheckThe following changed files need attention:
Please add tests for the uncovered files before merging. |
There was a problem hiding this comment.
Pull request overview
This PR adds a defensive workaround to the getConfiguration function to handle cases where it's called with a configuration object instead of a connection ID string. This issue manifests when using Service Provider connections with Managed Identity authentication in VS Code local development, causing dynamic dropdowns (like Service Bus queue names) to fail with network errors.
Changes:
- Added type check in
getConfigurationto detect and return configuration objects passed as input - Added explanatory comment documenting this as a workaround
| // WORKAROUND: If connectionId is already a configuration object (not a string), | ||
| // it means something is calling getConfiguration with the result of a previous call. | ||
| // In this case, just return the object as-is. | ||
| const connectionIdAsAny = connectionId as any; | ||
| if (typeof connectionIdAsAny === 'object' && connectionIdAsAny !== null) { | ||
| return connectionIdAsAny as Record<string, any>; | ||
| } |
There was a problem hiding this comment.
This workaround is masking the real bug in the VS Code connector service. The root cause is in apps/vs-code-react/src/app/designer/services/connector.ts lines 31 and 50, where getListDynamicValues() and getDynamicSchema() are passing the configuration object to _listDynamicValues() and _getDynamicSchema(), but these parent class methods expect connectionId (string) and configuration (object) as separate parameters respectively.
The proper fix would be to update connector.ts line 31 from:
return this._listDynamicValues(connectorId, operationId, parameters, dynamicState, configuration);
to:
return this._listDynamicValues(connectorId, operationId, parameters, dynamicState, connectionId);
And ensure line 50 passes the correct parameters to _getDynamicSchema() as well. This defensive check can remain as a safety measure, but the upstream bug should be fixed to prevent the double-calling pattern.
| const connectionIdAsAny = connectionId as any; | ||
| if (typeof connectionIdAsAny === 'object' && connectionIdAsAny !== null) { | ||
| return connectionIdAsAny as Record<string, any>; |
There was a problem hiding this comment.
The variable name connectionIdAsAny is misleading because it's not actually a connection ID - it's a parameter that might be either a connection ID (string) or a configuration object. A more accurate name would be connectionIdParam or connectionIdOrConfig to reflect that it could be either type.
| const connectionIdAsAny = connectionId as any; | |
| if (typeof connectionIdAsAny === 'object' && connectionIdAsAny !== null) { | |
| return connectionIdAsAny as Record<string, any>; | |
| const connectionIdOrConfig = connectionId as any; | |
| if (typeof connectionIdOrConfig === 'object' && connectionIdOrConfig !== null) { | |
| return connectionIdOrConfig as Record<string, any>; |
| getConfiguration: async (connectionId: string, manifest: OperationManifest | undefined): Promise<any> => { | ||
| try { | ||
| const configuration: Record<string, any> = {}; | ||
| // WORKAROUND: If connectionId is already a configuration object (not a string), | ||
| // it means something is calling getConfiguration with the result of a previous call. | ||
| // In this case, just return the object as-is. | ||
| const connectionIdAsAny = connectionId as any; | ||
| if (typeof connectionIdAsAny === 'object' && connectionIdAsAny !== null) { | ||
| return connectionIdAsAny as Record<string, any>; |
There was a problem hiding this comment.
Using as any to cast the parameter bypasses TypeScript's type checking. Since the function signature declares connectionId: string, this type mismatch should ideally be caught at compile time. Consider changing the function signature to connectionId: string | Record<string, any> to make the defensive check explicit in the type system, or use a more type-safe approach with a type guard.
| getConfiguration: async (connectionId: string, manifest: OperationManifest | undefined): Promise<any> => { | |
| try { | |
| const configuration: Record<string, any> = {}; | |
| // WORKAROUND: If connectionId is already a configuration object (not a string), | |
| // it means something is calling getConfiguration with the result of a previous call. | |
| // In this case, just return the object as-is. | |
| const connectionIdAsAny = connectionId as any; | |
| if (typeof connectionIdAsAny === 'object' && connectionIdAsAny !== null) { | |
| return connectionIdAsAny as Record<string, any>; | |
| getConfiguration: async ( | |
| connectionId: string | Record<string, any>, | |
| manifest: OperationManifest | undefined | |
| ): Promise<any> => { | |
| try { | |
| const configuration: Record<string, any> = {}; | |
| // WORKAROUND: If connectionId is already a configuration object (not a string), | |
| // it means something is calling getConfiguration with the result of a previous call. | |
| // In this case, just return the object as-is. | |
| if (typeof connectionId === 'object' && connectionId !== null) { | |
| return connectionId; |
2 participants
Commit Type
Risk Level
Summary
When using Service Provider connections with Managed Identity authentication in VS Code local development, dynamic dropdowns (e.g., Queue name for Service Bus) fail with a network error.
Root Cause
The
getConfigurationfunction was being called twice in the call chain:connectionId= string (e.g.,"/serviceProviders/serviceBus/connections/serviceBus-6") ✅connectionId= the configuration object returned from the first call ❌On the second call, the function tried to parse an object as a string, which caused
extractConnectionName()to fail, resulting in an empty configuration being sent to the backend.Fix
Added a type check at the start of
getConfigurationto detect when an object is passed instead of a string. If an object is passed, return it directly since it's already a valid configuration.Impact of Change
getConfigurationresilient to incorrect input types.Test Plan
Manual Test Steps
Notes