Skip to content

Commit af1b22f

Browse files
Eric-B-WuEric-B-Wu
authored
fix(designer): Fix OAuth adding extra parameters not specified on api registration (#8798)
* Fix obo * small fix * fix connection issue
1 parent 9e4f94a commit af1b22f

File tree

6 files changed

+208
-106
lines changed

6 files changed

+208
-106
lines changed

‎libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts‎

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ export const serializeOperation = async (
287287
}
288288

289289
let serializedOperation: LogicAppsV2.OperationDefinition;
290-
const isManagedMcpClient = operation.type?.toLowerCase() === 'mcpclienttool' && operation.kind?.toLowerCase() === "managed";
290+
const isManagedMcpClient = operation.type?.toLowerCase() === 'mcpclienttool' && operation.kind?.toLowerCase() === 'managed';
291291

292292
if (isManagedMcpClient) {
293293
serializedOperation = await serializeManagedMcpOperation(rootState, operationId);
@@ -432,9 +432,10 @@ const serializeManifestBasedOperation = async (rootState: RootState, operationId
432432
const hostInfo = serializeHost(operationId, manifest, rootState);
433433
const inputs = hostInfo !== undefined ? mergeHostWithInputs(hostInfo, inputPathValue) : inputPathValue;
434434
const operationFromWorkflow = getRecordEntry(rootState.workflow.operations, operationId) as LogicAppsV2.OperationDefinition;
435-
const runAfter = isRootNode(operationId, rootState.workflow.nodesMetadata) || manifest.properties.runAfter?.type === RunAfterType.NotSupported
436-
? undefined
437-
: getRunAfter(operationFromWorkflow, idReplacements);
435+
const runAfter =
436+
isRootNode(operationId, rootState.workflow.nodesMetadata) || manifest.properties.runAfter?.type === RunAfterType.NotSupported
437+
? undefined
438+
: getRunAfter(operationFromWorkflow, idReplacements);
438439
const recurrence =
439440
isTrigger && manifest.properties.recurrence && manifest.properties.recurrence.type !== RecurrenceType.None
440441
? constructInputValues('recurrence.$.recurrence', inputsToSerialize, false /* encodePathComponents */)
@@ -475,7 +476,7 @@ const serializeManagedMcpOperation = async (rootState: RootState, nodeId: string
475476
const nativeMcpOperationInfo = { connectorId: 'connectionProviders/mcpclient', operationId: 'nativemcpclient' };
476477
const manifest = await getOperationManifest(nativeMcpOperationInfo);
477478
const inputParameters = serializeParametersFromManifest(inputsToSerialize, manifest);
478-
479+
479480
const operationFromWorkflow = getRecordEntry(rootState.workflow.operations, nodeId) as LogicAppsV2.OperationDefinition;
480481

481482
const { parsedSwagger } = await getConnectorWithSwagger(connectorId);
@@ -859,7 +860,7 @@ interface AgentConnectionInfo {
859860
interface McpConnectionInfo {
860861
connectionReference: {
861862
connectionName: string;
862-
}
863+
};
863864
}
864865

865866
const serializeHost = (
@@ -945,8 +946,8 @@ const serializeHost = (
945946
case ConnectionReferenceKeyFormat.McpConnection:
946947
return {
947948
connectionReference: {
948-
connectionName: referenceKey
949-
}
949+
connectionName: referenceKey,
950+
},
950951
};
951952
default:
952953
throw new AssertionException(
@@ -1006,28 +1007,28 @@ const serializeNestedOperations = async (
10061007

10071008
if (subGraphDetail?.allowOperations) {
10081009
const operations = operationNodes.filter((graph) => graph.subGraphLocation === subGraphLocation);
1009-
const nestedOperationsPromises = operations.map((nestedOperation) =>
1010-
serializeOperation(rootState, nestedOperation.id)
1011-
) as Promise<LogicAppsV2.OperationDefinition>[];
1012-
const nestedOperations = await Promise.all(nestedOperationsPromises);
1013-
const idReplacements = rootState.workflow.idReplacements;
1014-
1015-
const newResult = {};
1016-
safeSetObjectPropertyValue(
1017-
newResult,
1018-
[subGraphLocation],
1019-
nestedOperations.reduce((actions: LogicAppsV2.Actions, action: LogicAppsV2.OperationDefinition, index: number) => {
1020-
if (!isNullOrEmpty(action)) {
1021-
const actionId = operations[index].id;
1022-
actions[getRecordEntry(idReplacements, actionId) ?? actionId] = action;
1023-
return actions;
1024-
}
1010+
const nestedOperationsPromises = operations.map((nestedOperation) =>
1011+
serializeOperation(rootState, nestedOperation.id)
1012+
) as Promise<LogicAppsV2.OperationDefinition>[];
1013+
const nestedOperations = await Promise.all(nestedOperationsPromises);
1014+
const idReplacements = rootState.workflow.idReplacements;
10251015

1016+
const newResult = {};
1017+
safeSetObjectPropertyValue(
1018+
newResult,
1019+
[subGraphLocation],
1020+
nestedOperations.reduce((actions: LogicAppsV2.Actions, action: LogicAppsV2.OperationDefinition, index: number) => {
1021+
if (!isNullOrEmpty(action)) {
1022+
const actionId = operations[index].id;
1023+
actions[getRecordEntry(idReplacements, actionId) ?? actionId] = action;
10261024
return actions;
1027-
}, {})
1028-
);
1025+
}
1026+
1027+
return actions;
1028+
}, {})
1029+
);
10291030

1030-
result = merge(result, newResult);
1031+
result = merge(result, newResult);
10311032
}
10321033

10331034
if (subGraphDetail?.isAdditive) {

‎libs/designer-v2/src/lib/core/parsers/BJSWorkflow/__test__/agentMcpWorkflowDefinition.ts‎

Lines changed: 68 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -9,62 +9,62 @@ export const agentMcpWorkflowDefinitionInput = {
99
contentVersion: '1.0.0.0',
1010
parameters: {},
1111
triggers: {
12-
'manual': {
12+
manual: {
1313
type: 'Request',
1414
kind: 'Http',
1515
inputs: {
16-
schema: {}
17-
}
18-
}
16+
schema: {},
17+
},
18+
},
1919
},
2020
actions: {
21-
'WorkflowAgent': {
21+
WorkflowAgent: {
2222
type: 'Agent',
2323
inputs: {
2424
parameters: {
2525
deploymentId: 'gpt-4o',
2626
messages: [
2727
{
2828
role: 'System',
29-
content: 'You are a helpful assistant that can use tools to accomplish tasks.'
29+
content: 'You are a helpful assistant that can use tools to accomplish tasks.',
3030
},
3131
{
3232
role: 'User',
33-
content: 'Help me manage files and data.'
34-
}
35-
]
36-
}
33+
content: 'Help me manage files and data.',
34+
},
35+
],
36+
},
3737
},
3838
tools: {
39-
'McpFileServer': {
40-
type: 'McpClientTool',
41-
kind: 'BuiltIn',
42-
inputs: {
43-
parameters: {
44-
mcpServerPath: '/servers/filesystem',
45-
toolName: 'list_files',
46-
}
47-
}
48-
}
39+
McpFileServer: {
40+
type: 'McpClientTool',
41+
kind: 'BuiltIn',
42+
inputs: {
43+
parameters: {
44+
mcpServerPath: '/servers/filesystem',
45+
toolName: 'list_files',
46+
},
47+
},
48+
},
4949
},
5050
runAfter: {},
5151
limit: {
5252
timeout: 'PT2H',
53-
count: 50
54-
}
53+
count: 50,
54+
},
5555
},
56-
'ResponseAction': {
56+
ResponseAction: {
5757
type: 'Response',
5858
inputs: {
5959
statusCode: 200,
60-
body: '@outputs("WorkflowAgent")'
60+
body: '@outputs("WorkflowAgent")',
6161
},
6262
runAfter: {
63-
'WorkflowAgent': ['SUCCEEDED']
64-
}
65-
}
63+
WorkflowAgent: ['SUCCEEDED'],
64+
},
65+
},
6666
},
67-
outputs: {}
67+
outputs: {},
6868
};
6969

7070
export const expectedAgentMcpWorkflowDefinitionOutput = {
@@ -92,16 +92,16 @@ export const expectedAgentMcpWorkflowDefinitionOutput = {
9292
children: [
9393
{
9494
height: 40,
95-
id: "WorkflowAgent-addCase-#subgraph",
96-
type: "SUBGRAPH_CARD_NODE",
95+
id: 'WorkflowAgent-addCase-#subgraph',
96+
type: 'SUBGRAPH_CARD_NODE',
9797
width: 200,
9898
},
9999
],
100100
edges: [],
101-
id: "WorkflowAgent-addCase",
101+
id: 'WorkflowAgent-addCase',
102102
subGraphLocation: undefined,
103-
type: "HIDDEN_NODE",
104-
},
103+
type: 'HIDDEN_NODE',
104+
},
105105
],
106106
edges: [
107107
{
@@ -124,15 +124,12 @@ export const expectedAgentMcpWorkflowDefinitionOutput = {
124124
},
125125
createWorkflowNode('ResponseAction'),
126126
],
127-
edges: [
128-
createWorkflowEdge('manual', 'WorkflowAgent'),
129-
createWorkflowEdge('WorkflowAgent', 'ResponseAction'),
130-
],
127+
edges: [createWorkflowEdge('manual', 'WorkflowAgent'), createWorkflowEdge('WorkflowAgent', 'ResponseAction')],
131128
},
132129
actionData: {
133130
manual: {
134131
inputs: {
135-
schema: {}
132+
schema: {},
136133
},
137134
kind: 'Http',
138135
type: 'Request',
@@ -144,58 +141,58 @@ export const expectedAgentMcpWorkflowDefinitionOutput = {
144141
messages: [
145142
{
146143
role: 'System',
147-
content: 'You are a helpful assistant that can use tools to accomplish tasks.'
144+
content: 'You are a helpful assistant that can use tools to accomplish tasks.',
148145
},
149146
{
150147
role: 'User',
151-
content: 'Help me manage files and data.'
152-
}
153-
]
154-
}
148+
content: 'Help me manage files and data.',
149+
},
150+
],
151+
},
155152
},
156153
tools: {
157-
'McpFileServer': {
158-
type: 'McpClientTool',
159-
kind: 'BuiltIn',
160-
inputs: {
161-
parameters: {
162-
mcpServerPath: '/servers/filesystem',
163-
toolName: 'list_files',
164-
}
165-
}
166-
}
154+
McpFileServer: {
155+
type: 'McpClientTool',
156+
kind: 'BuiltIn',
157+
inputs: {
158+
parameters: {
159+
mcpServerPath: '/servers/filesystem',
160+
toolName: 'list_files',
161+
},
162+
},
163+
},
167164
},
168165
limit: {
169166
timeout: 'PT2H',
170-
count: 50
167+
count: 50,
171168
},
172169
runAfter: {},
173170
type: 'Agent',
174171
},
175172
ResponseAction: {
176173
inputs: {
177174
statusCode: 200,
178-
body: '@outputs("WorkflowAgent")'
175+
body: '@outputs("WorkflowAgent")',
179176
},
180177
runAfter: {
181-
'WorkflowAgent': ['SUCCEEDED']
178+
WorkflowAgent: ['SUCCEEDED'],
182179
},
183180
type: 'Response',
184181
},
185182
McpFileServer: {
186-
type: 'McpClientTool',
187-
kind: 'BuiltIn',
188-
inputs: {
189-
parameters: {
190-
mcpServerPath: '/servers/filesystem',
191-
toolName: 'list_files',
192-
}
193-
}
183+
type: 'McpClientTool',
184+
kind: 'BuiltIn',
185+
inputs: {
186+
parameters: {
187+
mcpServerPath: '/servers/filesystem',
188+
toolName: 'list_files',
189+
},
190+
},
194191
},
195192
},
196193
nodesMetadata: {
197194
manual: { graphId: 'root', isRoot: true, isTrigger: true },
198-
WorkflowAgent: {
195+
WorkflowAgent: {
199196
graphId: 'root',
200197
actionCount: 1,
201198
parentNodeId: undefined,
@@ -204,13 +201,13 @@ export const expectedAgentMcpWorkflowDefinitionOutput = {
204201
McpFileServer: {
205202
graphId: 'WorkflowAgent',
206203
parentNodeId: 'WorkflowAgent',
207-
subgraphType: "MCP_CLIENT",
204+
subgraphType: 'MCP_CLIENT',
208205
},
209206
'WorkflowAgent-addCase': {
210-
actionCount: 0,
211-
graphId: 'WorkflowAgent',
212-
parentNodeId: 'WorkflowAgent',
213-
subgraphType: 'AGENT_ADD_CONDITON',
207+
actionCount: 0,
208+
graphId: 'WorkflowAgent',
209+
parentNodeId: 'WorkflowAgent',
210+
subgraphType: 'AGENT_ADD_CONDITON',
214211
},
215212
},
216-
};
213+
};

‎libs/designer-v2/src/lib/ui/panel/connectionsPanel/createConnection/__test__/createConnectionWrapper.spec.tsx‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,46 @@ describe('CreateConnectionWrapper', () => {
5252
expectedConnectionParameterSetValuesWithUndefined
5353
);
5454
});
55+
56+
it('filters out parameters not in validParameterKeys when provided', () => {
57+
const outputValues = {
58+
parameter1: 'value1',
59+
parameter2: 'value2',
60+
'token:tenantId': 'tenant-id-value', // This should be filtered out
61+
extraParam: 'extra-value', // This should be filtered out
62+
};
63+
64+
const validKeys = ['parameter1', 'parameter2'];
65+
66+
const result = getConnectionParameterSetValues(selectedParameterSetName, outputValues, validKeys);
67+
68+
expect(result).toEqual({
69+
name: selectedParameterSetName,
70+
values: {
71+
parameter1: { value: 'value1' },
72+
parameter2: { value: 'value2' },
73+
},
74+
});
75+
// Ensure filtered parameters are not included
76+
expect(result.values).not.toHaveProperty('token:tenantId');
77+
expect(result.values).not.toHaveProperty('extraParam');
78+
});
79+
80+
it('includes all parameters when validParameterKeys is not provided', () => {
81+
const outputValues = {
82+
parameter1: 'value1',
83+
'token:tenantId': 'tenant-id-value',
84+
};
85+
86+
const result = getConnectionParameterSetValues(selectedParameterSetName, outputValues);
87+
88+
expect(result).toEqual({
89+
name: selectedParameterSetName,
90+
values: {
91+
parameter1: { value: 'value1' },
92+
'token:tenantId': { value: 'tenant-id-value' },
93+
},
94+
});
95+
});
5596
});
5697
});

0 commit comments

Comments
 (0)