Skip to content

Conversation

@cameroncooke
Copy link
Collaborator

Move next-step definitions out of individual tool handlers and into YAML
manifests, introducing a declarative nextSteps array on each tool
manifest entry.

Previously every tool handler constructed its own nextSteps array
inline, duplicating structure and coupling presentation logic to tool
business logic. This made it hard to audit which tools suggest which
follow-up actions and required code changes to adjust next-step wording
or ordering.

The new system works in three layers:

  1. Manifest templates (manifests/tools/*.yaml) declare static
    next-step entries with a label, optional toolId, optional
    params (supporting ${arg} substitution from the invocation
    args), and optional priority.

  2. Template builder (buildTemplateNextSteps) resolves templates
    at invocation time, looking up target tools in the catalog and
    substituting params.

  3. Merge layer (mergeTemplateAndResponseNextSteps) combines
    template-driven steps with any dynamic nextStepParams the tool
    handler returns at runtime, so tools can still provide
    context-dependent param values without owning the full next-step
    structure.

Tool handlers now return only nextStepParams (a map of toolId to
param values) when they need dynamic data, rather than constructing
entire NextStep objects. Most tools no longer touch next-steps at all.

Includes a NEXT_STEPS_MIGRATION_TODO.md tracking remaining tools to
migrate and shared test helpers in src/test-utils/next-step-assertions.ts.

Move next-step definitions from inline tool logic into YAML manifests
using a new nextStepTemplates system. Templates support static params,
${arg} substitution, and runtime merging with dynamic response params.
Removes hardcoded next-step construction from individual tool handlers.
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

...templateStep,
params: fallbackStep.params,
};
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index misalignment between filtered template steps and original templates

High Severity

mergeTemplateAndResponseNextSteps accesses tool.nextStepTemplates by positional index from the filtered templateSteps array, but buildTemplateNextSteps can skip entries when a target tool isn't in the runtime catalog (e.g., filtered by predicates). This breaks the 1:1 index correspondence, causing consumeDynamicParams to look up the wrong toolId and apply incorrect params. For example, list_devices references build_device (predicated), test_device (predicated), and get_device_app_path. In XcodeAgentMode, the first two are excluded, so templateSteps[0] is the get_device_app_path step but templates[0] points to the build_device template — applying build_device's dynamic params to the wrong step.

Additional Locations (1)

Fix in Cursor Fix in Web

`Important: Before working on the project make sure to read the README.md file in the workspace root directory.`,
`Build for simulator: build_sim({ workspacePath: "${projectPath}/${params.customizeNames ? params.projectName : 'MyProject'}.xcworkspace", scheme: "${params.customizeNames ? params.projectName : 'MyProject'}", simulatorName: "iPhone 16" })`,
`Build and run on simulator: build_run_sim({ workspacePath: "${projectPath}/${params.customizeNames ? params.projectName : 'MyProject'}.xcworkspace", scheme: "${params.customizeNames ? params.projectName : 'MyProject'}", simulatorName: "iPhone 16" })`,
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scaffold tools lost dynamic workspace path and scheme params

Medium Severity

Both scaffold handlers removed the dynamic next-step information (computed workspace path, scheme name, simulator name) from the response body without returning equivalent nextStepParams. The manifest templates for build_sim/build_run_sim/build_macos/build_run_macos have no params or ${arg} substitutions, so the resulting next steps will render without any params — e.g., build_sim() instead of build_sim({ workspacePath: "...", scheme: "...", simulatorName: "iPhone 16" }).

Additional Locations (2)

Fix in Cursor Fix in Web

}

return step;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exported test helper function is never imported anywhere

Low Severity

expectNextStepByToolId is exported from the new next-step-assertions.ts utility file but is never imported or used anywhere in the codebase. The module also eagerly calls loadManifest() at module scope via createMcpToolToIdResolver(), meaning any test that imports this file pays the cost of parsing the full YAML manifest even if it doesn't use the helper.

Fix in Cursor Fix in Web

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 11, 2026

Open in StackBlitz

npm i https://pkg.pr.new/getsentry/XcodeBuildMCP/xcodebuildmcp@216

commit: 7c062ae

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant