Skip to content

Commit 28b7b34

Browse files
committed
fix: resolve PR inline next-step template issues
1 parent a4c15cc commit 28b7b34

File tree

7 files changed

+168
-82
lines changed

7 files changed

+168
-82
lines changed

src/mcp/tools/project-scaffolding/__tests__/scaffold_ios_project.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,18 @@ describe('scaffold_ios_project plugin', () => {
373373
),
374374
},
375375
],
376+
nextStepParams: {
377+
build_sim: {
378+
workspacePath: '/tmp/test-projects/TestIOSApp.xcworkspace',
379+
scheme: 'TestIOSApp',
380+
simulatorName: 'iPhone 16',
381+
},
382+
build_run_sim: {
383+
workspacePath: '/tmp/test-projects/TestIOSApp.xcworkspace',
384+
scheme: 'TestIOSApp',
385+
simulatorName: 'iPhone 16',
386+
},
387+
},
376388
});
377389
});
378390

@@ -411,6 +423,18 @@ describe('scaffold_ios_project plugin', () => {
411423
),
412424
},
413425
],
426+
nextStepParams: {
427+
build_sim: {
428+
workspacePath: '/tmp/test-projects/TestIOSApp.xcworkspace',
429+
scheme: 'TestIOSApp',
430+
simulatorName: 'iPhone 16',
431+
},
432+
build_run_sim: {
433+
workspacePath: '/tmp/test-projects/TestIOSApp.xcworkspace',
434+
scheme: 'TestIOSApp',
435+
simulatorName: 'iPhone 16',
436+
},
437+
},
414438
});
415439
});
416440

@@ -441,6 +465,18 @@ describe('scaffold_ios_project plugin', () => {
441465
),
442466
},
443467
],
468+
nextStepParams: {
469+
build_sim: {
470+
workspacePath: '/tmp/test-projects/MyProject.xcworkspace',
471+
scheme: 'MyProject',
472+
simulatorName: 'iPhone 16',
473+
},
474+
build_run_sim: {
475+
workspacePath: '/tmp/test-projects/MyProject.xcworkspace',
476+
scheme: 'MyProject',
477+
simulatorName: 'iPhone 16',
478+
},
479+
},
444480
});
445481
});
446482

src/mcp/tools/project-scaffolding/__tests__/scaffold_macos_project.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,16 @@ describe('scaffold_macos_project plugin', () => {
249249
),
250250
},
251251
],
252+
nextStepParams: {
253+
build_macos: {
254+
workspacePath: '/tmp/test-projects/TestMacApp.xcworkspace',
255+
scheme: 'TestMacApp',
256+
},
257+
build_run_macos: {
258+
workspacePath: '/tmp/test-projects/TestMacApp.xcworkspace',
259+
scheme: 'TestMacApp',
260+
},
261+
},
252262
});
253263

254264
// Verify template manager calls using manual tracking
@@ -284,6 +294,16 @@ describe('scaffold_macos_project plugin', () => {
284294
),
285295
},
286296
],
297+
nextStepParams: {
298+
build_macos: {
299+
workspacePath: '/tmp/test-projects/MyProject.xcworkspace',
300+
scheme: 'MyProject',
301+
},
302+
build_run_macos: {
303+
workspacePath: '/tmp/test-projects/MyProject.xcworkspace',
304+
scheme: 'MyProject',
305+
},
306+
},
287307
});
288308
});
289309

src/mcp/tools/project-scaffolding/scaffold_ios_project.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,9 @@ export async function scaffold_ios_projectLogic(
358358
const projectParams = { ...params, platform: 'iOS' };
359359
const projectPath = await scaffoldProject(projectParams, commandExecutor, fileSystemExecutor);
360360

361+
const generatedProjectName = params.customizeNames === false ? 'MyProject' : params.projectName;
362+
const workspacePath = `${projectPath}/${generatedProjectName}.xcworkspace`;
363+
361364
const response = {
362365
success: true,
363366
projectPath,
@@ -372,6 +375,18 @@ export async function scaffold_ios_projectLogic(
372375
text: JSON.stringify(response, null, 2),
373376
},
374377
],
378+
nextStepParams: {
379+
build_sim: {
380+
workspacePath,
381+
scheme: generatedProjectName,
382+
simulatorName: 'iPhone 16',
383+
},
384+
build_run_sim: {
385+
workspacePath,
386+
scheme: generatedProjectName,
387+
simulatorName: 'iPhone 16',
388+
},
389+
},
375390
};
376391
} catch (error) {
377392
log(

src/mcp/tools/project-scaffolding/scaffold_macos_project.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,9 @@ export async function scaffold_macos_projectLogic(
332332
const projectParams = { ...params, platform: 'macOS' as const };
333333
const projectPath = await scaffoldProject(projectParams, commandExecutor, fileSystemExecutor);
334334

335+
const generatedProjectName = params.customizeNames === false ? 'MyProject' : params.projectName;
336+
const workspacePath = `${projectPath}/${generatedProjectName}.xcworkspace`;
337+
335338
const response = {
336339
success: true,
337340
projectPath,
@@ -346,6 +349,16 @@ export async function scaffold_macos_projectLogic(
346349
text: JSON.stringify(response, null, 2),
347350
},
348351
],
352+
nextStepParams: {
353+
build_macos: {
354+
workspacePath,
355+
scheme: generatedProjectName,
356+
},
357+
build_run_macos: {
358+
workspacePath,
359+
scheme: generatedProjectName,
360+
},
361+
},
349362
};
350363
} catch (error) {
351364
log(

src/runtime/__tests__/tool-invoker.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,59 @@ describe('DefaultToolInvoker next steps post-processing', () => {
370370
]);
371371
});
372372

373+
it('maps dynamic params to the correct template tool after catalog filtering', async () => {
374+
const directHandler = vi.fn().mockResolvedValue({
375+
content: [{ type: 'text', text: 'ok' }],
376+
nextStepParams: {
377+
stop_sim_log_cap: { logSessionId: 'session-123' },
378+
},
379+
} satisfies ToolResponse);
380+
381+
const catalog = createToolCatalog([
382+
makeTool({
383+
id: 'start_sim_log_cap',
384+
cliName: 'start-simulator-log-capture',
385+
mcpName: 'start_sim_log_cap',
386+
workflow: 'logging',
387+
stateful: false,
388+
nextStepTemplates: [
389+
{
390+
label: 'Unavailable step',
391+
toolId: 'missing_tool',
392+
},
393+
{
394+
label: 'Stop capture and retrieve logs',
395+
toolId: 'stop_sim_log_cap',
396+
priority: 1,
397+
},
398+
],
399+
handler: directHandler,
400+
}),
401+
makeTool({
402+
id: 'stop_sim_log_cap',
403+
cliName: 'stop-simulator-log-capture',
404+
mcpName: 'stop_sim_log_cap',
405+
workflow: 'logging',
406+
stateful: true,
407+
handler: vi.fn().mockResolvedValue(textResponse('stop')),
408+
}),
409+
]);
410+
411+
const invoker = new DefaultToolInvoker(catalog);
412+
const response = await invoker.invoke('start-simulator-log-capture', {}, { runtime: 'cli' });
413+
414+
expect(response.nextSteps).toEqual([
415+
{
416+
tool: 'stop_sim_log_cap',
417+
label: 'Stop capture and retrieve logs',
418+
params: { logSessionId: 'session-123' },
419+
priority: 1,
420+
workflow: 'logging',
421+
cliTool: 'stop-simulator-log-capture',
422+
},
423+
]);
424+
});
425+
373426
it('keeps tool-provided next steps when template count does not match', async () => {
374427
const directHandler = vi.fn().mockResolvedValue({
375428
content: [{ type: 'text', text: 'ok' }],

src/runtime/tool-invoker.ts

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,28 @@ function resolveTemplateParams(
4343
return resolved;
4444
}
4545

46+
type BuiltTemplateNextStep = {
47+
step: NextStep;
48+
templateToolId?: string;
49+
};
50+
4651
function buildTemplateNextSteps(
4752
tool: ToolDefinition,
4853
args: Record<string, unknown>,
4954
catalog: ToolCatalog,
50-
): NextStep[] {
55+
): BuiltTemplateNextStep[] {
5156
if (!tool.nextStepTemplates || tool.nextStepTemplates.length === 0) {
5257
return [];
5358
}
5459

55-
const built: NextStep[] = [];
60+
const built: BuiltTemplateNextStep[] = [];
5661
for (const template of tool.nextStepTemplates) {
5762
if (!template.toolId) {
5863
built.push({
59-
label: template.label,
60-
priority: template.priority,
64+
step: {
65+
label: template.label,
66+
priority: template.priority,
67+
},
6168
});
6269
continue;
6370
}
@@ -68,10 +75,13 @@ function buildTemplateNextSteps(
6875
}
6976

7077
built.push({
71-
tool: target.mcpName,
72-
label: template.label,
73-
params: resolveTemplateParams(template.params ?? {}, args),
74-
priority: template.priority,
78+
step: {
79+
tool: target.mcpName,
80+
label: template.label,
81+
params: resolveTemplateParams(template.params ?? {}, args),
82+
priority: template.priority,
83+
},
84+
templateToolId: template.toolId,
7585
});
7686
}
7787

@@ -102,21 +112,27 @@ function consumeDynamicParams(
102112
}
103113

104114
function mergeTemplateAndResponseNextSteps(
105-
tool: ToolDefinition,
106-
templateSteps: NextStep[],
115+
templateSteps: BuiltTemplateNextStep[],
107116
responseParamsMap: NextStepParamsMap | undefined,
108117
responseSteps: NextStep[] | undefined,
109118
): NextStep[] {
110119
const consumedCounts = new Map<string, number>();
111-
const templates = tool.nextStepTemplates ?? [];
112120

113-
return templateSteps.map((templateStep, index) => {
114-
const template = templates[index];
115-
if (!template?.toolId || !templateStep.tool || hasTemplateParams(templateStep)) {
121+
return templateSteps.map((builtTemplateStep, index) => {
122+
const templateStep = builtTemplateStep.step;
123+
if (
124+
!builtTemplateStep.templateToolId ||
125+
!templateStep.tool ||
126+
hasTemplateParams(templateStep)
127+
) {
116128
return templateStep;
117129
}
118130

119-
const paramsFromMap = consumeDynamicParams(responseParamsMap, template.toolId, consumedCounts);
131+
const paramsFromMap = consumeDynamicParams(
132+
responseParamsMap,
133+
builtTemplateStep.templateToolId,
134+
consumedCounts,
135+
);
120136
if (paramsFromMap) {
121137
return {
122138
...templateStep,
@@ -192,7 +208,6 @@ function postProcessToolResponse(params: {
192208
? {
193209
...response,
194210
nextSteps: mergeTemplateAndResponseNextSteps(
195-
tool,
196211
templateSteps,
197212
response.nextStepParams,
198213
response.nextSteps,

src/test-utils/next-step-assertions.ts

Lines changed: 0 additions & 66 deletions
This file was deleted.

0 commit comments

Comments
 (0)