Skip to content

fix(mcp): stop empty UI-tool errors and tighten tool-usage guardrails#7911

Open
andypalmi wants to merge 14 commits into
mainfrom
fix/ui-navigate-empty-error
Open

fix(mcp): stop empty UI-tool errors and tighten tool-usage guardrails#7911
andypalmi wants to merge 14 commits into
mainfrom
fix/ui-navigate-empty-error

Conversation

@andypalmi

Copy link
Copy Markdown
Contributor

Stacked on #7908.

Summary

  • ui_navigate (and any other UI automation tool) could return Tool "X" failed: with nothing after the colon, because vue-router throws an Error with no message in production for an unknown route name, and the dispatcher's catch collapsed straight to err.message. The dispatcher now falls back to the error's own properties so the reason is never lost. ui_navigate also now catches resolve/push directly and returns a clean, structured failure. ui_list_routes no longer hides named redirect-only routes (like an application's landing route) from discovery.
  • Tightened a few tool descriptions so an id param can't be mistaken for a display name, and so platform_create_application / platform_create_remote_instance / platform_create_hosted_instance are explicit about checking for name collisions before creating and asking the user instead of guessing.

Closes

  • Closes FlowFuse/engineering#188
  • Closes FlowFuse/engineering#185
  • Closes FlowFuse/engineering#182
  • Closes FlowFuse/engineering#183

Test plan

  • npx vitest run --config ./config/vitest.config.ts test/unit/frontend/mcp/tools/navigation.spec.js test/unit/frontend/mcp/tools/routes.spec.js test/unit/frontend/services/automations.service.spec.js - all passing
  • npx eslint on all touched files - clean

andypalmi added 5 commits July 22, 2026 00:21
platform_list_stacks returned every stack across every instance type
with no way to tell which one was current, so "use the latest" was
ambiguous and a superseded Node-RED version could be selected.

Scope the tool to a chosen instance type, using the same projectType
filter as the real stacks endpoint, and surface that type's
defaultStack as the recommended latest version, both in the tool
description and in platform_create_hosted_instance's guidance.
The application-instances list carries no instance type, stack, or
template, and no tool description said so, so a request to read or
match an existing instance's specification had no reliable source and
could be answered with a guess reported as fact.

Note on the list tool that it omits the specification, and point both
it and the create-instance guidance at platform_get_hosted_instance,
which already returns the full specification.
router.resolve throws an Error with no message (only extra own
properties) for an unknown route name in a production build, which the
dispatcher's catch collapsed to `err.message` and lost entirely. The
dispatcher now falls back to the error's own properties when message is
empty. ui_navigate also now catches resolve/push directly and returns a
clean, structured failure instead of letting an unnamed-route lookup
throw past it, and ui_list_routes stops hiding named redirect-only
routes (like an application's landing route) from route discovery.

Closes FlowFuse/engineering#188
…isions

ui_navigate's id param now warns explicitly that it must be a real
entity id from a list/get tool, never a display name, and clarifies
that success: true only confirms the route matched - not that the
target entity exists.

platform_create_application now says to check platform_list_applications
for a name collision before creating one. platform_create_remote_instance
now says to assign the device to an application right away when the user
references one, instead of leaving it unassigned by default.
platform_create_hosted_instance makes the name-availability check
mandatory and says to offer the user alternative names on a collision
rather than picking one silently.

Closes FlowFuse/engineering#185, FlowFuse/engineering#182, FlowFuse/engineering#183
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.61%. Comparing base (e220d2e) to head (12f189a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7911      +/-   ##
==========================================
- Coverage   75.72%   75.61%   -0.11%     
==========================================
  Files         434      434              
  Lines       23198    23232      +34     
  Branches     6161     6172      +11     
==========================================
+ Hits        17567    17568       +1     
- Misses       5631     5664      +33     
Flag Coverage Δ
backend 75.61% <ø> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@andypalmi
andypalmi requested a review from cstns July 23, 2026 09:24
Comment thread forge/ee/lib/mcp/tools/applications.js Outdated
Comment thread forge/ee/lib/mcp/tools/devices.js Outdated
Comment thread frontend/src/mcp/tools/navigation.ts Outdated
Co-authored-by: Costin Serban <cstn.serban@gmail.com>
Co-authored-by: Costin Serban <cstn.serban@gmail.com>
andypalmi and others added 3 commits July 24, 2026 12:54
Replace platform_list_hosted_instance_types and platform_list_stacks
with a single tool that nests each instance type's stacks and flags
available/creatable per team, defaulting to creatable-only results.
Base automatically changed from fix/expert-mcp-latest-stack-and-spec to main July 24, 2026 10:58
andypalmi added 2 commits July 24, 2026 12:59
…dling

Move the error-describing fallback out of automations.service.ts into
frontend/src/mcp/tools/errors.ts and reuse it in ui_navigate, so a
NavigationFailure with no message surfaces its details instead of a
generic fallback string.
@andypalmi

Copy link
Copy Markdown
Contributor Author

@cstns I've already applied your suggestions in this PR.

The only additional change is the commit I just pushed: extracted the describeError fallback out of automations.service.ts into a shared frontend/src/mcp/tools/errors.ts helper, and reused it in ui_navigate so a NavigationFailure with no message surfaces its details instead of a generic string.

Could I get your review on this again, and if it looks good, an approve?

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.

2 participants